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 60CD2C433F5 for ; Thu, 4 Nov 2021 08:59:57 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 03BF46112E for ; Thu, 4 Nov 2021 08:59:56 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 03BF46112E Authentication-Results: mail.kernel.org; dmarc=fail (p=quarantine dis=none) header.from=suse.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id 6247B6B0072; Thu, 4 Nov 2021 04:59:56 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 5D4686B0073; Thu, 4 Nov 2021 04:59:56 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4C4206B0074; Thu, 4 Nov 2021 04:59:56 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0247.hostedemail.com [216.40.44.247]) by kanga.kvack.org (Postfix) with ESMTP id 3DE306B0072 for ; Thu, 4 Nov 2021 04:59:56 -0400 (EDT) Received: from smtpin03.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id F151B1853BCA8 for ; Thu, 4 Nov 2021 08:59:55 +0000 (UTC) X-FDA: 78770650350.03.63B5E8E Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by imf12.hostedemail.com (Postfix) with ESMTP id 754FC10000B2 for ; Thu, 4 Nov 2021 08:59:55 +0000 (UTC) Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id 34582212BD; Thu, 4 Nov 2021 08:59:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1636016394; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=JHgMSLSsZciqyDKovNJpJOLLyUz2f8yCrBxcbv99D2w=; b=P2VDLgHyqRtFyYFgUXtLqQcQJXO8Kl7PKzqj/1htCo3lXHnEtHDFbBqh96wV/73AEyKFsf j2J4FVJ/oKvDizQxfwEMMG8FZiIG9nm6bIxy0QhprMaV7NVtPKoqUBYhgMu5b0BakBIHfu LrXC4SL4e/HVywXwtwb58qyNCASWd/c= Received: from suse.cz (unknown [10.100.201.86]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id 76082A3BF2; Thu, 4 Nov 2021 08:59:53 +0000 (UTC) Date: Thu, 4 Nov 2021 09:59:48 +0100 From: Michal Hocko To: "Uladzislau Rezki (Sony)" Cc: Andrew Morton , linux-mm@kvack.org, LKML , Mel Gorman , Christoph Hellwig , Matthew Wilcox , Nicholas Piggin , Hillf Danton , Oleksiy Avramchenko , Steven Rostedt , Vasily Averin Subject: Re: [PATCH] mm/vmalloc: Eliminate an extra orig_gfp_mask Message-ID: References: <20211103200703.2265-1-urezki@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20211103200703.2265-1-urezki@gmail.com> X-Rspamd-Queue-Id: 754FC10000B2 X-Stat-Signature: 8y9gn1adty8xr8a1zt33hgagiy5jwfj9 Authentication-Results: imf12.hostedemail.com; dkim=pass header.d=suse.com header.s=susede1 header.b=P2VDLgHy; dmarc=pass (policy=quarantine) header.from=suse.com; spf=pass (imf12.hostedemail.com: domain of mhocko@suse.com designates 195.135.220.28 as permitted sender) smtp.mailfrom=mhocko@suse.com X-Rspamd-Server: rspam01 X-HE-Tag: 1636016395-918483 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: [Cc Vasily] On Wed 03-11-21 21:07:03, Uladzislau Rezki wrote: > That extra variable has been introduced just for keeping an original > passed gfp_mask because it is updated with __GFP_NOWARN on entry, thus > error handling messages were broken. I am not sure what you mean by "error handling messages were broken" part. It is true that the current Linus tree has a broken allocation failure reporting but that is not a fault of orig_gfp_mask. In fact patch which is fixing that "mm/vmalloc: repair warn_alloc()s in __vmalloc_area_node()" currently in akpm tree is adding the additional mask. > Instead we can keep an original gfp_mask without modifying it and add > an extra __GFP_NOWARN flag together with gfp_mask as a parameter to > the vm_area_alloc_pages() function. It will make it less confused. I would tend to agree that this is a better approach. There is already nested_gfp mask and one more doesn't add to the readbility. Maybe we should just drop the above patch and just go with one which doesn't introduce the intermediate step and an additional gfp mask. > Signed-off-by: Uladzislau Rezki (Sony) > --- > mm/vmalloc.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/mm/vmalloc.c b/mm/vmalloc.c > index d2a00ad4e1dd..3b549ff5c95e 100644 > --- a/mm/vmalloc.c > +++ b/mm/vmalloc.c > @@ -2920,7 +2920,6 @@ static void *__vmalloc_area_node(struct vm_struct *area, gfp_t gfp_mask, > int node) > { > const gfp_t nested_gfp = (gfp_mask & GFP_RECLAIM_MASK) | __GFP_ZERO; > - const gfp_t orig_gfp_mask = gfp_mask; > unsigned long addr = (unsigned long)area->addr; > unsigned long size = get_vm_area_size(area); > unsigned long array_size; > @@ -2928,7 +2927,7 @@ static void *__vmalloc_area_node(struct vm_struct *area, gfp_t gfp_mask, > unsigned int page_order; > > array_size = (unsigned long)nr_small_pages * sizeof(struct page *); > - gfp_mask |= __GFP_NOWARN; > + > if (!(gfp_mask & (GFP_DMA | GFP_DMA32))) > gfp_mask |= __GFP_HIGHMEM; > > @@ -2941,7 +2940,7 @@ static void *__vmalloc_area_node(struct vm_struct *area, gfp_t gfp_mask, > } > > if (!area->pages) { > - warn_alloc(orig_gfp_mask, NULL, > + warn_alloc(gfp_mask, NULL, > "vmalloc error: size %lu, failed to allocated page array size %lu", > nr_small_pages * PAGE_SIZE, array_size); > free_vm_area(area); > @@ -2951,8 +2950,8 @@ static void *__vmalloc_area_node(struct vm_struct *area, gfp_t gfp_mask, > set_vm_area_page_order(area, page_shift - PAGE_SHIFT); > page_order = vm_area_page_order(area); > > - area->nr_pages = vm_area_alloc_pages(gfp_mask, node, > - page_order, nr_small_pages, area->pages); > + area->nr_pages = vm_area_alloc_pages(gfp_mask | __GFP_NOWARN, > + node, page_order, nr_small_pages, area->pages); > > atomic_long_add(area->nr_pages, &nr_vmalloc_pages); > > @@ -2961,7 +2960,7 @@ static void *__vmalloc_area_node(struct vm_struct *area, gfp_t gfp_mask, > * allocation request, free them via __vfree() if any. > */ > if (area->nr_pages != nr_small_pages) { > - warn_alloc(orig_gfp_mask, NULL, > + warn_alloc(gfp_mask, NULL, > "vmalloc error: size %lu, page order %u, failed to allocate pages", > area->nr_pages * PAGE_SIZE, page_order); > goto fail; > @@ -2969,7 +2968,7 @@ static void *__vmalloc_area_node(struct vm_struct *area, gfp_t gfp_mask, > > if (vmap_pages_range(addr, addr + size, prot, area->pages, > page_shift) < 0) { > - warn_alloc(orig_gfp_mask, NULL, > + warn_alloc(gfp_mask, NULL, > "vmalloc error: size %lu, failed to map pages", > area->nr_pages * PAGE_SIZE); > goto fail; > -- > 2.17.1 -- Michal Hocko SUSE Labs