On Mon, 18 Apr 2011 16:58:09 +0400 Pavel Shilovsky <[email protected]> wrote:
> 2011/4/18 Jeff Layton <[email protected]>: > > On Mon, 18 Apr 2011 16:20:52 +0400 > > Pavel Shilovsky <[email protected]> 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 <[email protected]> > >> --- > >> 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 <[email protected]> -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
