* Thomas Gleixner <tglx@xxxxxxxxxxxxx> [230516 18:48]: > On Mon, May 15 2023 at 15:27, Liam R. Howlett wrote: > > * Thomas Gleixner <tglx@xxxxxxxxxxxxx> [230510 15:01]: > >> The documentation of mt_next() claims that it starts the search at the > >> provided index. That's incorrect as it starts the search after the provided > >> index. > >> > >> The documentation of mt_find() is slightly confusing. "Handles locking" is > >> not really helpful as it does not explain how the "locking" works. > > > > More locking notes can be found in Documentation/core-api/maple_tree.rst > > which lists mt_find() under the "Takes RCU read lock" list. I'm okay > > with duplicating the comment of taking the RCU read lock in here. > > Without a reference to the actual locking documentation such comments > are not super helpful. Noted. A reference to the larger document should probably be added. Thanks. > > >> Fix similar issues for mt_find_after() and mt_prev(). > >> > >> Remove the completely confusing and pointless "Note: Will not return the > >> zero entry." comment from mt_for_each() and document @__index correctly. > > > > The zero entry concept is an advanced API concept which allows you to > > store something that cannot be seen by the mt_* family of users, so it > > will not be returned and, instead, it will return NULL. Think of it as > > a reservation for an entry that isn't fully initialized. Perhaps it > > should read "Will not return the XA_ZERO_ENTRY" ? > >> > >> - * > >> - * Note: Will not return the zero entry. > > > > This function "will not return the zero entry", meaning it will return > > NULL if xa_is_zero(entry). > > If I understand correctly, this translates to: > > This iterator skips entries, which have been reserved for future use > but have not yet been fully initialized. > > Right? Well, that's one use of using the XA_ZERO_ENTRY, but it's really up to the user to decide why they are adding something that returns NULL in a specific range for the not-advanced API. It might be worth adding the XA_ZERO_ENTRY in here, since that's the only special value right now? > > >> @@ -6487,9 +6493,14 @@ EXPORT_SYMBOL(mtree_destroy); > >> * mt_find() - Search from the start up until an entry is found. > >> * @mt: The maple tree > >> * @index: Pointer which contains the start location of the search > >> - * @max: The maximum value to check > >> + * @max: The maximum value of the search range > >> + * > >> + * Takes RCU read lock internally to protect the search, which does not > >> + * protect the returned pointer after dropping RCU read lock. > >> * > >> - * Handles locking. @index will be incremented to one beyond the range. > >> + * In case that an entry is found @index contains the index of the found > >> + * entry plus one, so it can be used as iterator index to find the next > >> + * entry. > > > > What about: > > "In case that an entry is found @index contains the last index of the > > found entry plus one" > > Still confusing to the casual reader like me :) > > "In case that an entry is found @index is updated to point to the next > possible entry independent whether the found entry is occupying a > single index or a range if indices." > > Hmm? That makes sense to me. Thanks, Liam
