On Wed, 2012-12-12 at 09:53 +1100, NeilBrown wrote:
> On Wed, 12 Dec 2012 09:28:13 +1100 NeilBrown <neilb@xxxxxxx> wrote:
>
> >
> > Hi Trond et al,
> > we seem to have a regression introduced by
> >
> > commit 7b281ee026552f10862b617a2a51acf49c829554
> > NFS: fsync() must exit with an error if page writeback failed
> >
> > which has found it's way (in different form into -stable releases).
>
> Bit of a clarification here. It didn't get into -stable, but we have the bug
> in our 3.0 based SLES11-SP2 through a different route (I assumed it came
> through stable but was being too hasty).
>
> The bug first arrived in v3.1-rc1
>
> commit 02c24a82187d5a628c68edfe71ae60dc135cd178
> fs: push i_mutex and filemap_write_and_wait down into ->fsync() handlers
>
>
> was fixed by me in v3.3-rc1
>
> commit 2edb6bc3852c681c0d948245bd55108dc6407604
> NFS - fix recent breakage to NFS error handling.
>
> the code was then messed up a bit by
>
> commit a5c58892b427a2752e3ec44b0aad4ce9221dc63b
> NFS: Create a v4-specific fsync function
> in v3.6-rc1
>
> and that mess was fixed by
>
> commit 7b281ee026552f10862b617a2a51acf49c829554
> NFS: fsync() must exit with an error if page writeback failed
>
> which re-introduced the original problem in v3.6-rc6.
>
> That first patch has been backported to SLES11 so now I'm fixing the bug
> again and finding it in mainline again :-)
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...
--
Trond Myklebust
Linux NFS client maintainer
NetApp
Trond.Myklebust@xxxxxxxxxx
www.netapp.com
��.n��������+%������w��{.n�����{��w��� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
