| Message ID | 20240105152129.196824-3-aleksandr.mikhalitsyn@canonical.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fuse: a few small fixes | expand |
On Fri, 5 Jan 2024 at 16:21, Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com> 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. Are the rules about when to use __GFP_ACCOUNT and when not documented somewhere? I notice that most filesystem objects are allocated with __GFP_ACCOUNT, but struct super_block isn't. Is there a reason for that? Thanks, Miklos
On Mon, Feb 26, 2024 at 12:01 PM Miklos Szeredi <miklos@szeredi.hu> wrote: > > On Fri, 5 Jan 2024 at 16:21, Alexander Mikhalitsyn > <aleksandr.mikhalitsyn@canonical.com> 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. Hi Miklos, Sorry, this thread just got lost in my inbox. I was revisiting and rebasing fuse idmapped mounts support series and found this again. > > Are the rules about when to use __GFP_ACCOUNT and when not documented somewhere? The only doc I found is this (memory-allocation.rst): >Untrusted allocations triggered from userspace should be a subject >of kmem accounting and must have ``__GFP_ACCOUNT`` bit set. > > I notice that most filesystem objects are allocated with > __GFP_ACCOUNT, but struct super_block isn't. Is there a reason for > that? I guess that it just wasn't yet covered with memcg accounting. I can send a patch to account struct super_block too. These days, it's pretty safe to use __GFP_ACCOUNT almost anywhere, because even if memcg is not determined in a current caller context then memcg charge will be skipped (look into __memcg_slab_post_alloc_hook() function). Let's ask what our friends who take care of mmcontrol.c think about this. +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> I have also added Christian because he might be interested in accounting for struct super_block. Kind regards, Alex > > Thanks, > Miklos
On Thu, 18 Jul 2024 at 12:01, Aleksandr Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com> wrote: > I have also added Christian because he might be interested in > accounting for struct super_block. IMO doing it in the VFS as well makes much more sense than just the fuse specific parts. Thanks, Miklos
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c index 2a6d44f91729..b8636b5e79dc 100644 --- a/fs/fuse/inode.c +++ b/fs/fuse/inode.c @@ -1415,11 +1415,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. Cc: Miklos Szeredi <miklos@szeredi.hu> Cc: Amir Goldstein <amir73il@gmail.com> 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(-)