Re: [PATCH] CIFS: Add rwpidforward mount option (try #2)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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?

-- 
Best regards,
Pavel Shilovsky.
--
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


[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux