[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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

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


+	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.


+	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.


Bob Peterson
Red Hat Cluster Suite

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]