From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qt0-f170.google.com ([209.85.216.170]:36488 "EHLO mail-qt0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751256AbdCZKDP (ORCPT ); Sun, 26 Mar 2017 06:03:15 -0400 Received: by mail-qt0-f170.google.com with SMTP id r45so18726330qte.3 for ; Sun, 26 Mar 2017 03:03:14 -0700 (PDT) Message-ID: <1490522591.2698.4.camel@redhat.com> Subject: Re: [PATCH] fs/open.c: use ->f_mapping instead of ->f_mapping->host->i_mapping From: Jeff Layton To: Eric Biggers , linux-fsdevel@vger.kernel.org Cc: Alexander Viro , Eric Biggers , David Howells Date: Sun, 26 Mar 2017 06:03:11 -0400 In-Reply-To: <20170326032128.32721-1-ebiggers3@gmail.com> References: <20170326032128.32721-1-ebiggers3@gmail.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Sat, 2017-03-25 at 20:21 -0700, Eric Biggers wrote: > From: Eric Biggers > > 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 > --- > 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