Commit 6edda04ccc7c ("mm/kmemleak: prevent soft lockup in first object
iteration loop of kmemleak_scan()") adds cond_resched() in the first
object iteration loop of kmemleak_scan(). However, it turns that the
2nd objection iteration loop can still cause soft lockup to happen in
some cases. So add a cond_resched() call in the 2nd and 3rd loops as
well to prevent that and for completeness.
Signed-off-by: Waiman Long <[email protected]>
---
mm/kmemleak.c | 61 +++++++++++++++++++++++++++++++++++----------------
1 file changed, 42 insertions(+), 19 deletions(-)
diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index 1eddc0132f7f..613d34b57c5d 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -1463,6 +1463,27 @@ static void scan_gray_list(void)
WARN_ON(!list_empty(&gray_list));
}
+/*
+ * Conditionally call resched() in a object iteration loop while making sure
+ * that the given object won't go away without RCU read lock by performing a
+ * get_object() if !pinned.
+ *
+ * Return: false if can't do a cond_resched() due to get_object() failure
+ * true otherwise
+ */
+static bool kmemleak_cond_resched(struct kmemleak_object *object, bool pinned)
+{
+ if (!pinned && !get_object(object))
+ return false;
+
+ rcu_read_unlock();
+ cond_resched();
+ rcu_read_lock();
+ if (!pinned)
+ put_object(object);
+ return true;
+}
+
/*
* Scan data sections and all the referenced memory blocks allocated via the
* kernel's standard allocators. This function must be called with the
@@ -1474,7 +1495,7 @@ static void kmemleak_scan(void)
struct zone *zone;
int __maybe_unused i;
int new_leaks = 0;
- int loop1_cnt = 0;
+ int loop_cnt = 0;
jiffies_last_scan = jiffies;
@@ -1483,7 +1504,6 @@ static void kmemleak_scan(void)
list_for_each_entry_rcu(object, &object_list, object_list) {
bool obj_pinned = false;
- loop1_cnt++;
raw_spin_lock_irq(&object->lock);
#ifdef DEBUG
/*
@@ -1517,24 +1537,11 @@ static void kmemleak_scan(void)
raw_spin_unlock_irq(&object->lock);
/*
- * Do a cond_resched() to avoid soft lockup every 64k objects.
- * Make sure a reference has been taken so that the object
- * won't go away without RCU read lock.
+ * Do a cond_resched() every 64k objects to avoid soft lockup.
*/
- if (!(loop1_cnt & 0xffff)) {
- if (!obj_pinned && !get_object(object)) {
- /* Try the next object instead */
- loop1_cnt--;
- continue;
- }
-
- rcu_read_unlock();
- cond_resched();
- rcu_read_lock();
-
- if (!obj_pinned)
- put_object(object);
- }
+ if (!(++loop_cnt & 0xffff) &&
+ !kmemleak_cond_resched(object, obj_pinned))
+ loop_cnt--; /* Try again on next object */
}
rcu_read_unlock();
@@ -1601,7 +1608,15 @@ static void kmemleak_scan(void)
* scan and color them gray until the next scan.
*/
rcu_read_lock();
+ loop_cnt = 0;
list_for_each_entry_rcu(object, &object_list, object_list) {
+ /*
+ * Do a cond_resched() every 64k objects to avoid soft lockup.
+ */
+ if (!(++loop_cnt & 0xffff) &&
+ !kmemleak_cond_resched(object, false))
+ loop_cnt--; /* Try again on next object */
+
/*
* This is racy but we can save the overhead of lock/unlock
* calls. The missed objects, if any, should be caught in
@@ -1635,7 +1650,15 @@ static void kmemleak_scan(void)
* Scanning result reporting.
*/
rcu_read_lock();
+ loop_cnt = 0;
list_for_each_entry_rcu(object, &object_list, object_list) {
+ /*
+ * Do a cond_resched() every 64k objects to avoid soft lockup.
+ */
+ if (!(++loop_cnt & 0xffff) &&
+ !kmemleak_cond_resched(object, false))
+ loop_cnt--; /* Try again on next object */
+
/*
* This is racy but we can save the overhead of lock/unlock
* calls. The missed objects, if any, should be caught in
--
2.31.1
On Thu, 20 Oct 2022 13:56:19 -0400 Waiman Long <[email protected]> wrote:
> Commit 6edda04ccc7c ("mm/kmemleak: prevent soft lockup in first object
> iteration loop of kmemleak_scan()") adds cond_resched() in the first
> object iteration loop of kmemleak_scan(). However, it turns that the
> 2nd objection iteration loop can still cause soft lockup to happen in
> some cases. So add a cond_resched() call in the 2nd and 3rd loops as
> well to prevent that and for completeness.
>
Seems reasonable, although not an object of beauty.
We didn't include a Fixes: or cc:stable in 6edda04ccc7c, even though it
addresses softlockups. I think I'll add a cc:stable to this, with a
Fixes:6edda04ccc7c. So any kenrel which included 6edda04ccc7c will
hopefully add this as well.
On 10/20/22 20:58, Andrew Morton wrote:
> On Thu, 20 Oct 2022 13:56:19 -0400 Waiman Long <[email protected]> wrote:
>
>> Commit 6edda04ccc7c ("mm/kmemleak: prevent soft lockup in first object
>> iteration loop of kmemleak_scan()") adds cond_resched() in the first
>> object iteration loop of kmemleak_scan(). However, it turns that the
>> 2nd objection iteration loop can still cause soft lockup to happen in
>> some cases. So add a cond_resched() call in the 2nd and 3rd loops as
>> well to prevent that and for completeness.
>>
> Seems reasonable, although not an object of beauty.
>
> We didn't include a Fixes: or cc:stable in 6edda04ccc7c, even though it
> addresses softlockups. I think I'll add a cc:stable to this, with a
> Fixes:6edda04ccc7c. So any kenrel which included 6edda04ccc7c will
> hopefully add this as well.
That sounds good to me. I consider commit 6edda04ccc7c incomplete and
this patch complete it. Technically it can be considered a fix.
Thanks,
Longman
On Thu, Oct 20, 2022 at 01:56:19PM -0400, Waiman Long wrote:
> Commit 6edda04ccc7c ("mm/kmemleak: prevent soft lockup in first object
> iteration loop of kmemleak_scan()") adds cond_resched() in the first
> object iteration loop of kmemleak_scan(). However, it turns that the
> 2nd objection iteration loop can still cause soft lockup to happen in
> some cases. So add a cond_resched() call in the 2nd and 3rd loops as
> well to prevent that and for completeness.
>
> Signed-off-by: Waiman Long <[email protected]>
Acked-by: Catalin Marinas <[email protected]>