Re: [PATCH] block: Fix partition check for host-aware zoned block devices

[Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>]

On Oct 16, 2021 / 15:26, Matthew Wilcox wrote:
> On Sat, Oct 16, 2021 at 06:34:50AM +0200, Christoph Hellwig wrote:
> > On Fri, Oct 15, 2021 at 11:07:40AM +0900, Shin'ichiro Kawasaki wrote:
> > > To fix the issues, call the helper function disk_has_partitions() in
> > > place of disk->part_tbl empty check. Since the function was removed with
> > > the commit a33df75c6328, reimplement it to walk through entries in the
> > > xarray disk->part_tbl.
> > 
> > Looks good,
> > 
> > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > 
> > Matthew,
> > 
> > we talked about possiblig adding a xa_nr_entries helper a while ago.
> > This would be a good place for it, as we could just check
> > xa_nr_entries() > 1 for example.
> 
> Do I understand the problem correctly, that you don't actually want to
> know whether there's more than one entry in the array, but rather that
> there's an entry at an index other than 0?
> 
> If so, that's an easy question to answer, we just don't have a helper
> for it yet.  Something like this should do:
> 
> static inline bool xa_is_trivial(const struct xarray *xa)
> {
> 	void *entry = READ_ONCE(xa->xa_head);
> 
> 	return entry || !xa_is_node(entry);
> }
> 
> Probably needs a better name than that.

Thanks for the discussion. Based on the code above, I tried following hunk
below, and confirmed the new helper function can be used to fix the issue I
found. Good. To make it work, I needed to change the logical operator in the
function from OR to AND. As for the function name, my mere suggestion is
xa_has_single_entry(), but this may not be the best.

I would like to ask advice on the next action for the fix. If my original patch
can go to upstream first, I think the changes for this new xarray helper can be
done later. This approach would be good if the new helper will not be propagated
to the stable branches. Another approach is to do both of this new helper
introduction and the issue fix at this moment. Which approach is the better?


diff --git a/include/linux/xarray.h b/include/linux/xarray.h
index a91e3d90df8a..7a31c9423d01 100644
--- a/include/linux/xarray.h
+++ b/include/linux/xarray.h
@@ -1238,6 +1238,20 @@ static inline unsigned long xa_to_sibling(const void *entry)
 	return xa_to_internal(entry);
 }
 
+/**
+ * xa_has_single_entry() - Does it have an entry at an index other than 0?
+ * @entry: XArray entry.
+ *
+ * Context: Any context.
+ * Return: %true if there is no entry at an index other than 0.
+ */
+static inline bool xa_has_single_entry(const struct xarray *xa)
+{
+	const void *entry = READ_ONCE(xa->xa_head);
+
+	return entry && !xa_is_node(entry);
+}
+
 /**
  * xa_is_sibling() - Is the entry a sibling entry?
  * @entry: Entry retrieved from the XArray


-- 
Best Regards,
Shin'ichiro Kawasaki