All of lore.kernel.org
 help / color / mirror / Atom feed
From: Luis Henriques <luis@igalia.com>
To: Laura Promberger <laura.promberger@cern.ch>
Cc: Miklos Szeredi <miklos@szeredi.hu>,
	 Bernd Schubert <bschubert@ddn.com>,
	Dave Chinner <david@fromorbit.com>,
	 Matt Harvey <mharvey@jumptrading.com>,
	 "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
	 "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v8] fuse: add more control over cache invalidation behaviour
Date: Tue, 15 Apr 2025 11:34:38 +0100	[thread overview]
Message-ID: <87r01tn269.fsf@igalia.com> (raw)
In-Reply-To: <GV0P278MB07182F4A1BDFD2506E2F58AC85B62@GV0P278MB0718.CHEP278.PROD.OUTLOOK.COM> (Laura Promberger's message of "Fri, 11 Apr 2025 15:16:53 +0000")

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@igalia.com>
> Sent: Monday, March 17, 2025 12:28
> To: Miklos Szeredi <miklos@szeredi.hu>
> Cc: Laura Promberger <laura.promberger@cern.ch>; Bernd Schubert
> <bschubert@ddn.com>; Dave Chinner <david@fromorbit.com>; Matt Harvey
> <mharvey@jumptrading.com>; linux-fsdevel@vger.kernel.org
> <linux-fsdevel@vger.kernel.org>; linux-kernel@vger.kernel.org
> <linux-kernel@vger.kernel.org>
> 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@igalia.com> 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@igalia.com/
> [2] https://github.com/luis-henrix/libfuse
> [3] https://github.com/luis-henrix/cvmfs
>
> Cheers,
> --
> Luís


  reply	other threads:[~2025-04-15 10:34 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-26  9:14 [PATCH v8] fuse: add more control over cache invalidation behaviour Luis Henriques
2025-03-07 15:30 ` Luis Henriques
2025-03-10 16:42   ` Miklos Szeredi
2025-03-10 20:11     ` Bernd Schubert
2025-03-13 10:24       ` Miklos Szeredi
2025-03-11 11:08     ` Luis Henriques
2025-03-13 10:32       ` Miklos Szeredi
2025-03-13 11:25         ` Luis Henriques
2025-03-13 11:39           ` Miklos Szeredi
2025-03-13 12:11             ` Luis Henriques
2025-03-17 11:28     ` Luis Henriques
2025-04-11 15:14       ` Laura Promberger
2025-04-11 15:16       ` Laura Promberger
2025-04-15 10:34         ` Luis Henriques [this message]
2025-04-15 10:41           ` Miklos Szeredi
2025-04-15 10:49             ` Luis Henriques

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87r01tn269.fsf@igalia.com \
    --to=luis@igalia.com \
    --cc=bschubert@ddn.com \
    --cc=david@fromorbit.com \
    --cc=laura.promberger@cern.ch \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mharvey@jumptrading.com \
    --cc=miklos@szeredi.hu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.