linux-kernel - Re: [PATCH 1/2] mm: KSM: fix ksm

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <YPB+tT0AcOB4UZQ5@casper.infradead.org>
Date:   Thu, 15 Jul 2021 19:30:13 +0100
From:   Matthew Wilcox <willy@...radead.org>
To:     Pavel Tatashin <pasha.tatashin@...een.com>
Cc:     Zhansaya Bagdauletkyzy <zhansayabagdaulet@...il.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Tyler Hicks <tyhicks@...ux.microsoft.com>,
        linux-mm <linux-mm@...ck.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] mm: KSM: fix ksm_run data type

On Thu, Jul 15, 2021 at 02:21:21PM -0400, Pavel Tatashin wrote:
> On Thu, Jul 15, 2021 at 2:18 PM Matthew Wilcox <willy@...radead.org> wrote:
> >
> > On Fri, Jul 16, 2021 at 12:01:01AM +0600, Zhansaya Bagdauletkyzy wrote:
> > > +++ b/mm/ksm.c
> > > @@ -289,7 +289,7 @@ static int ksm_nr_node_ids = 1;
> > >  #define KSM_RUN_MERGE        1
> > >  #define KSM_RUN_UNMERGE      2
> > >  #define KSM_RUN_OFFLINE      4
> > > -static unsigned long ksm_run = KSM_RUN_STOP;
> > > +static unsigned int ksm_run = KSM_RUN_STOP;
> >
> > Should this be an enum instead?
> 
> I think "unsigned int" is OK here, as it is exposed as uint to users:
> Documentation/ABI/testing/sysfs-kernel-mm-ksm
> 
> /sys/kernel/mm/ksm/run
> 
> run: write 0 to disable ksm, read 0 while ksm is disabled.
> 
> - write 1 to run ksm, read 1 while ksm is running.
> - write 2 to disable ksm and unmerge all its pages.

The document is out of date then as it does not mention 'offline'.

Also, why does the call to kstrtouint() specify base 10?  If it is a
bitmap, then permitting 0x [1] is more natural.  I would expect to see
base 0 there.

[1] or even 0b, although I see that _parse_integer_fixup_radix does not
support the 0b notation.