Re: [PATCH v2 1/2] fuse: move fuse_forget_link allocation inside fuse_queue_forget()

From: Miklos Szeredi
Date: Mon Aug 26 2024 - 15:24:46 EST


On Sat, 24 Aug 2024 at 11:26, yangyun <yangyun50@xxxxxxxxxx> wrote:
>
> The `struct fuse_forget_link` is allocated outside `fuse_queue_forget()`
> before this patch. This requires the allocation in advance. In some
> cases, this struct is not needed but allocated, which contributes to
> memory usage and performance degradation. Besides, this messes up the
> code to some extent. So move the `fuse_forget_link` allocation inside
> fuse_queue_forget with __GFP_NOFAIL.
>
> `fuse_force_forget()` is used by `readdirplus` before this patch for
> the reason that we do not know how many 'fuse_forget_link' structures
> will be allocated in advance when error happens. After this patch, this
> function is not needed any more and can be removed. By this way, all
> FUSE_FORGET requests are sent by using `fuse_queue_forget()` function as
> e.g. virtiofs handles them differently from regular requests.

The patch is nice and clean. However, I'm a bit worried about the
inode eviction path, which can be triggered from memory reclaim.
Allocating a small structure shouldn't be an issue, yet I feel that
the old way of preallocating it on inode creation should be better.

What do you think?

Thanks,
Miklos