lkml.org 
[lkml]   [2014]   [Apr]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH RFC 2/2] mm: introdule compound_head_by_tail()
    On Sun 27-04-14 21:36:24, Jianyu Zhan wrote:
    > In put_comound_page(), we call compound_head() after !PageTail
    > check fails, so in compound_head() PageTail is quite likely to
    > be true, but instead it is checked with:
    >
    > if (unlikely(PageTail(page)))
    >
    > in this case, this unlikely macro is a negative hint for compiler.
    >
    > So this patch introduce compound_head_by_tail() which deal with
    > a possible tail page(though it could be spilt by a racy thread),
    > and make compound_head() a wrapper on it.
    >
    > Signed-off-by: Jianyu Zhan <nasa4836@gmail.com>

    I really fail to see how that helps. compound_head is inlined and the
    compiler should be clever enough to optimize the code properly. I
    haven't tried that to be honest but this looks like it only adds a code
    without any good reason. And I really hate the new name as well. What
    does it suppose to mean?

    > ---
    > include/linux/mm.h | 34 ++++++++++++++++++++++------------
    > mm/swap.c | 2 +-
    > 2 files changed, 23 insertions(+), 13 deletions(-)
    >
    > diff --git a/include/linux/mm.h b/include/linux/mm.h
    > index bf9811e..1bc7baf 100644
    > --- a/include/linux/mm.h
    > +++ b/include/linux/mm.h
    > @@ -405,20 +405,30 @@ static inline void compound_unlock_irqrestore(struct page *page,
    > #endif
    > }
    >
    > +/**
    > + * Note: this function must be called on a possible tail page,
    > + * this tail page may not be tail anymore upon we calling this funciton,
    > + * because we may race with __split_huge_page_refcount tearing down it.
    > + */
    > +static inline struct page *compound_head_by_tail(struct page *page)
    > +{
    > + struct page *head = page->first_page;
    > +
    > + /*
    > + * page->first_page may be a dangling pointer to an old
    > + * compound page, so recheck that it is still a tail
    > + * page before returning.
    > + */
    > + smp_rmb();
    > + if (likely(PageTail(page)))
    > + return head;
    > + return page;
    > +}
    > +
    > static inline struct page *compound_head(struct page *page)
    > {
    > - if (unlikely(PageTail(page))) {
    > - struct page *head = page->first_page;
    > -
    > - /*
    > - * page->first_page may be a dangling pointer to an old
    > - * compound page, so recheck that it is still a tail
    > - * page before returning.
    > - */
    > - smp_rmb();
    > - if (likely(PageTail(page)))
    > - return head;
    > - }
    > + if (unlikely(PageTail(page)))
    > + return compound_head_by_tail(page);
    > return page;
    > }
    >
    > diff --git a/mm/swap.c b/mm/swap.c
    > index 0d8d891..0b05355 100644
    > --- a/mm/swap.c
    > +++ b/mm/swap.c
    > @@ -256,7 +256,7 @@ static void put_compound_page(struct page *page)
    > * Case 3 is possible, as we may race with
    > * __split_huge_page_refcount tearing down a THP page.
    > */
    > - head_page = compound_head(page);
    > + head_page = compound_head_by_tail(page);
    > if (!__compound_tail_refcounted(head_page))
    > put_unrefcounted_compound_page(head_page, page);
    > else
    > --
    > 2.0.0-rc1
    >
    > --
    > 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>

    --
    Michal Hocko
    SUSE Labs


    \
     
     \ /
      Last update: 2014-04-28 17:41    [W:3.543 / U:0.548 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site