Re: [PATCH] mm, compaction: properly signal and act upon lock and need_sched() contention

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 05/12/2014 05:34 PM, Naoya Horiguchi wrote:
On Mon, May 12, 2014 at 04:15:11PM +0200, Vlastimil Babka wrote:
Compaction uses compact_checklock_irqsave() function to periodically check for
lock contention and need_resched() to either abort async compaction, or to
free the lock, schedule and retake the lock. When aborting, cc->contended is
set to signal the contended state to the caller. Two problems have been
identified in this mechanism.

First, compaction also calls directly cond_resched() in both scanners when no
lock is yet taken. This call either does not abort async compaction, or set
cc->contended appropriately. This patch introduces a new
compact_check_resched() function to achieve both.

Second, isolate_freepages() does not check if isolate_freepages_block()
aborted due to contention, and advances to the next pageblock. This violates
the principle of aborting on contention, and might result in pageblocks not
being scanned completely, since the scanning cursor is advanced. This patch
makes isolate_freepages_block() check the cc->contended flag and abort.

Reported-by: Joonsoo Kim <iamjoonsoo.kim@xxxxxxx>
Signed-off-by: Vlastimil Babka <vbabka@xxxxxxx>
Cc: Minchan Kim <minchan@xxxxxxxxxx>
Cc: Mel Gorman <mgorman@xxxxxxx>
Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@xxxxxxxxxxx>
Cc: Michal Nazarewicz <mina86@xxxxxxxxxx>
Cc: Naoya Horiguchi <n-horiguchi@xxxxxxxxxxxxx>
Cc: Christoph Lameter <cl@xxxxxxxxx>
Cc: Rik van Riel <riel@xxxxxxxxxx>
---
  mm/compaction.c | 40 +++++++++++++++++++++++++++++++++-------
  1 file changed, 33 insertions(+), 7 deletions(-)

diff --git a/mm/compaction.c b/mm/compaction.c
index 83ca6f9..b34ab7c 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -222,6 +222,27 @@ static bool compact_checklock_irqsave(spinlock_t *lock, unsigned long *flags,
  	return true;
  }

+/*
+ * Similar to compact_checklock_irqsave() (see its comment) for places where
+ * a zone lock is not concerned.
+ *
+ * Returns false when compaction should abort.
+ */
+static inline bool compact_check_resched(struct compact_control *cc)
+{
+	/* async compaction aborts if contended */
+	if (need_resched()) {
+		if (cc->mode == MIGRATE_ASYNC) {
+			cc->contended = true;

This changes the meaning of contended in struct compact_control (not just
indicating lock contention,) so please update the comment in mm/internal.h too.

It doesn't change it, since compact_checklock_irqsave() already has this semantic:
if (should_release_lock(lock) && cc->mode == MIGRATE_ASYNC)
	cc->contended = true;

and should_release_lock() is:
	need_resched() || spin_is_contended(lock)

So the comment was already outdated, I will update it in v2.

Reviewed-by: Naoya Horiguchi <n-horiguchi@xxxxxxxxxxxxx>

Thanks.

Thanks,
Naoya Horiguchi


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]