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  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Wed, 6 Mar 2019 13:17:38 -0800
From:   Dave Hansen <dave.hansen@...el.com>
To:     Alex Ghiti <alex@...ti.fr>, Vlastimil Babka <vbabka@...e.cz>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will.deacon@....com>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Paul Mackerras <paulus@...ba.org>,
        Michael Ellerman <mpe@...erman.id.au>,
        Martin Schwidefsky <schwidefsky@...ibm.com>,
        Heiko Carstens <heiko.carstens@...ibm.com>,
        Yoshinori Sato <ysato@...rs.sourceforge.jp>,
        Rich Felker <dalias@...c.org>,
        "David S . Miller" <davem@...emloft.net>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        "H . Peter Anvin" <hpa@...or.com>, x86@...nel.org,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Andy Lutomirski <luto@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Mike Kravetz <mike.kravetz@...cle.com>,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linuxppc-dev@...ts.ozlabs.org, linux-s390@...r.kernel.org,
        linux-sh@...r.kernel.org, sparclinux@...r.kernel.org,
        linux-mm@...ck.org
Subject: Re: [PATCH v5 4/4] hugetlb: allow to free gigantic pages regardless
 of the configuration

On 3/6/19 12:08 PM, Alex Ghiti wrote:
>>>
>>> +    /*
>>> +     * Gigantic pages allocation depends on the capability for large
>>> page
>>> +     * range allocation. If the system cannot provide
>>> alloc_contig_range,
>>> +     * allow users to free gigantic pages.
>>> +     */
>>> +    if (hstate_is_gigantic(h) && !IS_ENABLED(CONFIG_CONTIG_ALLOC)) {
>>> +        spin_lock(&hugetlb_lock);
>>> +        if (count > persistent_huge_pages(h)) {
>>> +            spin_unlock(&hugetlb_lock);
>>> +            return -EINVAL;
>>> +        }
>>> +        goto decrease_pool;
>>> +    }
>> We talked about it during the last round and I don't seen any mention of
>> it here in comments or the changelog: Why is this a goto?  Why don't we
>> just let the code fall through to the "decrease_pool" label?  Why is
>> this new block needed at all?  Can't we just remove the old check and
>> let it be?
> 
> I'll get rid of the goto, I don't know how to justify it properly in a
> comment,
> maybe because it is not necessary.
> This is not a new block, this means exactly the same as before (remember
> gigantic_page_supported() actually meant CONTIG_ALLOC before this series),
> except that now we allow a user to free boottime allocated gigantic pages.
> And no we cannot just remove the check and let it be since it would modify
> the current behaviour, which is to return an error when trying to allocate
> gigantic pages whereas alloc_contig_range is not defined. I thought it was
> clearly commented above, I can try to make it more explicit.

OK, that makes sense.  Could you get some of this in the changelog,
please?  Otherwise this all looks good to me.

Powered by blists - more mailing lists