[PATCH] mm/list_lru: Simplify __list_lru_walk_one()

[PATCH] mm/list_lru: Simplify __list_lru_walk_one()

[Kirill Tkhai]

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(-)

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();
 		}

Re: [PATCH] mm/list_lru: Simplify __list_lru_walk_one()

[Andrew Morton]

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.

Re: [PATCH] mm/list_lru: Simplify __list_lru_walk_one()

[Kirill Tkhai]

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