On Tue, 2012-05-01 at 13:16 -0400, Trond Myklebust wrote:
> ...and correct a bug when NFS_IOHDR_ERROR is set: we should not be
> setting PageUptodate/set_page_dirty on a page unless the number of
> bytes read <= hdr->good_bytes
>
> Signed-off-by: Trond Myklebust <Trond.Myklebust@xxxxxxxxxx>
> Cc: Fred Isaman <iisaman@xxxxxxxxxx>
> ---
> v2: Actually add the bugfix to direct.c...
>
> fs/nfs/direct.c | 46 +++++++++++++++++++---------------------------
> fs/nfs/read.c | 42 +++++++++++++++++-------------------------
> 2 files changed, 36 insertions(+), 52 deletions(-)
>
> diff --git a/fs/nfs/direct.c b/fs/nfs/direct.c
> index 78d1ead..34027d5 100644
> --- a/fs/nfs/direct.c
> +++ b/fs/nfs/direct.c
> @@ -253,36 +253,28 @@ static void nfs_direct_read_completion(struct nfs_pgio_header *hdr)
> dreq->count += hdr->good_bytes;
> spin_unlock(&dreq->lock);
>
> - if (!test_bit(NFS_IOHDR_ERROR, &hdr->flags)) {
> - while (!list_empty(&hdr->pages)) {
> - struct nfs_page *req = nfs_list_entry(hdr->pages.next);
> - struct page *page = req->wb_page;
> -
> - if (test_bit(NFS_IOHDR_EOF, &hdr->flags)) {
> - if (bytes > hdr->good_bytes)
> - zero_user(page, 0, PAGE_SIZE);
> - else if (hdr->good_bytes - bytes < PAGE_SIZE)
> - zero_user_segment(page,
> - hdr->good_bytes & ~PAGE_MASK,
> - PAGE_SIZE);
> - }
> - bytes += req->wb_bytes;
> - nfs_list_remove_request(req);
> - if (!PageCompound(page))
> - set_page_dirty(page);
> - nfs_direct_readpage_release(req);
> + while (!list_empty(&hdr->pages)) {
> + struct nfs_page *req = nfs_list_entry(hdr->pages.next);
> + struct page *page = req->wb_page;
> +
> + if (test_bit(NFS_IOHDR_EOF, &hdr->flags)) {
> + if (bytes > hdr->good_bytes)
> + zero_user(page, 0, PAGE_SIZE);
> + else if (hdr->good_bytes - bytes < PAGE_SIZE)
> + zero_user_segment(page,
> + hdr->good_bytes & ~PAGE_MASK,
> + PAGE_SIZE);
> }
> - } else {
> - while (!list_empty(&hdr->pages)) {
> - struct nfs_page *req = nfs_list_entry(hdr->pages.next);
> -
> - if (bytes < hdr->good_bytes)
> - if (!PageCompound(req->wb_page))
> + bytes += req->wb_bytes;
> + if (!PageCompound(page)) {
> + if (test_bit(NFS_IOHDR_ERROR, &hdr->flags)) {
> + if (bytes <= hdr->good_bytes)
> set_page_dirty(req->wb_page);
OK... Looking at this more closely, I think that I'm wrong in calling
the old behaviour a bug. Since we are saying that the buffer is good up
to and including hdr->good_bytes of read bytes, we should mark the page
as being dirty so long as it contains one or more good bytes.
I'll resend the cleanup without the erroneous fix...
--
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�����٥
