From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932399AbcGDAha (ORCPT ); Sun, 3 Jul 2016 20:37:30 -0400 Received: from linuxhacker.ru ([217.76.32.60]:59492 "EHLO fiona.linuxhacker.ru" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932182AbcGDAh2 convert rfc822-to-8bit (ORCPT ); Sun, 3 Jul 2016 20:37:28 -0400 Subject: Re: More parallel atomic_open/d_splice_alias fun with NFS and possibly more FSes. Mime-Version: 1.0 (Apple Message framework v1283) Content-Type: text/plain; charset=windows-1252 From: Oleg Drokin In-Reply-To: <20160704000802.GH14480@ZenIV.linux.org.uk> Date: Sun, 3 Jul 2016 20:37:22 -0400 Cc: Mailing List , "" Content-Transfer-Encoding: 8BIT Message-Id: <0145470E-667E-4A8D-AB79-F897322DA441@linuxhacker.ru> References: <20160617042914.GD14480@ZenIV.linux.org.uk> <20160703062917.GG14480@ZenIV.linux.org.uk> <20160704000802.GH14480@ZenIV.linux.org.uk> To: Al Viro X-Mailer: Apple Mail (2.1283) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Jul 3, 2016, at 8:08 PM, Al Viro wrote: > On Sun, Jul 03, 2016 at 07:29:46AM +0100, Al Viro wrote: > >> Tentative NFS patch follows; I don't understand Lustre well enough, but it >> looks like a plausible strategy there as well. > > Speaking of Lustre: WTF is > /* Open dentry. */ > if (S_ISFIFO(d_inode(dentry)->i_mode)) { > /* We cannot call open here as it would > * deadlock. > */ > if (it_disposition(it, DISP_ENQ_OPEN_REF)) > ptlrpc_req_finished( > (struct ptlrpc_request *) > it->d.lustre.it_data); > rc = finish_no_open(file, de); > } else { > about and why do we only do that to FIFOs? What about symlinks or device > nodes? Directories, for that matter... Shouldn't that be if (!S_ISREG(...)) > instead? Hm… This dates to sometime in 2006 and my memory is a bit hazy here. I think when we called into the open, it went into fifo open and stuck there waiting for the other opener. Something like that. And we cannot really be stuck here because we are holding some locks that need to be released in predictable time. This code is actually unreachable now because the server never returns an openhandle for special device nodes anymore (there's a comment about it in current staging tree, but I guess you are looking at some prior version). I imagine device nodes might have represented a similar risk too, but it did not occur to me to test it separately and the testsuite does not do it either. Directories do not get stuck when you open them so they are ok and we can atomically open them too, I guess. Symlinks are handled specially on the server and the open never returns the actual open handle for those, so this path is also unreachable with those. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Subject: Re: More parallel atomic_open/d_splice_alias fun with NFS and possibly more FSes. Mime-Version: 1.0 (Apple Message framework v1283) Content-Type: text/plain; charset=windows-1252 From: Oleg Drokin In-Reply-To: <20160704000802.GH14480@ZenIV.linux.org.uk> Date: Sun, 3 Jul 2016 20:37:22 -0400 Cc: Mailing List , "" Content-Transfer-Encoding: 8BIT Message-Id: <0145470E-667E-4A8D-AB79-F897322DA441@linuxhacker.ru> References: <20160617042914.GD14480@ZenIV.linux.org.uk> <20160703062917.GG14480@ZenIV.linux.org.uk> <20160704000802.GH14480@ZenIV.linux.org.uk> To: Al Viro Sender: linux-kernel-owner@vger.kernel.org List-ID: On Jul 3, 2016, at 8:08 PM, Al Viro wrote: > On Sun, Jul 03, 2016 at 07:29:46AM +0100, Al Viro wrote: > >> Tentative NFS patch follows; I don't understand Lustre well enough, but it >> looks like a plausible strategy there as well. > > Speaking of Lustre: WTF is > /* Open dentry. */ > if (S_ISFIFO(d_inode(dentry)->i_mode)) { > /* We cannot call open here as it would > * deadlock. > */ > if (it_disposition(it, DISP_ENQ_OPEN_REF)) > ptlrpc_req_finished( > (struct ptlrpc_request *) > it->d.lustre.it_data); > rc = finish_no_open(file, de); > } else { > about and why do we only do that to FIFOs? What about symlinks or device > nodes? Directories, for that matter... Shouldn't that be if (!S_ISREG(...)) > instead? Hm� This dates to sometime in 2006 and my memory is a bit hazy here. I think when we called into the open, it went into fifo open and stuck there waiting for the other opener. Something like that. And we cannot really be stuck here because we are holding some locks that need to be released in predictable time. This code is actually unreachable now because the server never returns an openhandle for special device nodes anymore (there's a comment about it in current staging tree, but I guess you are looking at some prior version). I imagine device nodes might have represented a similar risk too, but it did not occur to me to test it separately and the testsuite does not do it either. Directories do not get stuck when you open them so they are ok and we can atomically open them too, I guess. Symlinks are handled specially on the server and the open never returns the actual open handle for those, so this path is also unreachable with those.