lkml.org 
[lkml]   [2020]   [Jan]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    SubjectRe: [patch] mm, thp: fix defrag setting if newline is not used
    From
    Date
    On 1/15/20 2:58 AM, David Rientjes wrote:
    > If thp defrag setting "defer" is used and a newline is *not* used when
    > writing to the sysfs file, this is interpreted as the "defer+madvise"
    > option.
    >
    > This is because we do prefix matching and if five characters are written
    > without a newline, the current code ends up comparing to the first five
    > bytes of the "defer+madvise" option and using that instead.
    >
    > Find the length of what the user is writing and use that to guide our
    > decision on which string comparison to do.
    >
    > Fixes: 21440d7eb904 ("mm, thp: add new defer+madvise defrag option")
    > Signed-off-by: David Rientjes <rientjes@google.com>
    > ---
    > This can be done in *many* different ways including extracting logic to
    > a helper function. If someone would like this to be implemented
    > differently, please suggest it.

    I've come up with this:

    diff --git mm/huge_memory.c mm/huge_memory.c
    index 41a0fbddc96b..f36b93334874 100644
    --- mm/huge_memory.c
    +++ mm/huge_memory.c
    @@ -256,7 +256,7 @@ static ssize_t defrag_store(struct kobject *kobj,
    clear_bit(TRANSPARENT_HUGEPAGE_DEFRAG_KSWAPD_OR_MADV_FLAG, &transparent_hugepage_flags);
    clear_bit(TRANSPARENT_HUGEPAGE_DEFRAG_REQ_MADV_FLAG, &transparent_hugepage_flags);
    set_bit(TRANSPARENT_HUGEPAGE_DEFRAG_DIRECT_FLAG, &transparent_hugepage_flags);
    - } else if (!memcmp("defer+madvise", buf,
    + } else if (count > sizeof("defer")-1 && !memcmp("defer+madvise", buf,
    min(sizeof("defer+madvise")-1, count))) {
    clear_bit(TRANSPARENT_HUGEPAGE_DEFRAG_DIRECT_FLAG, &transparent_hugepage_flags);
    clear_bit(TRANSPARENT_HUGEPAGE_DEFRAG_KSWAPD_FLAG, &transparent_hugepage_flags);
    It's smaller, but more hacky. But it doesn't add new restrictions.
    E.g. this still works:

    # echo -n 'alw' > /sys/kernel/mm/transparent_hugepage/defrag
    # cat /sys/kernel/mm/transparent_hugepage/defrag
    [always] defer defer+madvise madvise never
    But whether anyone does that, I don't know (it doesn't work without -n).
    Also this still works:

    # echo -n 'defer ' > /sys/kernel/mm/transparent_hugepage/defrag
    # cat /sys/kernel/mm/transparent_hugepage/defrag
    always [defer] defer+madvise madvise never
    Ideally we would have had strict matching as you propose (no matching of prefixes)
    since the beginning and use e.g. strstrip() to remove all whitespace from buffer
    first. But it's 'const char *' and I'm not sure if it's null-terminated.

    \
     
     \ /
      Last update: 2020-01-15 13:45    [W:3.905 / U:0.028 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site