Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756987AbbDVNcU (ORCPT ); Wed, 22 Apr 2015 09:32:20 -0400 Received: from mx1.redhat.com ([209.132.183.28]:45725 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752208AbbDVNcQ (ORCPT ); Wed, 22 Apr 2015 09:32:16 -0400 Date: Wed, 22 Apr 2015 15:31:51 +0200 From: Mateusz Guzik To: Eric Dumazet Cc: Al Viro , Andrew Morton , "Paul E. McKenney" , Yann Droneaud , Konstantin Khlebnikov , linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [RFC PATCH] fs: use a sequence counter instead of file_lock in fd_install Message-ID: <20150422133149.GA10455@mguzik> References: <20150417230252.GE889@ZenIV.linux.org.uk> <20150420130633.GA2513@mguzik> <20150420134326.GC2513@mguzik> <20150420151054.GD2513@mguzik> <1429550126.7346.268.camel@edumazet-glaptop2.roam.corp.google.com> <1429562991.7346.290.camel@edumazet-glaptop2.roam.corp.google.com> <1429639543.7346.329.camel@edumazet-glaptop2.roam.corp.google.com> <20150421200624.GA16097@mguzik> <20150421201201.GB16097@mguzik> <1429650413.18561.10.camel@edumazet-glaptop2.roam.corp.google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <1429650413.18561.10.camel@edumazet-glaptop2.roam.corp.google.com> User-Agent: Mutt/1.5.23.1-rc1 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1839 Lines: 58 On Tue, Apr 21, 2015 at 02:06:53PM -0700, Eric Dumazet wrote: > On Tue, 2015-04-21 at 22:12 +0200, Mateusz Guzik wrote: > > > in dup_fd: > > for (i = open_files; i != 0; i--) { > > struct file *f = *old_fds++; > > if (f) { > > get_file(f); > > > > I see no new requirement here. f is either NULL or not. > multi threaded programs never had a guarantee dup_fd() would catch a non > NULL pointer here. > It's not about seeing NULL f or not, but using the right address for dereference. If I read memory-barriers.txt right (see 'DATA DEPENDENCY BARRIERS'), it is possible that cpus like alpha will see a non-NULL pointer and then proceed to dereference *the old* (here: NULL) value. Hence I suspect this needs smp_read_barrier_depends (along with ACCESS_ONCE). Other consumers (e.g. procfs code) use rcu_dereference macro which does ends up using lockless_dereference macro, which in turn does: #define lockless_dereference(p) \ ({ \ typeof(p) _________p1 = ACCESS_ONCE(p); \ smp_read_barrier_depends(); /* Dependency order vs. p above. */ \ (_________p1); \ }) That said memory barriers are not exactly my strong suit, but I do believe my suspicion here is justified enough to ask someone with solid memory barrier-fu to comment. > > > at least a data dependency barrier, or maybe smp_rmb for peace of mind > > > > similarly in do_dup2: > > tofree = fdt->fd[fd]; > > if (!tofree && fd_is_open(fd, fdt)) > > goto Ebusy; > > Same here. > > -- Mateusz Guzik -- 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/