Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1364976 > unrolled thread

Bloat caused by unnecessary calls to compound_head()?

Back to article view | Back to linux.kernel

Contents

  Bloat caused by unnecessary calls to compound_head()? Eric Biggers <ebiggers3@gmail.com> - 2016-03-26 20:00 +0100
    Re: Bloat caused by unnecessary calls to compound_head()? "Kirill A. Shutemov" <kirill@shutemov.name> - 2016-03-27 21:50 +0200
      Re: Bloat caused by unnecessary calls to compound_head()? Eric Biggers <ebiggers3@gmail.com> - 2016-04-01 03:40 +0200

#1364976 — Bloat caused by unnecessary calls to compound_head()?

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

[toc] | [next] | [standalone]

#1365174

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

[toc] | [prev] | [next] | [standalone]

#1368931

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?

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web