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

[Jan Kara]

On Wed 28-10-26 14:16:28, 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. Once the shrink has run for one
> second, call cond_resched() so the CPU can be scheduled before the
> scan continues.
> 
> 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..c9c70db5eb71 100644
> --- a/fs/ext4/extents_status.c
> +++ b/fs/ext4/extents_status.c
> @@ -1679,6 +1679,7 @@ static int __es_shrink(struct ext4_sb_info *sbi, int nr_to_scan,
>  	int nr_to_walk;
>  	int nr_shrunk = 0;
>  	int retried = 0, nr_skipped = 0;
> +	unsigned long end_jiffies = jiffies + HZ;
>  
>  	es_stats = &sbi->s_es_stats;
>  	start_time = ktime_get();
> @@ -1719,6 +1720,15 @@ static int __es_shrink(struct ext4_sb_info *sbi, int nr_to_scan,
>  		nr_shrunk += es_reclaim_extents(ei, &nr_to_scan);
>  		write_unlock(&ei->i_es_lock);
>  
> +		/*
> +		 * Call cond_resched to avoid soft lockup if a kernel with
> +		 * preempt disabled.
> +		 */
> +		if (time_after(jiffies, end_jiffies)) {
> +			cond_resched();
> +			end_jiffies = jiffies + HZ;
> +		}
> +

No need for this strange timing logic. Just add to this place (after the
nr_to_scan check):

	cond_resched_tasks_rcu_qs()

as we should also report RCU quiescent period here. Also I believe there
are some fixes to __es_shrink() flying around to fix nr_to_scan handling
because normally nr_to_scan should never be so large to trigger softlockups
(but __es_shrink() wasn't always obeying passed nr_to_scan).

								Honza

>  		if (nr_to_scan <= 0)
>  			goto out;
>  		spin_lock(&sbi->s_es_lock);
> -- 
> 2.43.7
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR