diff mbox series

[2/3] fs: improve naming for fsid helpers

Message ID 20210315145419.2612537-3-christian.brauner@ubuntu.com (mailing list archive)
State New
Headers show
Series tweak idmap helpers | expand

Commit Message

Christian Brauner March 15, 2021, 2:54 p.m. UTC
Vivek pointed out that the current naming scheme can be misleading as it
conflicts with some of the other helpers naming. So get rid of the
confusion by simply calling those helpers idmapped_fs{u,g}id() that
make it very clear that and idmapped fsuid/fsgid is used. xfs needs to
use them directly in the quota allocation codepaths.

Inspired-by: Vivek Goyal <vgoyal@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Darrick J. Wong <djwong@kernel.org>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: linux-fsdevel@vger.kernel.org
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
 fs/xfs/xfs_inode.c   | 8 ++++----
 fs/xfs/xfs_symlink.c | 4 ++--
 include/linux/fs.h   | 4 ++--
 3 files changed, 8 insertions(+), 8 deletions(-)

Comments

Christoph Hellwig March 18, 2021, 6:27 a.m. UTC | #1
> -static inline kuid_t fsuid_into_mnt(struct user_namespace *mnt_userns)
> +static inline kuid_t idmapped_fsuid(struct user_namespace *mnt_userns)
>  {
>  	return kuid_from_mnt(mnt_userns, current_fsuid());
>  }
>  
> -static inline kgid_t fsgid_into_mnt(struct user_namespace *mnt_userns)
> +static inline kgid_t idmapped_fsgid(struct user_namespace *mnt_userns)
>  {
>  	return kgid_from_mnt(mnt_userns, current_fsgid());
>  }

I'm not sure the naming is an improvement.  I always think of
identity mapped when reading it, which couldn't be further from what
it does..  But either way comments describing what these helpers do
would be very useful.
Al Viro March 18, 2021, 6:30 a.m. UTC | #2
On Thu, Mar 18, 2021 at 07:27:23AM +0100, Christoph Hellwig wrote:
> > -static inline kuid_t fsuid_into_mnt(struct user_namespace *mnt_userns)
> > +static inline kuid_t idmapped_fsuid(struct user_namespace *mnt_userns)
> >  {
> >  	return kuid_from_mnt(mnt_userns, current_fsuid());
> >  }
> >  
> > -static inline kgid_t fsgid_into_mnt(struct user_namespace *mnt_userns)
> > +static inline kgid_t idmapped_fsgid(struct user_namespace *mnt_userns)
> >  {
> >  	return kgid_from_mnt(mnt_userns, current_fsgid());
> >  }
> 
> I'm not sure the naming is an improvement.  I always think of
> identity mapped when reading it, which couldn't be further from what
> it does..  But either way comments describing what these helpers do
> would be very useful.

s/idmapped/mapped/?
Christian Brauner March 18, 2021, 11:40 a.m. UTC | #3
On Thu, Mar 18, 2021 at 06:30:50AM +0000, Al Viro wrote:
> On Thu, Mar 18, 2021 at 07:27:23AM +0100, Christoph Hellwig wrote:
> > > -static inline kuid_t fsuid_into_mnt(struct user_namespace *mnt_userns)
> > > +static inline kuid_t idmapped_fsuid(struct user_namespace *mnt_userns)
> > >  {
> > >  	return kuid_from_mnt(mnt_userns, current_fsuid());
> > >  }
> > >  
> > > -static inline kgid_t fsgid_into_mnt(struct user_namespace *mnt_userns)
> > > +static inline kgid_t idmapped_fsgid(struct user_namespace *mnt_userns)
> > >  {
> > >  	return kgid_from_mnt(mnt_userns, current_fsgid());
> > >  }
> > 
> > I'm not sure the naming is an improvement.  I always think of
> > identity mapped when reading it, which couldn't be further from what
> > it does..  But either way comments describing what these helpers do
> > would be very useful.
> 
> s/idmapped/mapped/?

Yeah, I think that's a good idea.

I'll also add comments.
Christian Brauner March 18, 2021, 11:40 a.m. UTC | #4
On Thu, Mar 18, 2021 at 07:27:23AM +0100, Christoph Hellwig wrote:
> > -static inline kuid_t fsuid_into_mnt(struct user_namespace *mnt_userns)
> > +static inline kuid_t idmapped_fsuid(struct user_namespace *mnt_userns)
> >  {
> >  	return kuid_from_mnt(mnt_userns, current_fsuid());
> >  }
> >  
> > -static inline kgid_t fsgid_into_mnt(struct user_namespace *mnt_userns)
> > +static inline kgid_t idmapped_fsgid(struct user_namespace *mnt_userns)
> >  {
> >  	return kgid_from_mnt(mnt_userns, current_fsgid());
> >  }
> 
> I'm not sure the naming is an improvement.  I always think of
> identity mapped when reading it, which couldn't be further from what
> it does..  But either way comments describing what these helpers do
> would be very useful.

Good point. I'll add comments!
diff mbox series

Patch

diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index f93370bd7b1e..8703408bd1aa 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1007,8 +1007,8 @@  xfs_create(
 	/*
 	 * Make sure that we have allocated dquot(s) on disk.
 	 */
-	error = xfs_qm_vop_dqalloc(dp, fsuid_into_mnt(mnt_userns),
-			fsgid_into_mnt(mnt_userns), prid,
+	error = xfs_qm_vop_dqalloc(dp, idmapped_fsuid(mnt_userns),
+			idmapped_fsgid(mnt_userns), prid,
 			XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT,
 			&udqp, &gdqp, &pdqp);
 	if (error)
@@ -1158,8 +1158,8 @@  xfs_create_tmpfile(
 	/*
 	 * Make sure that we have allocated dquot(s) on disk.
 	 */
-	error = xfs_qm_vop_dqalloc(dp, fsuid_into_mnt(mnt_userns),
-			fsgid_into_mnt(mnt_userns), prid,
+	error = xfs_qm_vop_dqalloc(dp, idmapped_fsuid(mnt_userns),
+			idmapped_fsgid(mnt_userns), prid,
 			XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT,
 			&udqp, &gdqp, &pdqp);
 	if (error)
diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c
index 7f368b10ded1..669e8517e2e1 100644
--- a/fs/xfs/xfs_symlink.c
+++ b/fs/xfs/xfs_symlink.c
@@ -182,8 +182,8 @@  xfs_symlink(
 	/*
 	 * Make sure that we have allocated dquot(s) on disk.
 	 */
-	error = xfs_qm_vop_dqalloc(dp, fsuid_into_mnt(mnt_userns),
-			fsgid_into_mnt(mnt_userns), prid,
+	error = xfs_qm_vop_dqalloc(dp, idmapped_fsuid(mnt_userns),
+			idmapped_fsgid(mnt_userns), prid,
 			XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT,
 			&udqp, &gdqp, &pdqp);
 	if (error)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index edcb1aa99fd6..189673721726 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1610,12 +1610,12 @@  static inline kgid_t kgid_from_mnt(struct user_namespace *mnt_userns,
 	return KGIDT_INIT(from_kgid(mnt_userns, kgid));
 }
 
-static inline kuid_t fsuid_into_mnt(struct user_namespace *mnt_userns)
+static inline kuid_t idmapped_fsuid(struct user_namespace *mnt_userns)
 {
 	return kuid_from_mnt(mnt_userns, current_fsuid());
 }
 
-static inline kgid_t fsgid_into_mnt(struct user_namespace *mnt_userns)
+static inline kgid_t idmapped_fsgid(struct user_namespace *mnt_userns)
 {
 	return kgid_from_mnt(mnt_userns, current_fsgid());
 }