* [PATCH] mm: use PageAnon() and PageKsm() helpers in page_anon_vma()
@ 2015-03-31 11:50 Kirill A. Shutemov
2015-03-31 13:11 ` Christoph Lameter
0 siblings, 1 reply; 9+ messages in thread
From: Kirill A. Shutemov @ 2015-03-31 11:50 UTC (permalink / raw
To: Andrew Morton
Cc: linux-mm, Konstantin Khlebnikov, Rik van Riel, Kirill A. Shutemov
page_anon_vma() directly checks PAGE_MAPPING_ANON and PAGE_MAPPING_KSM
bits on page->mapping to find out if page->mapping is anon_vma;
Let's use PageAnon() and PageKsm() helpers instead. It helps readability
and makes page_anon_vma() work correctly on tail pages.
Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
---
include/linux/rmap.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/include/linux/rmap.h b/include/linux/rmap.h
index 9c5ff69fa0cd..21f10e53bb9e 100644
--- a/include/linux/rmap.h
+++ b/include/linux/rmap.h
@@ -108,8 +108,7 @@ static inline void put_anon_vma(struct anon_vma *anon_vma)
static inline struct anon_vma *page_anon_vma(struct page *page)
{
- if (((unsigned long)page->mapping & PAGE_MAPPING_FLAGS) !=
- PAGE_MAPPING_ANON)
+ if (!PageAnon(page) || PageKsm(page))
return NULL;
return page_rmapping(page);
}
--
2.1.4
--
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 related [flat|nested] 9+ messages in thread
* Re: [PATCH] mm: use PageAnon() and PageKsm() helpers in page_anon_vma()
2015-03-31 11:50 [PATCH] mm: use PageAnon() and PageKsm() helpers in page_anon_vma() Kirill A. Shutemov
@ 2015-03-31 13:11 ` Christoph Lameter
2015-03-31 14:35 ` Kirill A. Shutemov
0 siblings, 1 reply; 9+ messages in thread
From: Christoph Lameter @ 2015-03-31 13:11 UTC (permalink / raw
To: Kirill A. Shutemov
Cc: Andrew Morton, linux-mm, Konstantin Khlebnikov, Rik van Riel
On Tue, 31 Mar 2015, Kirill A. Shutemov wrote:
> Let's use PageAnon() and PageKsm() helpers instead. It helps readability
> and makes page_anon_vma() work correctly on tail pages.
But it adds a branch due to the use of ||.
--
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] 9+ messages in thread
* Re: [PATCH] mm: use PageAnon() and PageKsm() helpers in page_anon_vma()
2015-03-31 13:11 ` Christoph Lameter
@ 2015-03-31 14:35 ` Kirill A. Shutemov
2015-03-31 20:33 ` Andrew Morton
0 siblings, 1 reply; 9+ messages in thread
From: Kirill A. Shutemov @ 2015-03-31 14:35 UTC (permalink / raw
To: Christoph Lameter
Cc: Kirill A. Shutemov, Andrew Morton, linux-mm,
Konstantin Khlebnikov, Rik van Riel
On Tue, Mar 31, 2015 at 08:11:02AM -0500, Christoph Lameter wrote:
> On Tue, 31 Mar 2015, Kirill A. Shutemov wrote:
>
> > Let's use PageAnon() and PageKsm() helpers instead. It helps readability
> > and makes page_anon_vma() work correctly on tail pages.
>
> But it adds a branch due to the use of ||.
Which caller is hot enough to care?
--
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] 9+ messages in thread
* Re: [PATCH] mm: use PageAnon() and PageKsm() helpers in page_anon_vma()
2015-03-31 14:35 ` Kirill A. Shutemov
@ 2015-03-31 20:33 ` Andrew Morton
2015-04-01 11:50 ` Kirill A. Shutemov
0 siblings, 1 reply; 9+ messages in thread
From: Andrew Morton @ 2015-03-31 20:33 UTC (permalink / raw
To: Kirill A. Shutemov
Cc: Christoph Lameter, Kirill A. Shutemov, linux-mm,
Konstantin Khlebnikov, Rik van Riel
On Tue, 31 Mar 2015 17:35:34 +0300 "Kirill A. Shutemov" <kirill@shutemov.name> wrote:
> On Tue, Mar 31, 2015 at 08:11:02AM -0500, Christoph Lameter wrote:
> > On Tue, 31 Mar 2015, Kirill A. Shutemov wrote:
> >
> > > Let's use PageAnon() and PageKsm() helpers instead. It helps readability
> > > and makes page_anon_vma() work correctly on tail pages.
> >
> > But it adds a branch due to the use of ||.
>
> Which caller is hot enough to care?
>
It's a surprisingly expensive patch.
text data bss dec hex filename
19984 1153 15192 36329 8de9 mm/ksm.o-before
20028 1153 15216 36397 8e2d mm/ksm.o-after
14728 116 5168 20012 4e2c mm/rmap.o-before
14763 116 5192 20071 4e67 mm/rmap.o-after
25723 1417 9776 36916 9034 mm/swapfile.o-before
25769 1417 9800 36986 907a mm/swapfile.o-after
197 bytes more text+bss, 125 bytes more text.
(Why the heck do changes like this allegedly affect bss size?)
--
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] 9+ messages in thread
* Re: [PATCH] mm: use PageAnon() and PageKsm() helpers in page_anon_vma()
2015-03-31 20:33 ` Andrew Morton
@ 2015-04-01 11:50 ` Kirill A. Shutemov
2015-04-01 19:57 ` Andrew Morton
0 siblings, 1 reply; 9+ messages in thread
From: Kirill A. Shutemov @ 2015-04-01 11:50 UTC (permalink / raw
To: Andrew Morton
Cc: Christoph Lameter, Kirill A. Shutemov, linux-mm,
Konstantin Khlebnikov, Rik van Riel
On Tue, Mar 31, 2015 at 01:33:38PM -0700, Andrew Morton wrote:
> On Tue, 31 Mar 2015 17:35:34 +0300 "Kirill A. Shutemov" <kirill@shutemov.name> wrote:
>
> > On Tue, Mar 31, 2015 at 08:11:02AM -0500, Christoph Lameter wrote:
> > > On Tue, 31 Mar 2015, Kirill A. Shutemov wrote:
> > >
> > > > Let's use PageAnon() and PageKsm() helpers instead. It helps readability
> > > > and makes page_anon_vma() work correctly on tail pages.
> > >
> > > But it adds a branch due to the use of ||.
> >
> > Which caller is hot enough to care?
> >
>
> It's a surprisingly expensive patch.
>
> text data bss dec hex filename
>
> 19984 1153 15192 36329 8de9 mm/ksm.o-before
> 20028 1153 15216 36397 8e2d mm/ksm.o-after
>
> 14728 116 5168 20012 4e2c mm/rmap.o-before
> 14763 116 5192 20071 4e67 mm/rmap.o-after
>
> 25723 1417 9776 36916 9034 mm/swapfile.o-before
> 25769 1417 9800 36986 907a mm/swapfile.o-after
>
> 197 bytes more text+bss, 125 bytes more text.
>
> (Why the heck do changes like this allegedly affect bss size?)
What about this?
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mm: use PageAnon() and PageKsm() helpers in page_anon_vma()
2015-04-01 11:50 ` Kirill A. Shutemov
@ 2015-04-01 19:57 ` Andrew Morton
2015-04-01 22:02 ` Kirill A. Shutemov
0 siblings, 1 reply; 9+ messages in thread
From: Andrew Morton @ 2015-04-01 19:57 UTC (permalink / raw
To: Kirill A. Shutemov
Cc: Christoph Lameter, Kirill A. Shutemov, linux-mm,
Konstantin Khlebnikov, Rik van Riel
On Wed, 1 Apr 2015 14:50:54 +0300 "Kirill A. Shutemov" <kirill@shutemov.name> wrote:
> >From adc384977898173d65c2567fc5eb421da9b272e0 Mon Sep 17 00:00:00 2001
> From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> Date: Wed, 1 Apr 2015 14:33:56 +0300
> Subject: [PATCH] mm: uninline and cleanup page-mapping related helpers
>
> Most-used page->mapping helper -- page_mapping() -- has already
> uninlined. Let's uninline also page_rmapping() and page_anon_vma().
> It saves us depending on configuration around 400 bytes in text:
>
> text data bss dec hex filename
> 660318 99254 410000 1169572 11d8a4 mm/built-in.o-before
> 659854 99254 410000 1169108 11d6d4 mm/built-in.o
Well, code size isn't the only thing to care about. Some functions
really should be inlined for performance reasons even if that makes the
overall code larger. But the changes you're proposing here look OK to
me.
> As side effect page_anon_vma() now works properly on tail pages.
Let's fix the bug in a separate patch, please. One which can be
backported to earlier kernels if that should be needed. ie: it should
precede any uninlining.
--
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] 9+ messages in thread
* Re: [PATCH] mm: use PageAnon() and PageKsm() helpers in page_anon_vma()
2015-04-01 19:57 ` Andrew Morton
@ 2015-04-01 22:02 ` Kirill A. Shutemov
2015-04-01 22:06 ` Andrew Morton
0 siblings, 1 reply; 9+ messages in thread
From: Kirill A. Shutemov @ 2015-04-01 22:02 UTC (permalink / raw
To: Andrew Morton
Cc: Christoph Lameter, Kirill A. Shutemov, linux-mm,
Konstantin Khlebnikov, Rik van Riel
On Wed, Apr 01, 2015 at 12:57:45PM -0700, Andrew Morton wrote:
> On Wed, 1 Apr 2015 14:50:54 +0300 "Kirill A. Shutemov" <kirill@shutemov.name> wrote:
>
> > >From adc384977898173d65c2567fc5eb421da9b272e0 Mon Sep 17 00:00:00 2001
> > From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> > Date: Wed, 1 Apr 2015 14:33:56 +0300
> > Subject: [PATCH] mm: uninline and cleanup page-mapping related helpers
> >
> > Most-used page->mapping helper -- page_mapping() -- has already
> > uninlined. Let's uninline also page_rmapping() and page_anon_vma().
> > It saves us depending on configuration around 400 bytes in text:
> >
> > text data bss dec hex filename
> > 660318 99254 410000 1169572 11d8a4 mm/built-in.o-before
> > 659854 99254 410000 1169108 11d6d4 mm/built-in.o
>
> Well, code size isn't the only thing to care about. Some functions
> really should be inlined for performance reasons even if that makes the
> overall code larger. But the changes you're proposing here look OK to
> me.
>
> > As side effect page_anon_vma() now works properly on tail pages.
>
> Let's fix the bug in a separate patch, please. One which can be
> backported to earlier kernels if that should be needed. ie: it should
> precede any uninlining.
The bug is not triggerable in current upsteam. AFAIK, we don't call
page_anon_vma() on tail pages of THP, since we don't map THP with PTEs
yet. For rest of cases we will get NULL, which is valid answer.
Do you still want "page = compound_head(page);" line in separate patch?
--
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] 9+ messages in thread
* Re: [PATCH] mm: use PageAnon() and PageKsm() helpers in page_anon_vma()
2015-04-01 22:02 ` Kirill A. Shutemov
@ 2015-04-01 22:06 ` Andrew Morton
2015-04-02 11:34 ` Kirill A. Shutemov
0 siblings, 1 reply; 9+ messages in thread
From: Andrew Morton @ 2015-04-01 22:06 UTC (permalink / raw
To: Kirill A. Shutemov
Cc: Christoph Lameter, Kirill A. Shutemov, linux-mm,
Konstantin Khlebnikov, Rik van Riel
On Thu, 2 Apr 2015 01:02:46 +0300 "Kirill A. Shutemov" <kirill@shutemov.name> wrote:
> On Wed, Apr 01, 2015 at 12:57:45PM -0700, Andrew Morton wrote:
> > On Wed, 1 Apr 2015 14:50:54 +0300 "Kirill A. Shutemov" <kirill@shutemov.name> wrote:
> >
> > > >From adc384977898173d65c2567fc5eb421da9b272e0 Mon Sep 17 00:00:00 2001
> > > From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> > > Date: Wed, 1 Apr 2015 14:33:56 +0300
> > > Subject: [PATCH] mm: uninline and cleanup page-mapping related helpers
> > >
> > > Most-used page->mapping helper -- page_mapping() -- has already
> > > uninlined. Let's uninline also page_rmapping() and page_anon_vma().
> > > It saves us depending on configuration around 400 bytes in text:
> > >
> > > text data bss dec hex filename
> > > 660318 99254 410000 1169572 11d8a4 mm/built-in.o-before
> > > 659854 99254 410000 1169108 11d6d4 mm/built-in.o
> >
> > Well, code size isn't the only thing to care about. Some functions
> > really should be inlined for performance reasons even if that makes the
> > overall code larger. But the changes you're proposing here look OK to
> > me.
> >
> > > As side effect page_anon_vma() now works properly on tail pages.
> >
> > Let's fix the bug in a separate patch, please. One which can be
> > backported to earlier kernels if that should be needed. ie: it should
> > precede any uninlining.
>
> The bug is not triggerable in current upsteam. AFAIK, we don't call
> page_anon_vma() on tail pages of THP, since we don't map THP with PTEs
> yet. For rest of cases we will get NULL, which is valid answer.
argh. It rather helps if you can tell me when this happens (and which
patch it fixes). I sometimes spend quite a bit of time runnnig around
in circles wondering what the heck tree/patch just got fixed.
> Do you still want "page = compound_head(page);" line in separate patch?
I think that would be best. That way the offending patch gets fixed
and doesn't get bundled up with an unrelated change.
--
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] 9+ messages in thread
* Re: [PATCH] mm: use PageAnon() and PageKsm() helpers in page_anon_vma()
2015-04-01 22:06 ` Andrew Morton
@ 2015-04-02 11:34 ` Kirill A. Shutemov
0 siblings, 0 replies; 9+ messages in thread
From: Kirill A. Shutemov @ 2015-04-02 11:34 UTC (permalink / raw
To: Andrew Morton
Cc: Christoph Lameter, Kirill A. Shutemov, linux-mm,
Konstantin Khlebnikov, Rik van Riel
On Wed, Apr 01, 2015 at 03:06:53PM -0700, Andrew Morton wrote:
> On Thu, 2 Apr 2015 01:02:46 +0300 "Kirill A. Shutemov" <kirill@shutemov.name> wrote:
>
> > On Wed, Apr 01, 2015 at 12:57:45PM -0700, Andrew Morton wrote:
> > > On Wed, 1 Apr 2015 14:50:54 +0300 "Kirill A. Shutemov" <kirill@shutemov.name> wrote:
> > >
> > > > >From adc384977898173d65c2567fc5eb421da9b272e0 Mon Sep 17 00:00:00 2001
> > > > From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> > > > Date: Wed, 1 Apr 2015 14:33:56 +0300
> > > > Subject: [PATCH] mm: uninline and cleanup page-mapping related helpers
> > > >
> > > > Most-used page->mapping helper -- page_mapping() -- has already
> > > > uninlined. Let's uninline also page_rmapping() and page_anon_vma().
> > > > It saves us depending on configuration around 400 bytes in text:
> > > >
> > > > text data bss dec hex filename
> > > > 660318 99254 410000 1169572 11d8a4 mm/built-in.o-before
> > > > 659854 99254 410000 1169108 11d6d4 mm/built-in.o
> > >
> > > Well, code size isn't the only thing to care about. Some functions
> > > really should be inlined for performance reasons even if that makes the
> > > overall code larger. But the changes you're proposing here look OK to
> > > me.
> > >
> > > > As side effect page_anon_vma() now works properly on tail pages.
> > >
> > > Let's fix the bug in a separate patch, please. One which can be
> > > backported to earlier kernels if that should be needed. ie: it should
> > > precede any uninlining.
> >
> > The bug is not triggerable in current upsteam. AFAIK, we don't call
> > page_anon_vma() on tail pages of THP, since we don't map THP with PTEs
> > yet. For rest of cases we will get NULL, which is valid answer.
>
> argh. It rather helps if you can tell me when this happens (and which
> patch it fixes). I sometimes spend quite a bit of time runnnig around
> in circles wondering what the heck tree/patch just got fixed.
Sorry for the mess.
> > Do you still want "page = compound_head(page);" line in separate patch?
>
> I think that would be best. That way the offending patch gets fixed
> and doesn't get bundled up with an unrelated change.
I've sent updated "mm: sanitize page->mapping for tail pages" patch that
care about this part. And this is uninlining patch on top of updated
patch.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-04-02 11:34 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-03-31 11:50 [PATCH] mm: use PageAnon() and PageKsm() helpers in page_anon_vma() Kirill A. Shutemov
2015-03-31 13:11 ` Christoph Lameter
2015-03-31 14:35 ` Kirill A. Shutemov
2015-03-31 20:33 ` Andrew Morton
2015-04-01 11:50 ` Kirill A. Shutemov
2015-04-01 19:57 ` Andrew Morton
2015-04-01 22:02 ` Kirill A. Shutemov
2015-04-01 22:06 ` Andrew Morton
2015-04-02 11:34 ` Kirill A. Shutemov
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.