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 618DFC54E58 for ; Thu, 21 Mar 2024 16:57:12 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D9B7D6B00A0; Thu, 21 Mar 2024 12:57:11 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D4B946B00A3; Thu, 21 Mar 2024 12:57:11 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C12F96B00C9; Thu, 21 Mar 2024 12:57:11 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id B1F8F6B00A0 for ; Thu, 21 Mar 2024 12:57:11 -0400 (EDT) Received: from smtpin17.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 73B411C0B59 for ; Thu, 21 Mar 2024 16:57:11 +0000 (UTC) X-FDA: 81921651462.17.C9479B3 Received: from mail-yw1-f177.google.com (mail-yw1-f177.google.com [209.85.128.177]) by imf28.hostedemail.com (Postfix) with ESMTP id 98A1EC001E for ; Thu, 21 Mar 2024 16:57:09 +0000 (UTC) Authentication-Results: imf28.hostedemail.com; dkim=none; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=kernel.org (policy=none); spf=pass (imf28.hostedemail.com: domain of dennisszhou@gmail.com designates 209.85.128.177 as permitted sender) smtp.mailfrom=dennisszhou@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1711040229; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=BKTbYwQ5gcXN3TRRJ5BC/ttApNw9m6DYa4ZEIkNEXPQ=; b=h5lY/wo5URfSkYoHBxEOfqh7wpQZ8IZAIm0BKJzEoKiEPi+QEABzkBwhEzCe0m2YZJF3O+ k32Mc7xO98IhaFD9zkbhhEBwBDQFR/3QitsHjED1o0569kJqBcifoEkhoIGSt4ZoAATIlP 8zEVlaNIhT0TEp4kcSnuYo/3I/9Rl2c= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=none; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=kernel.org (policy=none); spf=pass (imf28.hostedemail.com: domain of dennisszhou@gmail.com designates 209.85.128.177 as permitted sender) smtp.mailfrom=dennisszhou@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1711040229; a=rsa-sha256; cv=none; b=2bM7YaHjvIg9OSKxIDPltveZDJaNEH7TlwXiAQGkGYePHIGWjgVOZpJ+8evf1euAYpRq5j +9a5W9RJ/4tKCX2TLdRsySeZYWA/YFf/P1gnjKi+jFQzB62IFd4EDZ3rh32nGo+hH+i0qE wPJZPNathvj1BXnOGGc8qrTV8ZeI7I4= Received: by mail-yw1-f177.google.com with SMTP id 00721157ae682-60a104601dcso12694997b3.2 for ; Thu, 21 Mar 2024 09:57:09 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1711040228; x=1711645028; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=BKTbYwQ5gcXN3TRRJ5BC/ttApNw9m6DYa4ZEIkNEXPQ=; b=i4AtbK5M78Gr6QftNM6Zsd5571TRByrUska9kx2b8IgY6dx/NQIHOGc8OooxVbcjT0 bxX2qe2yzNhH/QJHLa27M1QkHs37V9PrwjSRF6b/JxaG0sK4oMu1hvo+11b3Qmsrmrni uyvLNimzXpxL5aaj3vyik0CB6gp87oNWCKp+voCSG1WAsMTn8nto6W2TUlyK+MLl+ym6 3Md4hJ91hJEQV6no8CV5aGxLZatqH/Yx/p4+2P9Ko6KP7COE5rr5Tq2PQP5DbzcybvK/ itOtqYDc50DEFPXVL2VAVPdlG/uDtU+a3lYLBcQ2bs6ifb1Q7XwK2RqyWsHR2pRbnJe1 QkWA== X-Forwarded-Encrypted: i=1; AJvYcCU2StACV7thn0oB10SCwGI8h35sCz3La2Zy5n3jxzoWCKOihYYk8PxfCK6t+EeYJzfv2bkMpWGY3Afl5a/K7/iMMZQ= X-Gm-Message-State: AOJu0YzGzLxO4PBB3LNt4rtyre7S6xnFsFZ5Ff70C/Gtu8ecUtz3Ig1b H73Dvgn1YFH7/G7mRjfj+cPnHpL6Efi9jKcdL7yb38TnFhyKM/jCX6QElweaOObegw== X-Google-Smtp-Source: AGHT+IE07hS1rv0Ld5GU41nHwA0qGGdhPzZKRg/h5j1OAB2Q9YTWqw8kONtdL3LlAT5Ov2WRcnAGeg== X-Received: by 2002:a0d:cdc5:0:b0:610:f906:d6c2 with SMTP id p188-20020a0dcdc5000000b00610f906d6c2mr4628198ywd.5.1711040228515; Thu, 21 Mar 2024 09:57:08 -0700 (PDT) Received: from V92F7Y9K0C (fixed-187-190-174-151.totalplay.net. [187.190.174.151]) by smtp.gmail.com with ESMTPSA id w125-20020a818683000000b0060cccc8a7b9sm7004ywf.69.2024.03.21.09.57.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 21 Mar 2024 09:57:08 -0700 (PDT) Date: Thu, 21 Mar 2024 09:57:05 -0700 From: Dennis Zhou To: Yosry Ahmed , Andrew Morton Cc: Dennis Zhou , Tejun Heo , Christoph Lameter , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Uladzislau Rezki , Christoph Hellwig , Lorenzo Stoakes Subject: Re: [PATCH] percpu: clean up all mappings when pcpu_map_pages() fails Message-ID: References: <20240311194346.2291333-1-yosryahmed@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Rspamd-Queue-Id: 98A1EC001E X-Rspam-User: X-Rspamd-Server: rspam04 X-Stat-Signature: o8pxr53dwr47mbz9ooukaz9pxayde6ys X-HE-Tag: 1711040229-817382 X-HE-Meta: U2FsdGVkX1/lF11kPzb6ThRESMYlqno6J6tCK9Hz2M+ovdDs3kiGqyJq40bCyFhcmbMVDWcOQfkYpgVG+gCuH30zSKFbCO3aCLvjrDnK+p0koa/AoGFCPmsi4BJ/NUSBXM3KS9dpUpR9FMYKA4rkdsUn9sn5iB6o1BnQG4412mDkW9g525CRvYJpjxg8ZP2YqpgvncOOMNXpaSsJgGfnQqgwKuQR+/r+CeKyqw3pwQ/DRWpn1ao3Yujh2rm6GKE3N64nlGI1XqkgKYBxEPJhNpdGXeetp8KyQuV5rccW4HY1qnhnsU6wYRuPQ5JU+EXT82QY48aRoUmrvGfsjVWCWBMRzvLUw/xBbNrAKx/G3vhWD/EcMdqiiS1ucvC6SYZ3D/qnH73Jj//2ZNx1N9Os/vSzgHk7ybATPmTB3IyseE1H44C0+xbSUrF5fHV1JAZFYxrfgtqAj2jVSIYJV4nUWgmPu+zXExf2cZin2slTEv3Ym5jn7JTiA+M6qrZJGhHAxeMqduE50imJrsObyUeQm55Gs9GWUzLNetn9tyJeuee0fQEmfdxRrLDzt4lQSp/e9E+IxIsyaqSUgkNRqY3Ip4DLE7sHX5s+dPOxv414oo1xQcsVCWMmuh2m1VSlM+5hSXeOysyRB3ILOwQPnM01IgwZiT1bVwe6qCLVETv9eqtW35bJi3RYg7A8j04Fsp6Z1nSNai1hvrJt1MDhSGDvYhi9mZ4kyZVBySVe/tfyF6N/i7T/oCXGHDpsr4AGDMjC4AegmYRn+rav+CUb2YlsVeZ1TodyYyUY+sWmK8cBNSBnCQsJgGI0E4WNSsxhVWHlO5e48fhneHV7v4h2zlNo38H9NUrkouuCR2Pgi0qhO6+YNsOnEQtvuJogxELQpekW9MNxHe5RatO9p1BdAmh1sWkM9tRY8Pzi3OIc0HX9qLET3EePQxW50H5Lnjq3RIfNYICtvYCmTQqCokRISXP Hw1ooQAf SXcNjo09kuCJkjiURmfay+MA1RsExuYWJHoqPJDy25diJXkr870foP1Q/50nTZuWTUQOmTY+rbUZCmVHtK8NpfPSoFUnAhSJAwwybornse3CpcKEkmHrfCPb8s2LcQMGeN1pfoeg7d/rq4u+ghkG0FJfA6W7RN87lEDBsbh597MSEdx5IMNeliFSpgtCFTQcZwa0yqI+nsXBfuzfvng5IhGQzvUQxJT3TQFv+d6ghRMPkvNywWlQDJn5DChS7P3xzL8MYz82dTWejJ96jlrBQM0zI6sgjiGL97qvq6mjInLcL3RyPx1+8i2LD5YhL9TSrDDriHajpIrAY1HD+iuj1GqYkx3p8k/2YEHKzy0ETYAAUsvOV5unzZ1dxouGDNiMa980yr47bC9CJCE8qfKbEEVS2bjKmZMK3Sijj7Z8QAwKWW9huJN9JzY11JTf55jTyBjQ8JbY22Cv+fwheQTx+9tPzTkxvrLlJ58z+2UXWfKqQcKXktkdNrASpbVtCTpXXnFl0SDdMBQiNDn2Jas9WdCgWXlXcoDgpRrk73PV17jnj7GSOYiUCSLFfykkDJJeVnWBq1UsO64VxqNKubuGCQbxWuSJk0ezFnXOdx+rNB36gLSR0PljlnGNy0Q== 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 Tue, Mar 19, 2024 at 01:49:17PM -0700, Yosry Ahmed wrote: > On Tue, Mar 19, 2024 at 1:32 PM Dennis Zhou wrote: > > > > Hi Yosry, > > > > On Tue, Mar 19, 2024 at 01:08:26PM -0700, Yosry Ahmed wrote: > > > On Mon, Mar 11, 2024 at 12:43 PM Yosry Ahmed wrote: > > > > > > > > In pcpu_map_pages(), if __pcpu_map_pages() fails on a CPU, we call > > > > __pcpu_unmap_pages() to clean up mappings on all CPUs where mappings > > > > were created, but not on the CPU where __pcpu_map_pages() fails. > > > > > > > > __pcpu_map_pages() and __pcpu_unmap_pages() are wrappers around > > > > vmap_pages_range_noflush() and vunmap_range_noflush(). All other callers > > > > of vmap_pages_range_noflush() call vunmap_range_noflush() when mapping > > > > fails, except pcpu_map_pages(). The reason could be that partial > > > > mappings may be left behind from a failed mapping attempt. > > > > > > > > Call __pcpu_unmap_pages() for the failed CPU as well in > > > > pcpu_map_pages(). > > > > > > > > This was found by code inspection, no failures or bugs were observed. > > > > > > > > Signed-off-by: Yosry Ahmed > > > > > > Any thoughts about this change? Should I resend next week after the > > > merge window? > > > > > > > Sorry for the delay. > > > > I'm looking at the code from mm/kmsan/hooks.c kmsan_ioremap_page_range(). > > It seems like __vunmap_range_noflush() is called on error for > > successfully mapped pages similar to how it's being done in percpu-vm.c. > > You picked an unconventional example to compare against :) > > > > > I haven't read in depth the expectations of vmap_pages_range_noflush() > > but on first glance it doesn't seem like percpu is operating out of the > > ordinary? > > I was looking at vm_map_ram(), vmap(), and __vmalloc_area_node(). They > all call vmap_pages_range()-> vmap_pages_range_noflush(). > > When vmap_pages_range() fails: > - vm_map_ram() calls > vm_unmap_ram()->free_unmap_vmap_area()->vunmap_range_noflush(). > - vmap() calls vunmap()->remove_vm_area()->free_unmap_vmap_area()-> > vunmap_range_noflush(). > - __vmalloc_area_node() calls > vfree()->remove_vm_area()->free_unmap_vmap_area()-> > vunmap_range_noflush(). > Okay so I had a moment to read it more closely. If we're mapping > 1 pages, and one of the latter pages fails. Then we could leave some mappings installed. @Andrew, I think this makes sense. Would you please be able to pick this up? I'm not running a tree this window. I will try to send out the percpu hotplug changes I've been forward porting for a while now and try to get that in a branch for-6.10. Acked-by: Dennis Zhou > I think it is needed to clean up any leftover partial mappings. I am > not sure about the kmsan example though. > Yeah the kmsan example seems like it could be wrong for the same reason, but I haven't inspected that more closely. Thanks, Dennis > Adding vmalloc reviewers here as well here. > > > > > > > > --- > > > > > > > > Perhaps the reason __pcpu_unmap_pages() is not currently being called > > > > for the failed CPU is that the size and alignment requirements make sure > > > > we never leave any partial mappings behind? I have no idea. Nonetheless, > > > > I think we want this change as that could be fragile, and is > > > > inconsistent with other callers. > > > > > > > > --- > > > > mm/percpu-vm.c | 4 ++-- > > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/mm/percpu-vm.c b/mm/percpu-vm.c > > > > index 2054c9213c433..cd69caf6aa8d8 100644 > > > > --- a/mm/percpu-vm.c > > > > +++ b/mm/percpu-vm.c > > > > @@ -231,10 +231,10 @@ static int pcpu_map_pages(struct pcpu_chunk *chunk, > > > > return 0; > > > > err: > > > > for_each_possible_cpu(tcpu) { > > > > - if (tcpu == cpu) > > > > - break; > > > > __pcpu_unmap_pages(pcpu_chunk_addr(chunk, tcpu, page_start), > > > > page_end - page_start); > > > > + if (tcpu == cpu) > > > > + break; > > > > } > > > > pcpu_post_unmap_tlb_flush(chunk, page_start, page_end); > > > > return err; > > > > -- > > > > 2.44.0.278.ge034bb2e1d-goog > > > >