Received-SPF: pass (google.com: domain of linux-kernel+bounces-132137-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Thu, 4 Apr 2024 14:23:26 -0700
From: Andrew Morton <akpm@linux-foundation.org>
To: Vishal Verma <vishal.l.verma@intel.com>
Cc: Dan Williams <dan.j.williams@intel.com>, Dave Jiang
 <dave.jiang@intel.com>, Alison Schofield <alison.schofield@intel.com>,
 linux-mm@kvack.org, nvdimm@lists.linux.dev, linux-cxl@vger.kernel.org,
 linux-kernel@vger.kernel.org
Subject: Re: [PATCH] dax/bus.c: replace WARN_ON_ONCE() with lockdep asserts
Message-Id: <20240404142326.b3d5b0f3f7b9f25f1a16288a@linux-foundation.org>
In-Reply-To: <20240402-vv-dax_abi_fixes-v1-1-c3e0fdbafba5@intel.com>
References: <20240402-vv-dax_abi_fixes-v1-1-c3e0fdbafba5@intel.com>
Precedence: bulk
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

On Tue, 02 Apr 2024 00:24:28 -0600 Vishal Verma <vishal.l.verma@intel.com> wrote:

> In [1], Dan points out that all of the WARN_ON_ONCE() usage in the
> referenced patch should be replaced with lockdep_assert_held(_write)().
> 
> Replace those, and additionally, replace a couple of other
> WARN_ON_ONCE() introduced in the same patch for actual failure
> cases (i.e. when acquiring a semaphore fails in a remove / unregister
> path) with dev_WARN_ONCE() as is the precedent here.
> 
> Recall that previously, unregistration paths was implicitly protected by
> overloading the device lock, which the patch in [1] sought to remove.
> This meant adding a semaphore acquisition in these unregistration paths.
> Since that can fail, and it doesn't make sense to return errors from
> these paths, retain the two instances of (now) dev_WARN_ONCE().
> 
> ...
>
> @@ -471,6 +471,7 @@ static void __unregister_dev_dax(void *dev)
>  
>  	dev_dbg(dev, "%s\n", __func__);
>  
> +	lockdep_assert_held_write(&dax_region_rwsem);
>  	kill_dev_dax(dev_dax);
>  	device_del(dev);
>  	free_dev_dax_ranges(dev_dax);

This is new and unchangelogged?

I'm taking Dan's reply to your patch as Not-A-Nack ;)