Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760780Ab3IDVbx (ORCPT ); Wed, 4 Sep 2013 17:31:53 -0400 Received: from mail-ee0-f44.google.com ([74.125.83.44]:61225 "EHLO mail-ee0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756214Ab3IDVbv (ORCPT ); Wed, 4 Sep 2013 17:31:51 -0400 MIME-Version: 1.0 In-Reply-To: <1378321523-40893-1-git-send-email-Waiman.Long@hp.com> References: <1378321523-40893-1-git-send-email-Waiman.Long@hp.com> Date: Wed, 4 Sep 2013 14:31:49 -0700 X-Google-Sender-Auth: 52ZhmyYVu1R31xsoEDkmfUYsjLQ Message-ID: Subject: Re: [PATCH] dcache: Translating dentry into pathname without taking rename_lock From: Linus Torvalds To: Waiman Long Cc: Alexander Viro , linux-fsdevel , Linux Kernel Mailing List , "Chandramouleeswaran, Aswin" , "Norton, Scott J" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1187 Lines: 28 On Wed, Sep 4, 2013 at 12:05 PM, Waiman Long wrote: > + rcu_read_unlock(); > + if (read_seqretry(&rename_lock, seq)) > + goto restart; Btw, you have this pattern twice, and while it's not necessarily incorrect, it's a bit worrisome for performance. The rcu_read_unlock() is very possibly going to trigger an immediate scheduling event, so checking the sequence lock after dropping the read-lock sounds like it would make it much easier to hit the race with some rename. I'm also a tiny bit worried about livelocking on the sequence lock in the presence of lots of renames, so I'm wondering if the locking should try to approximate what we do for the RCU lookup path: start off optimistically using just the RCU lock and a sequence point, but if that fails, fall back to taking the real lock. Maybe using a counter ("try twice, then get the rename lock for writing") Hmm? Linus -- 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/