[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87r3s5b1yw.fsf@linux.vnet.ibm.com>
Date: Tue, 31 Mar 2015 21:20:31 +0530
From: "Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>
To: "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Andrea Arcangeli <aarcange@...hat.com>
Cc: Dave Hansen <dave.hansen@...el.com>,
Hugh Dickins <hughd@...gle.com>, Mel Gorman <mgorman@...e.de>,
Rik van Riel <riel@...hat.com>,
Vlastimil Babka <vbabka@...e.cz>,
Christoph Lameter <cl@...two.org>,
Naoya Horiguchi <n-horiguchi@...jp.nec.com>,
Steve Capper <steve.capper@...aro.org>,
Johannes Weiner <hannes@...xchg.org>,
Michal Hocko <mhocko@...e.cz>,
Jerome Marchand <jmarchan@...hat.com>,
linux-kernel@...r.kernel.org, linux-mm@...ck.org,
"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
Subject: Re: [PATCHv4 20/24] mm, thp: remove compound_lock
....
....
static void put_compound_page(struct page *page)
> {
> struct page *page_head;
> - unsigned long flags;
>
> if (likely(!PageTail(page))) {
> if (put_page_testzero(page)) {
> @@ -108,58 +101,33 @@ static void put_compound_page(struct page *page)
> /* __split_huge_page_refcount can run under us */
> page_head = compound_head(page);
>
> - if (!compound_lock_needed(page_head)) {
> - /*
> - * If "page" is a THP tail, we must read the tail page flags
> - * after the head page flags. The split_huge_page side enforces
> - * write memory barriers between clearing PageTail and before
> - * the head page can be freed and reallocated.
> - */
> - smp_rmb();
> - if (likely(PageTail(page))) {
> - /* __split_huge_page_refcount cannot race here. */
> - VM_BUG_ON_PAGE(!PageHead(page_head), page_head);
> - VM_BUG_ON_PAGE(page_mapcount(page) != 0, page);
> - if (put_page_testzero(page_head)) {
> - /*
> - * If this is the tail of a slab compound page,
> - * the tail pin must not be the last reference
> - * held on the page, because the PG_slab cannot
> - * be cleared before all tail pins (which skips
> - * the _mapcount tail refcounting) have been
> - * released. For hugetlbfs the tail pin may be
> - * the last reference on the page instead,
> - * because PageHeadHuge will not go away until
> - * the compound page enters the buddy
> - * allocator.
> - */
> - VM_BUG_ON_PAGE(PageSlab(page_head), page_head);
> - __put_compound_page(page_head);
> - }
> - } else if (put_page_testzero(page))
> - __put_single_page(page);
> - return;
> - }
> -
> - flags = compound_lock_irqsave(page_head);
> - /* here __split_huge_page_refcount won't run anymore */
> - if (likely(page != page_head && PageTail(page))) {
> - bool free;
> -
> - free = put_page_testzero(page_head);
> - compound_unlock_irqrestore(page_head, flags);
> - if (free) {
> - if (PageHead(page_head))
> - __put_compound_page(page_head);
> - else
> - __put_single_page(page_head);
> + /*
> + * If "page" is a THP tail, we must read the tail page flags after the
> + * head page flags. The split_huge_page side enforces write memory
> + * barriers between clearing PageTail and before the head page can be
> + * freed and reallocated.
> + */
> + smp_rmb();
> + if (likely(PageTail(page))) {
> + /* __split_huge_page_refcount cannot race here. */
> + VM_BUG_ON_PAGE(!PageHead(page_head), page_head);
> + if (put_page_testzero(page_head)) {
> + /*
> + * If this is the tail of a slab compound page, the
> + * tail pin must not be the last reference held on the
> + * page, because the PG_slab cannot be cleared before
> + * all tail pins (which skips the _mapcount tail
> + * refcounting) have been released. For hugetlbfs the
> + * tail pin may be the last reference on the page
> + * instead, because PageHeadHuge will not go away until
> + * the compound page enters the buddy allocator.
> + */
> + VM_BUG_ON_PAGE(PageSlab(page_head), page_head);
> + __put_compound_page(page_head);
> }
The comment may need an update ? For THP also a tail pin may be the last
reference on the page right ?
> - } else {
> - compound_unlock_irqrestore(page_head, flags);
> - VM_BUG_ON_PAGE(PageTail(page), page);
> - if (put_page_testzero(page))
> - __put_single_page(page);
> - }
> + } else if (put_page_testzero(page))
> + __put_single_page(page);
> + return;
> }
>
> void put_page(struct page *page)
> @@ -178,42 +146,29 @@ EXPORT_SYMBOL(put_page);
> void __get_page_tail(struct page *page)
> {
> struct page *page_head = compound_head(page);
.....
....
> + smp_rmb();
> + if (likely(PageTail(page))) {
> + /*
> + * This is a hugetlbfs page or a slab page.
> + * __split_huge_page_refcount cannot race here.
> + */
This comment also need an update. This can be a THP tail page right ?
> + VM_BUG_ON_PAGE(!PageHead(page_head), page_head);
> + VM_BUG_ON(page_head != page->first_page);
> + VM_BUG_ON_PAGE(atomic_read(&page_head->_count) <= 0,
> + page);
> + atomic_inc(&page_head->_count);
> + } else {
> + /*
> + * __split_huge_page_refcount run before us, "page" was
> + * a thp tail. the split page_head has been freed and
> + * reallocated as slab or hugetlbfs page of smaller
> + * order (only possible if reallocated as slab on x86).
> + */
> VM_BUG_ON_PAGE(atomic_read(&page->_count) <= 0, page);
> atomic_inc(&page->_count);
> }
> - compound_unlock_irqrestore(page_head, flags);
> + return;
> }
> EXPORT_SYMBOL(__get_page_tail);
>
> --
> 2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists