Messages in this thread | | | From | KOSAKI Motohiro <> | Date | Mon, 7 Apr 2014 18:14:01 -0400 | Subject | Re: [PATCH 2/5] vrange: Add purged page detection on setting memory non-volatile |
| |
>> This change hwpoison and migration tag number. maybe ok, maybe not. > > Though depending on config can't these tag numbers change anyway?
I don't think distro disable any of these.
>> I'd suggest to use younger number than hwpoison. >> (That's why hwpoison uses younger number than migration) > > So I can, but the way these are defined makes the results seem pretty > terrible: > > #define SWP_MIGRATION_WRITE (MAX_SWAPFILES + SWP_HWPOISON_NUM \ > + SWP_MVOLATILE_PURGED_NUM + 1) > > Particularly when: > #define MAX_SWAPFILES ((1 << MAX_SWAPFILES_SHIFT) \ > - SWP_MIGRATION_NUM \ > - SWP_HWPOISON_NUM \ > - SWP_MVOLATILE_PURGED_NUM \ > ) > > Its a lot of unnecessary mental gymnastics. Yuck. > > Would a general cleanup like the following be ok to try to make this > more extensible? > > thanks > -john > > diff --git a/include/linux/swap.h b/include/linux/swap.h > index 3507115..21387df 100644 > --- a/include/linux/swap.h > +++ b/include/linux/swap.h > @@ -49,29 +49,38 @@ static inline int current_is_kswapd(void) > * actions on faults. > */ > > +enum { > + /* > + * NOTE: We use the high bits here (subtracting from > + * 1<<MAX_SWPFILES_SHIFT), so to preserve the values insert > + * new entries here at the top of the enum, not at the bottom > + */ > +#ifdef CONFIG_MEMORY_FAILURE > + SWP_HWPOISON_NR, > +#endif > +#ifdef CONFIG_MIGRATION > + SWP_MIGRATION_READ_NR, > + SWP_MIGRATION_WRITE_NR, > +#endif > + SWP_MAX_NR, > +}; > +#define MAX_SWAPFILES ((1 << MAX_SWAPFILES_SHIFT) - SWP_MAX_NR) > +
I don't see any benefit of this code. At least, SWP_MAX_NR is suck. The name doesn't match the actual meanings.
| |