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