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 94F55C54E4A for ; Thu, 7 Mar 2024 19:46:35 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DA06D6B0281; Thu, 7 Mar 2024 14:46:34 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id D503F6B0282; Thu, 7 Mar 2024 14:46:34 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C3EEC6B0283; Thu, 7 Mar 2024 14:46:34 -0500 (EST) 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 B25496B0281 for ; Thu, 7 Mar 2024 14:46:34 -0500 (EST) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 7E1171A06CC for ; Thu, 7 Mar 2024 19:46:34 +0000 (UTC) X-FDA: 81871275108.03.6D30271 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf11.hostedemail.com (Postfix) with ESMTP id 9485D4000F for ; Thu, 7 Mar 2024 19:46:32 +0000 (UTC) Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=linux-foundation.org header.s=korg header.b=eauGu7pU; dmarc=none; spf=pass (imf11.hostedemail.com: domain of akpm@linux-foundation.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=akpm@linux-foundation.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1709840793; 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=YKVj3AmYdIGS1hS5Bx8ducYL25JEqQhDXqMSV9sIYvU=; b=JEUlE4XQfHd9nzK/tHAq+OSvaVXBtpNMN2F0rOhGiUAV3vIKyxtBDWG6W1nZADm7zokJAF 3d+aXVm/vo3PLlhBe2sAOFphema4QvNM5y+c0m2H12LeQ1xYM6ikvnQvlkq0TAWZ8vlUkK dDWkw1Sv5I2nwoCtNwsBYekPC+VWKTQ= ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=pass header.d=linux-foundation.org header.s=korg header.b=eauGu7pU; dmarc=none; spf=pass (imf11.hostedemail.com: domain of akpm@linux-foundation.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=akpm@linux-foundation.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1709840793; a=rsa-sha256; cv=none; b=HyEoqHReD5fe9S4IvH1jtXf9TAc9iGSfUhrn5IWY/K5ykr2GyRONlPZCfTdJMQ7jJWrr4T l51L+bvA/hXj36J8gVcvILoG+zEzXpk6i3aAAEcKUdLrrlUpUCukShYh5wgRW9jh3eN5GV 9DQh60LcIm2PDl9r1lzdmxPAg3VNutY= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id E2812611F5; Thu, 7 Mar 2024 19:46:31 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 65E5CC433C7; Thu, 7 Mar 2024 19:46:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linux-foundation.org; s=korg; t=1709840791; bh=ZkoLriIFMqvbtvNzCDlOVN4ynWY6JZVhyoE0r2/dOLc=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=eauGu7pU2cnbm5PxdoOPGqcJrKJWzalQoLQXnotDT8SMjSUVteZDfVN0M4hjcp6h0 NkY5wvnlqgftIA9ras3eqMPnZ7Zl7YvRJSlUqpa8FE6Mb5kIbdDPnY6TuLkLieCcBR EGimGhHMtu4+jQFG2i4YN7Ri0mdT9wMGljK3Etdc= Date: Thu, 7 Mar 2024 11:46:30 -0800 From: Andrew Morton To: Waiman Long Cc: Catalin Marinas , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Audra Mitchell Subject: Re: [PATCH v2] mm/kmemleak: Don't hold kmemleak_lock when calling printk() Message-Id: <20240307114630.32702099ac24c182b91da517@linux-foundation.org> In-Reply-To: <20240307184707.961255-1-longman@redhat.com> References: <20240307184707.961255-1-longman@redhat.com> X-Mailer: Sylpheed 3.8.0beta1 (GTK+ 2.24.33; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Rspam-User: X-Stat-Signature: 7d4n75qozbri4ahjnb5pm1ynue53wkbw X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 9485D4000F X-HE-Tag: 1709840792-292407 X-HE-Meta: U2FsdGVkX18JW39Atfz6ojdwaRluqbOj7IHFnrnmlL5WZo2TUjKgv94gfcoj571HpTp5AfCge2yn6LPH6t4kSBawffvP+5AMQt+X9MauKyvOMO+1EYn1EV+rnP3ErlK11sH5On2bKURBZcWc6ZpjXwfhghoEbUP7d7TYgqdZ7YQcyL49u+WH6p+QhLYi03QZa3Hep+LbeFdvOo+mB/XoVxU1fjk3M+3xzR9RLyEzhQjb/0ke5OhEvvJIpnNdQFis+Hui2qdH/kLRW0Q0Tw7h4Kvm2n44RAfu6K509yJ07HJSlecIbV2NevuNp8JAoDAbTeoJ9olIGds2TI1vphAp2/O4xhJpnvF9WAn/NfClypAanuQiDcIZeI+3l+Bf7OU9U4QP4GmbJLEZs3Z/3OOezhbXKLg6tc+s17GMXAuyGywwbz5lU7eJhl3pLfp1xnNGWuas+ff4tbDdpVWxmOkT6b6MzUfmVia/fUwbwrN5RYk5V44hfVa9vvmx8P451WruWyDy4CWYMxZhu5k7zKrOq/B/GOOMjO6aTpqlRce9ozG2/ptwldmNeWm5NboSOWSZExy9NLj+v34uqhRotiYqFHt6slzOoXSDomnnNiZJ+QokQvHt+LdliWpcdKKnhFVoRjxgElldiD0tW4UpYoUnpQ6ZtUJ3DczfvYikehrtuk20GpePWAmz6P7sqrRxbjHj29Y51F2xwa8F8javJKrwTZPW0UvCDCgRKOF5MFyr0NSIehmmLVTkvJaDuaWG4wWxl35X5+zMzPBtGphKVo6YL+66YCFPUkDbkhUZulbbberc/enxF5gUg7/kX1a5OStH7xKhoxelq48VVzk06DY8SakMhiv5oILB/5KBdzMKzfdMfORrrk/JGK7pgyqHBOzA802oSy0BIN9ykQ1E3QnIUjkGeU2baqLVesoK1wyddzbc6XUosYEN3bdCC8mxHs4WOfgnI/AeSzYgDZQUx7/ r/AF/qdo JWbZX 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 Thu, 7 Mar 2024 13:47:07 -0500 Waiman Long wrote: > When some error conditions happen (like OOM), some kmemleak functions > call printk() to dump out some useful debugging information while holding > the kmemleak_lock. This may cause deadlock as the printk() function > may need to allocate additional memory leading to a create_object() > call acquiring kmemleak_lock again. > > An abbreviated lockdep splat is as follows: > > ... > > Fix this deadlock issue by making sure that printk() is only called > after releasing the kmemleak_lock. > > ... > > @@ -427,9 +442,19 @@ static struct kmemleak_object *__lookup_object(unsigned long ptr, int alias, > else if (untagged_objp == untagged_ptr || alias) > return object; > else { > + if (!get_object(object)) > + break; > + /* > + * Release kmemleak_lock temporarily to avoid deadlock > + * in printk(). dump_object_info() is called without > + * holding object->lock (race unlikely). > + */ > + raw_spin_unlock(&kmemleak_lock); > kmemleak_warn("Found object by alias at 0x%08lx\n", > ptr); > dump_object_info(object); > + put_object(object); > + raw_spin_lock(&kmemleak_lock); > break; Please include a full description of why this is safe. Once we've dropped that lock, the tree is in an unknown state and we shouldn't touch it again. This consideration should be added to the relevant functions' interface documentation and the code should be reviewed to ensure that we're actually adhering to this. Or something like that. To simply drop and reacquire a lock without supporting analysis and comments does not inspire confidence!