Hi Greg, "Greg KH" <gregkh@xxxxxxxxxxxxxxxxxxx> writes: > On Fri, Oct 04, 2024 at 04:52:24PM +0100, Gary Guo wrote: >> There is an operation needed by `block::mq`, atomically decreasing >> refcount from 2 to 0, which is not available through refcount.h, so >> I exposed `Refcount::as_atomic` which allows accessing the refcount >> directly. > > That's scary, and of course feels wrong on many levels, but: > > >> @@ -91,13 +95,17 @@ pub(crate) unsafe fn start_unchecked(this: &ARef<Self>) { >> /// C `struct request`. If the operation fails, `this` is returned in the >> /// `Err` variant. >> fn try_set_end(this: ARef<Self>) -> Result<*mut bindings::request, ARef<Self>> { >> - // We can race with `TagSet::tag_to_rq` >> - if let Err(_old) = this.wrapper_ref().refcount().compare_exchange( >> - 2, >> - 0, >> - Ordering::Relaxed, >> - Ordering::Relaxed, >> - ) { >> + // To hand back the ownership, we need the current refcount to be 2. >> + // Since we can race with `TagSet::tag_to_rq`, this needs to atomically reduce >> + // refcount to 0. `Refcount` does not provide a way to do this, so use the underlying >> + // atomics directly. >> + if this >> + .wrapper_ref() >> + .refcount() >> + .as_atomic() >> + .compare_exchange(2, 0, Ordering::Relaxed, Ordering::Relaxed) >> + .is_err() > > Why not just call rust_helper_refcount_set()? Or is the issue that you > think you might not be 2 here? And if you HAVE to be 2, why that magic > value (i.e. why not just always be 1 and rely on normal > increment/decrement?) > > I know some refcounts are odd in the kernel, but I don't see where the > block layer is caring about 2 as a refcount anywhere, what am I missing? It is in the documentation, rendered version available here [1]. Let me know if it is still unclear, then I guess we need to update the docs. Also, my session from Recipes has a little bit of discussion regarding this refcount and it's use [2]. Best regards, Andreas [1] https://rust.docs.kernel.org/kernel/block/mq/struct.Request.html#implementation-details [2] https://youtu.be/1LEvgkhU-t4?si=B1XnJhzCCNnUtRsI&t=1685