> 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
Neil
Jeeze, I got good news and bad news...don't you just hate reading
this first line. ;-)
Applied your patch, the problems does not occur.
Rebooted back to old 2.4.0-test12 image and cannot reproduce bug,
yet 2 days ago I could make it happen at will. I double checked
the compile dates of the kernel and I didn't mess up the patched
kernel with the old kernel. I'll let my tests run while i'm at
work and try to pound on it again this evening.
So without a definitive, yup it still happens w/o patch I can't be
sure I can tell you that your patch is a sure cure.
Martin
>
> --- 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