Re: [PATCH -v2 01/10] swap: Change SWAPFILE_CLUSTER to 512

From: Andrew Morton
Date: Fri Sep 02 2016 - 16:30:23 EST


On Thu, 01 Sep 2016 16:04:57 -0700 "Huang\, Ying" <ying.huang@xxxxxxxxx> wrote:

> >> }
> >>
> >> -#define SWAPFILE_CLUSTER 256
> >> +#define SWAPFILE_CLUSTER 512
> >> #define LATENCY_LIMIT 256
> >>
> >
> > What happens to architectures which have different HPAGE_SIZE and/or
> > PAGE_SIZE?
>
> For the architecture with HPAGE_SIZE / PAGE_SIZE == 512 (for example
> x86_64), the huge page swap optimizing will be turned on. For other
> architectures, it will be turned off as before.
>
> This mostly because I don't know whether it is a good idea to turn on
> THP swap optimizing for the architectures other than x86_64. For
> example, it appears that the huge page size is 8M (1<<23) on SPARC. But
> I don't know whether 8M is too big for a swap cluster. And it appears
> that the huge page size could be as large as 512M on MIPS.

This doesn't sounds very organized. If some architecture with some
config happens to have HPAGE_SIZE / PAGE_SIZE == 512 then the feature
will be turned on; otherwise it will be turned off. Nobody will even
notice that it happened.

Would it not be better to do

#ifdef CONFIG_SOMETHING
#define SWAPFILE_CLUSTER (HPAGE_SIZE / PAGE_SIZE)
#else
#define SWAPFILE_CLUSTER 256
#endif

and, by using CONFIG_SOMETHING in the other appropriate places, enable
the feature in the usual fashion?