[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 2/3] block: make BlockBackend->disable_request_queuing ato

From: Stefan Hajnoczi
Subject: Re: [PATCH v2 2/3] block: make BlockBackend->disable_request_queuing atomic
Date: Thu, 9 Mar 2023 07:31:18 -0500

On Thu, Mar 09, 2023 at 10:07:40AM +0100, Paolo Bonzini wrote:
> On 3/7/23 22:04, Stefan Hajnoczi wrote:
> > This field is accessed by multiple threads without a lock. Use explicit
> > qatomic_read()/qatomic_set() calls. There is no need for acquire/release
> > because blk_set_disable_request_queuing() doesn't provide any
> > guarantees (it helps that it's used at BlockBackend creation time and
> > not when there is I/O in flight).
> This in turn means itdoes not need to be atomic - atomics are only needed if
> there are concurrent writes.  No big deal; I am now resurrecting the series
> from the time I had noticed the queued_requests thread-safety problem, so
> this will be simplified in 8.1.  For now your version is okay, thanks for
> fixing it!

I was under the impression that variables accessed by multiple threads
outside a lock or similar primitive need memory_order_relaxed both as
documentation and to tell the compiler that they should indeed be atomic
(but without ordering guarantees).

I think memory_order_relaxed also tells the compiler to do a bit more,
like to generate just a single store to the variable for each occurrence
in the code ("speculative" and "out-of-thin air" stores).

It's the documentation part that's most interesting in this case. Do we
not want to identify variables that are accessed outside a lock and
therefore require some thought?


Attachment: signature.asc
Description: PGP signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]