Re: [PATCH v2 2/2] pseries/hotplug-memory: leverage xarray API to simplify code

[Nathan Lynch <nathanl@linux.ibm.com>]

Hi Scott,

Scott Cheloha <cheloha@linux.ibm.com> writes:
> -#define for_each_drmem_lmb_in_range(lmb, start, end)		\
> -	for ((lmb) = (start); (lmb) <= (end); (lmb)++)
> -
> -#define for_each_drmem_lmb(lmb)					\
> -	for_each_drmem_lmb_in_range((lmb),			\
> -		&drmem_info->lmbs[0],				\
> -		&drmem_info->lmbs[drmem_info->n_lmbs - 1])

A couple things.

This will conflict with "powerpc/pseries: Avoid NULL pointer dereference
when drmem is unavailable" which is in linuxppc/next-test:
   
   https://patchwork.ozlabs.org/patch/1231904/

Regardless, I don't think trading the iterator macros for open-coded
loops improve the code:

> -	for_each_drmem_lmb(lmb) {
> +	for (i = 0; i < drmem_info->n_lmbs; i++) {
> +		lmb = &drmem_info->lmbs[i];

[...]

> +struct xarray;
> +extern struct xarray *drmem_lmb_xa;

drmem_lmb_xa should go in the drmem_info structure if you can't make it
static in drmem.c.

>  
>  /*
>   * The of_drconf_cell_v1 struct defines the layout of the LMB data
> @@ -71,23 +66,6 @@ static inline u32 drmem_lmb_size(void)
>  	return drmem_info->lmb_size;
>  }
>  
> -#define DRMEM_LMB_RESERVED	0x80000000
> -
> -static inline void drmem_mark_lmb_reserved(struct drmem_lmb *lmb)
p> -{
> -	lmb->flags |= DRMEM_LMB_RESERVED;
> -}
> -
> -static inline void drmem_remove_lmb_reservation(struct drmem_lmb *lmb)
> -{
> -	lmb->flags &= ~DRMEM_LMB_RESERVED;
> -}
> -
> -static inline bool drmem_lmb_reserved(struct drmem_lmb *lmb)
> -{
> -	return lmb->flags & DRMEM_LMB_RESERVED;
> -}

The flag management is logically separate from the iterator changes, so
splitting that out would ease review.

Looking further... yes, this needs to be a series of smaller changes
please.