netdev - Re: [PATCH 08/11] nfs: disable data cache revalidation for swapfiles

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 16 Apr 2012 11:03:35 -0400
From:	Fred Isaman <iisaman@...app.com>
To:	Mel Gorman <mgorman@...e.de>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Linux-MM <linux-mm@...ck.org>,
	Linux-Netdev <netdev@...r.kernel.org>,
	Linux-NFS <linux-nfs@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	David Miller <davem@...emloft.net>,
	Trond Myklebust <Trond.Myklebust@...app.com>,
	Neil Brown <neilb@...e.de>,
	Christoph Hellwig <hch@...radead.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Mike Christie <michaelc@...wisc.edu>,
	Eric B Munson <emunson@...bm.net>
Subject: Re: [PATCH 08/11] nfs: disable data cache revalidation for swapfiles

On Mon, Apr 16, 2012 at 9:44 AM, Mel Gorman <mgorman@...e.de> wrote:
> On Mon, Apr 16, 2012 at 09:10:04AM -0400, Fred Isaman wrote:
>> > <SNIP>
>> > -static struct nfs_page *nfs_page_find_request_locked(struct page *page)
>> > +static struct nfs_page *
>> > +nfs_page_find_request_locked(struct nfs_inode *nfsi, struct page *page)
>> >  {
>> >        struct nfs_page *req = NULL;
>> >
>> > -       if (PagePrivate(page)) {
>> > +       if (PagePrivate(page))
>> >                req = (struct nfs_page *)page_private(page);
>> > -               if (req != NULL)
>> > -                       kref_get(&req->wb_kref);
>> > +       else if (unlikely(PageSwapCache(page))) {
>> > +               struct nfs_page *freq, *t;
>> > +
>> > +               /* Linearly search the commit list for the correct req */
>> > +               list_for_each_entry_safe(freq, t, &nfsi->commit_list, wb_list) {
>> > +                       if (freq->wb_page == page) {
>> > +                               req = freq;
>> > +                               break;
>> > +                       }
>> > +               }
>> > +
>> > +               BUG_ON(req == NULL);
>>
>> I suspect I am missing something, but why is it guaranteed that the
>> req is on the commit list?
>>
>
> It's a fair question and a statement about what I expected to happen.
> The commit list replaces the nfs_page_tree radix tree that used to exist
> and my understanding was that the req would exist in the radix tree until
> the swap IO was completed. I expected it to be the same for the commit
> list and the BUG_ON was based on that expectation. Are there cases where
> the req would not be found?
>
> Thanks.
>
> --
> Mel Gorman
> SUSE Labs
> --

A req is on the commit list only if it actually needs to be scheduled
for COMMIT.  In other words, only after it has been sent via WRITE and
the server did not return NFS_FILE_SYNC.

Thus dirtying a page, then trying to touch it again before the WRITE
is sent will not find the corresponding req on the commit_list.

Fred
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html