2024-01-05 15:21:59

by Aleksandr Mikhalitsyn

[permalink] [raw]
Subject: [PATCH v1 0/3] fuse: a few small fixes

Found by chance while working on support for idmapped mounts in fuse
(will send series soon :-)).

Cc: Miklos Szeredi <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
Signed-off-by: Alexander Mikhalitsyn <[email protected]>

Alexander Mikhalitsyn (3):
fuse: fix typo for fuse_permission comment
fuse: use GFP_KERNEL_ACCOUNT for allocations in fuse_dev_alloc
fuse: __kuid_val/__kgid_val helpers in fuse_fill_attr_from_inode()

fs/fuse/dir.c | 2 +-
fs/fuse/inode.c | 8 ++++----
2 files changed, 5 insertions(+), 5 deletions(-)

--
2.34.1



2024-01-05 15:22:27

by Aleksandr Mikhalitsyn

[permalink] [raw]
Subject: [PATCH v1 1/3] fuse: fix typo for fuse_permission comment

Found by chance while working on support for idmapped mounts in fuse.

Cc: Miklos Szeredi <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
Signed-off-by: Alexander Mikhalitsyn <[email protected]>
---
fs/fuse/dir.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index d19cbf34c634..6f5f9ff95380 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -1485,7 +1485,7 @@ static int fuse_perm_getattr(struct inode *inode, int mask)
*
* 1) Local access checking ('default_permissions' mount option) based
* on file mode. This is the plain old disk filesystem permission
- * modell.
+ * model.
*
* 2) "Remote" access checking, where server is responsible for
* checking permission in each inode operation. An exception to this
--
2.34.1


2024-01-05 15:22:36

by Aleksandr Mikhalitsyn

[permalink] [raw]
Subject: [PATCH v1 2/3] fuse: use GFP_KERNEL_ACCOUNT for allocations in fuse_dev_alloc

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 <[email protected]>
Cc: Amir Goldstein <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
Signed-off-by: Alexander Mikhalitsyn <[email protected]>
---
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 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;
--
2.34.1


2024-01-05 15:22:52

by Aleksandr Mikhalitsyn

[permalink] [raw]
Subject: [PATCH v1 3/3] fuse: __kuid_val/__kgid_val helpers in fuse_fill_attr_from_inode()

For the sake of consistency, let's use these helpers to extract
{u,g}id_t values from k{u,g}id_t ones.

There are no functional changes, just to make code cleaner.

Cc: Christian Brauner <[email protected]>
Cc: Miklos Szeredi <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
Signed-off-by: Alexander Mikhalitsyn <[email protected]>
---
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 b8636b5e79dc..ab824a8908b7 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1489,8 +1489,8 @@ static void fuse_fill_attr_from_inode(struct fuse_attr *attr,
.ctimensec = ctime.tv_nsec,
.mode = fi->inode.i_mode,
.nlink = fi->inode.i_nlink,
- .uid = fi->inode.i_uid.val,
- .gid = fi->inode.i_gid.val,
+ .uid = __kuid_val(fi->inode.i_uid),
+ .gid = __kgid_val(fi->inode.i_gid),
.rdev = fi->inode.i_rdev,
.blksize = 1u << fi->inode.i_blkbits,
};
--
2.34.1


2024-01-08 11:37:22

by Christian Brauner

[permalink] [raw]
Subject: Re: [PATCH v1 3/3] fuse: __kuid_val/__kgid_val helpers in fuse_fill_attr_from_inode()

On Fri, Jan 05, 2024 at 04:21:29PM +0100, Alexander Mikhalitsyn wrote:
> For the sake of consistency, let's use these helpers to extract
> {u,g}id_t values from k{u,g}id_t ones.
>
> There are no functional changes, just to make code cleaner.
>
> Cc: Christian Brauner <[email protected]>
> Cc: Miklos Szeredi <[email protected]>
> Cc: <[email protected]>
> Cc: <[email protected]>
> Signed-off-by: Alexander Mikhalitsyn <[email protected]>
> ---

Looks good to me,
Reviewed-by: Christian Brauner <[email protected]>

2024-01-29 12:04:42

by Aleksandr Mikhalitsyn

[permalink] [raw]
Subject: Re: [PATCH v1 0/3] fuse: a few small fixes

Gentle ping.

On Fri, Jan 5, 2024 at 4:21 PM Alexander Mikhalitsyn
<[email protected]> wrote:
>
> Found by chance while working on support for idmapped mounts in fuse
> (will send series soon :-)).
>
> Cc: Miklos Szeredi <[email protected]>
> Cc: <[email protected]>
> Cc: <[email protected]>
> Signed-off-by: Alexander Mikhalitsyn <[email protected]>
>
> Alexander Mikhalitsyn (3):
> fuse: fix typo for fuse_permission comment
> fuse: use GFP_KERNEL_ACCOUNT for allocations in fuse_dev_alloc
> fuse: __kuid_val/__kgid_val helpers in fuse_fill_attr_from_inode()
>
> fs/fuse/dir.c | 2 +-
> fs/fuse/inode.c | 8 ++++----
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> --
> 2.34.1
>

2024-01-29 12:15:56

by Aleksandr Mikhalitsyn

[permalink] [raw]
Subject: Re: [PATCH v1 3/3] fuse: __kuid_val/__kgid_val helpers in fuse_fill_attr_from_inode()

On Mon, 8 Jan 2024 12:37:07 +0100
Christian Brauner <[email protected]> wrote:

> On Fri, Jan 05, 2024 at 04:21:29PM +0100, Alexander Mikhalitsyn wrote:
> > For the sake of consistency, let's use these helpers to extract
> > {u,g}id_t values from k{u,g}id_t ones.
> >
> > There are no functional changes, just to make code cleaner.
> >
> > Cc: Christian Brauner <[email protected]>
> > Cc: Miklos Szeredi <[email protected]>
> > Cc: <[email protected]>
> > Cc: <[email protected]>
> > Signed-off-by: Alexander Mikhalitsyn <[email protected]>
> > ---
>
> Looks good to me,
> Reviewed-by: Christian Brauner <[email protected]>

Thanks!

Kind regards,
Alex

2024-02-26 11:36:26

by Miklos Szeredi

[permalink] [raw]
Subject: Re: [PATCH v1 2/3] fuse: use GFP_KERNEL_ACCOUNT for allocations in fuse_dev_alloc

On Fri, 5 Jan 2024 at 16:21, Alexander Mikhalitsyn
<[email protected]> 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

2024-03-05 14:32:20

by Miklos Szeredi

[permalink] [raw]
Subject: Re: [PATCH v1 1/3] fuse: fix typo for fuse_permission comment

On Fri, 5 Jan 2024 at 16:21, Alexander Mikhalitsyn
<[email protected]> wrote:
>
> Found by chance while working on support for idmapped mounts in fuse.
>
> Cc: Miklos Szeredi <[email protected]>
> Cc: <[email protected]>
> Cc: <[email protected]>
> Signed-off-by: Alexander Mikhalitsyn <[email protected]>

Applied, thanks.

2024-03-05 14:33:17

by Miklos Szeredi

[permalink] [raw]
Subject: Re: [PATCH v1 3/3] fuse: __kuid_val/__kgid_val helpers in fuse_fill_attr_from_inode()

On Fri, 5 Jan 2024 at 16:22, Alexander Mikhalitsyn
<[email protected]> wrote:
>
> For the sake of consistency, let's use these helpers to extract
> {u,g}id_t values from k{u,g}id_t ones.
>
> There are no functional changes, just to make code cleaner.
>
> Cc: Christian Brauner <[email protected]>
> Cc: Miklos Szeredi <[email protected]>
> Cc: <[email protected]>
> Cc: <[email protected]>
> Signed-off-by: Alexander Mikhalitsyn <[email protected]>

Applied, thanks.

Miklos