linux-kernel - Re: [PATCH -v2 01/10] swap: Change SWAPFILE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 2 Sep 2016 13:30:16 -0700
From:   Andrew Morton <akpm@...ux-foundation.org>
To:     "Huang\, Ying" <ying.huang@...el.com>
Cc:     <tim.c.chen@...el.com>, <dave.hansen@...el.com>,
        <andi.kleen@...el.com>, <aaron.lu@...el.com>, <linux-mm@...ck.org>,
        <linux-kernel@...r.kernel.org>, Hugh Dickins <hughd@...gle.com>,
        Shaohua Li <shli@...nel.org>, Minchan Kim <minchan@...nel.org>,
        Rik van Riel <riel@...hat.com>
Subject: Re: [PATCH -v2 01/10] swap: Change SWAPFILE_CLUSTER to 512

On Thu, 01 Sep 2016 16:04:57 -0700 "Huang\, Ying" <ying.huang@...el.com> 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?