lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240827093426.3397154-1-yangyun50@huawei.com>
Date: Tue, 27 Aug 2024 17:34:26 +0800
From: yangyun <yangyun50@...wei.com>
To: <miklos@...redi.hu>
CC: <josef@...icpanda.com>, <linux-fsdevel@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <lixiaokeng@...wei.com>,
	<yangyun50@...wei.com>
Subject: Re: [PATCH v2 1/2] fuse: move fuse_forget_link allocation inside fuse_queue_forget()

On Mon, Aug 26, 2024 at 09:24:22PM +0200, Miklos Szeredi wrote:
> On Sat, 24 Aug 2024 at 11:26, yangyun <yangyun50@...wei.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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ