Re: [PATCH 1/2] mm: KSM: fix ksm_run data type

From: Matthew Wilcox
Date: Thu Jul 15 2021 - 14:31:15 EST


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