Re: block: be more careful about status in __bio_chain_endio

From: Hannes Reinecke
Date: Mon Jun 17 2019 - 03:37:34 EST


On 6/12/19 9:01 AM, Christoph Hellwig wrote:
> On Tue, Jun 11, 2019 at 10:56:42PM -0400, John Dorminy wrote:
>> I believe the second of these might, but is not guaranteed to,
>> preserve the first error observed in a child; I believe if you want to
>> definitely save the first error you need an atomic.
>
> Is there any reason not to simply use a cmpxchg? Yes, it is a
> relatively expensive operation, but once we are chaining bios we are out
> of the super hot path anyway. We do something similar in xfs and iomap
> already.
>
Agree.
Thing is, we need to check if the parent status is NULL, _and_ the
parent status might be modified asynchronously.
So even a READ_ONCE() wouldn't cut it, as it would tell us that the
parent status _was_ NULL, not that the parent status _is_ NULL by the
time we're setting it.
So cmpxchg() is it.

Cheers,

Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@xxxxxxx +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 NÃrnberg
GF: Felix ImendÃrffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG NÃrnberg)