[PATCH] fs/open.c: use ->f_mapping instead of ->f_mapping->host->i_mapping

[PATCH] fs/open.c: use ->f_mapping instead of ->f_mapping->host->i_mapping

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

In do_dentry_open(), initialize the readahead state using ->f_mapping
instead of ->f_mapping->host->i_mapping.  This is equivalent, even for
block device files; we don't need the extra indirection because
->f_mapping already represents the data read/written by the file (which
for block device nodes is the underlying block device mapping).

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/open.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/open.c b/fs/open.c
index 949cef29c3bb..32e19fda24d2 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -763,7 +763,7 @@ static int do_dentry_open(struct file *f,
 
 	f->f_flags &= ~(O_CREAT | O_EXCL | O_NOCTTY | O_TRUNC);
 
-	file_ra_state_init(&f->f_ra, f->f_mapping->host->i_mapping);
+	file_ra_state_init(&f->f_ra, f->f_mapping);
 
 	return 0;
 
-- 
2.12.1

Re: [PATCH] fs/open.c: use ->f_mapping instead of ->f_mapping->host->i_mapping

[Jeff Layton]

On Sat, 2017-03-25 at 20:21 -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> In do_dentry_open(), initialize the readahead state using ->f_mapping
> instead of ->f_mapping->host->i_mapping.  This is equivalent, even for
> block device files; we don't need the extra indirection because
> ->f_mapping already represents the data read/written by the file (which
> for block device nodes is the underlying block device mapping).
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
>  fs/open.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/open.c b/fs/open.c
> index 949cef29c3bb..32e19fda24d2 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -763,7 +763,7 @@ static int do_dentry_open(struct file *f,
>  
>  	f->f_flags &= ~(O_CREAT | O_EXCL | O_NOCTTY | O_TRUNC);
>  
> -	file_ra_state_init(&f->f_ra, f->f_mapping->host->i_mapping);
> +	file_ra_state_init(&f->f_ra, f->f_mapping);
>  
>  	return 0;
> 

(cc'ing David Howells...)

Yeah, that certainly looks circular to me.

I was chatting with David the other day, and I asked him a similar
question: When is f_mapping set to anything else other than what this
would point to?

    filp->f_path.dentry->d_inode->i_mapping

It seems like we might not need f_mapping at all, though David seemed to
think that coda might sometimes set it to something different. Do you
have any insight there?

In any case:

Reviewed-by: Jeff Layton <jlayton@redhat.com>