Message-ID: <gta2wjt4tkcvyu7g56gqyipiigbmbk2ca3biqzmxc2npch3rk7@ccwzwgnk7mcz>
Date: Wed, 11 Feb 2026 13:21:26 -0500
From: "Liam R. Howlett" <Liam.Howlett@...cle.com>
To: Andreas Hindborg <a.hindborg@...nel.org>
Cc: Tamir Duberstein <tamird@...il.com>, Miguel Ojeda <ojeda@...nel.org>,
        Alex Gaynor <alex.gaynor@...il.com>, Boqun Feng <boqun.feng@...il.com>,
        Gary Guo <gary@...yguo.net>,
        Björn Roy Baron <bjorn3_gh@...tonmail.com>,
        Benno Lossin <lossin@...nel.org>, Alice Ryhl <aliceryhl@...gle.com>,
        Trevor Gross <tmgross@...ch.edu>, Danilo Krummrich <dakr@...nel.org>,
        Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
        Vlastimil Babka <vbabka@...e.cz>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Christoph Lameter <cl@...two.org>,
        David Rientjes <rientjes@...gle.com>,
        Roman Gushchin <roman.gushchin@...ux.dev>,
        Harry Yoo <harry.yoo@...cle.com>, Daniel Gomez <da.gomez@...nel.org>,
        rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-mm@...ck.org
Subject: Re: [PATCH v3 03/12] rust: xarray: add `contains_index` method

* Andreas Hindborg <a.hindborg@...nel.org> [260211 02:41]:
> "Liam R. Howlett" <Liam.Howlett@...cle.com> writes:
> 
> > * Andreas Hindborg <a.hindborg@...nel.org> [260209 14:38]:
> >> Add a convenience method `contains_index` to check whether an element
> >> exists at a given index in the XArray. This method provides a more
> >> ergonomic API compared to calling `get` and checking for `Some`.
> >
> > I think this is going to result in less efficient code for most uses.
> >
> > Most users use the results returned, not just checking if there is or is
> > not a value.  So if you find the value an xarray state and then just
> > throw it away and find it again, it'll be less efficient.
> >
> > If there are users that do use the xarray to just check if something
> > exists or not (which there probably are?), then it should be in a
> > wrapper for that code and not the generic API.  Otherwise we will have
> > users pop up to use this method when they should not.
> 
> I agree in that I would prefer matching on the result of a lookup and
> using that result. This is not always possible due to a limitation in
> the current implementation of the borrow checker. Please see my response
> to Tamir [1], it has all the pointers.
> 
> Since we cannot use a match statement in certain situations, we have to
> fall back to something that does not borrow from the collections, like
> `array.get(index).is_some()`. I would argue that
> 
>   if array.contains_index(foo) { ... }
> 
> is easier to read than
> 
>   if array.get(foo).is_some() { ... }
> 
> And I don't think `array.contains()` is going to have worse codegen than
> `array.get(index).is_some()`.

This is probably my lack of rust knowledge...


My concern is around API usage.  I am concerned people will use xas as a
throw-away lookup with this API and cause more walks of the xarray to
the same location.

In the normal API, we have lookups like this; you take a lock, look
something up, drop the lock and return it.  Since the life cycle of the
stored information is outside the scope of the xarray, the user is
dependent on the entry being stable by some other means after the xarray
lock is dropped.

In the advanced API, we do more within the locked area, usually.

Usually, applications don't just print out there is a value, they do
something with it.  So I would expect a real example to be something
like (this horrible psudo-c/rust mangled mess):

let entry = array.get_mut(foo);

if (entry.is_some()) {
        /* do something with entry */
        send_to_party(entry);
} else {
        /* deal with it not existing */
}

What I don't want to do:

if (array.contains_index(foo)) {
        entry = array.get_mut(foo);
} else {
        ...
}

Where contains_index(foo) sets up an xas, walks to the location, returns
the entry (or not) and then translates into a boolean.. then if it's
true we set up another xas to walk to the same location.

That is, the worst code gen would come from this:
if (array.get(foo).is_some()) { array.get_mut(foo).. }

>From what you said here and the link, you are saying we need to do this
in certain situations due to rust's borrow checker and the lifetime, but
I cannot see why we would need to walk the xarray twice from the example
provided.

And making it easier to do this could result in a lot more users
doubling xarray walks without realising that it's a bad idea (unless
it's this special case).

...
> 
> [1] https://lore.kernel.org/rust-for-linux/20260209-xarray-entry-send-v3-0-f777c65b8ae2@kernel.org/T/#m95fb90870c511491f4f487dbf852c689cd0733f4
> 

I have trouble following 'the taken arm' in your link.  I think you mean
one of the branches based on the existence of the entry, but I don't
know which is the 'taken' and how 'self' is out of scope.

Other links off the above link seem to indicate it is a problem with the
rust borrow checking hitting a false positive.

It seems we need to look up things twice to work around the false
positive - or implement something like get_or_insert()?

Or, wait for the new checker to be released - but that doesn't fix all
the false positives, just this one?

So, do all users of the xarray suffer from this false positive?

Thanks,
Liam

Powered by blists - more mailing lists