[PATCH v3] powerpc/pseries: explicitly reschedule during drmem_lmb list traversal

[Christophe Leroy]

Le 13/08/2020 à 17:11, Nathan Lynch a écrit :
> The drmem lmb list can have hundreds of thousands of entries, and
> unfortunately lookups take the form of linear searches. As long as
> this is the case, traversals have the potential to monopolize the CPU
> and provoke lockup reports, workqueue stalls, and the like unless
> they explicitly yield.
> 
> Rather than placing cond_resched() calls within various
> for_each_drmem_lmb() loop blocks in the code, put it in the iteration
> expression of the loop macro itself so users can't omit it.
> 
> Introduce a drmem_lmb_next() iteration helper function which calls
> cond_resched() at a regular interval during array traversal. Each
> iteration of the loop in DLPAR code paths can involve around ten RTAS
> calls which can each take up to 250us, so this ensures the check is
> performed at worst every few milliseconds.
> 
> Fixes: 6c6ea53725b3 ("powerpc/mm: Separate ibm, dynamic-memory data from DT format")
> Signed-off-by: Nathan Lynch <nathanl at linux.ibm.com>

Looks a lot better to me than v2.

Reviewed-by: Christophe Leroy <christophe.leroy at csgroup.eu>

> ---
> 
> Notes:
>      Changes since v2:
>      * Make drmem_lmb_next() more general.
>      * Adjust reschedule interval for better code generation.
>      * Add commentary to drmem_lmb_next() to explain the cond_resched()
>        call.
>      * Remove bounds assertions.
>      
>      Changes since v1:
>      * Add bounds assertions in drmem_lmb_next().
>      * Call cond_resched() in the iterator on only every 20th element
>        instead of on every iteration, to reduce overhead in tight loops.
> 
>   arch/powerpc/include/asm/drmem.h | 18 +++++++++++++++++-
>   1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/include/asm/drmem.h b/arch/powerpc/include/asm/drmem.h
> index 17ccc6474ab6..6fb928605ed1 100644
> --- a/arch/powerpc/include/asm/drmem.h
> +++ b/arch/powerpc/include/asm/drmem.h
> @@ -8,6 +8,8 @@
>   #ifndef _ASM_POWERPC_LMB_H
>   #define _ASM_POWERPC_LMB_H
>   
> +#include <linux/sched.h>
> +
>   struct drmem_lmb {
>   	u64     base_addr;
>   	u32     drc_index;
> @@ -26,8 +28,22 @@ struct drmem_lmb_info {
>   
>   extern struct drmem_lmb_info *drmem_info;
>   
> +static inline struct drmem_lmb *drmem_lmb_next(struct drmem_lmb *lmb,
> +					       const struct drmem_lmb *start)
> +{
> +	/*
> +	 * DLPAR code paths can take several milliseconds per element
> +	 * when interacting with firmware. Ensure that we don't
> +	 * unfairly monopolize the CPU.
> +	 */
> +	if (((++lmb - start) % 16) == 0)
> +		cond_resched();
> +
> +	return lmb;
> +}
> +
>   #define for_each_drmem_lmb_in_range(lmb, start, end)		\
> -	for ((lmb) = (start); (lmb) < (end); (lmb)++)
> +	for ((lmb) = (start); (lmb) < (end); lmb = drmem_lmb_next(lmb, start))
>   
>   #define for_each_drmem_lmb(lmb)					\
>   	for_each_drmem_lmb_in_range((lmb),			\
>