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 4E789C71153 for ; Mon, 4 Sep 2023 08:29:50 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 3724E8E0020; Mon, 4 Sep 2023 04:29:49 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 323378E001C; Mon, 4 Sep 2023 04:29:49 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1C3958E0020; Mon, 4 Sep 2023 04:29:49 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 0C0348E001C for ; Mon, 4 Sep 2023 04:29:49 -0400 (EDT) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id D2A29B3102 for ; Mon, 4 Sep 2023 08:29:48 +0000 (UTC) X-FDA: 81198241656.23.F2E6E58 Received: from mail-lf1-f48.google.com (mail-lf1-f48.google.com [209.85.167.48]) by imf30.hostedemail.com (Postfix) with ESMTP id CAA2680021 for ; Mon, 4 Sep 2023 08:29:46 +0000 (UTC) Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=gmail.com header.s=20221208 header.b=ruxWJLLt; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf30.hostedemail.com: domain of urezki@gmail.com designates 209.85.167.48 as permitted sender) smtp.mailfrom=urezki@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1693816186; a=rsa-sha256; cv=none; b=yQxYXUmvytl12mcoJRmJPO+UVrMSAOUgVROSAFMtwDOl5wbe9Cei4T6yT+Ne/d9V4He1RV UP3g7HxpDWp+XhOVbxgqR8tByhAqbpMRjcUW1C7WaBSFqZfs1g6zQcDOi7J5NmRJ43iuXW ZpV6kLk+pcCsVbZDX2oi0qOylOC9CmQ= ARC-Authentication-Results: i=1; imf30.hostedemail.com; dkim=pass header.d=gmail.com header.s=20221208 header.b=ruxWJLLt; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf30.hostedemail.com: domain of urezki@gmail.com designates 209.85.167.48 as permitted sender) smtp.mailfrom=urezki@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1693816186; 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:dkim-signature; bh=FSQN7fQrN24zOcl+Pz1ofkIl/DUjbLFfNjlnwngy+q8=; b=hwB21opCLecz7O+i2uuA4f2HGzbfYgSDqkt3rCtQKdrK2ZqQS0b81KjBLOOPrkYlII5T7m TtlHR+TloQ6ImDmr+vRvzJgw0TTKdV/4QrC3f9dRaZVeq6wpHS9gM8JK9ytG08w4IOyNsT t8xPLv4c13FmaHDQhhIp+cGWkEj41aU= Received: by mail-lf1-f48.google.com with SMTP id 2adb3069b0e04-5007f3d3235so1796588e87.2 for ; Mon, 04 Sep 2023 01:29:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1693816185; x=1694420985; darn=kvack.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:date:from:from:to :cc:subject:date:message-id:reply-to; bh=FSQN7fQrN24zOcl+Pz1ofkIl/DUjbLFfNjlnwngy+q8=; b=ruxWJLLtzVl+NOpA1CLmnn+6LRM/kjgYTkyKoJOxLBnFW8hgazIvfNLXCR1j8x4gkg rGclTEnUygujTLKiHTE91m5jzguGLuMSmOLcpi96Qihf2G3DtuFRQgp45nXSc7qSIMec 5dEDXZ12gWn6gqxAIaMWBTqncTOlXLgA2KboQodLQvdwXuFbdEEeQ8CG8fBkk8vtmgQF JCQTPdayQhT/qji9fBNP4X2wiGvlaau61ekQe/Da2OJob7l9CQJvy5eYYcyvMOt4g/2j n0qL7CzvLj97wnqt1cx8YbJLieAjLNFTIgzX1Xqif+68eTnGcl55P0HEuYi0nhInTf2k X6mQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1693816185; x=1694420985; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:date:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=FSQN7fQrN24zOcl+Pz1ofkIl/DUjbLFfNjlnwngy+q8=; b=TXwUxZ9ADbm1rVPfEVx/eZg5AXpUmIRu0EMaXhEDxYQ0n3Lp+yrG/lKkJ9Mx7na7o1 bGZ/eASAmvn93pxtVLJdfpDkQSb0fE8Q1IEUVFQe94YbqIG118HnLC/2OzPJ5CNipVM+ TSGOPmpac0368jhkLsb6zHiIjGQMHfgQKa6wbpTPN3+EAQBd2kcbXxXT9e0dbE8P0miA w6wo1vEBV3z0rJ6NcYiJonqvHGLzRlP3ofNgf18Ups2iA57epT+HXkrRjbM4cJLQ5mKn aEAemHbtrdKyS44KXtMlBsH/Ac5su2Fx0yIRMz99PbLNfUGJypWnDgcM0ZBsXHXzkKCT MsQw== X-Gm-Message-State: AOJu0Yy/UgvOemkJLUYAPKveK62M/CdehWUiq76VhiEoi000sKK0A2qQ YdoAnzAtRUupGDNtw55d5F0= X-Google-Smtp-Source: AGHT+IElqWJcui3EeYMMBUsZPvO53t34k8zy+aGUzwGn20FizbdtafmlhAUIwiHLVvPxfX8faVTnyQ== X-Received: by 2002:ac2:4f0a:0:b0:500:ca0f:605e with SMTP id k10-20020ac24f0a000000b00500ca0f605emr6690362lfr.6.1693816184732; Mon, 04 Sep 2023 01:29:44 -0700 (PDT) Received: from pc636 (host-90-235-20-237.mobileonline.telia.com. [90.235.20.237]) by smtp.gmail.com with ESMTPSA id c19-20020ac244b3000000b005008b5191aesm1604815lfm.284.2023.09.04.01.29.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 04 Sep 2023 01:29:44 -0700 (PDT) From: Uladzislau Rezki X-Google-Original-From: Uladzislau Rezki Date: Mon, 4 Sep 2023 10:29:41 +0200 To: Joel Fernandes Cc: Uladzislau Rezki , linux-kernel@vger.kernel.org, Andrew Morton , Christoph Hellwig , Lorenzo Stoakes , Zhen Lei , "Paul E . McKenney" , rcu@vger.kernel.org, Zqiang , Matthew Wilcox , linux-mm@kvack.org Subject: Re: [PATCH v2 1/2] mm/vmalloc: Add a safer version of find_vm_area() for debug Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: CAA2680021 X-Stat-Signature: 3xbaickswbqyrwirwxg1xkctaxgu9qs3 X-HE-Tag: 1693816186-932937 X-HE-Meta: U2FsdGVkX1+uf/A5nXmPP+/Mc6AHRu0fCaXYq+QJDAzljZiuOIhBq+ByRcITRSD/NcY06TzbLXSq6ulTpzkVyoYTZC/3XcfPp23RX+ZLlR2/DCMf09SM5D9wBDwcpoXz118pItCwYppFEEiKg6uT49vFjPBHAS0AvSt9ZGfo7/YNtHQ9bKnoRGNq4xWlD4F4Pw1sUwGxJLJ5r8Ahjtjxmwpxm+u1s/VmAmGk1qnfmGcCl7lRRC2YO0qrXSaFtsRfxEQiNAAxYJMligARdrXrONv/Iw9KOpSlhs++DKghiyA7fl+mhoqQbzELqPLXoM8T/gtiQzaY4VKuUqMzelBjQU0aam3DMfAjnt6IzSf/k8+LRo1d1hDTYffDbNLqE1xBLJPsW3i1Wy+bzMd+w7152YU29K33e/0lxq4GudOFJN4ehJ4rIyS6rdnF7bHBRMBvb0UFSV2Z+H16y0MkuoIOWLelQwSmDtVFW90ZJ2Ew0SADru55b/VZIv5gAZfsCY8/QFEo12SWEOuKhO3x+DzSqYwVR576nojyjIzHsnfl7eNNFMVCVMOKMAfOsvjcCoCpU7ZPUCghU6ReRkRtacBh8XVwfYNgTAGqngH7TvLP8X7Q+5qkEoQHRXCtX9br7M/YSMX8TE8rPZ4B7Q/qk/ChXejq/e2CBBjurEFYJbWi+nYeLl/YX0XLeulQoKCURVygonDN+KBRdAoQ+uAxcYpozna9EZTSsm1/5+CxPHDPMCGNGrgaUnvvQjk3U56L+D8wNvKqxbUrkgUMoIlPa4ZzEp3C4rnn1eSseUDMVExHJ3RmnrlAL8fqUG8GrmN7UKdJW9GfEQF5QOgjprQY2hsig+o7y5pm98KWfYAF3vAb16crlebMKbAPalSfJNqcQ08orUNQAuTnLrG7bEyoa1B39vnGD3jQ7JkCmAQUmwwjrUl4maOdzY5Xd8dWznd2uO8O1+OL+mbGIR0b6qU8G5E yP9VYjeQ uEMFQuA6rXJFpVi/lp/R1ETJthbOrBu0DgoSS0gh8m66I+3gJeCOSkSTUx19sQG+BFMkbWpxFOIbjlW4FZgZm2y243jDNEoq412Au+lwhHcHAz8fknrMr6W3mPixpEDWIaKCTz4qE3pmWGqPeopREO7k1/T2ljVbYpM9/z9gfU4aHg4CNimuEy3uvgWeFLmJdgQ3OZeK0omoMKHnwrkFA4uE5VVm1Y3NCP0ItQP4YlzwLh5Ig8B5Rde1EBqyKJJP2VFGcmWmPUJyIf5wJW+K8LQLCqHZewefzbWZHVb/nIrQaRGApnwluWCQB+HxZJDFtAOawLEPBvK/BRvcOVoux0YD+ThPGW5Ll4GKy7CeEXFdhV/XP1hgJzswW1bk2v90RQtKTSfU9YEqidBVYjYlW+NEPkjsiUFEF1yI1vUZDSQVE8pyEzHGszKqMdlY5mZ3RphFO8EAzHyi9PtE9IkgzNrB5PAAcLXhqFduF+QOlqQB+tuDkyLIqXSXu0UG61t/X1kJwFy99Kl473V2NgGozTJ+e+k01EaLNY5neh/FBnWcRhDmQhofPRCKlRZJPelpIS5CX658tq8Mmv7/knKevDrqHIhpm/NDTwRQhKrjDoytgIIIIRhXgdd/eMbhWzMaGbmn+ijlarIR2vE+8B/WSd0JQYL7XCCtHcaFCuFiVckI5XphXM0MDwp6IznnEOeRDYHtUkY5kUxPHV27vKiKw15l86qxQuSD4iMweqoKvvUqUCgp2rIGhZEQfioXwNtRiv6Txm3eusxb9598jNztw/JxPdYKNbkAkc3rhYSEHtMIStXzQvi4DGfgBnQ== 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: On Fri, Sep 01, 2023 at 12:41:24PM -0400, Joel Fernandes wrote: > > > > On Sep 1, 2023, at 8:48 AM, Uladzislau Rezki wrote: > > > > On Fri, Sep 01, 2023 at 12:33:21AM +0000, Joel Fernandes wrote: > >>> On Fri, Sep 01, 2023 at 12:19:17AM +0000, Joel Fernandes wrote: > >>> On Thu, Aug 31, 2023 at 09:47:52PM +0200, Uladzislau Rezki wrote: > >>>> On Thu, Aug 31, 2023 at 05:18:25PM +0000, Joel Fernandes (Google) wrote: > >>>>> It is unsafe to dump vmalloc area information when trying to do so from > >>>>> some contexts. Add a safer trylock version of the same function to do a > >>>>> best-effort VMA finding and use it from vmalloc_dump_obj(). > >>>>> > >>>>> [apply test robot feedback on unused function fix.] > >>>>> > >>>>> Reported-by: Zhen Lei > >>>>> Cc: Paul E. McKenney > >>>>> Cc: rcu@vger.kernel.org > >>>>> Cc: Zqiang > >>>>> Reviewed-by: Matthew Wilcox (Oracle) > >>>>> Signed-off-by: Joel Fernandes (Google) > >>>>> --- > >>>>> v1->v2: Apply review tags and test robot feedback. > >>>>> > >>>>> mm/vmalloc.c | 39 ++++++++++++++++++++++++++++++++++++++- > >>>>> 1 file changed, 38 insertions(+), 1 deletion(-) > >>>>> > >>>>> diff --git a/mm/vmalloc.c b/mm/vmalloc.c > >>>>> index 93cf99aba335..f09e882ae3b8 100644 > >>>>> --- a/mm/vmalloc.c > >>>>> +++ b/mm/vmalloc.c > >>>>> @@ -1865,6 +1865,20 @@ struct vmap_area *find_vmap_area(unsigned long addr) > >>>>> return va; > >>>>> } > >>>>> > >>>>> +#ifdef CONFIG_PRINTK > >>>>> +static struct vmap_area *find_vmap_area_trylock(unsigned long addr) > >>>>> +{ > >>>>> + struct vmap_area *va; > >>>>> + > >>>>> + if (!spin_trylock(&vmap_area_lock)) > >>>>> + return NULL; > >>>>> + va = __find_vmap_area(addr, &vmap_area_root); > >>>>> + spin_unlock(&vmap_area_lock); > >>>>> + > >>>>> + return va; > >>>>> +} > >>>>> +#endif > >>>>> + > >>>>> static struct vmap_area *find_unlink_vmap_area(unsigned long addr) > >>>>> { > >>>>> struct vmap_area *va; > >>>>> @@ -2671,6 +2685,29 @@ struct vm_struct *find_vm_area(const void *addr) > >>>>> return va->vm; > >>>>> } > >>>>> > >>>>> +/** > >>>>> + * try_to_find_vm_area - find a continuous kernel virtual area > >>>>> + * @addr: base address > >>>>> + * > >>>>> + * This function is the same as find_vm_area() except that it is > >>>>> + * safe to call if vmap_area_lock is already held and returns NULL > >>>>> + * if it is. See comments in find_vmap_area() for other details. > >>>>> + * > >>>>> + * Return: the area descriptor on success or %NULL on failure. > >>>>> + */ > >>>>> +#ifdef CONFIG_PRINTK > >>>>> +static struct vm_struct *try_to_find_vm_area(const void *addr) > >>>>> +{ > >>>>> + struct vmap_area *va; > >>>>> + > >>>>> + va = find_vmap_area_trylock((unsigned long)addr); > >>>>> + if (!va) > >>>>> + return NULL; > >>>>> + > >>>>> + return va->vm; > >>>>> +} > >>>>> +#endif > >>>>> + > >>>>> /** > >>>>> * remove_vm_area - find and remove a continuous kernel virtual area > >>>>> * @addr: base address > >>>>> @@ -4277,7 +4314,7 @@ bool vmalloc_dump_obj(void *object) > >>>>> struct vm_struct *vm; > >>>>> void *objp = (void *)PAGE_ALIGN((unsigned long)object); > >>>>> > >>>>> - vm = find_vm_area(objp); > >>>>> + vm = try_to_find_vm_area(objp); > >>>>> if (!vm) > >>>>> return false; > >>>>> pr_cont(" %u-page vmalloc region starting at %#lx allocated at %pS\n", > >>> > >>> Hi Vlad, > >>> Thanks for taking a look. > >>> > >>>> I am not sure if this patch makes a lot of sense. I agree, this is a > >>>> problem and it mitigates it. But it is broken in terms of once you drop > >>>> the lock, the VA should not be accessed. > >>> > >>> Just to note the lockless-access issue you are referring to is not introduced > >>> by this patch but is rather in the existing code. Also just to note this is > >>> debug code. > >>> > >>>> Is that a real issue or it gets triggered due to some syntetic test case? > >>> > >>> It is a real issue. See 2/2. > >>> > >>>> If i were you, i would go with open-coded version of trylock. Because > >>>> there is only one user so far. > >>> > >>> Taking your open coding and locking suggestions, I came up with the below > >>> which actually results in a smaller patch. Does it look good to you? > >>> > >>> diff --git a/mm/vmalloc.c b/mm/vmalloc.c > >>> index 93cf99aba335..aaf6bad997a7 100644 > >> > >> And with some trivial compiler errors fixed (sorry should have build tested > >> but wanted to just share the idea earlier): > >> > >> diff --git a/mm/vmalloc.c b/mm/vmalloc.c > >> index 93cf99aba335..2c6a0e2ff404 100644 > >> --- a/mm/vmalloc.c > >> +++ b/mm/vmalloc.c > >> @@ -4274,14 +4274,32 @@ void pcpu_free_vm_areas(struct vm_struct **vms, int nr_vms) > >> #ifdef CONFIG_PRINTK > >> bool vmalloc_dump_obj(void *object) > >> { > >> - struct vm_struct *vm; > >> void *objp = (void *)PAGE_ALIGN((unsigned long)object); > >> + const void *caller; > >> + struct vm_struct *vm; > >> + struct vmap_area *va; > >> + unsigned long addr; > >> + unsigned int nr_pages; > >> > >> - vm = find_vm_area(objp); > >> - if (!vm) > >> + if (!spin_trylock(&vmap_area_lock)) > >> + return false; > >> + va = __find_vmap_area((unsigned long)objp, &vmap_area_root); > >> + if (!va) { > >> + spin_unlock(&vmap_area_lock); > >> return false; > >> + } > >> + > >> + vm = va->vm; > >> + if (!vm) { > >> + spin_unlock(&vmap_area_lock); > >> + return false; > >> + } > >> + addr = (unsigned long)vm->addr; > >> + caller = vm->caller; > >> + nr_pages = vm->nr_pages; > >> + spin_unlock(&vmap_area_lock); > >> pr_cont(" %u-page vmalloc region starting at %#lx allocated at %pS\n", > >> - vm->nr_pages, (unsigned long)vm->addr, vm->caller); > >> + nr_pages, addr, caller); > >> return true; > >> } > >> #endif > >> > > Looks good to me and thank you for fixing a locking issue :) > > I think you will re-spin and resend it one more time? > > Yes. May I add your Reviewed-by tag to both patches after re-spinning as mentioned above? > Reviewed-by: Uladzislau Rezki (Sony) -- Uladzislau Rezki