From: Boaz Harrosh <ooo@electrozaur.com>
To: Hugh Dickins <hughd@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Trond Myklebust <trond.myklebust@primarydata.com>,
Christoph Lameter <cl@linux.com>,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-nfs@vger.kernel.org, linux-mm@kvack.org,
osd-dev@open-osd.org
Subject: Re: [PATCH] osd fs: __r4w_get_page rely on PageUptodate for uptodate
Date: Mon, 02 Nov 2015 13:05:32 +0200 [thread overview]
Message-ID: <5637437C.4070306@electrozaur.com> (raw)
In-Reply-To: <alpine.LSU.2.11.1511011513240.11427@eggly.anvils>
On 11/02/2015 01:39 AM, Hugh Dickins wrote:
<>
>> This patch is not correct!
>
> I think you have actually confirmed that the patch is correct:
> why bother to test PageDirty or PageWriteback when PageUptodate
> already tells you what you need?
>
> Or do these filesystems do something unusual with PageUptodate
> when PageDirty is set? I didn't find it.
>
This is kind of delicate stuff. It took me a while to get it right
when I did it. I don't remember all the details.
But consider this option:
exofs_write_begin on a full PAGE_CACHE_SIZE, the page is instantiated
new in page-cache is that PageUptodate(page) then? I thought not.
(exofs does not set that)
Now that page I do not want to read in. The latest data is in memory.
(Same when this page is in writeback, dirty-bit is cleared)
So for sure if page is dirty or writeback then we surly do not need a read.
only if not then we need to consider the PageUptodate(page) state.
Do you think the code is actually wrong as is?
BTW: Very similar code is in fs/nfs/objlayout/objio_osd.c::__r4w_get_page
> Thanks,
> Hugh
>
<>
Thanks
Boaz
WARNING: multiple messages have this Message-ID (diff)
From: Boaz Harrosh <ooo@electrozaur.com>
To: Hugh Dickins <hughd@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Trond Myklebust <trond.myklebust@primarydata.com>,
Christoph Lameter <cl@linux.com>,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-nfs@vger.kernel.org, linux-mm@kvack.org,
osd-dev@open-osd.org
Subject: Re: [PATCH] osd fs: __r4w_get_page rely on PageUptodate for uptodate
Date: Mon, 02 Nov 2015 13:05:32 +0200 [thread overview]
Message-ID: <5637437C.4070306@electrozaur.com> (raw)
In-Reply-To: <alpine.LSU.2.11.1511011513240.11427@eggly.anvils>
On 11/02/2015 01:39 AM, Hugh Dickins wrote:
<>
>> This patch is not correct!
>
> I think you have actually confirmed that the patch is correct:
> why bother to test PageDirty or PageWriteback when PageUptodate
> already tells you what you need?
>
> Or do these filesystems do something unusual with PageUptodate
> when PageDirty is set? I didn't find it.
>
This is kind of delicate stuff. It took me a while to get it right
when I did it. I don't remember all the details.
But consider this option:
exofs_write_begin on a full PAGE_CACHE_SIZE, the page is instantiated
new in page-cache is that PageUptodate(page) then? I thought not.
(exofs does not set that)
Now that page I do not want to read in. The latest data is in memory.
(Same when this page is in writeback, dirty-bit is cleared)
So for sure if page is dirty or writeback then we surly do not need a read.
only if not then we need to consider the PageUptodate(page) state.
Do you think the code is actually wrong as is?
BTW: Very similar code is in fs/nfs/objlayout/objio_osd.c::__r4w_get_page
> Thanks,
> Hugh
>
<>
Thanks
Boaz
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2015-11-02 11:05 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-29 18:43 [PATCH] osd fs: __r4w_get_page rely on PageUptodate for uptodate Hugh Dickins
2015-10-29 18:43 ` Hugh Dickins
2015-11-01 10:00 ` Boaz Harrosh
2015-11-01 10:00 ` Boaz Harrosh
2015-11-01 23:39 ` Hugh Dickins
2015-11-01 23:39 ` Hugh Dickins
2015-11-01 23:39 ` Hugh Dickins
2015-11-02 11:05 ` Boaz Harrosh [this message]
2015-11-02 11:05 ` Boaz Harrosh
2015-11-03 2:49 ` Hugh Dickins
2015-11-03 2:49 ` Hugh Dickins
2015-11-03 9:24 ` Boaz Harrosh
2015-11-03 9:24 ` Boaz Harrosh
2015-11-03 15:39 ` Hugh Dickins
2015-11-03 15:39 ` Hugh Dickins
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=5637437C.4070306@electrozaur.com \
--to=ooo@electrozaur.com \
--cc=akpm@linux-foundation.org \
--cc=cl@linux.com \
--cc=hughd@google.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-nfs@vger.kernel.org \
--cc=osd-dev@open-osd.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.