From mboxrd@z Thu Jan 1 00:00:00 1970 From: Cyrill Gorcunov Subject: Re: [patch 0/3] A few patches for dcache Date: Mon, 15 Aug 2011 11:42:30 +0400 Message-ID: <20110815074229.GL2182@sun> References: <20110728131219.146414619@openvz.org> <20110729032503.GD5404@dastard> <20110729055918.GB15883@sun> <20110729065951.GE5404@dastard> <20110729072441.GA2203@ZenIV.linux.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Dave Chinner , LINUXFS-ML , Andrew Morton , Christoph Hellwig , James Bottomley , xemul@openvz.org To: Al Viro Return-path: Received: from mail-bw0-f46.google.com ([209.85.214.46]:43349 "EHLO mail-bw0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751406Ab1HOHmg (ORCPT ); Mon, 15 Aug 2011 03:42:36 -0400 Received: by bke11 with SMTP id 11so2837196bke.19 for ; Mon, 15 Aug 2011 00:42:35 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20110729072441.GA2203@ZenIV.linux.org.uk> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Fri, Jul 29, 2011 at 08:24:41AM +0100, Al Viro wrote: ... > > > > I'll defer to Al on that one - the intricacies of the rename locking > > are way over my head. > > I'm not sure that's safe. Note that one use of rename_lock is that > we allow hash lookup to race with d_move(). Which can move object > from one hash chain to another, so hash lookup may end up jumping > from one chain to another and getting a false negative. That's > why __d_lookup() is not safe without read_seqretry loop (or seq_writelock, > of course). > > But look what happens if we do per-sb locks - d_move() derailing the > hash lookup might happen on *any* filesystem. They all share the > same hash table. So just checking that we hadn't done renames on > our filesystem is not enough to make sure we hadn't hit a false > negative. > > Unless we go for making the hashtable itself per-superblock (and I really > doubt that it's a good idea), I don't see any obvious ways to avoid that > kind of race. IOW, how would you implement safe d_lookup()? Hi Al, (a bit late reply actually ;) but what about first two patches (without per-sb locks)? They simply introduce read_seqlock/unlock without modification of sl->sequence. The functions which were calling for read_seqretry -- still remains and do continue calling for read_seqretry. The following functions were converted to read_seqlock/unlock - have_submounts (still have the read_seqretry on unlocked path) - select_parent (still have the read_seqretry on unlocked path) - __d_path - d_path - d_path_with_unreachable - dentry_path_raw - dentry_path - getcwd - d_genocide (still have the read_seqretry on unlocked path) While both d_move() and d_materialise_unique() remains using write_seqlock. So it seems I'm a bit confused/screwed why can't we do so... Cyrill