linux-kernel - Re: [RFC 2/3] mm/vmalloc: add support for __GFP

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]

Date:   Mon, 25 Oct 2021 16:30:23 +0200
From:   Uladzislau Rezki <urezki@...il.com>
To:     Michal Hocko <mhocko@...e.com>
Cc:     NeilBrown <neilb@...e.de>,
        Linux Memory Management List <linux-mm@...ck.org>,
        Dave Chinner <david@...morbit.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Christoph Hellwig <hch@...radead.org>,
        linux-fsdevel@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>,
        Ilya Dryomov <idryomov@...il.com>,
        Jeff Layton <jlayton@...nel.org>
Subject: Re: [RFC 2/3] mm/vmalloc: add support for __GFP_NOFAIL

>
> I would really prefer if this was not the main point of arguing here.
> Unless you feel strongly about msleep I would go with schedule_timeout
> here because this is a more widely used interface in the mm code and
> also because I feel like that relying on the rounding behavior is just
> subtle. Here is what I have staged now.
>
I have a preference but do not have a strong opinion here. You can go
either way you want.

>
> Are there any other concerns you see with this or other patches in the
> series?
>
it is better if you could send a new vX version because it is hard to
combine every "folded"
into one solid commit. One comment below:

> ---
> commit c1a7e40e6b56fed5b9e716de7055b77ea29d89d0
> Author: Michal Hocko <mhocko@...e.com>
> Date:   Wed Oct 20 10:12:45 2021 +0200
>
>     fold me "mm/vmalloc: add support for __GFP_NOFAIL"
>
>     Add a short sleep before retrying. 1 jiffy is a completely random
>     timeout. Ideally the retry would wait for an explicit event - e.g.
>     a change to the vmalloc space change if the failure was caused by
>     the space fragmentation or depletion. But there are multiple different
>     reasons to retry and this could become much more complex. Keep the retry
>     simple for now and just sleep to prevent from hogging CPUs.
>
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 0fb5413d9239..a866db0c9c31 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -2944,6 +2944,7 @@ static void *__vmalloc_area_node(struct vm_struct *area, gfp_t gfp_mask,
>         do {
>                 ret = vmap_pages_range(addr, addr + size, prot, area->pages,
>                         page_shift);
> +               schedule_timeout_uninterruptible(1);
>
We do not want to schedule_timeout_uninterruptible(1); every time.
Only when an error is detected.

-- 
Uladzislau Rezki