Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 15 Jul 2021 19:30:13 +0100
From:   Matthew Wilcox <willy@infradead.org>
To:     Pavel Tatashin <pasha.tatashin@soleen.com>
Cc:     Zhansaya Bagdauletkyzy <zhansayabagdaulet@gmail.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Tyler Hicks <tyhicks@linux.microsoft.com>,
        linux-mm <linux-mm@kvack.org>,
        LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/2] mm: KSM: fix ksm_run data type
Message-ID: <YPB+tT0AcOB4UZQ5@casper.infradead.org>
References: <cover.1626371112.git.zhansayabagdaulet@gmail.com>
 <343394260f599d940cacc37f1dcc0309239ae220.1626371112.git.zhansayabagdaulet@gmail.com>
 <YPB7rBlU1SinK6FR@casper.infradead.org>
 <CA+CK2bCPMmbr+=h4evTkbJoEFQu_th_NOe0Gp11hU7xz3fLZ8A@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CA+CK2bCPMmbr+=h4evTkbJoEFQu_th_NOe0Gp11hU7xz3fLZ8A@mail.gmail.com>
Precedence: bulk

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@infradead.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.