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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1F4BEC27C53 for ; Wed, 19 Jun 2024 09:26:33 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 60D9B6B040C; Wed, 19 Jun 2024 05:26:32 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 463A56B040D; Wed, 19 Jun 2024 05:26:32 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 28BFB6B040E; Wed, 19 Jun 2024 05:26:32 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 089736B040C for ; Wed, 19 Jun 2024 05:26:32 -0400 (EDT) Received: from smtpin21.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id ADF5E805A0 for ; Wed, 19 Jun 2024 09:26:31 +0000 (UTC) X-FDA: 82247107782.21.534649F Received: from sin.source.kernel.org (sin.source.kernel.org [145.40.73.55]) by imf14.hostedemail.com (Postfix) with ESMTP id 62C9D100009 for ; Wed, 19 Jun 2024 09:26:29 +0000 (UTC) Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=Ywh1PGmY; spf=pass (imf14.hostedemail.com: domain of rppt@kernel.org designates 145.40.73.55 as permitted sender) smtp.mailfrom=rppt@kernel.org; dmarc=pass (policy=none) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1718789179; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=5S+kMTP0rVWJmVAHfxNBBhAoF7MCTAMI7nzl/GnbuYs=; b=vksT00S0uFoLKq5BhhSvaVpiqpEfC0A8Ph8SkN724iuHKvRpQeY93+zssODzSwQXoCgWqc 1fh4ePVEKcPLj4ectFpIzdpvdVI0ziM4KeJoDr55hOxtRFBzvloPMncs5L8bLeGgrnmSt7 ZazKyH3doLS4z25IEFuxA9bCr//6YeU= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=Ywh1PGmY; spf=pass (imf14.hostedemail.com: domain of rppt@kernel.org designates 145.40.73.55 as permitted sender) smtp.mailfrom=rppt@kernel.org; dmarc=pass (policy=none) header.from=kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1718789179; a=rsa-sha256; cv=none; b=x6+7wfimnvE4LIHKZ9X5RhiPAwNJl0jmyJxTse3vZlDt4vBaMP/fBQZhttILjKIiKmVZ8k GgZGLw3u79eATWOgGYwKxmmXtP01exixbso1h/3Zt3SgRx9pWSbi+b0B3XPh1UWt4dAnzW aWqB12/OI2H8dtLBPH3K2QrKddrjBMc= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id 0A8E0CE1DF5; Wed, 19 Jun 2024 09:26:26 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 90D1AC2BBFC; Wed, 19 Jun 2024 09:26:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1718789185; bh=66o7eKaXW7UoOMZm3DSXuJpSFK2xxkY0YlrIARKvwmw=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Ywh1PGmYz6kwnYVpS1LbOyH0smBbKcT5eOCc1Z3qLrpNuYqxxAzDkfv5QEcH9rZNO WdgmOtycIl/4n0g7uQLS6ZwPwjk0D3Nd7HgrifSVmLyqfYAypd62HdVwM/xCyhp5Jr C2usHD8Fk/mH71GgY+ai4RfZxYc9ncN6int7sHojPQWgcrv9J8JMKsxB/WEiLsNyJm IOw5hejQviP3ufACa9UKC9vcWMFHfetFU4U91SfSBQayxHD9xQ61JVxqz4XYqq+s5n V0lRbF33uihfC+w1MZB91+MicCXOtWLs3d+HJ6Te0mGyXL9/S+00WSX1H2/ZMpvHN9 XgZ8bY1aiSjzA== Date: Wed, 19 Jun 2024 12:24:09 +0300 From: Mike Rapoport To: "Gowans, James" Cc: "linux-mm@kvack.org" , "Graf (AWS), Alexander" , "linux-kernel@vger.kernel.org" , "akpm@linux-foundation.org" Subject: Re: [PATCH] memblocks: Move late alloc warning down to phys alloc Message-ID: References: <20240614133016.134150-1-jgowans@amazon.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 62C9D100009 X-Stat-Signature: b7o5ho9esgdepfiyw3bgsc3grurr1up4 X-Rspam-User: X-HE-Tag: 1718789189-700526 X-HE-Meta: U2FsdGVkX18ofJIZJP7yphZUplzhfPjyH/6InuTn4DjYPF5JBNlnraHBAf8WiE0FW7ccbUI5CgmWuHrROr38BadxKQuKMoR3ZPQb6pkHcbJDYQWCBUtYSQx1O/St/DytXBCzz41mKQhnMiAijsdLL67qj7HWYmpnecSEpJ+DUEpxs2QvSTfw811yvIfuejYz714AY38gABca+qoK2wKh0c8ioUUVqf93dQged2QAI3aKGTf2It2AubD6Xh+zW9Vnmwenmz+D4HdH0G763sbc8cR8lrI5Mi/Q1lb54wRudfMsONV+Z95pGTtzCEGOeRhY9aIwEm4CYcSV5bekG9yVqKRN4Cavm90xz7Gp5Bo8HjK7ZOK1On0SyEkcCSrMbC3psTpyPYnoWsjLbu+YgdYkqRLxNXkSP8EvYgfwqymcS4ExA++EBLtBPVvXZgCSpJlGU4kJv/xItqTCHSQBYtbSVfRu12e31ayeUa5873pFAHGeU7mM9sNiF8IRY38lfJh3gJR5NWriIyAnLU5bt+spZMKSUespG5nNiWIZP4l6wAcXqkb3YT2Pcj8DmELjv9M1Vr5KGE5BrPl7hYqCdVZIasiuoRZRPPzBYg3jSBQZrIPlMURDtssVV5zvHsk+6Tu8bkJ3MhWcKwA/HsB2IvLdeyI9R9EEWbP+EIZBJDqnJ7Yg/cuIHtD5ppZ0zSUNwhIeunO6oryuGwBWApG2YCevQQBYAx8ohVqf59me9zzPrRH71AcCcgeZDyRu8GF0drDS23tfA8EQKwJ94gXOZAH+959nJX9CYjWXvP/yrATK5+1ExhuGat7Cg8/YdGi3g3jMO86S8QVsNqkuzy5VO9MF6vw3pdGZ0WKkyXOyKa96/OOdFdYS3r5fSNmqw9JH/IXIUTzrKzJm6jNGrB1gLsvinZ8GWLG32eX9BE5GxyTNOqfeV0RYq3vKhCv+l8cqM79CJDD1ABbnSBSVJWme25Y 8NM0a13Q /EJJxDNLH9uandbu/UP9E4o6iCONKobOrZJvzn/L0OwEWZxHDL6eeHdOQhvR/Fx0EvkMSNRHhat3eA3JQZBHbAE7k+kTsHBsqiwcJr6RyeIT8WjHaJ65RbuSVt6RDlwY4ogKuuZOWDddRZBWRy0dBCA8uBK7llp03iyFsNE8N4t0Xjrw87yfog9waCpFoikEffnjWQKweoYjfguR5TaCOq3H/1Hidkk17sniXi+hdokvnAeAC+Vmr2bT06qWWT55KsQUcCq4rRIJawER2In1m5rpj7rhOywvfVhZ0XiF3/g0JQBnHGDDH+jX6YjPz6A6/HmUDCZOvApqzwAB5ctbDS2c8mNvWPmd8y8obNff49aTeToEDutjH9IylDmLO84luiKlyBQeKe4VIPark1DX7Bgy5cg== 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: List-Subscribe: List-Unsubscribe: On Wed, Jun 19, 2024 at 07:53:14AM +0000, Gowans, James wrote: > On Tue, 2024-06-18 at 14:02 +0300, Mike Rapoport wrote: > > On Fri, Jun 14, 2024 at 03:30:16PM +0200, James Gowans wrote: > > > Subject: [PATCH] memblocks: Move late alloc warning down to phys alloc > > > > Nit: memblock > > Ack! > > > > > > If a driver/subsystem tries to do an allocation after memblocks have And another one here :) ^ > > > been freed and the memory handed to the buddy allocator, it will not > > > actually be legal to use that allocation - the buddy allocator owns the > > > memory. This is handled by the memblocks function which does allocations > > > and returns virtual addresses by printing a warning and doing a kmalloc > > > instead. However, the physical allocation function does not to do this > > > check - callers of the physical alloc function are unprotected against > > > mis-use. > > > > Did you see such misuse or this is a theoretical issue? > > Yeah, I was driving the memblock allocator badly when prototyping > something. Allocating a large contiguous block of memory for an in- > memory filesystem and I was doing the allocation in an initcall, but by > that point it was too late. The memblock allocator happily gave me a > large chunk of memory, but it was already in use by the buddy allocator, > so ended up with memory corruption. Oops! Getting this warning would > have made the problem immediately obvious. > > > > > > Improve the error catching here by moving the check into the physical > > > allocation function which is used by the virtual addr allocation > > > function. > > > > > > Signed-off-by: James Gowans > > > Cc: Mike Rapoport > > > Cc: Andrew Morton > > > Cc: Alex Graf > > > --- > > > mm/memblock.c | 18 +++++++++++------- > > > 1 file changed, 11 insertions(+), 7 deletions(-) > > > > > > diff --git a/mm/memblock.c b/mm/memblock.c > > > index d09136e040d3..dd4f237dc1fc 100644 > > > --- a/mm/memblock.c > > > +++ b/mm/memblock.c > > > @@ -1457,6 +1457,17 @@ phys_addr_t __init memblock_alloc_range_nid(phys_addr_t size, > > > align = SMP_CACHE_BYTES; > > > } > > > > > > + /* > > > + * Detect any accidental use of these APIs after slab is ready, as at > > > + * this moment memblock may be deinitialized already and its > > > + * internal data may be destroyed (after execution of memblock_free_all) > > > + */ > > > + if (WARN_ON_ONCE(slab_is_available())) { > > > + void *vaddr = kzalloc_node(size, GFP_NOWAIT, nid); > > > + > > > + return vaddr ? virt_to_phys(vaddr) : 0; > > > + } > > > > I'd move this before alignment check. > > Ack, will do in V2. > > Anything else or should I make the tweaks and post V2? Looks ok to me. > JG -- Sincerely yours, Mike.