Thread (19 messages) 19 messages, 4 authors, 2021-09-01

Re: [PATCH v4] staging: greybus: Convert uart.c from IDR to XArray

From: Johan Hovold <johan@kernel.org>
Date: 2021-08-31 08:16:53
Also in: lkml

On Mon, Aug 30, 2021 at 02:31:35PM +0100, Matthew Wilcox wrote:
On Mon, Aug 30, 2021 at 02:33:05PM +0200, Johan Hovold wrote:
quoted
On Mon, Aug 30, 2021 at 01:16:07PM +0100, Matthew Wilcox wrote:
quoted
On Mon, Aug 30, 2021 at 01:52:48PM +0200, Johan Hovold wrote:
quoted
Whether the API is better is debatable. As I said, almost no drivers use
the new XArray interface, and perhaps partly because the new interface
isn't as intuitive as has been claimed (e.g. xa_load() instead of
ida_find()). And IDR/IDA isn't marked/documented as deprecated as far as
I know.
quoted
quoted
Why do you think that idr_find() is more intuitive than xa_load()?
The 'find' verb means that you search for something.  But it doesn't
search for anything; it just returns the pointer at that index.
'find' should return the next non-NULL pointer at-or-above a given
index.
We're looking up a minor number which may or may not exist. "Find" (or
"lookup" or "search") seems to describe this much better than "load"
(even if that may better reflect the implementation of XArray).
It's not the _implementation_ that it fits, it's the _idiom_.
The implementation is a lookup in a trie.  The idiom of the XArray
is that it's a sparse array, and so it's a load.
Ok, but it still stands out in the conversions since it is in no way
obvious that idr_find() should be replaced by xa_load() from just
looking at the diff. You need to look up the interface for that.
quoted
And no, I would not expect a find implementation to return the next
entry if the requested entry does not exist (and neither does idr_find()
or radix_tree_lookup()).
Oh dear.  You've been corrupted by the bad naming of the IDR functions
;-(
Heh. Don't flatter yourself. Just look up any text book on data
structures.

Johan
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help