Re: [PATCH] Avoid RMW of flags2 outside lock (BZ #27842)

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On 19/05/2022 13:26, Wilco Dijkstra wrote:
> Hi Adhemerval,
>  
>> I don't think this is correct because if the caller issues pthread_create 
>> after flockfile, funlockfile will not issues the correct operations.  I
> 
> No, the idea of switching off the single-threaded optimization before the lock is
> precisely to ensure that you never could get that situation. Note that neither of
> the locks in flockfile and funlockfile use _IO_FLAGS2_NEED_LOCK currently, so 
> this is just being extremely conservative - in principle we could remove the
> update or move it after the lock.

I am trying to see why exactly we need to disable single-thread optimization
on flockfile, since there is no FILE operation that takes a callback where
pthread_create might be called beween _IO_acquire_lock.  Can't we just remove
the _IO_FLAGS2_NEED_LOCK set on flockfile?

> 
>> have a fix that uses a different locking mechanism where the _IO_FLAGS2_NEED_LOCK
>> is removed by moving both the thread id and single-thread optimization to the
>> locks itself (on Linux tid has at maximum 30-bits, we can use 1 bits for the
>> single-thread optimization and 1 bits for congestion optimization).
> 
> Right so you mean moving NEED_LOCK bit into the lock variable?

Yes, and making the lock smaller on linux (just a word plus the recursive counter).

> 
>> I would say that with currency scheme where _IO_FLAGS2_NEED_LOCK is stick,
>> this is a benign data race (although still undesirable). 
> 
> You mean as it is now? It is a real bug since various functions update flags2
> behind a lock, so it is possible for this RMW to cause corruption (but only if
> you are already multithreaded, which the update is pointless anyway and we
> can just skip it).

I don't think it would be possible to corrupt because once pthread_create
is called, _IO_FLAGS2_NEED_LOCK will be always set (so RMW won't see a
__flags2 without _IO_FLAGS2_NEED_LOCK being set).