* Bloat caused by unnecessary calls to compound_head()?
@ 2016-03-26 18:50 Eric Biggers
2016-03-27 19:46 ` Kirill A. Shutemov
0 siblings, 1 reply; 6+ messages in thread
From: Eric Biggers @ 2016-03-26 18:50 UTC (permalink / raw)
To: linux-mm; +Cc: linux-kernel, kirill.shutemov
Hi,
I noticed that after the recent "page-flags" patchset, there are an excessive
number of calls to compound_head() in certain places.
For example, the frequently executed mark_page_accessed() function already
starts out by calling compound_head(), but then each time it tests a page flag
afterwards, there is an extra, seemingly unnecessary, call to compound_head().
This causes a series of instructions like the following to appear no fewer than
10 times throughout the function:
ffffffff81119db4: 48 8b 53 20 mov 0x20(%rbx),%rdx
ffffffff81119db8: 48 8d 42 ff lea -0x1(%rdx),%rax
ffffffff81119dbc: 83 e2 01 and $0x1,%edx
ffffffff81119dbf: 48 0f 44 c3 cmove %rbx,%rax
ffffffff81119dc3: 48 8b 00 mov (%rax),%rax
Part of the problem, I suppose, is that the compiler doesn't know that the pages
can't be linked more than one level deep.
Is this a known tradeoff, and have any possible solutions been considered?
Eric
--
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>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Bloat caused by unnecessary calls to compound_head()?
2016-03-26 18:50 Eric Biggers
@ 2016-03-27 19:46 ` Kirill A. Shutemov
2016-04-01 1:33 ` Eric Biggers
0 siblings, 1 reply; 6+ messages in thread
From: Kirill A. Shutemov @ 2016-03-27 19:46 UTC (permalink / raw)
To: Eric Biggers; +Cc: linux-mm, linux-kernel, kirill.shutemov, Hugh Dickins
On Sat, Mar 26, 2016 at 01:50:49PM -0500, Eric Biggers wrote:
> Hi,
>
> I noticed that after the recent "page-flags" patchset, there are an excessive
> number of calls to compound_head() in certain places.
>
> For example, the frequently executed mark_page_accessed() function already
> starts out by calling compound_head(), but then each time it tests a page flag
> afterwards, there is an extra, seemingly unnecessary, call to compound_head().
> This causes a series of instructions like the following to appear no fewer than
> 10 times throughout the function:
>
> ffffffff81119db4: 48 8b 53 20 mov 0x20(%rbx),%rdx
> ffffffff81119db8: 48 8d 42 ff lea -0x1(%rdx),%rax
> ffffffff81119dbc: 83 e2 01 and $0x1,%edx
> ffffffff81119dbf: 48 0f 44 c3 cmove %rbx,%rax
> ffffffff81119dc3: 48 8b 00 mov (%rax),%rax
>
> Part of the problem, I suppose, is that the compiler doesn't know that the pages
> can't be linked more than one level deep.
>
> Is this a known tradeoff, and have any possible solutions been considered?
<I'm sick, so my judgment may be off>
Yes, it's known problem. And I've tried to approach it few times without
satisfying results.
Your mail made me try again.
The idea is to introduce new type to indicate head page --
'struct head_page' -- it's compatible with struct page on memory layout,
but distinct from C point of view. compound_head() should return pointer
of that type. For the proof-of-concept I've introduced new helper --
compound_head_t().
Then we can make page-flag helpers to accept both types, by converting
them to macros and use __builtin_types_compatible_p().
When a page-flag helper sees pointer to 'struct head_page' as an argument,
it can safely assume that it deals with head or non-compound page and therefore
can bypass all policy restrictions and get rid of compound_head() calls.
I'll send proof-of-concept patches in reply to this message. The code is
not pretty. I myself consider the idea rather ugly.
Any comments are welcome.
--
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>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Bloat caused by unnecessary calls to compound_head()?
@ 2016-03-27 20:33 George Spelvin
2016-03-27 20:44 ` Kirill A. Shutemov
0 siblings, 1 reply; 6+ messages in thread
From: George Spelvin @ 2016-03-27 20:33 UTC (permalink / raw)
To: ebiggers3; +Cc: kirill, linux, linux-kernel, linux-mm
Could you just mark compound_head __pure? That would tell the compiler
that it's safe to re-use the return value as long as there is no memory
mutation in between.
--
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>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Bloat caused by unnecessary calls to compound_head()?
2016-03-27 20:33 Bloat caused by unnecessary calls to compound_head()? George Spelvin
@ 2016-03-27 20:44 ` Kirill A. Shutemov
0 siblings, 0 replies; 6+ messages in thread
From: Kirill A. Shutemov @ 2016-03-27 20:44 UTC (permalink / raw)
To: George Spelvin; +Cc: ebiggers3, linux-kernel, linux-mm
On Sun, Mar 27, 2016 at 04:33:04PM -0400, George Spelvin wrote:
> Could you just mark compound_head __pure? That would tell the compiler
> that it's safe to re-use the return value as long as there is no memory
> mutation in between.
>
Hm. It has some positive impact, but it's not dramatic. For instance,
mm/swap.o results:
add/remove: 0/0 grow/shrink: 0/2 up/down: 0/-43 (-43)
function old new delta
__page_cache_release 319 298 -21
release_pages 722 700 -22
mark_page_accessed() problem was not fixed by that.
--
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>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Bloat caused by unnecessary calls to compound_head()?
2016-03-27 19:46 ` Kirill A. Shutemov
@ 2016-04-01 1:33 ` Eric Biggers
2016-04-04 10:39 ` Kirill A. Shutemov
0 siblings, 1 reply; 6+ messages in thread
From: Eric Biggers @ 2016-04-01 1:33 UTC (permalink / raw)
To: Kirill A. Shutemov; +Cc: linux-mm, linux-kernel, kirill.shutemov, Hugh Dickins
On Sun, Mar 27, 2016 at 10:46:49PM +0300, Kirill A. Shutemov wrote:
> The idea is to introduce new type to indicate head page --
> 'struct head_page' -- it's compatible with struct page on memory layout,
> but distinct from C point of view. compound_head() should return pointer
> of that type. For the proof-of-concept I've introduced new helper --
> compound_head_t().
>
Well, it's good for optimizing the specific case of mark_page_accessed(). I'm
more worried about the general level of bloat, since the Page* macros are used
in so many places. And generating page-flags.h with a script is something to be
avoided if at all possible.
I wasn't following the discussion around the original page-flags patchset. Can
you point me to a discussion of the benefits of the page "policy" checks --- why
are they suddenly needed when they weren't before? Or any helpful comments in
the code?
--
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>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Bloat caused by unnecessary calls to compound_head()?
2016-04-01 1:33 ` Eric Biggers
@ 2016-04-04 10:39 ` Kirill A. Shutemov
0 siblings, 0 replies; 6+ messages in thread
From: Kirill A. Shutemov @ 2016-04-04 10:39 UTC (permalink / raw)
To: Eric Biggers; +Cc: linux-mm, linux-kernel, kirill.shutemov, Hugh Dickins
On Thu, Mar 31, 2016 at 08:33:29PM -0500, Eric Biggers wrote:
> On Sun, Mar 27, 2016 at 10:46:49PM +0300, Kirill A. Shutemov wrote:
> > The idea is to introduce new type to indicate head page --
> > 'struct head_page' -- it's compatible with struct page on memory layout,
> > but distinct from C point of view. compound_head() should return pointer
> > of that type. For the proof-of-concept I've introduced new helper --
> > compound_head_t().
> >
>
> Well, it's good for optimizing the specific case of mark_page_accessed(). I'm
> more worried about the general level of bloat, since the Page* macros are used
> in so many places. And generating page-flags.h with a script is something to be
> avoided if at all possible.
I think it can be done without generating page-flags.h. We can generate
with preprocessor Head* helpers in addition to Page*. New heplers would
opperate with struct head_page rather than struct page.
> I wasn't following the discussion around the original page-flags patchset. Can
> you point me to a discussion of the benefits of the page "policy" checks --- why
> are they suddenly needed when they weren't before? Or any helpful comments in
> the code?
Recent THP refcounting rework (went into v4.5) made possible mapping part
of huge page with PTEs. Basically, we now have page table entries which
point to tail pages. It means we have tail pages in codepaths where we
haven't before and we need to deal with this.
Many of the flags apply to whole compound page, not a subpage. So we need
to redirect page flag operation to head page.
--
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>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-04-04 10:39 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-27 20:33 Bloat caused by unnecessary calls to compound_head()? George Spelvin
2016-03-27 20:44 ` Kirill A. Shutemov
-- strict thread matches above, loose matches on Subject: below --
2016-03-26 18:50 Eric Biggers
2016-03-27 19:46 ` Kirill A. Shutemov
2016-04-01 1:33 ` Eric Biggers
2016-04-04 10:39 ` Kirill A. Shutemov
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).