linux-kernel - Re: [patch] mm, thp: add new background defrag option

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: <20170105155053.GW21618@dhcp22.suse.cz>
Date:   Thu, 5 Jan 2017 16:50:54 +0100
From:   Michal Hocko <mhocko@...nel.org>
To:     Vlastimil Babka <vbabka@...e.cz>
Cc:     Mel Gorman <mgorman@...hsingularity.net>,
        David Rientjes <rientjes@...gle.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Jonathan Corbet <corbet@....net>,
        "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
        linux-kernel@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [patch] mm, thp: add new background defrag option

On Thu 05-01-17 14:58:47, Vlastimil Babka wrote:
[...]
> I'm not a fan of either name, so I've tried to implement my own
> suggestion. Turns out it was easier than expected, as there's no kernel
> boot option for "defer", just for "enabled", so that particular worry
> was unfounded.
> 
> And personally I think that it's less confusing when one can enable defer
> and madvise together (and not any other combination), than having to dig
> up the difference between "defer" and "background".
> 
> I have only tested the sysfs manipulation, not actual THP, but seems to me
> that alloc_hugepage_direct_gfpmask() already happens to process the flags
> in a way that it works as expected.

IMHO this looks indeed much simpler implementation wise, more consistent
from the semantic point of view and less confusing from the usage POV.
 
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 10eedbf14421..cc5ae86169a8 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -150,7 +150,16 @@ static ssize_t triple_flag_store(struct kobject *kobj,
>  				 enum transparent_hugepage_flag deferred,
>  				 enum transparent_hugepage_flag req_madv)
>  {
> -	if (!memcmp("defer", buf,
> +	if (!memcmp("defer madvise", buf,
> +			min(sizeof("defer madvise")-1, count))
> +	    || !memcmp("madvise defer", buf,
> +			min(sizeof("madvise defer")-1, count))) {
> +		if (enabled == deferred)
> +			return -EINVAL;
> +		clear_bit(enabled, &transparent_hugepage_flags);
> +		set_bit(req_madv, &transparent_hugepage_flags);
> +		set_bit(deferred, &transparent_hugepage_flags);
> +	} else if (!memcmp("defer", buf,
>  		    min(sizeof("defer")-1, count))) {
>  		if (enabled == deferred)
>  			return -EINVAL;
> @@ -251,9 +260,12 @@ static ssize_t defrag_show(struct kobject *kobj,
>  {
>  	if (test_bit(TRANSPARENT_HUGEPAGE_DEFRAG_DIRECT_FLAG, &transparent_hugepage_flags))
>  		return sprintf(buf, "[always] defer madvise never\n");
> -	if (test_bit(TRANSPARENT_HUGEPAGE_DEFRAG_KSWAPD_FLAG, &transparent_hugepage_flags))
> -		return sprintf(buf, "always [defer] madvise never\n");
> -	else if (test_bit(TRANSPARENT_HUGEPAGE_DEFRAG_REQ_MADV_FLAG, &transparent_hugepage_flags))
> +	if (test_bit(TRANSPARENT_HUGEPAGE_DEFRAG_KSWAPD_FLAG, &transparent_hugepage_flags)) {
> +		if (test_bit(TRANSPARENT_HUGEPAGE_DEFRAG_REQ_MADV_FLAG, &transparent_hugepage_flags))
> +			return sprintf(buf, "always [defer] [madvise] never\n");
> +		else
> +			return sprintf(buf, "always [defer] madvise never\n");
> +	} else if (test_bit(TRANSPARENT_HUGEPAGE_DEFRAG_REQ_MADV_FLAG, &transparent_hugepage_flags))
>  		return sprintf(buf, "always defer [madvise] never\n");
>  	else
>  		return sprintf(buf, "always defer madvise [never]\n");

-- 
Michal Hocko
SUSE Labs