On Fri, Dec 29, 2023 at 9:13 PM Paulo Alcantara <pc@xxxxxxxxxxxxx> wrote: > > meetakshisetiyaoss@xxxxxxxxx writes: > > > From: Meetakshi Setiya <msetiya@xxxxxxxxxxxxx> > > > > There is a shortcoming in the current implementation of the file > > lease mechanism exposed when the lease keys were attempted to be reused > > for unlink, rename and set_path_size operations for a client. As per > > MS-SMB2, lease keys are associated with the file name. Linux cifs client > > maintains lease keys with the inode. If the file has any hardlinks, > > it is possible that the lease for a file be wrongly reused for an > > operation on the hardlink or vice versa. In these cases, the mentioned > > compound operations fail with STATUS_INVALID_PARAMETER. > > This patch adds a fallback to the old mechanism of not sending any > > lease with these compound operations if the request with lease key fails > > with STATUS_INVALID_PARAMETER. Resending the same request without lease > > key should not hurt any functionality, but might impact performance > > especially in cases where the error is not because of the usage of wrong > > lease key and we might end up doing an extra roundtrip. > > What's the problem with checking ->i_nlink to decide whether reusing > lease key? As per the discussion with Tom on the previous version of the changes, I conferred with Shyam and Steve about possible workarounds and this seemed like a choice which did the job without much perf drawbacks and code changes. One highlighted difference between the two could be that in the previous version, lease would not be reused for any file with hardlinks at all, even though the inode may hold the correct lease for that particular file. The current changes would take care of this by sending the lease at least once, irrespective of the number of hardlinks. Thanks Meetakshi
