Linux CIFS filesystem development
 help / color / mirror / Atom feed
* Directory Leases
@ 2024-10-26 13:13 Ralph Boehme
  2024-10-27  3:24 ` Steve French
  0 siblings, 1 reply; 7+ messages in thread
From: Ralph Boehme @ 2024-10-26 13:13 UTC (permalink / raw)
  To: linux-cifs

Hi!

I have implemented Directory Leases in Samba and wanted to do a few test 
with real world clients.

Mounting with

# mount.cifs //localhost/test /mnt/test/ -o 
username=slow,password=x,handlecache

(no idea if "handlecache" is needed, found it by skimming the code)

I don't see any usage of Directory Leases on the wire when I do an ls.

Do I need to pass any additional mount options? Anything else?

Kernel version is 6.11.3-200.fc40.x86_64.

Thanks!
-slow

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Directory Leases
  2024-10-26 13:13 Directory Leases Ralph Boehme
@ 2024-10-27  3:24 ` Steve French
  2024-10-27 14:16   ` Ralph Boehme
  0 siblings, 1 reply; 7+ messages in thread
From: Steve French @ 2024-10-27  3:24 UTC (permalink / raw)
  To: Ralph Boehme; +Cc: linux-cifs, Meetakshi Setiya

I built and installed Samba with your recent directory lease series
and tried some experiments with cifs.ko to it and I do see directory
lease requested by the client and held for 30 seconds for the
directory (or directories) that I do ls on, but I don't see cifs.ko
using the cached directory contents when multiple ls on the same
directory are attempted within 30 seconds of one another.   One theory
is that this has to do with some compounding changes a few months ago
- but I want to investigate more - will try some experiments (repeated
ls, looking to see how directory cached contents are being used) to
Windows from cifs.ko as well.

From the debugging and traces I looked at tonight it does look
possible that the client has a perf regression in not reusing cached
directory contents when we have a directory lease.  Will update when I
narrow it down more.

On Sat, Oct 26, 2024 at 8:25 AM Ralph Boehme <slow@samba.org> wrote:
>
> Hi!
>
> I have implemented Directory Leases in Samba and wanted to do a few test
> with real world clients.
>
> Mounting with
>
> # mount.cifs //localhost/test /mnt/test/ -o
> username=slow,password=x,handlecache
>
> (no idea if "handlecache" is needed, found it by skimming the code)
>
> I don't see any usage of Directory Leases on the wire when I do an ls.
>
> Do I need to pass any additional mount options? Anything else?
>
> Kernel version is 6.11.3-200.fc40.x86_64.
>
> Thanks!
> -slow
>


-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Directory Leases
  2024-10-27  3:24 ` Steve French
@ 2024-10-27 14:16   ` Ralph Boehme
  2024-10-28 21:11     ` Steve French
  0 siblings, 1 reply; 7+ messages in thread
From: Ralph Boehme @ 2024-10-27 14:16 UTC (permalink / raw)
  To: Steve French; +Cc: linux-cifs, Meetakshi Setiya

On 10/27/24 4:24 AM, Steve French wrote:
> I built and installed Samba with your recent directory lease series
> and tried some experiments with cifs.ko to it and I do see directory
> lease requested by the client and held for 30 seconds for the
> directory (or directories) that I do ls on,...

hm, guess I was not looking close enough, I rechecked and now I can see 
the client requesting directory leases and the server granting them.

Two things seem odd:

- the client only requests a READ lease without a HANDLE lease,

- the client opens a directory with R lease, does a query-info on it and 
then opens the directory a second time, without lease, and uses that 
second handle for the directory listing.

In my understanding a directory lease without H lease is useless, as it 
limits lifetime of the cache to the lifetime of the handle and you can't 
defer the close on the directory handle without a H lease.

Cf the presentation "SMB2.2 Advancements for WAN" from SDC 2011 page 20:

"Without H leases, the R lease is of no value."

open_cached_dir() seems to be the function requesting the directory 
lease and it requests SMB2_OPLOCK_LEVEL_II which is mapped to 
SMB2_LEASE_READ_CACHING_LE.

Thanks!
-slow

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Directory Leases
  2024-10-27 14:16   ` Ralph Boehme
@ 2024-10-28 21:11     ` Steve French
  2024-10-29 10:05       ` Ralph Boehme
  0 siblings, 1 reply; 7+ messages in thread
From: Steve French @ 2024-10-28 21:11 UTC (permalink / raw)
  To: Ralph Boehme; +Cc: linux-cifs, Meetakshi Setiya, ronnie sahlberg

Doing some additional experiments to Windows and also to the updated
Samba branch from Ralph, I see the directory lease request, and
I see that after ls (which will cache the directory contents for about
30 second) we do get a big benefit from the metadata of the directory
entries being cached e.g. "ls /mnt ; sleep 10; stat /mnt/file ; sleep
15 stat /mnt/file2 ; sleep 10 /mnt/file"  - we only get the roundtrips
for the initial ls - the stat calls don't cause any network traffic
since the directory is cached.

> the client opens a directory with R lease, does a query-info on it and
> then opens the directory a second time, without lease, and uses that
> second handle for the directory listing.

We get two calls from the kernel here for that "ls"
That query info is for "revalidate" and the second roundtrip is for
the readdir, but
it does look like a bug in the querydir not reusing the handle.

On Sun, Oct 27, 2024 at 9:16 AM Ralph Boehme <slow@samba.org> wrote:
>
> On 10/27/24 4:24 AM, Steve French wrote:
> > I built and installed Samba with your recent directory lease series
> > and tried some experiments with cifs.ko to it and I do see directory
> > lease requested by the client and held for 30 seconds for the
> > directory (or directories) that I do ls on,...
>
> hm, guess I was not looking close enough, I rechecked and now I can see
> the client requesting directory leases and the server granting them.
>
> Two things seem odd:
>
> - the client only requests a READ lease without a HANDLE lease,
>
> - the client opens a directory with R lease, does a query-info on it and
> then opens the directory a second time, without lease, and uses that
> second handle for the directory listing.
>
> In my understanding a directory lease without H lease is useless, as it
> limits lifetime of the cache to the lifetime of the handle and you can't
> defer the close on the directory handle without a H lease.
>
> Cf the presentation "SMB2.2 Advancements for WAN" from SDC 2011 page 20:
>
> "Without H leases, the R lease is of no value."
>
> open_cached_dir() seems to be the function requesting the directory
> lease and it requests SMB2_OPLOCK_LEVEL_II which is mapped to
> SMB2_LEASE_READ_CACHING_LE.
>
> Thanks!
> -slow



-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Directory Leases
  2024-10-28 21:11     ` Steve French
@ 2024-10-29 10:05       ` Ralph Boehme
       [not found]         ` <CAH2r5mtkuCihp9hRp16RSyV=g0xcPyYuUBbBipBAtdw_CbiTKQ@mail.gmail.com>
  2024-11-19  2:51         ` Steve French
  0 siblings, 2 replies; 7+ messages in thread
From: Ralph Boehme @ 2024-10-29 10:05 UTC (permalink / raw)
  To: Steve French; +Cc: linux-cifs, Meetakshi Setiya, ronnie sahlberg

Hi Steve,

On 10/28/24 10:11 PM, Steve French wrote:
> Doing some additional experiments to Windows and also to the updated
> Samba branch from Ralph, I see the directory lease request, and
> I see that after ls (which will cache the directory contents for about
> 30 second) we do get a big benefit from the metadata of the directory
> entries being cached e.g. "ls /mnt ; sleep 10; stat /mnt/file ; sleep
> 15 stat /mnt/file2 ; sleep 10 /mnt/file"  - we only get the roundtrips
> for the initial ls - the stat calls don't cause any network traffic
> since the directory is cached.
indeed, I can confirm that some cache is used for stat. Unfortunately it 
isn't used for readddir.

Also, coming back on the issue that the client is deferring a close on 
the directory with having a H lease:

In my understanding that's at least going to cause problems if other 
clients want to do anything on the server that is not allowed if there 
are conflicting opens like renaming a directory (which is not allowed if 
there are any opens below recursively). Unlinks will also be deferred as 
long as the client sticks to its handle.

The client should acquire a RH lease on directories if it wants to cache 
the handle and that's a prerequisite in order to cache readdir.

Afair the kernel is currently caching for 30 seconds. Increasing this 
time should not be done without also having a H lease.

-slow

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Fwd: Directory Leases
       [not found]         ` <CAH2r5mtkuCihp9hRp16RSyV=g0xcPyYuUBbBipBAtdw_CbiTKQ@mail.gmail.com>
@ 2024-11-02  6:29           ` Steve French
  0 siblings, 0 replies; 7+ messages in thread
From: Steve French @ 2024-11-02  6:29 UTC (permalink / raw)
  To: CIFS

Ralph,
Looking at the problem you noted with multiple directory listings (ie
'readdir') for the same directory not being served out of the
directory cache on the Linux client (as the 'stat' calls after
'readdir' will be) - I noticed that in cifs_readdir() in
fs/smb/client/readdir.c we are failing here - dirents.is_valid is not
set.  I want to dig into this more in the next few days and fix that
(and the missing 'H' in the lease request)

        /*
         * If we already have the entire directory cached then
         * we can just serve the cache.
         */
        if (cfid->dirents.is_valid) {
...

On Tue, Oct 29, 2024 at 5:05 AM Ralph Boehme <slow@samba.org> wrote:
>
> Hi Steve,
>
> On 10/28/24 10:11 PM, Steve French wrote:
> > Doing some additional experiments to Windows and also to the updated
> > Samba branch from Ralph, I see the directory lease request, and
> > I see that after ls (which will cache the directory contents for about
> > 30 second) we do get a big benefit from the metadata of the directory
> > entries being cached e.g. "ls /mnt ; sleep 10; stat /mnt/file ; sleep
> > 15 stat /mnt/file2 ; sleep 10 /mnt/file"  - we only get the roundtrips
> > for the initial ls - the stat calls don't cause any network traffic
> > since the directory is cached.
> indeed, I can confirm that some cache is used for stat. Unfortunately it
> isn't used for readddir.
>
> Also, coming back on the issue that the client is deferring a close on
> the directory with having a H lease:
>
> In my understanding that's at least going to cause problems if other
> clients want to do anything on the server that is not allowed if there
> are conflicting opens like renaming a directory (which is not allowed if
> there are any opens below recursively). Unlinks will also be deferred as
> long as the client sticks to its handle.
>
> The client should acquire a RH lease on directories if it wants to cache
> the handle and that's a prerequisite in order to cache readdir.
>
> Afair the kernel is currently caching for 30 seconds. Increasing this
> time should not be done without also having a H lease.
>
> -slow



-- 
Thanks,

Steve


-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Directory Leases
  2024-10-29 10:05       ` Ralph Boehme
       [not found]         ` <CAH2r5mtkuCihp9hRp16RSyV=g0xcPyYuUBbBipBAtdw_CbiTKQ@mail.gmail.com>
@ 2024-11-19  2:51         ` Steve French
  1 sibling, 0 replies; 7+ messages in thread
From: Steve French @ 2024-11-19  2:51 UTC (permalink / raw)
  To: Ralph Boehme; +Cc: linux-cifs, Meetakshi Setiya, ronnie sahlberg

[-- Attachment #1: Type: text/plain, Size: 1871 bytes --]

Here is a one line patch to make sure we request HANDLE leases on
cached directories.
I also want to make sure (in followon patch) that we check in the open
response (ie the lease
state granted) that we don't defer close of directory if we don't get
both HANDLE and READ
in response.


On Tue, Oct 29, 2024 at 5:05 AM Ralph Boehme <slow@samba.org> wrote:
>
> Hi Steve,
>
> On 10/28/24 10:11 PM, Steve French wrote:
> > Doing some additional experiments to Windows and also to the updated
> > Samba branch from Ralph, I see the directory lease request, and
> > I see that after ls (which will cache the directory contents for about
> > 30 second) we do get a big benefit from the metadata of the directory
> > entries being cached e.g. "ls /mnt ; sleep 10; stat /mnt/file ; sleep
> > 15 stat /mnt/file2 ; sleep 10 /mnt/file"  - we only get the roundtrips
> > for the initial ls - the stat calls don't cause any network traffic
> > since the directory is cached.
> indeed, I can confirm that some cache is used for stat. Unfortunately it
> isn't used for readddir.
>
> Also, coming back on the issue that the client is deferring a close on
> the directory with having a H lease:
>
> In my understanding that's at least going to cause problems if other
> clients want to do anything on the server that is not allowed if there
> are conflicting opens like renaming a directory (which is not allowed if
> there are any opens below recursively). Unlinks will also be deferred as
> long as the client sticks to its handle.
>
> The client should acquire a RH lease on directories if it wants to cache
> the handle and that's a prerequisite in order to cache readdir.
>
> Afair the kernel is currently caching for 30 seconds. Increasing this
> time should not be done without also having a H lease.
>
> -slow



--
Thanks,

Steve

[-- Attachment #2: 0001-smb3-request-handle-caching-when-caching-directories.patch --]
[-- Type: application/x-patch, Size: 1361 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2024-11-19  2:51 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-26 13:13 Directory Leases Ralph Boehme
2024-10-27  3:24 ` Steve French
2024-10-27 14:16   ` Ralph Boehme
2024-10-28 21:11     ` Steve French
2024-10-29 10:05       ` Ralph Boehme
     [not found]         ` <CAH2r5mtkuCihp9hRp16RSyV=g0xcPyYuUBbBipBAtdw_CbiTKQ@mail.gmail.com>
2024-11-02  6:29           ` Fwd: " Steve French
2024-11-19  2:51         ` Steve French

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox