[Date Prev][Date Next]
[Chronological]
[Thread]
[Top]
Re: (ITS#6104) race condition with cancel operation
hyc@symas.com writes:
>> Bitflags should be unnecessary as such, just need a set of values.
>> Though it wouldn't hurt if they were chosen so one could use bit
>> operations to reduce checks for "o_abandon == A or B". "Suppress
>> response" value mentioned in ITS#6138 also needed, maybe that must be a
>> bitflag when combined with Cancel stuff.
>
> Yes, it still seems some bitflags will be more appropriate.
As long as we decide which flag combinations are (not) valid. Problem
with "unrestricted" bitflags compared to a list of values is that they
allow and thus require support for flag combinations we have no real
need to support.
>> Question:
>> syncprov_op_abandon() holds&si->si_ops_mutex when setting
>> so->s_op->o_abandon. Does it need that? The function needs to
>> grab op->o_conn->c_mutex when called as Cancel, but must not do
>> that while holding&si->si_ops_mutex since Abandon grabs the
>> locks in the opposite order.
>
> syncprov_op_abandon() can remove ops from the list, so yes, it must
> hold si_ops_mutex.
Right, but does it need si_ops_mutex while setting o_abandon? Cancel
could remove the op, unlock si_ops_mutex, lock c_mutex, update
o_abandon. Alternative: Lock c_mutex first (that must be safe since
Abandon is doing it anyway), then lock si_ops_mutex. So it's not
a critical difference, just that a lock must be held a bit longer.
--
Hallvard