All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To: linux-mm@kvack.org
Cc: tglx@linutronix.de, Andrew Morton <akpm@linux-foundation.org>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Subject: [PATCH 3/3] mm: list_lru: Add lock_irq member to __list_lru_init()
Date: Fri, 22 Jun 2018 17:12:21 +0200	[thread overview]
Message-ID: <20180622151221.28167-4-bigeasy@linutronix.de> (raw)
In-Reply-To: <20180622151221.28167-1-bigeasy@linutronix.de>

scan_shadow_nodes() is the only user of __list_lru_walk_one() which
disables interrupts before invoking it. The reason is that nlru->lock is
nesting inside IRQ-safe i_pages lock. Some functions unconditionally
acquire the lock with the _irq() suffix.

__list_lru_walk_one() can't acquire the lock unconditionally with _irq()
suffix because it might invoke a callback which unlocks the nlru->lock
and invokes a sleeping function without enabling interrupts.

Add an argument to __list_lru_init() which identifies wheather the
nlru->lock needs to be acquired with disabling interrupts or without.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 include/linux/list_lru.h | 12 ++++++++----
 mm/list_lru.c            | 14 ++++++++++----
 mm/workingset.c          | 12 ++++--------
 3 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/include/linux/list_lru.h b/include/linux/list_lru.h
index 96def9d15b1b..c2161c3a1809 100644
--- a/include/linux/list_lru.h
+++ b/include/linux/list_lru.h
@@ -51,18 +51,22 @@ struct list_lru_node {
 
 struct list_lru {
 	struct list_lru_node	*node;
+	bool			lock_irq;
 #if defined(CONFIG_MEMCG) && !defined(CONFIG_SLOB)
 	struct list_head	list;
 #endif
 };
 
 void list_lru_destroy(struct list_lru *lru);
-int __list_lru_init(struct list_lru *lru, bool memcg_aware,
+int __list_lru_init(struct list_lru *lru, bool memcg_aware, bool lock_irq,
 		    struct lock_class_key *key);
 
-#define list_lru_init(lru)		__list_lru_init((lru), false, NULL)
-#define list_lru_init_key(lru, key)	__list_lru_init((lru), false, (key))
-#define list_lru_init_memcg(lru)	__list_lru_init((lru), true, NULL)
+#define list_lru_init(lru)		__list_lru_init((lru), false, false, \
+							NULL)
+#define list_lru_init_key(lru, key)	__list_lru_init((lru), false, false, \
+							(key))
+#define list_lru_init_memcg(lru)	__list_lru_init((lru), true, false, \
+							NULL)
 
 int memcg_update_all_list_lrus(int num_memcgs);
 void memcg_drain_all_list_lrus(int src_idx, int dst_idx);
diff --git a/mm/list_lru.c b/mm/list_lru.c
index fcfb6c89ed47..1c49d48078e4 100644
--- a/mm/list_lru.c
+++ b/mm/list_lru.c
@@ -204,7 +204,10 @@ __list_lru_walk_one(struct list_lru *lru, int nid, int memcg_idx,
 	struct list_head *item, *n;
 	unsigned long isolated = 0;
 
-	spin_lock(&nlru->lock);
+	if (lru->lock_irq)
+		spin_lock_irq(&nlru->lock);
+	else
+		spin_lock(&nlru->lock);
 	l = list_lru_from_memcg_idx(nlru, memcg_idx);
 restart:
 	list_for_each_safe(item, n, &l->list) {
@@ -251,7 +254,10 @@ __list_lru_walk_one(struct list_lru *lru, int nid, int memcg_idx,
 		}
 	}
 
-	spin_unlock(&nlru->lock);
+	if (lru->lock_irq)
+		spin_unlock_irq(&nlru->lock);
+	else
+		spin_unlock(&nlru->lock);
 	return isolated;
 }
 
@@ -553,7 +559,7 @@ static void memcg_destroy_list_lru(struct list_lru *lru)
 }
 #endif /* CONFIG_MEMCG && !CONFIG_SLOB */
 
-int __list_lru_init(struct list_lru *lru, bool memcg_aware,
+int __list_lru_init(struct list_lru *lru, bool memcg_aware, bool lock_irq,
 		    struct lock_class_key *key)
 {
 	int i;
@@ -580,7 +586,7 @@ int __list_lru_init(struct list_lru *lru, bool memcg_aware,
 		lru->node = NULL;
 		goto out;
 	}
-
+	lru->lock_irq = lock_irq;
 	list_lru_register(lru);
 out:
 	memcg_put_cache_ids();
diff --git a/mm/workingset.c b/mm/workingset.c
index 529480c21f93..23ce00f48212 100644
--- a/mm/workingset.c
+++ b/mm/workingset.c
@@ -480,13 +480,8 @@ static enum lru_status shadow_lru_isolate(struct list_head *item,
 static unsigned long scan_shadow_nodes(struct shrinker *shrinker,
 				       struct shrink_control *sc)
 {
-	unsigned long ret;
-
-	/* list_lru lock nests inside the IRQ-safe i_pages lock */
-	local_irq_disable();
-	ret = list_lru_shrink_walk(&shadow_nodes, sc, shadow_lru_isolate, NULL);
-	local_irq_enable();
-	return ret;
+	return list_lru_shrink_walk(&shadow_nodes, sc, shadow_lru_isolate,
+				    NULL);
 }
 
 static struct shrinker workingset_shadow_shrinker = {
@@ -523,7 +518,8 @@ static int __init workingset_init(void)
 	pr_info("workingset: timestamp_bits=%d max_order=%d bucket_order=%u\n",
 	       timestamp_bits, max_order, bucket_order);
 
-	ret = __list_lru_init(&shadow_nodes, true, &shadow_nodes_key);
+	/* list_lru lock nests inside the IRQ-safe i_pages lock */
+	ret = __list_lru_init(&shadow_nodes, true, true, &shadow_nodes_key);
 	if (ret)
 		goto err;
 	ret = register_shrinker(&workingset_shadow_shrinker);
-- 
2.18.0

  parent reply	other threads:[~2018-06-22 15:12 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-22 15:12 [PATCH 0/3] mm: use irq locking suffix instead local_irq_disable() Sebastian Andrzej Siewior
2018-06-22 15:12 ` [PATCH 1/3] mm: workingset: remove local_irq_disable() from count_shadow_nodes() Sebastian Andrzej Siewior
2018-06-24 19:51   ` Vladimir Davydov
2018-06-25 10:36   ` Kirill Tkhai
2018-06-22 15:12 ` [PATCH 2/3] mm: workingset: make shadow_lru_isolate() use locking suffix Sebastian Andrzej Siewior
2018-06-24 19:57   ` Vladimir Davydov
2018-06-26 21:25     ` Sebastian Andrzej Siewior
2018-06-27  8:50       ` Vladimir Davydov
2018-06-27  9:20         ` Sebastian Andrzej Siewior
2018-06-28  9:30           ` Vladimir Davydov
2018-07-02 22:38             ` Sebastian Andrzej Siewior
2018-06-22 15:12 ` Sebastian Andrzej Siewior [this message]
2018-06-24 20:09   ` [PATCH 3/3] mm: list_lru: Add lock_irq member to __list_lru_init() Vladimir Davydov
2018-07-03 14:52     ` Sebastian Andrzej Siewior
2018-07-03 14:52       ` [PATCH 1/4] mm/list_lru: use list_lru_walk_one() in list_lru_walk_node() Sebastian Andrzej Siewior
2018-07-03 14:52       ` [PATCH 2/4] mm/list_lru: Move locking from __list_lru_walk_one() to its caller Sebastian Andrzej Siewior
2018-07-03 14:52       ` [PATCH 3/4] mm/list_lru: Pass struct list_lru_node as an argument __list_lru_walk_one() Sebastian Andrzej Siewior
2018-07-03 14:52       ` [PATCH 4/4] mm/list_lru: Introduce list_lru_shrink_walk_irq() Sebastian Andrzej Siewior
2018-07-03 21:14       ` Andrew Morton
2018-07-03 21:44         ` Re: Sebastian Andrzej Siewior
2018-07-04 14:44           ` Re: Vladimir Davydov
2018-06-22 21:39 ` [PATCH 0/3] mm: use irq locking suffix instead local_irq_disable() Andrew Morton
2018-06-24 20:10   ` Vladimir Davydov

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=20180622151221.28167-4-bigeasy@linutronix.de \
    --to=bigeasy@linutronix.de \
    --cc=akpm@linux-foundation.org \
    --cc=linux-mm@kvack.org \
    --cc=tglx@linutronix.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.