Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 19 Apr 2022 15:59:38 +0200
From:   Christian Brauner <brauner@kernel.org>
To:     Yang Xu <xuyang2018.jy@fujitsu.com>
Cc:     linux-fsdevel@vger.kernel.org, ceph-devel@vger.kernel.org,
        linux-nfs@vger.kernel.org, linux-xfs@vger.kernel.org,
        viro@zeniv.linux.org.uk, david@fromorbit.com, djwong@kernel.org,
        jlayton@kernel.org, ntfs3@lists.linux.dev, chao@kernel.org,
        linux-f2fs-devel@lists.sourceforge.net
Subject: Re: [PATCH v4 4/8] NFSv3: only do posix_acl_create under
 CONFIG_NFS_V3_ACL
Message-ID: <20220419135938.flv776f36v6xb6sj@wittgenstein>
References: <1650368834-2420-1-git-send-email-xuyang2018.jy@fujitsu.com>
 <1650368834-2420-4-git-send-email-xuyang2018.jy@fujitsu.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <1650368834-2420-4-git-send-email-xuyang2018.jy@fujitsu.com>
Precedence: bulk

On Tue, Apr 19, 2022 at 07:47:10PM +0800, Yang Xu wrote:
> Since nfs3_proc_create/nfs3_proc_mkdir/nfs3_proc_mknod these rpc ops are called
> by nfs_create/nfs_mkdir/nfs_mkdir these inode ops, so they are all in control of
> vfs.
> 
> nfs3_proc_setacls does nothing in the !CONFIG_NFS_V3_ACL case, so we put
> posix_acl_create under CONFIG_NFS_V3_ACL and it also doesn't affect
> sattr->ia_mode value because vfs has did umask strip.
> 
> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
> ---

I have the same comment as on the xfs patch. If the filesystem has opted
out of acls and SB_POSIXACL isn't set in sb->s_flags then
posix_acl_create() is a nop. Why bother placing it under an ifdef?

It adds visual noise and it implies that posix_acl_create() actually
does something even if the filesystem doesn't support posix acls.

Unless this actually fixes something I'd drop this patch too.