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 67AFCC83F3E for ; Tue, 5 Sep 2023 11:47:14 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 96FA794000E; Tue, 5 Sep 2023 07:47:13 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 91EC48E001A; Tue, 5 Sep 2023 07:47:13 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7E61B94000E; Tue, 5 Sep 2023 07:47:13 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 6B7ED8E001A for ; Tue, 5 Sep 2023 07:47:13 -0400 (EDT) Received: from smtpin16.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 3D7891A0C3E for ; Tue, 5 Sep 2023 11:47:13 +0000 (UTC) X-FDA: 81202367946.16.D4E66BA Received: from mail-io1-f44.google.com (mail-io1-f44.google.com [209.85.166.44]) by imf15.hostedemail.com (Postfix) with ESMTP id 6A638A001F for ; Tue, 5 Sep 2023 11:47:11 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=joelfernandes.org header.s=google header.b=IzvTDVQs; spf=pass (imf15.hostedemail.com: domain of joel@joelfernandes.org designates 209.85.166.44 as permitted sender) smtp.mailfrom=joel@joelfernandes.org; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1693914431; 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=+kMHasHVGTn26MkV0Q3sK5DIWqCalskCsDPU3m7M6q0=; b=Xy4hg1UeqnU3oc2+x1/VFo45BDn8fx0+wjdem887pvhwCYrG6VLssMp7JamUZrXvdd2UKi 2ECPN51E+lect/1Mt9CN+fDkUstV/dhVzYpb6+lzK/7iSgffzfk8oTcnFHwxaACI6wVIUj 4EboC0aVcm+QYAk+OrIAHCliH4/oqZk= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1693914431; a=rsa-sha256; cv=none; b=Vtlwr94kK5GzqFk2770trRIsMA64hU8/eQV3ph61MQOju3lUg+i75KSlJqvo9eNrrYTrFH Y59nK9DC8fW24ewdjHRiTUhB4L+y6IGE+9MG8kZcWg4zN1LaEg0zIHA65HOkgMmdYIoTVo +jyikRvfTb34KfBnSgFyWhi3XeRrlgM= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=pass header.d=joelfernandes.org header.s=google header.b=IzvTDVQs; spf=pass (imf15.hostedemail.com: domain of joel@joelfernandes.org designates 209.85.166.44 as permitted sender) smtp.mailfrom=joel@joelfernandes.org; dmarc=none Received: by mail-io1-f44.google.com with SMTP id ca18e2360f4ac-79275d86bc3so97021439f.0 for ; Tue, 05 Sep 2023 04:47:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=joelfernandes.org; s=google; t=1693914430; x=1694519230; darn=kvack.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=+kMHasHVGTn26MkV0Q3sK5DIWqCalskCsDPU3m7M6q0=; b=IzvTDVQsUci7iTYPw/5jKT2ApqhBRjm/mip42BTJFFpk/fBZsJHgtQt/YbHn1R5Pr7 Da5gX8FJSRqMtSgiOCUczZe3R597mMypu48bc4jyL5qpXujQ/zjdkaKzFeS8k+KRx9MS aS+f3OXsp1y07ZGYyPwT+J+dlnlFeYttYdvVg= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1693914430; x=1694519230; h=in-reply-to: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=+kMHasHVGTn26MkV0Q3sK5DIWqCalskCsDPU3m7M6q0=; b=VO+pHzPoUAmuuH0GLYYMAVLQoVbBfWsk8Ds4OXrEij4K86S5oFgb8+ncEHgCYSem5P o/XD1eMJJNBk7CCzPrICzOc0FBtpWSNY+PXILMDjsnJmrXdl2KcsDe/yzCA73rNFl5DA fDReSkhJNuIxyrA8fcksJzcqFnQdy3UJuC0wqQSHkQwyyjOjyyddDrSLpEnvF6vV8iEK PQfC/o4f0j54rW6DqmjVQOJvxDNuD4CH8InoHR2M/m8JUtjGvLesVSluJs6BKBVhPOzq kAwyY5UZCiDSXGq39s/OxvW4H7fGPw/WRcLtWTzG8uffxQbaHCuH+6ZhdXsnVxi2aaip F3Ww== X-Gm-Message-State: AOJu0Yw+Fz6bTpoLRRhrWTW2cNgYE5xhte+qKNmYmidepTTPR7NCui/V Aj3Jps3xa2AEeE7em5LT/JVdDA== X-Google-Smtp-Source: AGHT+IHDwYBN4jbjbJ2InS/JWRLSveg7Bkkv+CFcAJjUWQW7JPDz2+SoDzDcEpMiiPdIjwRdBiw2KQ== X-Received: by 2002:a5e:dc49:0:b0:785:5917:a35f with SMTP id s9-20020a5edc49000000b007855917a35fmr14731736iop.8.1693914430477; Tue, 05 Sep 2023 04:47:10 -0700 (PDT) Received: from localhost (156.190.123.34.bc.googleusercontent.com. [34.123.190.156]) by smtp.gmail.com with ESMTPSA id k26-20020a6b401a000000b0077e3566a801sm4266287ioa.29.2023.09.05.04.47.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 05 Sep 2023 04:47:09 -0700 (PDT) Date: Tue, 5 Sep 2023 11:47:09 +0000 From: Joel Fernandes To: Lorenzo Stoakes Cc: linux-kernel@vger.kernel.org, Andrew Morton , Uladzislau Rezki , Christoph Hellwig , "Paul E. McKenney" , Vlastimil Babka , Zhen Lei , rcu@vger.kernel.org, Zqiang , stable@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH v3 1/2] mm/vmalloc: Add a safer version of find_vm_area() for debug Message-ID: <20230905114709.GA3881391@google.com> References: <20230904180806.1002832-1-joel@joelfernandes.org> <571d4a4a-0674-4c84-b714-8e7582699e30@lucifer.local> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <571d4a4a-0674-4c84-b714-8e7582699e30@lucifer.local> X-Rspamd-Queue-Id: 6A638A001F X-Rspam-User: X-Rspamd-Server: rspam11 X-Stat-Signature: q4ypo84q6jb3zd57w83kdim3bia9irjc X-HE-Tag: 1693914431-321953 X-HE-Meta: U2FsdGVkX19mPJ/aFD0UDiy07s+/lDOh7FmFAF1yJM8JlJ+pY82lJ/E9BYmAgB0Lo4l7EztqlgoimCFGstT604oVwtf+uTvCZfqw3S23JoTcQlooccA1L2EISqMP2+A/mu94vRK2Mm30mQYfRQKi2JMLP1Y76uhFpHhL6GOZKkpvEMA4lx9h7VT8kcCghrUPvQ6f3PszxdVNMR4XqXusPsXHli49zk3sRTjHTBYTsFk1XYzeRTvkeLqaUYAT2SqYPIGs9Vs7qLuH2R5+TM0nrNRElErTSgV17tKn++7i2OHVI9oAsnX5uqYk1JvqnKyGRxGMZoLGLZ71nl0YXK4wwmhvZPsrMp496i0gRvQfoh/BJ4K2adJm5InEJZ7+V33GN3hvwUuy+Zg3iK006E9YdiNx8HtelBV+6dCOB3aNauH0+GzMvTnrPM2iibotXTLkN56bOY+4UOQMX+U2QGBaK9pQk4bhhkg0moXrTsYRFPcXUN0bCiRUpMLYdGtQ4XyePwUO2DAOnTl1D6NCBWW77n8R3F1FaRcHL719xUlVR9dnxuLk1M0y7viF/T8E+iuM7ZEMu1j6HrVVQCyM3YAbwWR1wn+kDhhaoI0dGsA6NTXgZ1s646+AudueIDpCqkTHMcUIOthGB8lBI8MxS5AxQw3LICkMqopF8UxonhjEARC215It7Rdq0lIZOBeEFKWS4dTx9B/bp3SRlpPbvOk4brgLOOdK2DJoVNKu4pzheNIovpv31T52AT+jPCr+RJSq1bZljFPyB3FRwsTvj05VfOZuf9ia2yCRoFxSnM2xorVPpf89/nd4tUjdFLViyfKAN8ALeG/UWMsOhaKK2Hiv2ckZnja3x48vNCqz6PUBqSlcjDDYMi1SMcG9xa35qo4VUojfCDFyQ3CSCgyovFBfxip8NsL963GB6v5JR8eA7sbai0PIsnhjd/df3Lh64ua7KIALgkNa5mI9k/8E6jg Gn7zgQGG h2al9vDncWkqpMGN3dvZdFV+5+LRkNt54REnnNU+5VcPDuuMJCY/1RFA2INJMFKZJ/3WKTXVty2veBwOkeblCIItfZTwgXgFaHCqtQWaKJBmJo24BRrSyrijbfpFCPPsRQAq/N1MBbZrrSIURvjW1qb5H7N72iGydpe9qBxRd8rKI3s3DkWa0to/RNLWNmSxFslAdGnRKT88bRqi/YV0HQC0VTmbgl/phIUh7lQ0ezhm6S8aA9sl+gl5rRpMEt71LGSFJRBSxoUkllFA7k+dzHNx9wdA3ZX9yi/LAGQsIYepIvp6B+a5/MsmRfbKO+wZXSGf8dm0jCX6oteG3H70HLrTyi64l4AWcJWIpdAk78GAmvufBTCzmY6Kj98FTSAC8aNetRcyNCTQrHp8Yzk7C/m/hjrykEe3+iN/iEIK7y78YbQC53d2ElDxQC5SdTYpbBwMBFg5GCEVp3QL8nidpbM2PEKqJX1e4JAYgGjl74Ykw0ItjmbvZ51esfFSmOV5oTdtFCgmfC86ZMJi1FUWOKHsnxCj97NettRPNTCjjGW1NCoocUmPj3KMFB3hZG0HV+QTwh80VrugFBCIRc9rTMYJ7tXceTuw60A6Ec+9aNbWirgstihubnE7xofn0IZ2R/gbs 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 Tue, Sep 05, 2023 at 08:09:16AM +0100, Lorenzo Stoakes wrote: > On Mon, Sep 04, 2023 at 06:08:04PM +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(). > > It'd be nice to have more details as to precisely which contexts and what this > resolves. True. I was hoping the 'trylock' mention would be sufficient (example hardirq context interrupting a lock-held region) but you're right. > > [applied test robot feedback on unused function fix.] > > [applied Uladzislau feedback on locking.] > > > > Reported-by: Zhen Lei > > Cc: Paul E. McKenney > > Cc: rcu@vger.kernel.org > > Cc: Zqiang > > Reviewed-by: Uladzislau Rezki (Sony) > > Fixes: 98f180837a89 ("mm: Make mem_dump_obj() handle vmalloc() memory") > > Cc: stable@vger.kernel.org > > Signed-off-by: Joel Fernandes (Google) > > --- > > mm/vmalloc.c | 26 ++++++++++++++++++++++---- > > 1 file changed, 22 insertions(+), 4 deletions(-) > > > > 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; > > It'd be good to have a comment here explaining why we must trylock here. I am > also concerned that in the past this function would return false only if the > address was not a vmalloc one, but now it might just return false due to lock > contention and the user has no idea which it is? > > I'd want to at least output "vmalloc region cannot lookup lock contention" > vs. the below cannot find case. In the patch 2/2 we do print if the address looks like a vmalloc address even if the vmalloc look up fails. Also the reporter's usecase is not a common one. We only attempt to dump information if there was a debug objects failure (example if somebody did a double call_rcu). In such a situation, the patch will prevent a deadlock and still print something about the address. > Under heavy lock contention aren't you potentially breaking the ability to > introspect vmalloc addresses? Wouldn't it be better to explicitly detect the > contexts under which acquiring this spinlock is not appropriate? Yes this is a good point, but there's another case as well: PREEMPT_RT can sleep on lock contention (as spinlocks are sleeping) and we can't sleep from call_rcu() as it may be called in contexts that cannot sleep. So we handle that also using trylock. Thanks for the review! - Joel > > > + 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 > > -- > > 2.42.0.283.g2d96d420d3-goog > >