On Thu, Sep 12, 2024 at 12:53:40PM GMT, Jan Kara wrote:
> On Thu 12-09-24 17:15:48, Yafang Shao wrote:
> > Commit 681ce8623567 ("vfs: Delete the associated dentry when deleting a
> > file") introduced an unconditional deletion of the associated dentry when a
> > file is removed. However, this led to performance regressions in specific
> > benchmarks, such as ilebench.sum_operations/s [0], prompting a revert in
> > commit 4a4be1ad3a6e ("Revert 'vfs: Delete the associated dentry when
> > deleting a file'").
> >
> > This patch seeks to reintroduce the concept conditionally, where the
> > associated dentry is deleted only when the user explicitly opts for it
> > during file removal.
> >
> > There are practical use cases for this proactive dentry reclamation.
> > Besides the Elasticsearch use case mentioned in commit 681ce8623567,
> > additional examples have surfaced in our production environment. For
> > instance, in video rendering services that continuously generate temporary
> > files, upload them to persistent storage servers, and then delete them, a
> > large number of negative dentries—serving no useful purpose—accumulate.
> > Users in such cases would benefit from proactively reclaiming these
> > negative dentries. This patch provides an API allowing users to actively
> > delete these unnecessary negative dentries.
> >
> > Link: https://lore.kernel.org/linux-fsdevel/202405291318.4dfbb352-oliver.sang@xxxxxxxxx [0]
> > Signed-off-by: Yafang Shao <laoar.shao@xxxxxxxxx>
> > Cc: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
> > Cc: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
> > Cc: Christian Brauner <brauner@xxxxxxxxxx>
> > Cc: Jan Kara <jack@xxxxxxx>
>
> Umm, I don't think we want to burn a FMODE flag and expose these details of
> dentry reclaim to userspace. BTW, if we wanted to do this, we already have
> d_mark_dontcache() for in-kernel users which we could presumably reuse.
>
> But I would not completely give up on trying to handle this in an automated
> way inside the kernel. The original solution you mention above was perhaps
> too aggressive but maybe d_delete() could just mark the dentry with a
> "deleted" flag, retain_dentry() would move such dentries into a special LRU
> list which we'd prune once in a while (say once per 5 s). Also this list
> would be automatically pruned from prune_dcache_sb(). This way if there's
> quick reuse of a dentry, it will get reused and no harm is done, if it
> isn't quickly reused, we'll free them to not waste memory.
>
> What do people think about such scheme?
I agree that a new open flag is not the right way to go and it also
wastes a PF_* flag.
I think it'll probably be rather difficult to ensure that we don't see
performance regressions for some benchmark. Plus there will be users
that want aggressive negative dentry reclaim being fine with possibly
increased lookup cost independent of the directory case.
So imo the simple option is to add 681ce8623567 back behind a sysctl
that users that need aggressive negative dentry reclaim can simply turn on.
