Re: [PATCH] cifs: Reuse file lease key in compound operations

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

 



On Wed, Dec 6, 2023 at 1:02 AM Paulo Alcantara <pc@xxxxxxxxxxxxx> wrote:
>
> meetakshisetiyaoss@xxxxxxxxx writes:
>
> > From: Meetakshi Setiya <msetiya@xxxxxxxxxxxxx>
> >
> > Lock contention during unlink operation causes cifs lease break ack
> > worker thread to block and delay sending lease break acks to server.
> > This case occurs when multiple threads perform unlink, write and lease
> > break acks on the same file. Thhis patch fixes the problem by reusing
> > the existing lease keys for rename, unlink and set path size compound
> > operations so that the client does not break its own lease.
> >
> > Signed-off-by: Meetakshi Setiya <msetiya@xxxxxxxxxxxxx>
> > ---
> >  fs/smb/client/cifsglob.h  |  6 ++---
> >  fs/smb/client/cifsproto.h |  8 +++----
> >  fs/smb/client/cifssmb.c   |  6 ++---
> >  fs/smb/client/inode.c     | 12 +++++-----
> >  fs/smb/client/smb2inode.c | 49 +++++++++++++++++++++++++--------------
> >  fs/smb/client/smb2proto.h |  8 +++----
> >  6 files changed, 51 insertions(+), 38 deletions(-)
>
> NAK.  This patch broke some xfstests.
>
> Consider this reproducer:
>
> $ cat repro.sh
> #!/bin/sh
>
> umount /mnt/1 &>/dev/null
> mount.cifs //srv/share /mnt/1 -o ...,vers=3.1.1
> rm /mnt/1/* &>/dev/null
> pushd /mnt/1 >/dev/null
> touch foo
> ln -v foo bar
> rm -v bar
> popd >/dev/null
> umount /mnt/1 &>/dev/null
> $ ./repro.sh
> 'bar' => 'foo'
> rm: cannot remove 'bar': Invalid argument
>
> This is what going on
>
> - client creates 'foo' with RHW lease granted.
> - client creates hardlink file 'bar'.
>
> At this point, we have two positive dentries (foo & bar) which share
> same inode.
>
> - The client then attempts to remove 'bar' by re-using lease key from
> 'foo' through compound request CREATE(DELETE)+CLOSE, which fails with
> STATUS_INVALID_PARAMETER.

That's interesting. I'm assuming that the STATUS_INVALID_PARAMETER is
due to the server not recognizing the lease id for the file bar.
I'm not sure that this is a client bug.
If the server supports hard links, then should it not be aware that
foo and bar are the same files? AFAIK, file lease is associated with a
file, and not the dentry.
Meetakshi, can you please follow the repro steps provided by Paulo
(against Windows file server) and check why the invalid parameter
error is being returned?

-- 
Regards,
Shyam





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

  Powered by Linux