From patchwork Thu Jul 5 08:38:27 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Bartlett X-Patchwork-Id: 1158631 Return-Path: X-Original-To: patchwork-cifs-client@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork2.kernel.org (Postfix) with ESMTP id 93EA3DFB7C for ; Thu, 5 Jul 2012 08:45:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752109Ab2GEIpT (ORCPT ); Thu, 5 Jul 2012 04:45:19 -0400 Received: from fn.samba.org ([216.83.154.106]:49520 "EHLO mail.samba.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751025Ab2GEIpR (ORCPT ); Thu, 5 Jul 2012 04:45:17 -0400 X-Greylist: delayed 410 seconds by postgrey-1.27 at vger.kernel.org; Thu, 05 Jul 2012 04:45:17 EDT Received: from [127.0.0.1] (unknown [216.83.154.106]) by mail.samba.org (Postfix) with ESMTP id 28B8EAD413 for ; Thu, 5 Jul 2012 02:38:25 -0600 (MDT) Message-ID: <1341477507.22307.21.camel@obed> Subject: [PATCH] Always update the dentry cache with fresh readdir() results From: Andrew Bartlett To: linux-cifs@vger.kernel.org Date: Thu, 05 Jul 2012 18:38:27 +1000 X-Mailer: Evolution 3.2.3 (3.2.3-3.fc16) Mime-Version: 1.0 Sender: linux-cifs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-cifs@vger.kernel.org When we do a readdir() in CIFS, we are potentially efficiently collecting a great deal of current, catchable stat information. It is important that we always keep the dentry cache current for two reasons: - the information may have changed (within the actime timeout). - if we still have a dentry cache value after that timeout, it is quite expensive (1xRTT per entry) to find out if it was still correct. This hits folks who are using CIFS over a WAN very badly. For example on an emulated 50ms delay I would have ls --color complete in .1 seconds, and a second run take 4.5 seconds, as each stat() (for the colouring) would create a trans2 query_path_info query for each file, right after getting the same information in the trans2 find_first2. This patch implements the simplest approach, I would welcome a correction on if there is a better approach than d_drop() and dput(). Tested on 3.4.4-3.cifsrevalidate.fc17.i686 with a 50ms WANem emulated WAN against Samba 4.0 beta3. Thanks, Andrew Bartlett From 4478a902d3606205313eb37a225da22712841bff Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Thu, 5 Jul 2012 15:48:08 +1000 Subject: [PATCH] fs/cifs: Always recreate the cifs dentry cache when we have fresh information When we do a readdir() in CIFS, we are potentially efficiently collecting a great deal of current, cache-able stat information. It is important that we always keep the dentry cache current for two reasons: - the information may have changed (within the actime timeout). - if we still have a dentry cache value after that timeout, it is quite expensive (1xRTT per entry) to find out if it was still correct. This hits folks who are using CIFS over a WAN very badly. For example on an emulated 50ms delay I would have ls --color complete in .1 seconds, and a second run take 4.5 seconds, as each stat() (for the colouring) would create a trans2 query_path_info query for each file, right after getting the same information in the trans2 find_first2. This patch implements the simplest approach, I would welcome a correction on if there is a better approach than d_drop() and dput(). Tested on 3.4.4-3.cifsrevalidate.fc17.i686 against Samba 4.0 beta3. Andrew Bartlett Signed-off-by: Andrew Bartlett --- fs/cifs/readdir.c | 11 ++++++----- 1 files changed, 6 insertions(+), 5 deletions(-) diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c index 0a8224d..f964b40 100644 --- a/fs/cifs/readdir.c +++ b/fs/cifs/readdir.c @@ -66,9 +66,13 @@ static inline void dump_cifs_file_struct(struct file *file, char *label) #endif /* DEBUG2 */ /* - * Find the dentry that matches "name". If there isn't one, create one. If it's - * a negative dentry or the uniqueid changed, then drop it and recreate it. + * Find the dentry that matches "name". If there isn't one, create + * one. Otherwise recreate it fresh so we start the timeout for + * revalidation again. Local locks are much faster than the network + * ops to revalidate it, particularly as revalidation will be + * per-inode, but here we have a whole directory at once! */ + static struct dentry * cifs_readdir_lookup(struct dentry *parent, struct qstr *name, struct cifs_fattr *fattr) @@ -86,9 +90,6 @@ cifs_readdir_lookup(struct dentry *parent, struct qstr *name, dentry = d_lookup(parent, name); if (dentry) { - /* FIXME: check for inode number changes? */ - if (dentry->d_inode != NULL) - return dentry; d_drop(dentry); dput(dentry); } -- 1.7.7.6