From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0D784C433EF for ; Fri, 29 Oct 2021 14:05:48 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 8EB556115C for ; Fri, 29 Oct 2021 14:05:47 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 8EB556115C Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id 9F6C16B0071; Fri, 29 Oct 2021 10:05:46 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 9A5336B0072; Fri, 29 Oct 2021 10:05:46 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 894996B0073; Fri, 29 Oct 2021 10:05:46 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0133.hostedemail.com [216.40.44.133]) by kanga.kvack.org (Postfix) with ESMTP id 63A776B0071 for ; Fri, 29 Oct 2021 10:05:46 -0400 (EDT) Received: from smtpin07.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 03306284A7 for ; Fri, 29 Oct 2021 14:05:46 +0000 (UTC) X-FDA: 78749648292.07.A43BD04 Received: from mail-ed1-f44.google.com (mail-ed1-f44.google.com [209.85.208.44]) by imf05.hostedemail.com (Postfix) with ESMTP id 863AD508B661 for ; Fri, 29 Oct 2021 14:05:33 +0000 (UTC) Received: by mail-ed1-f44.google.com with SMTP id r4so38459406edi.5 for ; Fri, 29 Oct 2021 07:05:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=hvaOZ+L7Q/hsGmgFIj1Fd0g2K7g1gQBbyCtdTWJts7g=; b=k3ESfV5YtenEJ/NOkxsM84lYLQWIN00Dm9aqiA9jgI4swsqfLAEkI/ab/eKHSqMRQr Zj6jmYK3kTsyMGKTF46IYBnC65esbPtG9eEg6U7YAKzp5N3GvpBL7m57hAAQ2hClX4oL 0XtFIryldx8HXL7f4vipSifahc6nJv6O/REYNJB9+04h4Yp2kRwNK/u3tDU+ZzV/FjrO 2PaoSXeQpwQ3EhZKPi4HSaQWhasznxo33DgG2xTeSJof53lpP4JqDCPOZGCNGPLVBvkw RRvkCxr1Gj1KxPALv6tPGhGscizSde7P2504Tp9EpMbS0ki3n59anIoFZcvNSHrfmJqM yW5Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=hvaOZ+L7Q/hsGmgFIj1Fd0g2K7g1gQBbyCtdTWJts7g=; b=Cc4rhAgbf7rGCZrEAGeTu/InksviTskY6ai4t3kCtCZSmEP0KH4A37M3le33V4YJZm eD9L82zkoHxwYKQd7Dp2nfNMEJjUt5lUsDYPYirsJaZyFGWhcR30y/4rlpl7hUQKoYF+ AfQ4F6oOu2ad1+KgE66qvO8HSbibnd4e7nIBc9bA2aFcGcqF24/hYwYnHcE6u0CVUhaS /MkvrUKhJbHzJRE31Tcw0YtgkiT/bsguE5Y2DAPVtEcX+4Qqw49KDZ3JB7tUNwILJD3F UfO3GWyAWUf9Atr7VXXjXA3EZehwCw6ICfYjYfOFOgGzZZnaqkYpy2sPR7pob6k0dpjJ RLaw== X-Gm-Message-State: AOAM533pHyjBITRrGnu8vqAo6BMwIGqg1VErpOBCxd/Lfokin6I77XA0 wHBilfKWvEzckG2i8Cwc6ZX9H4V6GapHtIEN7Cqp7/s9MmORt0k4 X-Google-Smtp-Source: ABdhPJwtVXLFoFhFJUkbD073vlIqpwXOQ4Dd3cpc71Nl6L3ircqX5Unm2vzoFJ+Zbc907xuOR/EA9TYdqu94eALaABk= X-Received: by 2002:a17:906:58d3:: with SMTP id e19mr13384663ejs.350.1635516343380; Fri, 29 Oct 2021 07:05:43 -0700 (PDT) MIME-Version: 1.0 References: <20211025150223.13621-1-mhocko@kernel.org> <20211025150223.13621-3-mhocko@kernel.org> <20211026193315.GA1860@pc638.lan> <20211027175550.GA1776@pc638.lan> In-Reply-To: From: Uladzislau Rezki Date: Fri, 29 Oct 2021 16:05:32 +0200 Message-ID: Subject: Re: [PATCH 2/4] mm/vmalloc: add support for __GFP_NOFAIL To: Michal Hocko Cc: Linux Memory Management List , Dave Chinner , Neil Brown , Andrew Morton , Christoph Hellwig , linux-fsdevel@vger.kernel.org, LKML , Ilya Dryomov , Jeff Layton Content-Type: text/plain; charset="UTF-8" Authentication-Results: imf05.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=k3ESfV5Y; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf05.hostedemail.com: domain of urezki@gmail.com designates 209.85.208.44 as permitted sender) smtp.mailfrom=urezki@gmail.com X-Stat-Signature: cjjfopmcwpetfqx3nttin4q86iozy81s X-Rspamd-Queue-Id: 863AD508B661 X-Rspamd-Server: rspam01 X-HE-Tag: 1635516333-963509 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: > > diff --git a/mm/vmalloc.c b/mm/vmalloc.c > > index d77830ff604c..f4b7927e217e 100644 > > --- a/mm/vmalloc.c > > +++ b/mm/vmalloc.c > > @@ -2889,8 +2889,14 @@ static void *__vmalloc_area_node(struct vm_struct *area, gfp_t gfp_mask, > > unsigned long array_size; > > unsigned int nr_small_pages = size >> PAGE_SHIFT; > > unsigned int page_order; > > + unsigned long flags; > > + int ret; > > > > array_size = (unsigned long)nr_small_pages * sizeof(struct page *); > > + > > + /* > > + * This is i do not understand why we do not want to see warning messages. > > + */ > > gfp_mask |= __GFP_NOWARN; > > I suspect this is becauser vmalloc wants to have its own failure > reporting. > But as i see it is broken. All three warn_alloc() reports in the __vmalloc_area_node() are useless because the __GFP_NOWARN is added on top of gfp_mask: void warn_alloc(gfp_t gfp_mask, nodemask_t *nodemask, const char *fmt, ...) { struct va_format vaf; va_list args; static DEFINE_RATELIMIT_STATE(nopage_rs, 10*HZ, 1); if ((gfp_mask & __GFP_NOWARN) || !__ratelimit(&nopage_rs)) return; ... everything with the __GFP_NOWARN is just reverted. > [...] > > @@ -3010,16 +3037,22 @@ void *__vmalloc_node_range(unsigned long size, unsigned long align, > > area = __get_vm_area_node(real_size, align, shift, VM_ALLOC | > > VM_UNINITIALIZED | vm_flags, start, end, node, > > gfp_mask, caller); > > - if (!area) { > > - warn_alloc(gfp_mask, NULL, > > - "vmalloc error: size %lu, vm_struct allocation failed", > > - real_size); > > - goto fail; > > - } > > + if (area) > > + addr = __vmalloc_area_node(area, gfp_mask, prot, shift, node); > > + > > + if (!area || !addr) { > > + if (gfp_mask & __GFP_NOFAIL) { > > + schedule_timeout_uninterruptible(1); > > + goto again; > > + } > > + > > + if (!area) > > + warn_alloc(gfp_mask, NULL, > > + "vmalloc error: size %lu, vm_struct allocation failed", > > + real_size); > > > > - addr = __vmalloc_area_node(area, gfp_mask, prot, shift, node); > > - if (!addr) > > goto fail; > > + } > > > > /* > > * In this function, newly allocated vm_struct has VM_UNINITIALIZED > > > > OK, this looks easier from the code reading but isn't it quite wasteful > to throw all the pages backing the area (all of them allocated as > __GFP_NOFAIL) just to then fail to allocate few page tables pages and > drop all of that on the floor (this will happen in __vunmap AFAICS). > > I mean I do not care all that strongly but it seems to me that more > changes would need to be done here and optimizations can be done on top. > > Is this something you feel strongly about? > Will try to provide some motivations :) It depends on how to look at it. My view is as follows a more simple code is preferred. It is not considered as a hot path and it is rather a corner case to me. I think "unwinding" has some advantage. At least one motivation is to release a memory(on failure) before a delay that will prevent holding of extra memory in case of __GFP_NOFAIL infinitelly does not succeed, i.e. if a process stuck due to __GFP_NOFAIL it does not "hold" an extra memory forever. -- Uladzislau Rezki