From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 90787C43334 for ; Tue, 21 Jun 2022 10:20:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1344495AbiFUKUd (ORCPT ); Tue, 21 Jun 2022 06:20:33 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53302 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235564AbiFUKUb (ORCPT ); Tue, 21 Jun 2022 06:20:31 -0400 Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1E34B28712 for ; Tue, 21 Jun 2022 03:20:30 -0700 (PDT) Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id C914E21C4B; Tue, 21 Jun 2022 10:20:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1655806828; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=I6r4e52RQP0Q1y6nki9OqLdXE7jND5ciqzenalCNd0c=; b=qnuyGsW3MkjNjRm7hNdHMdpRzJlTKIgvqk3ki+jDkivVdBWDZhJhhIBZ+MfgqRmzsZxUuU 8fQKeHiyH0Jmqf1A4ID1+WMzi/r2STAWfar/GuBsYZfb0elL3I7Z+uN7lehkE20EqolnDT 4HoJy3a6N0C4aanm054LKUd9ptp2zuQ= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1655806828; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=I6r4e52RQP0Q1y6nki9OqLdXE7jND5ciqzenalCNd0c=; b=FUVk7Px7qeZN06ZWC47/AB1vBNewqVxK4HCWd0k6kAkzuMoBcUh2PD+tN67MpKmMF48Bno FCJ08vkeTRjW3KDw== Received: from quack3.suse.cz (unknown [10.100.224.230]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id 862F42C141; Tue, 21 Jun 2022 10:20:28 +0000 (UTC) Received: by quack3.suse.cz (Postfix, from userid 1000) id E2DDAA062B; Tue, 21 Jun 2022 12:20:27 +0200 (CEST) Date: Tue, 21 Jun 2022 12:20:27 +0200 From: Jan Kara To: Christian Brauner Cc: Christoph Hellwig , linux-fsdevel@vger.kernel.org, Seth Forshee , Aleksa Sarai , Linus Torvalds , Al Viro , Jan Kara Subject: Re: [PATCH 6/8] quota: port quota helpers mount ids Message-ID: <20220621102027.iw6yoo5lrr4oe3m6@quack3.lan> References: <20220620134947.2772863-1-brauner@kernel.org> <20220620134947.2772863-7-brauner@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220620134947.2772863-7-brauner@kernel.org> Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org On Mon 20-06-22 15:49:45, Christian Brauner wrote: > Port the is_quota_modification() and dqout_transfer() helper to type > safe kmnt{g,u}id_t. Since these helpers are only called by a few > filesystems don't introduce a new helper but simply extend the existing > helpers to pass down the mount's idmapping. > > Note, that this is a non-functional change, i.e. nothing will have > happened here or at the end of this series to how quota are done! This > a change necessary because we will at the end of this series make > ownership changes easier to reason about by keeping the original value > in struct iattr for both non-idmapped and idmapped mounts. > > For now we always pass the initial idmapping which makes the idmapping > functions these helpers call nops. > > This is done because we currently always pass the actual value to be > written to i_{g,u}id via struct iattr. While this allowed us to treat > the {g,u}id values in struct iattr as values that can be directly > written to inode->i_{g,u}id it also increases the potential for > confusion for filesystems. > > Now that we are about to introduce dedicated types to prevent this > confusion we will ultimately only map the value from the idmapped mount > into a filesystem value that can be written to inode->i_{g,u}id when the > filesystem actually updates the inode. So pass down the initial > idmapping until we finished that conversion. > > Since struct iattr uses an anonymous union with overlapping types as > supported by the C filesystems that haven't converted to ia_mnt{g,u}id > won't see any difference and things will continue to work as before. > In other words, no functional changes intended with this change. > > Cc: Seth Forshee > Cc: Christoph Hellwig > Cc: Jan Kara > Cc: Aleksa Sarai > Cc: Linus Torvalds > Cc: Al Viro > CC: linux-fsdevel@vger.kernel.org > Signed-off-by: Christian Brauner (Microsoft) Yeah, this looks fairly innocent. Feel free to add: Reviewed-by: Jan Kara Just when do you plan to make changes actually using the passed namespace? Honza > --- > fs/ext2/inode.c | 4 ++-- > fs/ext4/inode.c | 4 ++-- > fs/f2fs/file.c | 4 ++-- > fs/f2fs/recovery.c | 2 +- > fs/jfs/file.c | 4 ++-- > fs/ocfs2/file.c | 2 +- > fs/quota/dquot.c | 3 ++- > fs/reiserfs/inode.c | 4 ++-- > fs/zonefs/super.c | 2 +- > include/linux/quotaops.h | 9 ++++++--- > 10 files changed, 21 insertions(+), 17 deletions(-) > > diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c > index 6dc66ab97d20..593b79416e0e 100644 > --- a/fs/ext2/inode.c > +++ b/fs/ext2/inode.c > @@ -1679,14 +1679,14 @@ int ext2_setattr(struct user_namespace *mnt_userns, struct dentry *dentry, > if (error) > return error; > > - if (is_quota_modification(inode, iattr)) { > + if (is_quota_modification(&init_user_ns, inode, iattr)) { > error = dquot_initialize(inode); > if (error) > return error; > } > if (i_uid_needs_update(&init_user_ns, iattr, inode) || > i_gid_needs_update(&init_user_ns, iattr, inode)) { > - error = dquot_transfer(inode, iattr); > + error = dquot_transfer(&init_user_ns, inode, iattr); > if (error) > return error; > } > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c > index 05d932f81c53..72f08c184768 100644 > --- a/fs/ext4/inode.c > +++ b/fs/ext4/inode.c > @@ -5350,7 +5350,7 @@ int ext4_setattr(struct user_namespace *mnt_userns, struct dentry *dentry, > if (error) > return error; > > - if (is_quota_modification(inode, attr)) { > + if (is_quota_modification(&init_user_ns, inode, attr)) { > error = dquot_initialize(inode); > if (error) > return error; > @@ -5374,7 +5374,7 @@ int ext4_setattr(struct user_namespace *mnt_userns, struct dentry *dentry, > * counts xattr inode references. > */ > down_read(&EXT4_I(inode)->xattr_sem); > - error = dquot_transfer(inode, attr); > + error = dquot_transfer(&init_user_ns, inode, attr); > up_read(&EXT4_I(inode)->xattr_sem); > > if (error) { > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c > index a35d6b12bd63..02b2d56d4edc 100644 > --- a/fs/f2fs/file.c > +++ b/fs/f2fs/file.c > @@ -915,7 +915,7 @@ int f2fs_setattr(struct user_namespace *mnt_userns, struct dentry *dentry, > if (err) > return err; > > - if (is_quota_modification(inode, attr)) { > + if (is_quota_modification(&init_user_ns, inode, attr)) { > err = f2fs_dquot_initialize(inode); > if (err) > return err; > @@ -923,7 +923,7 @@ int f2fs_setattr(struct user_namespace *mnt_userns, struct dentry *dentry, > if (i_uid_needs_update(&init_user_ns, attr, inode) || > i_gid_needs_update(&init_user_ns, attr, inode)) { > f2fs_lock_op(F2FS_I_SB(inode)); > - err = dquot_transfer(inode, attr); > + err = dquot_transfer(&init_user_ns, inode, attr); > if (err) { > set_sbi_flag(F2FS_I_SB(inode), > SBI_QUOTA_NEED_REPAIR); > diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c > index 3cb7f8a43b4d..8e5a089f1ac8 100644 > --- a/fs/f2fs/recovery.c > +++ b/fs/f2fs/recovery.c > @@ -266,7 +266,7 @@ static int recover_quota_data(struct inode *inode, struct page *page) > if (!attr.ia_valid) > return 0; > > - err = dquot_transfer(inode, &attr); > + err = dquot_transfer(&init_user_ns, inode, &attr); > if (err) > set_sbi_flag(F2FS_I_SB(inode), SBI_QUOTA_NEED_REPAIR); > return err; > diff --git a/fs/jfs/file.c b/fs/jfs/file.c > index 1d732fd223d4..c18569b9895d 100644 > --- a/fs/jfs/file.c > +++ b/fs/jfs/file.c > @@ -95,14 +95,14 @@ int jfs_setattr(struct user_namespace *mnt_userns, struct dentry *dentry, > if (rc) > return rc; > > - if (is_quota_modification(inode, iattr)) { > + if (is_quota_modification(&init_user_ns, inode, iattr)) { > rc = dquot_initialize(inode); > if (rc) > return rc; > } > if ((iattr->ia_valid & ATTR_UID && !uid_eq(iattr->ia_uid, inode->i_uid)) || > (iattr->ia_valid & ATTR_GID && !gid_eq(iattr->ia_gid, inode->i_gid))) { > - rc = dquot_transfer(inode, iattr); > + rc = dquot_transfer(&init_user_ns, inode, iattr); > if (rc) > return rc; > } > diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c > index 7497cd592258..0e09cd8911da 100644 > --- a/fs/ocfs2/file.c > +++ b/fs/ocfs2/file.c > @@ -1146,7 +1146,7 @@ int ocfs2_setattr(struct user_namespace *mnt_userns, struct dentry *dentry, > if (status) > return status; > > - if (is_quota_modification(inode, attr)) { > + if (is_quota_modification(&init_user_ns, inode, attr)) { > status = dquot_initialize(inode); > if (status) > return status; > diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c > index 6cec2bfbf51b..df9af1ce2851 100644 > --- a/fs/quota/dquot.c > +++ b/fs/quota/dquot.c > @@ -2085,7 +2085,8 @@ EXPORT_SYMBOL(__dquot_transfer); > /* Wrapper for transferring ownership of an inode for uid/gid only > * Called from FSXXX_setattr() > */ > -int dquot_transfer(struct inode *inode, struct iattr *iattr) > +int dquot_transfer(struct user_namespace *mnt_userns, struct inode *inode, > + struct iattr *iattr) > { > struct dquot *transfer_to[MAXQUOTAS] = {}; > struct dquot *dquot; > diff --git a/fs/reiserfs/inode.c b/fs/reiserfs/inode.c > index 0cffe054b78e..1e89e76972a0 100644 > --- a/fs/reiserfs/inode.c > +++ b/fs/reiserfs/inode.c > @@ -3284,7 +3284,7 @@ int reiserfs_setattr(struct user_namespace *mnt_userns, struct dentry *dentry, > /* must be turned off for recursive notify_change calls */ > ia_valid = attr->ia_valid &= ~(ATTR_KILL_SUID|ATTR_KILL_SGID); > > - if (is_quota_modification(inode, attr)) { > + if (is_quota_modification(&init_user_ns, inode, attr)) { > error = dquot_initialize(inode); > if (error) > return error; > @@ -3367,7 +3367,7 @@ int reiserfs_setattr(struct user_namespace *mnt_userns, struct dentry *dentry, > reiserfs_write_unlock(inode->i_sb); > if (error) > goto out; > - error = dquot_transfer(inode, attr); > + error = dquot_transfer(&init_user_ns, inode, attr); > reiserfs_write_lock(inode->i_sb); > if (error) { > journal_end(&th); > diff --git a/fs/zonefs/super.c b/fs/zonefs/super.c > index 053299758deb..dd422b1d7320 100644 > --- a/fs/zonefs/super.c > +++ b/fs/zonefs/super.c > @@ -616,7 +616,7 @@ static int zonefs_inode_setattr(struct user_namespace *mnt_userns, > !uid_eq(iattr->ia_uid, inode->i_uid)) || > ((iattr->ia_valid & ATTR_GID) && > !gid_eq(iattr->ia_gid, inode->i_gid))) { > - ret = dquot_transfer(inode, iattr); > + ret = dquot_transfer(&init_user_ns, inode, iattr); > if (ret) > return ret; > } > diff --git a/include/linux/quotaops.h b/include/linux/quotaops.h > index 61ee34861ca2..0342ff6584fd 100644 > --- a/include/linux/quotaops.h > +++ b/include/linux/quotaops.h > @@ -20,7 +20,8 @@ static inline struct quota_info *sb_dqopt(struct super_block *sb) > } > > /* i_mutex must being held */ > -static inline bool is_quota_modification(struct inode *inode, struct iattr *ia) > +static inline bool is_quota_modification(struct user_namespace *mnt_userns, > + struct inode *inode, struct iattr *ia) > { > return ((ia->ia_valid & ATTR_SIZE) || > i_uid_needs_update(&init_user_ns, ia, inode) || > @@ -115,7 +116,8 @@ int dquot_set_dqblk(struct super_block *sb, struct kqid id, > struct qc_dqblk *di); > > int __dquot_transfer(struct inode *inode, struct dquot **transfer_to); > -int dquot_transfer(struct inode *inode, struct iattr *iattr); > +int dquot_transfer(struct user_namespace *mnt_userns, struct inode *inode, > + struct iattr *iattr); > > static inline struct mem_dqinfo *sb_dqinfo(struct super_block *sb, int type) > { > @@ -234,7 +236,8 @@ static inline void dquot_free_inode(struct inode *inode) > { > } > > -static inline int dquot_transfer(struct inode *inode, struct iattr *iattr) > +static inline int dquot_transfer(struct user_namespace *mnt_userns, > + struct inode *inode, struct iattr *iattr) > { > return 0; > } > -- > 2.34.1 > -- Jan Kara SUSE Labs, CR