Re: [PATCHv4 20/24] mm, thp: remove compound_lock

["Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>]

....
....

  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, 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>