On Sat, 1 May 2021 14:59:01 +0200 Tobias Burnus <tob...@codesourcery.com> wrote:
> As this patch is rather obvious, I intent to commit it tomorrow > as obvious, unless there is an OK earlier or other comments :-) > (And backport to GCC 11 in a couple of days.) > > Before PR100352 (r11-7647), > st_write_done did: > st_write_done_worker (dtd) > unlock_unit (dtp->u.p.current_unit); > > but st_write_done_worker did: > LOCK (&unit_lock); > newunit_free (dtp->common.unit); > UNLOCK (&unit_lock); > > And this locking could lead to a deadlock. > > Hence, 'unlock_unit' has been moved to st_write_done_worker > before the LOCK (&unit_lock). > > That solved the issue, but async I/O does not use this lock > but, in async.c's async_io(): > LOCK (&au->lock); > st_write_done_worker (au->pdt); > UNLOCK (&au->io_lock); > > Which worked before the previous patch fine, but now > there is a bogus unlock_unit() alias UNLOCK (&u->lock); > although the unit is not locked. > > Solution: Guard the unlock_unit. Doesn't look all that pretty TBH. Doesn't it suggest that the worker's callers should eventually take care of the locking (and newunit_free()ing) ? I.e. have the workers return a bool whether to free the newunit or not. But then, neither did i look closely nor do i volunteer to provide a fix.. HTH,
