[PATCH v2 2/2] pseries/hotplug-memory: leverage xarray API to simplify code
Nathan Lynch
nathanl at linux.ibm.com
Sat Feb 22 07:03:02 AEDT 2020
Hi Scott,
Scott Cheloha <cheloha at 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.
More information about the Linuxppc-dev
mailing list