linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Andrea Arcangeli <aarcange@redhat.com>,
	Andrew Morton <akpm@linux-foundation.org>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Dave Jones <davej@redhat.com>,
	Darren Hart <dvhart@linux.intel.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Mel Gorman <mgorman@suse.de>,
	linux-kernel@vger.kernel.org
Subject: [PATCH -mm 6/7] mm: thp: introduce get_lock_thp_head()
Date: Wed, 18 Dec 2013 20:20:05 +0100	[thread overview]
Message-ID: <20131218192005.GA8340@redhat.com> (raw)
In-Reply-To: <20131218191913.GA6464@redhat.com>

Both __put_page_tail() and __get_page_tail() need to carefully
take a reference on page_head, take compound_lock() and recheck
PageTail(page) under this lock.

This patch extracts this code into the new helper, it will have
another user. This also means that __get_page_tail() gets the
same VM_BUG_ON() checks.

Note: this change can also help if we decide to change the locking,
perhaps it makes sense to change __split_huge_page_refcount() to
also do compound_lock/unlock(page_tail) in a loop. In this case it
would be simple to adapt this helper and its usage.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 mm/swap.c |   99 +++++++++++++++++++++++++++---------------------------------
 1 files changed, 45 insertions(+), 54 deletions(-)

diff --git a/mm/swap.c b/mm/swap.c
index 5f3dda6..972923d 100644
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -93,12 +93,45 @@ static void __put_nontail_page(struct page *page)
 	}
 }
 
-static void __put_page_tail(struct page *page)
+static bool get_lock_thp_head(struct page *head, struct page *tail,
+				unsigned long *flags)
 {
-	struct page *page_head;
+	if (unlikely(head == tail || !get_page_unless_zero(head)))
+		return false;
+
+	/*
+	 * page_head wasn't a dangling pointer but it may not be a head
+	 * page anymore by the time we obtain the lock. That is ok as
+	 * long as it can't be freed from under us.
+	 */
+	*flags = compound_lock_irqsave(head);
+	if (likely(PageTail(tail))) {
+		VM_BUG_ON(head != tail->first_page);
+		VM_BUG_ON(atomic_read(&head->_count) <= 1);
+		VM_BUG_ON(atomic_read(&tail->_count) != 0);
+		VM_BUG_ON(page_mapcount(tail) <= 1);
+		return true;
+	}
+
+	compound_unlock_irqrestore(head, *flags);
+	/*
+	 * The head page may have been freed and reallocated as a compound
+	 * page of smaller order and then freed again. All we know is that
+	 * it cannot have become: a THP page, a compound page of higher
+	 * order, a tail page. That is because we still hold the refcount
+	 * of the split THP tail and page_head was the THP head before the
+	 * split.
+	 */
+	__put_nontail_page(head);
+	return false;
+}
 
+
+static void __put_page_tail(struct page *page)
+{
 	/* __split_huge_page_refcount can run under us */
-	page_head = compound_trans_head(page);
+	struct page *page_head = compound_trans_head(page);
+	unsigned long flags;
 
 	/*
 	 * THP can not break up slab pages so avoid taking
@@ -150,45 +183,16 @@ static void __put_page_tail(struct page *page)
 		return;
 	}
 
-	if (likely(page != page_head && get_page_unless_zero(page_head))) {
-		unsigned long flags;
-
-		/*
-		 * page_head wasn't a dangling pointer but it may not
-		 * be a head page anymore by the time we obtain the
-		 * lock. That is ok as long as it can't be freed from
-		 * under us.
-		 */
-		flags = compound_lock_irqsave(page_head);
-		if (unlikely(!PageTail(page))) {
-			/* __split_huge_page_refcount run before us */
-			compound_unlock_irqrestore(page_head, flags);
-			/*
-			 * The head page may have been freed and reallocated
-			 * as a compound page of smaller order and then freed
-			 * again. All we know is that it cannot have become:
-			 * a THP page, a compound page of higher order, a tail
-			 * page. That is because we still hold the refcount of
-			 * the split THP tail and page_head was the THP head
-			 * before the split.
-			 */
-			__put_nontail_page(page_head);
-			goto out_put_single;
-		}
-		VM_BUG_ON(page_head != page->first_page);
+	if (likely(get_lock_thp_head(page_head, page, &flags))) {
 		/*
-		 * We can release the refcount taken by
-		 * get_page_unless_zero() now that
-		 * __split_huge_page_refcount() is blocked on the
+		 * We can release the refcount taken by get_lock_thp_head()
+		 * now that __split_huge_page_refcount() is blocked on the
 		 * compound_lock.
 		 */
 		if (put_page_testzero(page_head))
 			VM_BUG_ON(1);
 		/* __split_huge_page_refcount will wait now */
-		VM_BUG_ON(page_mapcount(page) <= 0);
 		atomic_dec(&page->_mapcount);
-		VM_BUG_ON(atomic_read(&page_head->_count) <= 0);
-		VM_BUG_ON(atomic_read(&page->_count) != 0);
 		compound_unlock_irqrestore(page_head, flags);
 
 		__put_nontail_page(page_head);
@@ -224,9 +228,8 @@ bool __get_page_tail(struct page *page)
 	 * proper PT lock that already serializes against
 	 * split_huge_page().
 	 */
-	unsigned long flags;
-	bool got;
 	struct page *page_head = compound_trans_head(page);
+	unsigned long flags;
 
 	/* Ref to __put_page_tail() comment. */
 	if (!__compound_tail_refcounted(page_head)) {
@@ -254,25 +257,13 @@ bool __get_page_tail(struct page *page)
 		}
 	}
 
-	got = false;
-	if (likely(page != page_head && get_page_unless_zero(page_head))) {
-		/*
-		 * page_head wasn't a dangling pointer but it
-		 * may not be a head page anymore by the time
-		 * we obtain the lock. That is ok as long as it
-		 * can't be freed from under us.
-		 */
-		flags = compound_lock_irqsave(page_head);
-		/* here __split_huge_page_refcount won't run anymore */
-		if (likely(PageTail(page))) {
-			__get_page_tail_foll(page, false);
-			got = true;
-		}
+	if (likely(get_lock_thp_head(page_head, page, &flags))) {
+		__get_page_tail_foll(page, false);
 		compound_unlock_irqrestore(page_head, flags);
-		if (unlikely(!got))
-			__put_nontail_page(page_head);
+		return true;
 	}
-	return got;
+
+	return false;
 }
 EXPORT_SYMBOL(__get_page_tail);
 
-- 
1.5.5.1


  parent reply	other threads:[~2013-12-18 19:19 UTC|newest]

Thread overview: 114+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-10 15:47 process 'stuck' at exit Dave Jones
2013-12-10 18:40 ` Linus Torvalds
2013-12-10 19:18   ` Thomas Gleixner
2013-12-10 19:55     ` Linus Torvalds
2013-12-10 20:27       ` Dave Jones
2013-12-10 20:34         ` Thomas Gleixner
2013-12-10 20:55           ` Dave Jones
2013-12-10 21:25             ` Darren Hart
2013-12-10 21:28               ` Thomas Gleixner
2013-12-10 21:39                 ` Steven Rostedt
2013-12-10 20:33       ` Thomas Gleixner
2013-12-10 20:43         ` Linus Torvalds
2013-12-10 21:17           ` Thomas Gleixner
2013-12-10 20:35       ` Oleg Nesterov
2013-12-10 20:49         ` Dave Jones
2013-12-10 21:06           ` Darren Hart
2013-12-10 21:12             ` Dave Jones
2013-12-10 21:18             ` Linus Torvalds
2013-12-10 21:24               ` Linus Torvalds
2013-12-10 21:32               ` Dave Jones
2013-12-10 21:49                 ` Linus Torvalds
2013-12-10 21:56                   ` Dave Jones
2013-12-10 21:59                     ` Linus Torvalds
2013-12-10 22:07                       ` Dave Jones
2013-12-11 12:45                   ` Ingo Molnar
2013-12-10 21:34           ` Oleg Nesterov
2013-12-10 21:41             ` Dave Jones
2013-12-10 21:57               ` Linus Torvalds
2013-12-10 22:02                 ` Dave Jones
2013-12-10 22:09                 ` Oleg Nesterov
2013-12-10 22:19                 ` Linus Torvalds
2013-12-10 22:33                   ` Linus Torvalds
2013-12-10 22:38                     ` Darren Hart
2013-12-10 22:57                     ` Thomas Gleixner
2013-12-10 23:05                       ` Linus Torvalds
2013-12-10 23:28                         ` Thomas Gleixner
2013-12-10 23:31                         ` Al Viro
2013-12-11 17:08                     ` Oleg Nesterov
2013-12-11 17:18                       ` Thomas Gleixner
2013-12-11 17:56                         ` Oleg Nesterov
2013-12-11 19:18                           ` PATCH? introduce get_compound_page (Was: process 'stuck' at exit) Oleg Nesterov
2013-12-13 15:10                             ` Andrea Arcangeli
2013-12-13 16:22                               ` Oleg Nesterov
2013-12-13 17:34                                 ` Andrea Arcangeli
2013-12-16 18:36                                   ` Oleg Nesterov
2013-12-16 20:19                                     ` Andrea Arcangeli
2013-12-16 20:46                                       ` Oleg Nesterov
2013-12-17 16:53                                         ` Oleg Nesterov
2013-12-17 18:06                                           ` Andrea Arcangeli
2013-12-18 19:19                                             ` [PATCH -mm 0/7] (Was: introduce get_compound_page) Oleg Nesterov
2013-12-18 19:19                                               ` [PATCH -mm 1/7] mm: thp: introduce __put_nontail_page() Oleg Nesterov
2013-12-18 19:19                                               ` [PATCH -mm 2/7] mm: thp: change __get_page_tail() to use __put_nontail_page() Oleg Nesterov
2013-12-18 19:19                                               ` [PATCH -mm 3/7] mm: change release_pages() to use put_page() rather than put_compound_page() Oleg Nesterov
2013-12-18 19:19                                               ` [PATCH -mm 4/7] mm: thp: turn put_compound_page() into __put_page_tail() Oleg Nesterov
2013-12-18 19:36                                                 ` Peter Zijlstra
2013-12-18 19:50                                                   ` Oleg Nesterov
2013-12-18 19:20                                               ` [PATCH -mm 5/7] mm: thp: reorganize out_put_single code in __put_page_tail() Oleg Nesterov
2013-12-18 19:20                                               ` Oleg Nesterov [this message]
2013-12-18 21:37                                                 ` [PATCH -mm 6/7] mm: thp: introduce get_lock_thp_head() Linus Torvalds
2013-12-19 16:29                                                   ` Oleg Nesterov
2013-12-18 19:20                                               ` [PATCH -mm 7/7] mm: thp: introduce compound_head_put_tail(), change get_futex_key() to use it Oleg Nesterov
2013-12-18 19:28                                                 ` Peter Zijlstra
2013-12-18 19:40                                                   ` Oleg Nesterov
2013-12-19 19:08                                       ` [PATCH 0/1] mm: fix the theoretical compound_lock() vs prep_new_page() race Oleg Nesterov
2013-12-19 19:09                                         ` [PATCH 1/1] " Oleg Nesterov
2013-12-23 11:43                                           ` Andrea Arcangeli
2014-01-03 19:55                                             ` [PATCH v2 0/1] " Oleg Nesterov
2014-01-03 19:55                                               ` [PATCH v2 1/1] " Oleg Nesterov
2014-01-03 21:00                                                 ` Andrew Morton
2014-01-04 16:43                                                   ` Oleg Nesterov
2014-01-08 11:54                                                     ` Mel Gorman
2014-01-08 13:14                                                       ` Mel Gorman
2014-01-08 16:13                                                       ` Oleg Nesterov
2014-01-08 18:02                                                         ` Mel Gorman
2014-01-08 19:04                                                           ` Oleg Nesterov
2014-01-09 11:27                                                             ` Mel Gorman
2014-01-09 14:04                                                               ` Oleg Nesterov
2014-01-09 18:52                                                                 ` Andrea Arcangeli
2014-01-09 19:43                                                                   ` Oleg Nesterov
2014-01-09 21:36                                                                     ` Andrea Arcangeli
2014-01-10 16:12                                                                       ` Oleg Nesterov
2014-01-10 16:50                                                                         ` Peter Zijlstra
2014-01-10 16:12                                                                 ` Mel Gorman
2013-12-20 14:19                                         ` [PATCH 0/1] " Martin Schwidefsky
2013-12-16 20:19                                   ` [PATCH 0/2] mm: thp: get_huge_page_tail() cleanups Oleg Nesterov
2013-12-16 20:19                                     ` [PATCH -mm 1/2] mm: thp: __get_page_tail_foll() can use get_huge_page_tail() Oleg Nesterov
2013-12-16 20:19                                     ` [PATCH -mm 2/2] mm: thp: turn compound_head() into BUG_ON(!PageTail) in get_huge_page_tail() Oleg Nesterov
2013-12-16 20:27                                     ` [PATCH 0/2] mm: thp: get_huge_page_tail() cleanups Andrea Arcangeli
2013-12-10 22:42                   ` process 'stuck' at exit Thomas Gleixner
2013-12-10 22:48                     ` Linus Torvalds
2013-12-10 22:58                       ` Darren Hart
2013-12-10 23:01                         ` Dave Jones
2013-12-10 23:00                       ` Dave Jones
2013-12-11  0:05                         ` Steven Rostedt
2013-12-11  0:23                           ` Dave Jones
2013-12-11  0:55                             ` Dave Jones
2013-12-14 20:17                               ` Oleg Nesterov
2013-12-11  4:09                       ` Dave Jones
2013-12-12  4:26                       ` Dave Jones
2013-12-12  5:29                         ` Darren Hart
2013-12-10 22:51                     ` Darren Hart
2013-12-10 23:24                     ` Al Viro
2013-12-11 16:31                     ` Mel Gorman
2013-12-11 16:38                       ` Thomas Gleixner
2013-12-11 17:57                         ` Mel Gorman
2013-12-12 19:00                 ` Andrea Arcangeli
2013-12-12 19:03                   ` Linus Torvalds
2013-12-10 22:09               ` Steven Rostedt
2013-12-10 22:16                 ` Dave Jones
2013-12-10 22:21                   ` Steven Rostedt
2013-12-10 22:27                     ` Dave Jones
2013-12-11  1:02     ` Mel Gorman
2013-12-10 20:57   ` Darren Hart
2013-12-10 21:09     ` Dave Jones

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20131218192005.GA8340@redhat.com \
    --to=oleg@redhat.com \
    --cc=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=davej@redhat.com \
    --cc=dvhart@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mgorman@suse.de \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).