From: "Kirill A. Shutemov" <kirill@shutemov.name>
To: Michal Hocko <mhocko@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Minchan Kim <minchan@kernel.org>,
linux-mm@kvack.org, LKML <linux-kernel@vger.kernel.org>
Subject: Re: isolate_lru_page on !head pages
Date: Mon, 14 Dec 2015 14:04:56 +0200 [thread overview]
Message-ID: <20151214120456.GA4201@node.shutemov.name> (raw)
In-Reply-To: <20151209130204.GD30907@dhcp22.suse.cz>
On Wed, Dec 09, 2015 at 02:02:05PM +0100, Michal Hocko wrote:
> Hi Kirill,
[ sorry for late reply, just back from vacation. ]
> while looking at the issue reported by Minchan [1] I have noticed that
> there is nothing to prevent from "isolating" a tail page from LRU because
> isolate_lru_page checks PageLRU which is
> PAGEFLAG(LRU, lru, PF_HEAD)
> so it is checked on the head page rather than the given page directly
> but the rest of the operation is done on the given (tail) page.
Looks like most (all?) callers already exclude PTE-mapped THP already one
way or another.
Probably, VM_BUG_ON_PAGE(PageTail(page), page) in isolate_lru_page() would
be appropriate.
> This is really subtle because this expects that every caller of this
> function checks for the tail page otherwise we would clobber statistics
> and who knows what else (I haven't checked that in detail) as the page
> cannot be on the LRU list and the operation makes sense only on the head
> page.
>
> Would it make more sense to make PageLRU PF_ANY? That would return
> false for PageLRU on any tail page and so it would be ignored by
> isolate_lru_page.
I don't think this is right way to go. What we put on LRU is compound
page, not 4k subpages. PageLRU() should return true if the compound page
is on LRU regardless if you ask for head or tail page.
False-negatives PageLRU() can be as bad as bug Minchan reported, but
perhaps more silent.
> I haven't checked other flags but there might be a similar situation. I
> am wondering whether it is really a good idea to perform a flag check on
> a different page then the operation which depends on the result of the
> test in general. It sounds like a maintenance horror to me.
>
> [1] http://lkml.kernel.org/r/20151201133455.GB27574@bbox
--
Kirill A. Shutemov
--
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-12-14 12:04 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-12-09 13:02 isolate_lru_page on !head pages Michal Hocko
2015-12-14 12:04 ` Kirill A. Shutemov [this message]
2015-12-15 8:52 ` Michal Hocko
2015-12-15 12:03 ` Kirill A. Shutemov
2015-12-15 16:59 ` Michal Hocko
2015-12-22 15:47 ` Vlastimil Babka
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=20151214120456.GA4201@node.shutemov.name \
--to=kirill@shutemov.name \
--cc=akpm@linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@kernel.org \
--cc=minchan@kernel.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).