lkml.org 
[lkml]   [2009]   [Jun]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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.


\
 
 \ /
  Last update: 2009-06-26 07:45    [W:0.072 / U:1.252 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site