Re: [PATCH v2] ext4: fix soft lockup while shrinking the extent status cache

[Jan Kara]

On Wed 04-11-26 11:35:39, Jianping Liu wrote:
> From: Jianping Liu <frankjpliu@xxxxxxxxxxx>
> 
> __es_shrink() walks sbi->s_es_list and reclaims extents from each
> inode. On a non-preemptible kernel that walk never gives up the CPU.
> 
> There is a soft lockup report on our company's machine. The machine
> with millions of active inodes, 256 CPUs, and heavy memory pressure,
> concurrent shrinkers run too long.
> 
> i_es_lock is dropped after es_reclaim_extents(), and s_es_lock is not
> taken again until the next inode. When nr_to_scan remains, call
> cond_resched_tasks_rcu_qs() before continuing the scan.
> 
> cond_resched() does not report the RCU quiescent state needed here.
> It is compiled out when CONFIG_PREEMPTION=y. Otherwise it schedules
> through preempt_schedule(), and RCU records that switch as a
> preemption. An RCU-tasks read-side section ends only at a voluntary
> context switch, cond_resched_tasks_rcu_qs(), idle, or return to
> userspace. The helper reports that voluntary quiescent state and still
> lets a non-preemptible kernel reschedule.
> 
> Changes from v1:
> - Delete time out logic.
> - Change "cond_resched" to "cond_resched_tasks_rcu_qs".

These three lines belong below the ---. Otherwise feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

								Honza

> 
> Link: https://patchwork.ozlabs.org/project/linux-ext4/patch/20261028061628.1209508-1-frankljpliu@xxxxxxxxx/
> Signed-off-by: Jianping Liu <frankjpliu@xxxxxxxxxxx>
> ---
>  fs/ext4/extents_status.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
> index 018b6146b9d8..62346eaf4350 100644
> --- a/fs/ext4/extents_status.c
> +++ b/fs/ext4/extents_status.c
> @@ -1721,6 +1721,16 @@ static int __es_shrink(struct ext4_sb_info *sbi, int nr_to_scan,
>  
>  		if (nr_to_scan <= 0)
>  			goto out;
> +
> +		/*
> +		 * Report a voluntary RCU-tasks quiescent state before continuing.
> +		 * cond_resched() is not enough: it is compiled out under
> +		 * CONFIG_PREEMPTION, and otherwise switches via preemption, which
> +		 * does not end an RCU-tasks read-side section.
> +		 * This also lets a non-preemptible kernel reschedule.
> +		 */
> +		cond_resched_tasks_rcu_qs();
> +
>  		spin_lock(&sbi->s_es_lock);
>  	}
>  	spin_unlock(&sbi->s_es_lock);
> -- 
> 2.43.7
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR