Hi Laura, On Fri, Apr 11 2025, Laura Promberger wrote: > Hello Miklos, Luis, > > I tested Luis NOTIFY_INC_EPOCH patch (kernel, libfuse, cvmfs) on RHEL9 and can > confirm that in combination with your fix to the symlink truncate it solves all > the problem we had with cvmfs when applying a new revision and at the same time > hammering a symlink with readlink() that would change its target. ( > https://github.com/cvmfs/cvmfs/issues/3626 ) > > With those two patches we no longer end up with corrupted symlinks or get stuck on an old revision. > (old revision was possible because the kernel started caching the old one again during the update due to the high access rate and the asynchronous evict of inodes) > > As such we would be very happy if this patch could be accepted. Even though this patch and the one that fixed the symlinks corruption [1] aren't really related, it's always good to have extra testing. Thanks a lot for your help, Laura. In the meantime, I hope to send a refreshed v9 of this patch soon (maybe today) as it doesn't apply cleanly to current master anymore. And I also plan to send v2 of the (RFC) patch that adds the workqueue to clean-up expired cache entries. [1] That's commit b4c173dfbb6c ("fuse: don't truncate cached, mutated symlink"), which has been merged already. Cheers, -- Luís > > Have a nice weekend > Laura > > > ________________________________________ > From: Luis Henriques <luis@xxxxxxxxxx> > Sent: Monday, March 17, 2025 12:28 > To: Miklos Szeredi <miklos@xxxxxxxxxx> > Cc: Laura Promberger <laura.promberger@xxxxxxx>; Bernd Schubert > <bschubert@xxxxxxx>; Dave Chinner <david@xxxxxxxxxxxxx>; Matt Harvey > <mharvey@xxxxxxxxxxxxxxx>; linux-fsdevel@xxxxxxxxxxxxxxx > <linux-fsdevel@xxxxxxxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx > <linux-kernel@xxxxxxxxxxxxxxx> > Subject: Re: [PATCH v8] fuse: add more control over cache invalidation behaviour > > Hi Miklos, > > [ adding Laura to CC, something I should have done before ] > > On Mon, Mar 10 2025, Miklos Szeredi wrote: > >> On Fri, 7 Mar 2025 at 16:31, Luis Henriques <luis@xxxxxxxxxx> wrote: >> >>> Any further feedback on this patch, or is it already OK for being merged? >> >> The patch looks okay. I have ideas about improving the name, but that can wait. >> >> What I think is still needed is an actual use case with performance numbers. > > As requested, I've run some tests on CVMFS using this kernel patch[1]. > For reference, I'm also sharing the changes I've done to libfuse[2] and > CVMFS[3] in order to use this new FUSE operation. The changes to these > two repositories are in a branch named 'wip-notify-inc-epoch'. > > As for the details, basically what I've done was to hack the CVMFS loop in > FuseInvalidator::MainInvalidator() so that it would do a single call to > the libfuse operation fuse_lowlevel_notify_increment_epoch() instead of > cycling through the inodes list. The CVMFS patch is ugly, it just > short-circuiting the loop, but I didn't want to spend any more time with > it at this stage. The real patch will be slightly more complex in order > to deal with both approaches, in case the NOTIFY_INC_EPOCH isn't > available. > > Anyway, my test environment was a small VM, where I have two scenarios: a > small file-system with just a few inodes, and a larger one with around > 8000 inodes. The test approach was to simply mount the filesystem, load > the caches with 'find /mnt' and force a flush using the cvmfs_swissknife > tool, with the 'ingest' command. > > [ Disclosure: my test environment actually uses a fork of upstream cvmfs, > but for the purposes of these tests that shouldn't really make any > difference. ] > > The numbers in the table below represent the average time (tests were run > 100 times) it takes to run the MainInvalidator() function. As expected, > using the NOTIFY_INC_EPOCH is much faster, as it's a single operation, a > single call into FUSE. Using the NOTIFY_INVAL_* is much more expensive -- > it requires calling into the kernel several times, depending on the number > of inodes on the list. > > |------------------+------------------+----------------| > | | small filesystem | "big" fs | > | | (~20 inodes) | (~8000 inodes) | > |------------------+------------------+----------------| > | NOTIFY_INVAL_* | 330 us | 4300 us | > | NOTIFY_INC_EPOCH | 40 us | 45 us | > |------------------+------------------+----------------| > > Hopefully these results help answering Miklos questions regarding the > cvmfs use-case. > > [1] https://lore.kernel.org/all/20250226091451.11899-1-luis@xxxxxxxxxx/ > [2] https://github.com/luis-henrix/libfuse > [3] https://github.com/luis-henrix/cvmfs > > Cheers, > -- > Luís