[Cluster-devel] [GFS2 Patch][TRY 2] GFS2: Extend the life of the reservations structure

[Bob Peterson]

----- Original Message -----
| > @@ -722,7 +731,7 @@ static int gfs2_create_inode(struct inode *dir,
| > struct dentry *dentry,
| >  	gfs2_trans_end(sdp);
| >  	/* Check if we reserved space in the rgrp. Function link_dinode
| >  	may
| >  	   not, depending on whether alloc is required. */
| > -	if (dip->i_res)
| > +	if (gfs2_mb_reserved(dip))
| >  		gfs2_inplace_release(dip);
| >  	gfs2_quota_unlock(dip);
| >  	gfs2_qadata_put(dip);
| > @@ -740,6 +749,7 @@ fail_gunlock:
| >  		iput(inode);
| >  	}
| >  fail:
| > +	gfs2_rs_delete(dip);
| 
| Should we really be dropping the ip->i_res here I wonder? I'm not
| sure
| that we want to forget this information just because (for example)
| someone has tried to create a new inode and it has been failed for
| some
| reason or other.
| 
| Otherwise I think this looks good,
| 
| Steve.

Hi,

In this implementation, function gfs2_inplace_release just unlocks the
rgrp glock and sets rs_requested to zero. In fact, the reservation still
hangs around, attached to the inode until function gfs2_rs_delete is called
from gfs2_release or gfs2_evict_inode. So instead of having two functions
for allocation and deallocation, we now have four:

gfs2_rs_alloc:        kmem_cache_zalloc
gfs2_inplace_reserve: hold rgrp glock, rs_requested = reservation needed
gfs2_inplace_release: dq rgrp glock, rs_requested = 0
gfs2_rs_delete:       kmem_cache_free

I can change the names (suggestions welcome), because "gfs2_inplace_reserve"
and its counterpart have changed their roles over the years. I'm just
conditioned to look for those function names from years of conditioning. :)

Something more like "get_local_rgrp" is a more accurate reflection of
what's happening, but that name is already used.  I've never liked how
get_local_rgrp and gfs2_inplace_reserve both have retry loops. 
Since they're now pretty small functions, and one calls the other, perhaps
I should combine them into one function by the name "get_local_rgrp"?
And perhaps that should be done in a separate patch?

Regards,

Bob Peterson
Red Hat File Systems