[Cluster-devel] [PATCH] GFS2: kernel changes to support new gfs2_grow command (Try 3)

[Robert Peterson]

David Teigland wrote:
> On Wed, May 02, 2007 at 08:57:08PM -0500, Robert Peterson wrote:
>> +	__u64 fs_total, new_free, new_free_readable;
> 
> use u64, __u64 is for structs shared with user space

Okay.

>> +	printk(KERN_WARNING "GFS2: File system extended by %llu "
>> +	       "blocks (%llu%cB)\n", new_free, new_free_readable,
>> +	       stg_abbrev[factor]);
> 
> use fs_warn(), and unless there's some precedent elsewhere I don't think
> the human-readable translation is appropriate in the kernel.

Okay.
 
>> +	if (!gfs2_glock_is_held_excl(ip->i_gl) &&
>> +	    !gfs2_glock_is_held_shrd(ip->i_gl) &&
> 
> Be extremely wary of using these "is_held" functions algorithmically;
> needing to use them is usually an indication that you're doing something
> wrong (I'll have to study this case a bit more to say anything helpful).

I was using these functions to determine if someone was doing
writes to the rindex file through the meta_fs, in which case I
don't want to exit with a consistency error.  If the rindex is being
written to, there may be partial pieces of rindex entries
left on a page (i.e. between two gfs2_write_commits) before the
writing is over.  In these cases, it's normal and we should ignore
the partial entries (and re-read them when the writing is complete
and the version number has changed).

My concern here is if a different node decides to do an ri_update
while a gfs2_grow is happening.  For example, if someone does a
mount of the file system on a different node, the rindex may
briefly be in an inconsistent state.

> It's instructional to note that the glock_is_held functions are _never_
> used outside of assertions in all of gfs1 and gfs2!  There's an exception
> to every rule, but...

Yeah, I noticed that.

> Dave
 
Bob Peterson
Red Hat Cluster Suite