On Tue, 2012-05-01 at 15:12 -0400, Fred Isaman wrote:
> On Tue, May 1, 2012 at 2:40 PM, Trond Myklebust
> <Trond.Myklebust@xxxxxxxxxx> wrote:
> > Signed-off-by: Trond Myklebust <Trond.Myklebust@xxxxxxxxxx>
> > Cc: Fred Isaman <iisaman@xxxxxxxxxx>
> > ---
> > 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 f17e469..a1d366b 100644
> > --- a/fs/nfs/direct.c
> > +++ b/fs/nfs/direct.c
> > @@ -243,36 +243,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))
> > + if (!PageCompound(page)) {
> > + if (test_bit(NFS_IOHDR_ERROR, &hdr->flags)) {
> > + if (bytes < hdr->good_bytes)
> > set_page_dirty(req->wb_page);
>
> You can s/req->wb_page/page/
Yep. Will do...
> Other than that, looks fine, but I'll note that my intent in writing
> it as it was was to move the test_bit out of the loop.
> But given the EOF test that was still there, I guess it doesn't make
> much difference.
The other thing is that it becomes unnecessarily difficult to read
because of the loops that do mostly the same thing, except for this one
little bit in the middle.
--
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�����٥
