On 12/6/2023 12:31 AM, Shyam Prasad N wrote:
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.
Lease keys are tied to the _filename_, including the full path. They
are basically guid's, and are used as lookup keys in the lease list,
from which the lease itself is the resulting value. The value is then
inspected for a match to the filename for which it was created.
They are not about the _handle_, which is what is apparently being
assumed here.
MS-SMB2 section 3.3.5.9.8 says this, which the server in question is
correctly enforcing [emphasis added]:
The server MUST attempt to locate a Lease by performing a lookup in the LeaseTable.LeaseList
using the LeaseKey in the SMB2_CREATE_REQUEST_LEASE as the lookup key. If a lease is found,
Lease.FileDeleteOnClose is FALSE, and *Lease.Filename does not match the file name* for the
incoming request, the request MUST be failed with STATUS_INVALID_PARAMETER
IOW, hard links are, from a protocol leasing standpoint, not the
same "file".
Tom.
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?
