From: Tom Haynes <thomas.haynes@primarydata.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Jens Axboe <axboe@fb.com>,
Trond Myklebust <trond.myklebust@primarydata.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [GIT PULL] Backing device changes for 3.20
Date: Thu, 12 Feb 2015 15:53:55 -0800 [thread overview]
Message-ID: <20150212235355.GB38603@kitty.kitty> (raw)
In-Reply-To: <CA+55aFxnisiCLB0pHWU7dmzXWBG16gPB9cEycGRUgB+RcbJGnA@mail.gmail.com>
On Thu, Feb 12, 2015 at 02:05:00PM -0800, Linus Torvalds wrote:
> On Thu, Feb 12, 2015 at 12:38 PM, Jens Axboe <axboe@fb.com> wrote:
> >
> > This pull request contains a cleanup of how the backing device is
> > handled, in preparation for a rework of the life time rules. In this
> > part, the most important change is to split the unrelated nommu mmap
> > flags from it, but also removing a backing_dev_info pointer from the
> > address_space (and inode), and a cleanup of other various minor bits.
>
> Ugh, so this has a semantic conflict with the NFS client, that has
> this particular code:
>
> if (!cinfo->dreq) {
> inc_zone_page_state(req->wb_page, NR_UNSTABLE_NFS);
> - inc_bdi_stat(page_file_mapping(req->wb_page)->backing_dev_info,
> +
> inc_bdi_stat(inode_to_bdi(page_file_mapping(req->wb_page)->host),
> BDI_RECLAIMABLE);
> __mark_inode_dirty(req->wb_context->dentry->d_inode,
> I_DIRTY_DATASYNC);
> }
>
> duplicated several times, and now one more time in the new
> fs/nfs/flexfilelayout/flexfilelayout.c file.
>
> I fixed it the same way it was fixed everywhere else, but while fixing
> and looking at the cases, I *really* feel like the nfs code needs some
> cleaning up.
>
> That insane complicated and unexplained code exists three times: in
> filelayout/filelayout.c, flexfilelayout/flexfilelayout.c and in
> write.c.
>
> The "reverse" case (which does the decrements, and doesn't mark the
> inode dirty) exists a few more times.
>
> Could we make that a helper function, with a few comments.
Yes, I'll make a helper function.
And for the "reverse" case, the second instance can actually call
nfs_clear_page_commit().
> For example, looking at it, I wonder if
>
> - page_file_mapping(req->wb_page)->host
> - req->wb_context->dentry->d_inode
>
> are the same inode?
Will resolve...
>
> Hmm?
>
> Linus
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
prev parent reply other threads:[~2015-02-12 23:53 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-02-12 20:38 [GIT PULL] Backing device changes for 3.20 Jens Axboe
2015-02-12 22:05 ` Linus Torvalds
2015-02-12 22:37 ` Stephen Rothwell
2015-02-12 23:53 ` Tom Haynes [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150212235355.GB38603@kitty.kitty \
--to=thomas.haynes@primarydata.com \
--cc=axboe@fb.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nfs@vger.kernel.org \
--cc=torvalds@linux-foundation.org \
--cc=trond.myklebust@primarydata.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.