Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753721AbZFZIKl (ORCPT ); Fri, 26 Jun 2009 04:10:41 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1751856AbZFZIKZ (ORCPT ); Fri, 26 Jun 2009 04:10:25 -0400 Received: from mail-fx0-f213.google.com ([209.85.220.213]:58087 "EHLO mail-fx0-f213.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751225AbZFZIKW (ORCPT ); Fri, 26 Jun 2009 04:10:22 -0400 DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; b=vj8VuYybx0ai5MkAy5cWLSUbYPSRzDzkaAB0+uQWZgK3/gv1CAAT0WOA512p3AhKpC JPcAu6XWBONoJz98111gaDoWQyDMu+bikwYDUyVwdTBS3qMyNZt987wuSSqTjgAkVf7d Z7Zc+Hx8D3GfRvghrBAQjrdPXn7JuBYroem5I= Date: Fri, 26 Jun 2009 08:10:19 +0000 From: Jarek Poplawski To: Davide Libenzi Cc: Eric Dumazet , Oleg Nesterov , Jiri Olsa , netdev@vger.kernel.org, Linux Kernel Mailing List , fbl@redhat.com, nhorman@redhat.com, davem@redhat.com, Tejun Heo Subject: Re: [PATCH] net: fix race in the receive/select Message-ID: <20090626081018.GC5204@ff.dom.local> References: <20090626054230.GA5204@ff.dom.local> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20090626054230.GA5204@ff.dom.local> User-Agent: Mutt/1.5.18 (2008-05-17) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1903 Lines: 58 On Fri, Jun 26, 2009 at 05:42:30AM +0000, Jarek Poplawski wrote: > 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)? Hmm... But considering Eric's arguments I see it would be hard /impossible to do it with the current api, so let's forget. Jarek P. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/