On Mon, 18 Apr 2011 16:58:09 +0400
Pavel Shilovsky <piastry@xxxxxxxxxxx> wrote:
> 2011/4/18 Jeff Layton <jlayton@xxxxxxxxxx>:
> > On Mon, 18 Apr 2011 16:20:52 +0400
> > Pavel Shilovsky <piastry@xxxxxxxxxxx> wrote:
> >
> >> Add rwpidforward mount option that switches on a mode when we forward
> >> pid of a process who opened a file to any read and write operation.
> >>
> >> This can prevent applications like WINE from failing on read or write
> >> operation on a previously locked file region from the same netfd from
> >> another process if we use mandatory brlock style.
> >> It is actual for WINE because during a run of WINE program two processes
> >> work on the same netfd - share the same file struct between several VFS
> >> fds:
> >> 1) WINE-server does open and lock;
> >> 2) WINE-application does read and write.
> >>
> >> Signed-off-by: Pavel Shilovsky <piastry@xxxxxxxxxxx>
> >> ---
> >> Âfs/cifs/README Â Â Â | Â Â3 ++
> >> Âfs/cifs/cifs_fs_sb.h | Â Â1 +
> >> Âfs/cifs/cifsfs.c   |  Â8 ++++++
> >> Âfs/cifs/cifsproto.h Â| Â 21 ++++++++-------
> >> Âfs/cifs/cifssmb.c  Â|  31 +++++++++++++++-------
> >> Âfs/cifs/connect.c  Â|  Â5 +++
> >> Âfs/cifs/dir.c    Â|  Â4 +-
> >> Âfs/cifs/file.c    |  69 ++++++++++++++++++++++++++++++++++++++-----------
> >> Âfs/cifs/inode.c   Â|  10 ++++---
> >> Âfs/cifs/link.c    |  Â6 ++--
> >> Â10 files changed, 113 insertions(+), 45 deletions(-)
> >>
> >> diff --git a/fs/cifs/README b/fs/cifs/README
> >> index 4a3ca0e..33f4ba0 100644
> >> --- a/fs/cifs/README
> >> +++ b/fs/cifs/README
> >> @@ -457,6 +457,9 @@ A partial list of the supported mount options follows:
> >> Â Â Â Â Â Â Â otherwise - read from the server. All written data are stored
> >> Â Â Â Â Â Â Â in the cache, but if the client doesn't have Exclusive Oplock,
> >> Â Â Â Â Â Â Â it writes the data to the server.
> >> + Ârwpidforward ÂForward pid of a process who opened a file to any read or write
> >> + Â Â Â Â Â Â operation on that file. This revent application like WINE
> > Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ^^^^^^
> > Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â"prevents applications"
>
> Ok, thanks, will fix it.
>
> >
> >> + Â Â Â Â Â Â from failing on read and write if we use mandatory brlock style.
> >>  Âacl    ÂAllow setfacl and getfacl to manage posix ACLs if server
> >> Â Â Â Â Â Â Â supports them. Â(default)
> >>  Ânoacl   ÂDo not allow setfacl and getfacl calls on this mount
> >
> > [...]
> >
> >> @@ -1112,6 +1119,7 @@ static int cifs_writepages(struct address_space *mapping,
> >> Â Â Â struct pagevec pvec;
> >> Â Â Â int rc = 0;
> >> Â Â Â int scanned = 0;
> >> + Â Â __u32 netpid;
> >> Â Â Â int xid;
> >>
> >> Â Â Â cifs_sb = CIFS_SB(mapping->host->i_sb);
> >> @@ -1251,10 +1259,15 @@ retry_write:
> >> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â cERROR(1, "No writable handles for inode");
> >> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â rc = -EBADF;
> >> Â Â Â Â Â Â Â Â Â Â Â } else {
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (cifs_sb->mnt_cifs_flags &
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â CIFS_MOUNT_RWPIDFORWARD)
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â netpid = open_file->pid;
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â else
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â netpid = current->tgid;
> >> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â rc = CIFSSMBWrite2(xid, tcon, open_file->netfid,
> >> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âbytes_to_write, offset,
> >> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â&bytes_written, iov, n_iov,
> >> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â0);
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Ânetpid, bytes_to_write,
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âoffset, &bytes_written,
> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âiov, n_iov, 0);
> >> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â cifsFileInfo_put(open_file);
> >> Â Â Â Â Â Â Â Â Â Â Â }
> >>
> > Â Â Â Â Â Â Â Â^^^^^^^^^^^^^^^
> > When flushing data for writeback (writepage or writepages), I don't
> > think we should care if a lock is held or by whom. The kernel has
> > memory that needs to be freed and it wants to perform writes to make
> > that happen.
> >
> > I don't think these codepaths ought to care about the RWPIDFORWARD flag
> > at all. I think you ought to try to make sure that writes in those
> > cases use whatever pid will allow the writes to succeed.
>
> Ok, we have an oplock, set brlock and do write - it succeed (stored in
> cache). Then we close a file, do flushing, and failes (because of
> wrong pid). Why not to forward a pid in this situation?
>
I think we should do whatever is necessary for that write to succeed.
If that means forwarding the pid then that's fine. I just don't see why
the RWPIDFORWARD flag should matter at all in that case.
--
Jeff Layton <jlayton@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
