All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mel Gorman <mgorman@techsingularity.net>
To: "??????(Caspar)" <jinli.zjl@alibaba-inc.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	green@linuxhacker.ru, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org,
	"??????(??????)" <zhiche.yy@alibaba-inc.com>,
	?????? <shidao.ytt@alibaba-inc.com>
Subject: Re: [PATCH] mm/fadvise: discard partial pages iff endbyte is also eof
Date: Thu, 4 Jan 2018 11:34:51 +0000	[thread overview]
Message-ID: <20180104113451.j7dwal6mxbelt4p4@techsingularity.net> (raw)
In-Reply-To: <7dd95219-f0be-b30a-0a43-2aadcc61899c@alibaba-inc.com>

On Thu, Jan 04, 2018 at 02:13:43PM +0800, ??????(Caspar) wrote:
> 
> 
> On 2018/1/3 18:48, Mel Gorman wrote:
> > On Wed, Jan 03, 2018 at 02:53:43PM +0800, ??????(Caspar) wrote:
> > > 
> > > 
> > > > ?? 2017??12??23????12:16?????? <shidao.ytt@alibaba-inc.com> ??????
> > > > 
> > > > From: "shidao.ytt" <shidao.ytt@alibaba-inc.com>
> > > > 
> > > > in commit 441c228f817f7 ("mm: fadvise: document the
> > > > fadvise(FADV_DONTNEED) behaviour for partial pages") Mel Gorman
> > > > explained why partial pages should be preserved instead of discarded
> > > > when using fadvise(FADV_DONTNEED), however the actual codes to calcuate
> > > > end_index was unexpectedly wrong, the code behavior didn't match to the
> > > > statement in comments; Luckily in another commit 18aba41cbf
> > > > ("mm/fadvise.c: do not discard partial pages with POSIX_FADV_DONTNEED")
> > > > Oleg Drokin fixed this behavior
> > > > 
> > > > Here I come up with a new idea that actually we can still discard the
> > > > last parital page iff the page-unaligned endbyte is also the end of
> > > > file, since no one else will use the rest of the page and it should be
> > > > safe enough to discard.
> > > 
> > > +akpm...
> > > 
> > > Hi Mel, Andrew:
> > > 
> > > Would you please take a look at this patch, to see if this proposal
> > > is reasonable enough, thanks in advance!
> > > 
> > 
> > I'm backlogged after being out for the Christmas. Superficially the patch
> > looks ok but I wondered how often it happened in practice as we already
> > would discard files smaller than a page on DONTNEED. It also requires
> 
> Actually, we would *not*. Let's look into the codes.
> 

You're right of course. I suggest updating the changelog with what you
found and the test case. I think it's reasonable to special case the
discarding of partial pages if it's the end of a file with the potential
addendum of checking if the endbyte is past the end of the file. The man
page should also be updated.

-- 
Mel Gorman
SUSE Labs

--
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>

WARNING: multiple messages have this Message-ID (diff)
From: Mel Gorman <mgorman@techsingularity.net>
To: "??????(Caspar)" <jinli.zjl@alibaba-inc.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	green@linuxhacker.ru, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org,
	"??????(??????)" <zhiche.yy@alibaba-inc.com>,
	?????? <shidao.ytt@alibaba-inc.com>
Subject: Re: [PATCH] mm/fadvise: discard partial pages iff endbyte is also eof
Date: Thu, 4 Jan 2018 11:34:51 +0000	[thread overview]
Message-ID: <20180104113451.j7dwal6mxbelt4p4@techsingularity.net> (raw)
In-Reply-To: <7dd95219-f0be-b30a-0a43-2aadcc61899c@alibaba-inc.com>

On Thu, Jan 04, 2018 at 02:13:43PM +0800, ??????(Caspar) wrote:
> 
> 
> On 2018/1/3 18:48, Mel Gorman wrote:
> > On Wed, Jan 03, 2018 at 02:53:43PM +0800, ??????(Caspar) wrote:
> > > 
> > > 
> > > > ?? 2017??12??23????12:16?????? <shidao.ytt@alibaba-inc.com> ??????
> > > > 
> > > > From: "shidao.ytt" <shidao.ytt@alibaba-inc.com>
> > > > 
> > > > in commit 441c228f817f7 ("mm: fadvise: document the
> > > > fadvise(FADV_DONTNEED) behaviour for partial pages") Mel Gorman
> > > > explained why partial pages should be preserved instead of discarded
> > > > when using fadvise(FADV_DONTNEED), however the actual codes to calcuate
> > > > end_index was unexpectedly wrong, the code behavior didn't match to the
> > > > statement in comments; Luckily in another commit 18aba41cbf
> > > > ("mm/fadvise.c: do not discard partial pages with POSIX_FADV_DONTNEED")
> > > > Oleg Drokin fixed this behavior
> > > > 
> > > > Here I come up with a new idea that actually we can still discard the
> > > > last parital page iff the page-unaligned endbyte is also the end of
> > > > file, since no one else will use the rest of the page and it should be
> > > > safe enough to discard.
> > > 
> > > +akpm...
> > > 
> > > Hi Mel, Andrew:
> > > 
> > > Would you please take a look at this patch, to see if this proposal
> > > is reasonable enough, thanks in advance!
> > > 
> > 
> > I'm backlogged after being out for the Christmas. Superficially the patch
> > looks ok but I wondered how often it happened in practice as we already
> > would discard files smaller than a page on DONTNEED. It also requires
> 
> Actually, we would *not*. Let's look into the codes.
> 

You're right of course. I suggest updating the changelog with what you
found and the test case. I think it's reasonable to special case the
discarding of partial pages if it's the end of a file with the potential
addendum of checking if the endbyte is past the end of the file. The man
page should also be updated.

-- 
Mel Gorman
SUSE Labs

  parent reply	other threads:[~2018-01-04 11:34 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-23  4:16 [PATCH] mm/fadvise: discard partial pages iff endbyte is also eof 十刀
2017-12-23  4:16 ` 十刀
2018-01-03  6:53 ` 夷则(Caspar)
2018-01-03  6:53   ` 夷则(Caspar)
2018-01-03 10:48   ` Mel Gorman
2018-01-03 10:48     ` Mel Gorman
2018-01-04  0:17     ` Andrew Morton
2018-01-04  0:17       ` Andrew Morton
2018-01-04  8:17       ` 夷则(Caspar)
2018-01-04  8:17         ` 夷则(Caspar)
2018-01-04 22:54         ` Andrew Morton
2018-01-04 22:54           ` Andrew Morton
2018-01-04 10:05       ` Mel Gorman
2018-01-04 10:05         ` Mel Gorman
2018-01-04  6:13     ` 夷则(Caspar)
2018-01-04  6:13       ` 夷则(Caspar)
2018-01-04  7:44       ` 夷则(Caspar)
2018-01-04  7:44         ` 夷则(Caspar)
2018-01-04 11:34       ` Mel Gorman [this message]
2018-01-04 11:34         ` Mel Gorman
2018-01-04 11:38         ` 夷则(Caspar)
2018-01-04 11:38           ` 夷则(Caspar)
2018-01-05  6:10 ` [PATCH v2] mm/fadvise: discard partial page if endbyte is also EOF 夷则(Caspar)
2018-01-05  6:10   ` 夷则(Caspar)

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=20180104113451.j7dwal6mxbelt4p4@techsingularity.net \
    --to=mgorman@techsingularity.net \
    --cc=akpm@linux-foundation.org \
    --cc=green@linuxhacker.ru \
    --cc=jinli.zjl@alibaba-inc.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=shidao.ytt@alibaba-inc.com \
    --cc=zhiche.yy@alibaba-inc.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.