Re: [PATCH] kasan: avoid unnecessary quarantine lock cycling

[Andrey Ryabinin]

Hui Su <sh_def@xxxxxxx> writes:

> kasan_quarantine_remove_cache() drops quarantine_lock and restores
> interrupts after scanning each non-empty quarantine batch so that
> cond_resched() can schedule during a long scan.
>
> When no reschedule is pending, the unlock/IRQ restore and subsequent
> IRQ save/relock cycle is unnecessary.
>
> Check need_resched() before cycling quarantine_lock to avoid this
> overhead when rescheduling is not needed.
>
> Suggested-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Hui Su <sh_def@xxxxxxx>
> ---
>  mm/kasan/quarantine.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/mm/kasan/quarantine.c b/mm/kasan/quarantine.c
> index 6958aa713c67..db7170f6f1b2 100644
> --- a/mm/kasan/quarantine.c
> +++ b/mm/kasan/quarantine.c
> @@ -368,10 +368,12 @@ void kasan_quarantine_remove_cache(struct kmem_cache *cache)
>  		if (qlist_empty(&global_quarantine[i]))
>  			continue;
>  		qlist_move_cache(&global_quarantine[i], &to_free, cache);
> -		/* Scanning whole quarantine can take a while. */
> -		raw_spin_unlock_irqrestore(&quarantine_lock, flags);
> -		cond_resched();
> -		raw_spin_lock_irqsave(&quarantine_lock, flags);
> +		if (need_resched()) {

I'm not sure about this one and inclined to agree with sashiko here.
See https://sashiko.dev/#/patchset/20260808191230.3345930-2-sh_def%40163.com

With local IRQs disabled, the local timer tick cannot run, so it cannot
set TIF_NEED_RESCHED due to time-slice expiration. Perhaps another CPU may set
the flag remotely, but AFAIU nothing guarantees that this will happen.

If need_resched() is initially false, it may remain false for the entire scan.
IRQs would then remain disabled throughout, potentially long enough to trigger
a HARDLOCKUP.

Current code should be fine, the bulk of the work is done by qlist_move_cache(),
and the unlock/reschedule/relock sequence occurs only after each
non-empty batch,
at most QUARANTINE_BATCHES times.

> +			/* Scanning whole quarantine can take a while. */
> +			raw_spin_unlock_irqrestore(&quarantine_lock, flags);
> +			cond_resched();
> +			raw_spin_lock_irqsave(&quarantine_lock, flags);
> +		}
>  	}
>  	raw_spin_unlock_irqrestore(&quarantine_lock, flags);
>
> --
> 2.43.0