Now I am confused. I thought the code where empty and skip are inside
the wait_event_timeout leads to empty beeing evaluated every time that
the waiting threads gets awoken.
And since some other thread might change /ar->htt.num_pending_tx/ it is
necessary to check this every time we get awoken, rather than once
before we go to sleep.
Also the locking part arround empty seems to support my guess (why sync
if you do not have multiple threads accessing a variable).
These are only guesses without looking at the surrounding code.
Could you please explain why you think it is sufficient to evaluate the
condition only once before sleeping on it? (empty || skip) is a constant
if you do not update either empty or skip, at least from my point of view.
On 11/03/15 14:40, [email protected] wrote:
On Wed, 11 Mar 2015 15:17:44 +0100, Nicholas Mc Guire said:
So the wait_event_timeout condition here ends up being (empty || skip)
but what is the point of puting this code into the parameter list of
wait_event_timeout() ?
Would it not be equivalent to:
bool empty;
...
spin_lock_bh(&ar->htt.tx_lock);
empty = (ar->htt.num_pending_tx == 0);
spin_unlock_bh(&ar->htt.tx_lock);
skip = (ar->state == ATH10K_STATE_WEDGED) ||
test_bit(ATH10K_FLAG_CRASH_FLUSH,
&ar->dev_flags);
ret = wait_event_timeout(ar->htt.empty_tx_wq, (empty || skip),
ATH10K_FLUSH_TIMEOUT_HZ);
What am I missing here ?
Umm... a Signed-off-by: and formatting it as an actual patch? :)
Seriously - you're right, it's ugly code that needs fixing...
_______________________________________________
Kernelnewbies mailing list
[email protected]
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
_______________________________________________
Kernelnewbies mailing list
[email protected]
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
