| Message ID | 20240814112356.112329-1-aleksandr.mikhalitsyn@canonical.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | fuse: use GFP_KERNEL_ACCOUNT for allocations in fuse_dev_alloc | expand |
Hi Alexander, On Wed, Aug 14, 2024 at 01:23:56PM GMT, Alexander Mikhalitsyn wrote: > fuse_dev_alloc() is called from the process context and it makes > sense to properly account allocated memory to the kmemcg as these > allocations are for long living objects. > > Link: https://lore.kernel.org/all/20240105152129.196824-3-aleksandr.mikhalitsyn@canonical.com/ > > Cc: Miklos Szeredi <miklos@szeredi.hu> > Cc: Amir Goldstein <amir73il@gmail.com> > Cc: Christian Brauner <brauner@kernel.org> > Cc: Johannes Weiner <hannes@cmpxchg.org> > Cc: Michal Hocko <mhocko@kernel.org> > Cc: Roman Gushchin <roman.gushchin@linux.dev> > Cc: Shakeel Butt <shakeel.butt@linux.dev> > Cc: <linux-fsdevel@vger.kernel.org> > Cc: <linux-kernel@vger.kernel.org> > Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com> > --- > fs/fuse/inode.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c > index ed4c2688047f..6dae007186e1 100644 > --- a/fs/fuse/inode.c > +++ b/fs/fuse/inode.c > @@ -1486,11 +1486,11 @@ struct fuse_dev *fuse_dev_alloc(void) > struct fuse_dev *fud; > struct list_head *pq; > > - fud = kzalloc(sizeof(struct fuse_dev), GFP_KERNEL); > + fud = kzalloc(sizeof(struct fuse_dev), GFP_KERNEL_ACCOUNT); > if (!fud) > return NULL; > > - pq = kcalloc(FUSE_PQ_HASH_SIZE, sizeof(struct list_head), GFP_KERNEL); > + pq = kcalloc(FUSE_PQ_HASH_SIZE, sizeof(struct list_head), GFP_KERNEL_ACCOUNT); > if (!pq) { > kfree(fud); > return NULL; No objection from me but let me ask couple of questions to make sure we know the impact of this change. It seems like this function is called during mount() operation. Is that correct? If yes then basically the admin process or node controller is being charged for this memory. Nothing bad but this info should be in commit message. Also what is the lifetime of these objects? From mount to unmount? Please add that info as well. There are other unaccounted allocations in the fuse fs. Is there a followup plan to include those as well? thanks, Shakeel
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c index ed4c2688047f..6dae007186e1 100644 --- a/fs/fuse/inode.c +++ b/fs/fuse/inode.c @@ -1486,11 +1486,11 @@ struct fuse_dev *fuse_dev_alloc(void) struct fuse_dev *fud; struct list_head *pq; - fud = kzalloc(sizeof(struct fuse_dev), GFP_KERNEL); + fud = kzalloc(sizeof(struct fuse_dev), GFP_KERNEL_ACCOUNT); if (!fud) return NULL; - pq = kcalloc(FUSE_PQ_HASH_SIZE, sizeof(struct list_head), GFP_KERNEL); + pq = kcalloc(FUSE_PQ_HASH_SIZE, sizeof(struct list_head), GFP_KERNEL_ACCOUNT); if (!pq) { kfree(fud); return NULL;
fuse_dev_alloc() is called from the process context and it makes sense to properly account allocated memory to the kmemcg as these allocations are for long living objects. Link: https://lore.kernel.org/all/20240105152129.196824-3-aleksandr.mikhalitsyn@canonical.com/ Cc: Miklos Szeredi <miklos@szeredi.hu> Cc: Amir Goldstein <amir73il@gmail.com> Cc: Christian Brauner <brauner@kernel.org> Cc: Johannes Weiner <hannes@cmpxchg.org> Cc: Michal Hocko <mhocko@kernel.org> Cc: Roman Gushchin <roman.gushchin@linux.dev> Cc: Shakeel Butt <shakeel.butt@linux.dev> Cc: <linux-fsdevel@vger.kernel.org> Cc: <linux-kernel@vger.kernel.org> Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com> --- fs/fuse/inode.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)