| Message ID | 155308075272.10600.3895589023886665456.stgit@localhost.localdomain (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm/list_lru: Simplify __list_lru_walk_one() | expand |
On Wed, 20 Mar 2019 14:19:27 +0300 Kirill Tkhai <ktkhai@virtuozzo.com> wrote: > 1)Spinlock must be locked in any case, so assert_spin_locked() > are moved above the switch; This isn't true. When the ->isolate() handler xfs_buftarg_wait_rele() (at least) returns LRU_SKIP, the lock is not held.
On 20.03.2019 21:52, Andrew Morton wrote: > On Wed, 20 Mar 2019 14:19:27 +0300 Kirill Tkhai <ktkhai@virtuozzo.com> wrote: > >> 1)Spinlock must be locked in any case, so assert_spin_locked() >> are moved above the switch; > > This isn't true. When the ->isolate() handler xfs_buftarg_wait_rele() > (at least) returns LRU_SKIP, the lock is not held. Oh, I missed that :( Thanks for pointing. Kirill
diff --git a/mm/list_lru.c b/mm/list_lru.c index 0730bf8ff39f..5f9fc84f1046 100644 --- a/mm/list_lru.c +++ b/mm/list_lru.c @@ -232,33 +232,26 @@ __list_lru_walk_one(struct list_lru_node *nlru, int memcg_idx, --*nr_to_walk; ret = isolate(item, l, &nlru->lock, cb_arg); + lockdep_assert_held(&nlru->lock); switch (ret) { case LRU_REMOVED_RETRY: - assert_spin_locked(&nlru->lock); - /* fall through */ case LRU_REMOVED: isolated++; nlru->nr_items--; + if (ret == LRU_REMOVED) + break; + /* fall through */ + case LRU_RETRY: /* - * If the lru lock has been dropped, our list - * traversal is now invalid and so we have to - * restart from scratch. + * The lru lock has been dropped, our list traversal is + * now invalid and so we have to restart from scratch. */ - if (ret == LRU_REMOVED_RETRY) - goto restart; - break; + goto restart; case LRU_ROTATE: list_move_tail(item, &l->list); break; case LRU_SKIP: break; - case LRU_RETRY: - /* - * The lru lock has been dropped, our list traversal is - * now invalid and so we have to restart from scratch. - */ - assert_spin_locked(&nlru->lock); - goto restart; default: BUG(); }
1)Spinlock must be locked in any case, so assert_spin_locked() are moved above the switch; 2)Replace assert_spin_locked() with lockdep_assert_held(), since it is enabled in debug kernel only and it does not affect on runtime in other cases; 3)Reorder switch cases to make duplicate comment not needed. Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com> --- mm/list_lru.c | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-)