Re: [PATCH v10 1/9] dax: Introduce holder for dax_device

From: Christoph Hellwig <hch@infradead.org>
To: Shiyang Ruan <ruansy.fnst@fujitsu.com>
Cc: linux-kernel@vger.kernel.org, linux-xfs@vger.kernel.org,
	nvdimm@lists.linux.dev, linux-mm@kvack.org,
	linux-fsdevel@vger.kernel.org, djwong@kernel.org,
	dan.j.williams@intel.com, david@fromorbit.com, hch@infradead.org,
	jane.chu@oracle.com
Subject: Re: [PATCH v10 1/9] dax: Introduce holder for dax_device
Date: Wed, 2 Feb 2022 05:03:43 -0800	[thread overview]
Message-ID: <YfqBLxjr5zz1TU91@infradead.org> (raw)
In-Reply-To: <20220127124058.1172422-2-ruansy.fnst@fujitsu.com>

On Thu, Jan 27, 2022 at 08:40:50PM +0800, Shiyang Ruan wrote:
> +void dax_register_holder(struct dax_device *dax_dev, void *holder,
> +		const struct dax_holder_operations *ops)
> +{
> +	if (!dax_alive(dax_dev))
> +		return;
> +
> +	dax_dev->holder_data = holder;
> +	dax_dev->holder_ops = ops;

This needs to return an error if there is another holder already.  And
some kind of locking to prevent concurrent registrations.

Also please add kerneldoc comments for the new exported functions.

> +void *dax_get_holder(struct dax_device *dax_dev)
> +{
> +	if (!dax_alive(dax_dev))
> +		return NULL;
> +
> +	return dax_dev->holder_data;
> +}
> +EXPORT_SYMBOL_GPL(dax_get_holder);

get tends to imply getting a reference.  Maybe just dax_holder()?
That being said I can't see where we'd even want to use the holder
outside of this file.