Re: [PATCH] net: fix race in the receive/select

Davide Libenzi
2009-06-26T03:19:01Z

On Fri, 26 Jun 2009, Eric Dumazet wrote:

> I wont argue with you David, just try to correct bugs.
>
> fs/ext4/ioctl.c line 182
>
> set_current_state(TASK_INTERRUPTIBLE);
> add_wait_queue(&EXT4_SB(sb)->ro_wait_queue, &wait);
> if (timer_pending(&EXT4_SB(sb)->turn_ro_timer)) {
> schedule();
>
> Another example of missing barrier after add_wait_queue()
>
> Because add_wait_queue() misses a barrier, we have to add one after each call.
>
> Maybe it would be safer to add barrier in add_wait_queue() itself, not in _pollwait().

Not all the code that uses add_wait_queue() does need to have the MB,
like code that does the most common pattern:

xxx_poll(...) {
poll_wait(...);
lock();
flags = calc_flags(->status);
unlock();
return flags;
}

xxx_update(...) {
lock();
->status = ...;
unlock();
if (waitqueue_active())
wake_up();
}

It's the code that does the lockless flags calculation in ->poll that
might need it.
I dunno what the amount of changes are, but cross-matching MB across
subsystems does not look nice.
IMHO that's a detail of the subsystem locking, and should be confined
inside the subsystem itself.
No?

- Davide