Re: Bloat caused by unnecessary calls to compound_head()?

Re: Bloat caused by unnecessary calls to compound_head()?

[George Spelvin]

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>

Re: Bloat caused by unnecessary calls to compound_head()?

[Kirill A. Shutemov]

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>

Bloat caused by unnecessary calls to compound_head()?

[Eric Biggers]

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>

Re: Bloat caused by unnecessary calls to compound_head()?

[Kirill A. Shutemov]

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>

Re: Bloat caused by unnecessary calls to compound_head()?

[Eric Biggers]

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>

Re: Bloat caused by unnecessary calls to compound_head()?

[Kirill A. Shutemov]

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>