Robert S Peterson <rpeterso@xxxxxxxxxx> wrote: > > Use normal write file operations rather than AOPS prepare_write and > commit_write when the backing filesystem requires special locking. > > .. > > --- a/drivers/block/loop.c > +++ b/drivers/block/loop.c > @@ -44,6 +44,11 @@ > * backing filesystem. > * Anton Altaparmakov, 16 Feb 2005 > * > + * Extension of Anton's idea: Use normal write file operations rather than > + * prepare_write and commit_write when the backing filesystem requires > + * special locking. > + * Robert Peterson <rpeterso@xxxxxxxxxx>, 01 Mar 2006 > + * The preferred convention is not to put changelogs into .c files. The revision control system is where such info is kept. > * Still To Fix: > * - Advisory locking is ignored here. > * - Should use an own CAP_* category instead of CAP_SYS_ADMIN > @@ -74,6 +79,7 @@ > #include <linux/completion.h> > #include <linux/highmem.h> > #include <linux/gfp.h> > +#include <linux/mount.h> > > #include <asm/uaccess.h> > > @@ -791,7 +797,8 @@ static int loop_set_fd(struct loop_devic > */ > if (!file->f_op->sendfile) > goto out_putf; > - if (aops->prepare_write && aops->commit_write) > + if (!(file->f_vfsmnt->mnt_sb->s_type->fs_flags & FS_AOPS_NEED_LOCKING) && > + aops->prepare_write && aops->commit_write) > lo_flags |= LO_FLAGS_USE_AOPS; > if (!(lo_flags & LO_FLAGS_USE_AOPS) && !file->f_op->write) > lo_flags |= LO_FLAGS_READ_ONLY; > diff --git a/include/linux/fs.h b/include/linux/fs.h > index 9d96749..3def72e 100644 > --- a/include/linux/fs.h > +++ b/include/linux/fs.h > @@ -88,6 +88,7 @@ extern int dir_notify_enable; > /* public flags for file_system_type */ > #define FS_REQUIRES_DEV 1 > #define FS_BINARY_MOUNTDATA 2 > +#define FS_AOPS_NEED_LOCKING 4 /* Filesystem aops have special locking needs */ > #define FS_REVAL_DOT 16384 /* Check the paths ".", ".." for staleness */ > #define FS_ODD_RENAME 32768 /* Temporary stuff; will go away as soon > * as nfs_rename() will be cleaned up FS_AOPS_NEED_LOCKING is too poorly defined. "locking" of what?? All that should be defined with some precision and documented at least in the changelog and preferably in a code comment above the FS_AOPS_NEED_LOCKING definition site. And the name "FS_AOPS_NEED_LOCKING" itself is very vague. Plus we have no in-kernel users of this new flag from which to glean some understanding of what it means, so the documentation requirements become higher. I don't think the fact that the filesystem does or doesn't use locking is relevant to this patch. Why not call the thing FS_LOOP_USE_READ_WRITE? AFter all, that's what it does. I assume this new flag is needed for some out-of-tree filesystem? If so, the changelog should have described which one, and why it needs this flag, and how it will be using it, etc. I'm not averse to putting some tweaks into core kernel to support out-of-tree GPL code - if it's of significant benefit to the owners of that code (like: our code will now run when loaded into unmodified vendor kernels) and has a minor impact on the kernel.org tree, then why not? But it does need to be a good change, and one which is carefully and completely described, please. - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
