From patchwork Wed Dec 12 00:05:54 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: NeilBrown X-Patchwork-Id: 1864161 Return-Path: X-Original-To: patchwork-linux-nfs@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 2CE73DFAC4 for ; Wed, 12 Dec 2012 00:06:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754411Ab2LLAGI (ORCPT ); Tue, 11 Dec 2012 19:06:08 -0500 Received: from cantor2.suse.de ([195.135.220.15]:40113 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754344Ab2LLAGI (ORCPT ); Tue, 11 Dec 2012 19:06:08 -0500 Received: from relay2.suse.de (unknown [195.135.220.254]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx2.suse.de (Postfix) with ESMTP id E8EE7A4EB7; Wed, 12 Dec 2012 01:06:05 +0100 (CET) Date: Wed, 12 Dec 2012 11:05:54 +1100 From: NeilBrown To: "Myklebust, Trond" Cc: NFS Subject: Re: NFS regression - EIO is returned instead of ENOSPC Message-ID: <20121212110554.19d0d7da@notabene.brown> In-Reply-To: <4FA345DA4F4AE44899BD2B03EEEC2FA911923813@SACEXCMBX04-PRD.hq.netapp.com> References: <20121212092813.23afbc5f@notabene.brown> <20121212095304.368c9708@notabene.brown> <1355267807.23203.16.camel@lade.trondhjem.org> <4FA345DA4F4AE44899BD2B03EEEC2FA911923813@SACEXCMBX04-PRD.hq.netapp.com> X-Mailer: Claws Mail 3.8.1 (GTK+ 2.24.10; x86_64-suse-linux-gnu) Mime-Version: 1.0 Sender: linux-nfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org On Tue, 11 Dec 2012 23:20:38 +0000 "Myklebust, Trond" wrote: > On Tue, 2012-12-11 at 18:16 -0500, Trond Myklebust wrote: > > Hmm... I can see 2 places where we're setting the PageError flag. > > > > 1. nfs_updatepage(): in this case, the error occurred when we tried > > to change the page contents. Since we're holding the page lock, > > and so rather than mark the page as bad, we could probably just > > write back existing dirty areas (using nfs_wb_page()) and then > > remove it from the mapping. > > 2. nfs_write_completion(): here the writeback error applies to the > > entire dirty area on the page, and there is no point in try to > > write back again. Better just evict the page from the page cache > > (which is what nfs_zap_mapping() is supposed to do). While > > setting the PageError flag does cause some of the writeback > > functions to return EIO, that's not really what we're after; we > > already report errors more completely via the open context. > > > > So for now, can't we just change nfs_set_pageerror() to not bother > > setting the PG_error flag? Then in the future we might want to make > > nfs_updatepage a bit more sophisticated in how it deals with those > > errors... > > Ultimately, what I'm saying is that PageError is a hack for passing > errors around. Since we have our own hack for doing the same, then why > use PageError at all? > Sounds like a real possibility. I just tested with and I get the correct "ENOSPC" error, so that's a good sign. Tested-by: NeilBrown if you like. I've haven't tried to examine all the consequences of the change to ensure there are no unintended side effect, but you know the code better than me so if you think it is safe - I suspect it is. Thanks, NeilBrown diff --git a/fs/nfs/write.c b/fs/nfs/write.c index 9347ab7..e5da5e8 100644 --- a/fs/nfs/write.c +++ b/fs/nfs/write.c @@ -202,7 +202,6 @@ out: /* A writeback failed: mark the page as bad, and invalidate the page cache */ static void nfs_set_pageerror(struct page *page) { - SetPageError(page); nfs_zap_mapping(page_file_mapping(page)->host, page_file_mapping(page)); }