From patchwork Tue Jul 19 00:09:15 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Trond Myklebust X-Patchwork-Id: 987932 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter1.kernel.org (8.14.4/8.14.4) with ESMTP id p6J08Kjj032619 for ; Tue, 19 Jul 2011 00:09:22 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751114Ab1GSAJV (ORCPT ); Mon, 18 Jul 2011 20:09:21 -0400 Received: from mx2.netapp.com ([216.240.18.37]:22489 "EHLO mx2.netapp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750877Ab1GSAJV convert rfc822-to-8bit (ORCPT ); Mon, 18 Jul 2011 20:09:21 -0400 X-IronPort-AV: E=Sophos;i="4.67,225,1309762800"; d="scan'208";a="563709579" Received: from smtp1.corp.netapp.com ([10.57.156.124]) by mx2-out.netapp.com with ESMTP; 18 Jul 2011 17:09:20 -0700 Received: from svlrsexc2-prd.hq.netapp.com (svlrsexc2-prd.hq.netapp.com [10.57.115.31]) by smtp1.corp.netapp.com (8.13.1/8.13.1/NTAP-1.6) with ESMTP id p6J09KI2026476; Mon, 18 Jul 2011 17:09:20 -0700 (PDT) Received: from SACMVEXC2-PRD.hq.netapp.com ([10.99.115.17]) by svlrsexc2-prd.hq.netapp.com with Microsoft SMTPSVC(6.0.3790.3959); Mon, 18 Jul 2011 17:09:20 -0700 Received: from 10.99.190.120 ([10.99.190.120]) by SACMVEXC2-PRD.hq.netapp.com ([10.99.115.16]) with Microsoft Exchange Server HTTP-DAV ; Tue, 19 Jul 2011 00:09:19 +0000 Date: Mon, 18 Jul 2011 20:09:15 -0400 Subject: Re: [PATCH] nfs: open-associated setattr shouldn't invalidate own cache Message-ID: <02f301cc45a8$1ad15db6$78be630a@hq.netapp.com> From: "Myklebust, Trond" X-MimeOLE: Produced By Microsoft Exchange V6.5 Thread-Topic: [PATCH] nfs: open-associated setattr shouldn't invalidate own cache To: "J. Bruce Fields" , Thread-Index: AcxFqBrRUclCG/dERCG06VSnyQMtIw== Cc: MIME-Version: 1.0 X-OriginalArrivalTime: 19 Jul 2011 00:09:20.0123 (UTC) FILETIME=[1B44FCB0:01CC45A8] Sender: linux-nfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter1.kernel.org [140.211.167.41]); Tue, 19 Jul 2011 00:09:22 +0000 (UTC) We should already optimize away the unnecessary setting of the size. The problem is that truncate() still requires you to set the ctime, whereas ftruncate() does not iirc. "J. Bruce Fields" wrote: From: "J. Bruce Fields" We recently noticed that NFSv4 iozone read tests that should have been performing at local-cache speeds were running at wire speeds, and found that currently, open(O_RDWR|O_TRUNC) write() close() open(O_RDONLY) read() results in an over-the-wire read (due to a setattr triggered by O_TRUNC). Even non-O_TRUNC opens send setattrs (of timestamps) in some cases, causing the same problem. In discussion with Trond, he blames a VFS bug for breaking what should be a single open into an open followed by a setattr. However, it may make sense even in a case like open(O_RDWR) write() setattr() close() open(O_RDONLY) read() to treat the setattr similarly to the write, and not invalidate the client's own cache as long as the setattr is performed under a write open. (That said, I don't know if this is the correct place in the code to implement that behavior.) Signed-off-by: J. Bruce Fields --- fs/nfs/inode.c | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-) diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c index 6f4850d..d4eed06 100644 --- a/fs/nfs/inode.c +++ b/fs/nfs/inode.c @@ -438,8 +438,13 @@ nfs_setattr(struct dentry *dentry, struct iattr *attr) if ((attr->ia_valid & (ATTR_MODE|ATTR_UID|ATTR_GID)) != 0) nfs_inode_return_delegation(inode); error = NFS_PROTO(inode)->setattr(dentry, fattr, attr); - if (error == 0) + if (error) + goto out_free; + if (attr->ia_valid & ATTR_FILE) + nfs_post_op_update_inode_force_wcc(inode, fattr); + else nfs_refresh_inode(inode, fattr); +out_free: nfs_free_fattr(fattr); out: return error;