On Monday December 18, trond.myklebust@fys.uio.no wrote:
> >>>>> " " == M H VanLeeuwen <vanl@megsinet.net> writes:
>
> > Trond, Neil I don't know if this is a loopback bug or an NFS
> > bug but since nfs_fs.h was implicated so I thought one of you
> > may be interested.
>
> > Could you let me know if you know this problem has already been
> > fixed or if you need more info.
>
> Hi,
>
> As far as I'm concerned, it's a loopback bug.
I read it the same way.
Actually, I cannot see the point of copying the "struct file"! Why
not just take a reference to it? The comment tries to justify it, but
I don't buy it.
Would you mind trying the following patch (totally untested, but
obviously correct :-)
NeilBrown
--- drivers/block/loop.c 2000/12/18 10:25:22 1.1
+++ drivers/block/loop.c 2000/12/18 11:23:52
@@ -441,37 +441,18 @@
if (!aops->prepare_write || !aops->commit_write)
lo->lo_flags |= LO_FLAGS_READ_ONLY;
- error = get_write_access(inode);
- if (error)
- goto out_putf;
/* Backed by a regular file - we need to hold onto a file
- structure for this file. Friggin' NFS can't live without
- it on write and for reading we use do_generic_file_read(),
- so... We create a new file structure based on the one
- passed to us via 'arg'. This is to avoid changing the file
- structure that the caller is using */
+ structure for this file. There is no reliable way to
+ copy it (due to a filesys private field that may be ref-counted)
+ so we just keep a reference to the file like a "dup" would.
+ */
lo->lo_device = inode->i_dev;
lo->lo_flags = LO_FLAGS_DO_BMAP;
- error = -ENFILE;
- lo->lo_backing_file = get_empty_filp();
- if (lo->lo_backing_file == NULL) {
- put_write_access(inode);
- goto out_putf;
- }
-
- lo->lo_backing_file->f_mode = file->f_mode;
- lo->lo_backing_file->f_pos = file->f_pos;
- lo->lo_backing_file->f_flags = file->f_flags;
- lo->lo_backing_file->f_owner = file->f_owner;
- lo->lo_backing_file->f_dentry = file->f_dentry;
- lo->lo_backing_file->f_vfsmnt = mntget(file->f_vfsmnt);
- lo->lo_backing_file->f_op = fops_get(file->f_op);
- lo->lo_backing_file->private_data = file->private_data;
- file_moveto(lo->lo_backing_file, file);
-
+ lo->lo_backing_file = file;
+ get_file(file);
error = 0;
}
@@ -539,8 +520,6 @@
if (lo->lo_backing_file != NULL) {
struct file *filp = lo->lo_backing_file;
- if ((filp->f_mode & FMODE_WRITE) == 0)
- put_write_access(filp->f_dentry->d_inode);
fput(filp);
lo->lo_backing_file = NULL;
} else {
>
> Somebody appears to be trying to copy a 'struct file' in the routine
> 'loop_set_fd'. This will cause havoc in any and all filesystems that
> rely on f_ops->open() , f_ops->release() to maintain internal data.
> In this case, it's the file's RPC authorizations, that are getting
> garbage-collected from beneath you once the original struct file gets
> fput() at the end of the routine.
>
> Cheers,
> Trond
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Sat Dec 23 2000 - 21:00:21 EST