Messages in this thread | | | Date | Fri, 26 Jun 2009 05:42:30 +0000 | From | Jarek Poplawski <> | Subject | Re: [PATCH] net: fix race in the receive/select |
| |
On 26-06-2009 05:14, Davide Libenzi wrote: > 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?
How about poll_wait_mb() and waitqueue_active_mb() (with mb and additional check for NULL of wait_queue_head)?
Jarek P.
| |