[PATCHES] file_inode() and ->f_mapping cleanups

[PATCHES] file_inode() and ->f_mapping cleanups

[Al Viro]

	Another whack-a-mole pile - open-coding file_inode()
and file->f_mapping.  All of them are independent from each
other; this stuff sits in vfs.git #work.file_inode, but
if maintainers of an affected subsystems would prefer to have
some of that in their trees - just say so.

[PATCH 1/8] ibmvmc: don't open-code file_inode()

[Al Viro]

badly, at that...

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 drivers/misc/ibmvmc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/ibmvmc.c b/drivers/misc/ibmvmc.c
index c0fe3295c330..cbaf6d35e854 100644
--- a/drivers/misc/ibmvmc.c
+++ b/drivers/misc/ibmvmc.c
@@ -1039,6 +1039,7 @@ static unsigned int ibmvmc_poll(struct file *file, poll_table *wait)
 static ssize_t ibmvmc_write(struct file *file, const char *buffer,
 			    size_t count, loff_t *ppos)
 {
+	struct inode *inode;
 	struct ibmvmc_buffer *vmc_buffer;
 	struct ibmvmc_file_session *session;
 	struct crq_server_adapter *adapter;
@@ -1122,8 +1123,9 @@ static ssize_t ibmvmc_write(struct file *file, const char *buffer,
 	if (p == buffer)
 		goto out;
 
-	file->f_path.dentry->d_inode->i_mtime = current_time(file_inode(file));
-	mark_inode_dirty(file->f_path.dentry->d_inode);
+	inode = file_inode(file);
+	inode->i_mtime = current_time(inode);
+	mark_inode_dirty(inode);
 
 	dev_dbg(adapter->dev, "write: file = 0x%lx, count = 0x%lx\n",
 		(unsigned long)file, (unsigned long)count);
-- 
2.30.2

[PATCH 2/8] exfat_iterate(): don't open-code file_inode(file)

[Al Viro]

and it's file, not filp...

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/exfat/dir.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c
index a27b55ec060a..0fc08fdcba73 100644
--- a/fs/exfat/dir.c
+++ b/fs/exfat/dir.c
@@ -212,9 +212,9 @@ static void exfat_free_namebuf(struct exfat_dentry_namebuf *nb)
 
 /* skip iterating emit_dots when dir is empty */
 #define ITER_POS_FILLED_DOTS    (2)
-static int exfat_iterate(struct file *filp, struct dir_context *ctx)
+static int exfat_iterate(struct file *file, struct dir_context *ctx)
 {
-	struct inode *inode = filp->f_path.dentry->d_inode;
+	struct inode *inode = file_inode(file);
 	struct super_block *sb = inode->i_sb;
 	struct inode *tmp;
 	struct exfat_dir_entry de;
@@ -228,7 +228,7 @@ static int exfat_iterate(struct file *filp, struct dir_context *ctx)
 	mutex_lock(&EXFAT_SB(sb)->s_lock);
 
 	cpos = ctx->pos;
-	if (!dir_emit_dots(filp, ctx))
+	if (!dir_emit_dots(file, ctx))
 		goto unlock;
 
 	if (ctx->pos == ITER_POS_FILLED_DOTS) {
-- 
2.30.2

[PATCH 3/8] sgx: use ->f_mapping...

[Al Viro]

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 arch/x86/kernel/cpu/sgx/encl.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c
index 24c1bb8eb196..6de17468ca16 100644
--- a/arch/x86/kernel/cpu/sgx/encl.c
+++ b/arch/x86/kernel/cpu/sgx/encl.c
@@ -906,8 +906,7 @@ const cpumask_t *sgx_encl_cpumask(struct sgx_encl *encl)
 static struct page *sgx_encl_get_backing_page(struct sgx_encl *encl,
 					      pgoff_t index)
 {
-	struct inode *inode = encl->backing->f_path.dentry->d_inode;
-	struct address_space *mapping = inode->i_mapping;
+	struct address_space *mapping = encl->backing->f_mapping;
 	gfp_t gfpmask = mapping_gfp_mask(mapping);
 
 	return shmem_read_mapping_page_gfp(mapping, index, gfpmask);
-- 
2.30.2

Re: [PATCH 3/8] sgx: use ->f_mapping...

[Jarkko Sakkinen]

On Sat, Aug 20, 2022 at 09:16:01PM +0100, Al Viro wrote:
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
>  arch/x86/kernel/cpu/sgx/encl.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c
> index 24c1bb8eb196..6de17468ca16 100644
> --- a/arch/x86/kernel/cpu/sgx/encl.c
> +++ b/arch/x86/kernel/cpu/sgx/encl.c
> @@ -906,8 +906,7 @@ const cpumask_t *sgx_encl_cpumask(struct sgx_encl *encl)
>  static struct page *sgx_encl_get_backing_page(struct sgx_encl *encl,
>  					      pgoff_t index)
>  {
> -	struct inode *inode = encl->backing->f_path.dentry->d_inode;
> -	struct address_space *mapping = inode->i_mapping;
> +	struct address_space *mapping = encl->backing->f_mapping;
>  	gfp_t gfpmask = mapping_gfp_mask(mapping);
>  
>  	return shmem_read_mapping_page_gfp(mapping, index, gfpmask);
> -- 
> 2.30.2
> 

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

BR, Jarkko

[PATCH 4/8] bprm_fill_uid(): don't open-code file_inode()

[Al Viro]

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/exec.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index f793221f4eb6..c1867122204a 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1595,7 +1595,7 @@ static void bprm_fill_uid(struct linux_binprm *bprm, struct file *file)
 {
 	/* Handle suid and sgid on files */
 	struct user_namespace *mnt_userns;
-	struct inode *inode;
+	struct inode *inode = file_inode(file);
 	unsigned int mode;
 	kuid_t uid;
 	kgid_t gid;
@@ -1606,7 +1606,6 @@ static void bprm_fill_uid(struct linux_binprm *bprm, struct file *file)
 	if (task_no_new_privs(current))
 		return;
 
-	inode = file->f_path.dentry->d_inode;
 	mode = READ_ONCE(inode->i_mode);
 	if (!(mode & (S_ISUID|S_ISGID)))
 		return;
-- 
2.30.2

[PATCH 5/8] nfs_finish_open(): don't open-code file_inode()

[Al Viro]

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/nfs/dir.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index dbab3caa15ed..bcb2500c49b8 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -2022,7 +2022,7 @@ static int nfs_finish_open(struct nfs_open_context *ctx,
 	err = finish_open(file, dentry, do_open);
 	if (err)
 		goto out;
-	if (S_ISREG(file->f_path.dentry->d_inode->i_mode))
+	if (S_ISREG(file_inode(file)->i_mode))
 		nfs_file_set_open_context(file, ctx);
 	else
 		err = -EOPENSTALE;
-- 
2.30.2

[PATCH 6/8] dma_buf: no need to bother with file_inode()->i_mapping

[Al Viro]

->f_mapping will do just fine

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 drivers/dma-buf/udmabuf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
index 38e8767ec371..210473d927d8 100644
--- a/drivers/dma-buf/udmabuf.c
+++ b/drivers/dma-buf/udmabuf.c
@@ -210,7 +210,7 @@ static long udmabuf_create(struct miscdevice *device,
 		memfd = fget(list[i].memfd);
 		if (!memfd)
 			goto err;
-		mapping = file_inode(memfd)->i_mapping;
+		mapping = memfd->f_mapping;
 		if (!shmem_mapping(mapping) && !is_file_hugepages(memfd))
 			goto err;
 		seals = memfd_fcntl(memfd, F_GET_SEALS, 0);
-- 
2.30.2

[PATCH 7/8] _nfs42_proc_copy(): use ->f_mapping instead of file_inode()->i_mapping

[Al Viro]

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/nfs/nfs42proc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
index 068c45b3bc1a..542502199005 100644
--- a/fs/nfs/nfs42proc.c
+++ b/fs/nfs/nfs42proc.c
@@ -336,7 +336,7 @@ static ssize_t _nfs42_proc_copy(struct file *src,
 			return status;
 		}
 	}
-	status = nfs_filemap_write_and_wait_range(file_inode(src)->i_mapping,
+	status = nfs_filemap_write_and_wait_range(src->f_mapping,
 			pos_src, pos_src + (loff_t)count - 1);
 	if (status)
 		return status;
-- 
2.30.2

[PATCH 8/8] orangefs: use ->f_mapping

[Al Viro]

... and don't check for impossible conditions - file_inode() is
never NULL in anything seen by ->release() and neither is its
->i_mapping.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/orangefs/file.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/fs/orangefs/file.c b/fs/orangefs/file.c
index 86810e5d7914..732661aa2680 100644
--- a/fs/orangefs/file.c
+++ b/fs/orangefs/file.c
@@ -417,9 +417,7 @@ static int orangefs_file_release(struct inode *inode, struct file *file)
 	 * readahead cache (if any); this forces an expensive refresh of
 	 * data for the next caller of mmap (or 'get_block' accesses)
 	 */
-	if (file_inode(file) &&
-	    file_inode(file)->i_mapping &&
-	    mapping_nrpages(&file_inode(file)->i_data)) {
+	if (mapping_nrpages(file->f_mapping)) {
 		if (orangefs_features & ORANGEFS_FEATURE_READAHEAD) {
 			gossip_debug(GOSSIP_INODE_DEBUG,
 			    "calling flush_racache on %pU\n",
-- 
2.30.2

Re: [PATCH 8/8] orangefs: use ->f_mapping

[Mike Marshall]

I added this patch to one of the 6.0 rc's that I've been
running through xfstests, no regressions... so...

You can add tested by me if you'd like...

Thanks!

-Mike


On Sat, Aug 20, 2022 at 4:20 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> ... and don't check for impossible conditions - file_inode() is
> never NULL in anything seen by ->release() and neither is its
> ->i_mapping.
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
>  fs/orangefs/file.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/fs/orangefs/file.c b/fs/orangefs/file.c
> index 86810e5d7914..732661aa2680 100644
> --- a/fs/orangefs/file.c
> +++ b/fs/orangefs/file.c
> @@ -417,9 +417,7 @@ static int orangefs_file_release(struct inode *inode, struct file *file)
>          * readahead cache (if any); this forces an expensive refresh of
>          * data for the next caller of mmap (or 'get_block' accesses)
>          */
> -       if (file_inode(file) &&
> -           file_inode(file)->i_mapping &&
> -           mapping_nrpages(&file_inode(file)->i_data)) {
> +       if (mapping_nrpages(file->f_mapping)) {
>                 if (orangefs_features & ORANGEFS_FEATURE_READAHEAD) {
>                         gossip_debug(GOSSIP_INODE_DEBUG,
>                             "calling flush_racache on %pU\n",
> --
> 2.30.2
>

Re: [PATCHES] file_inode() and ->f_mapping cleanups

[Christian Brauner]

On Sat, Aug 20, 2022 at 09:12:36PM +0100, Al Viro wrote:
> 	Another whack-a-mole pile - open-coding file_inode()
> and file->f_mapping.  All of them are independent from each
> other; this stuff sits in vfs.git #work.file_inode, but
> if maintainers of an affected subsystems would prefer to have
> some of that in their trees - just say so.
> 

Oh sweet, I wondered whether I should bother with a series like that a
few weeks ago...

Reviewed-by: Christian Brauner (Microsoft) <brauner@kernel.org>