Re: [PATCH v3 5/8] block/write-threshold: don't use aio context lock

[Vladimir Sementsov-Ogievskiy]

07.05.2021 16:45, Paolo Bonzini wrote:
> On 06/05/21 11:06, Vladimir Sementsov-Ogievskiy wrote:
> >   void bdrv_write_threshold_check_write(BlockDriverState *bs, int64_t offset,
> >                                         int64_t bytes)
> >   {
> >       int64_t end = offset + bytes;
> > -    uint64_t wtr = bs->write_threshold_offset;
> > +    uint64_t wtr;
> > -    if (wtr > 0 && end > wtr) {
> > -        qapi_event_send_block_write_threshold(bs->node_name, end - wtr, wtr);
> > +retry:
> > +    wtr = bdrv_write_threshold_get(bs);
> > +    if (wtr == 0 || wtr >= end) {
> > +        return;
> > +    }
> > -        /* autodisable to avoid flooding the monitor */
> > -        bdrv_write_threshold_set(bs, 0);
> > +    /* autodisable to avoid flooding the monitor */
> > +    if (qatomic_cmpxchg(&bs->write_threshold_offset, wtr, 0) != wtr) {
> > +        /* bs->write_threshold_offset changed in parallel */
> > +        goto retry;
> >       }
> > +
> > +    /* We have cleared bs->write_threshold_offset, so let's send event */
> > +    qapi_event_send_block_write_threshold(bs->node_name, end - wtr, wtr);
> >   }
>
> This has the problem that 64-bit atomics are not always possible on 32-bit 
> builds.  We can use a spinlock (and probably just drop this patch for now).
>
> Paolo

OK, let's just drop it for now, the series originally not intended to make 
something thread-safe, but only to clear the way for.

(And honestly I doubt that write-threshold worth the complexity of this atomic 
cmpxchg retry loop, mutex would be simpler anyway)

--
Best regards,
Vladimir