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

[yangyun <yangyun50@huawei.com>]

On Mon, Aug 26, 2024 at 09:24:22PM +0200, Miklos Szeredi wrote:
> On Sat, 24 Aug 2024 at 11:26, yangyun <yangyun50@huawei.com> 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?
You are right. Since flag __GFP_NOFAIL would block the system, especailly in 
the memeory reclaim situaion. The preallocating of this struct is necessary
because it will be exactly used in the inode eviction path. I'm sorry for not 
considering the risk of __GFP_NOFAIL seriously as a beginner.

Thanks for the reminder. Update soon.
> 
> Thanks,
> Miklos