Return-Path: linux-nfs-owner@vger.kernel.org Received: from mail-pb0-f46.google.com ([209.85.160.46]:60981 "EHLO mail-pb0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753743Ab2FMNyU convert rfc822-to-8bit (ORCPT ); Wed, 13 Jun 2012 09:54:20 -0400 Received: by pbbrp8 with SMTP id rp8so2321039pbb.19 for ; Wed, 13 Jun 2012 06:54:19 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <4FD1EC54.9070909@panasas.com> References: <4FD1CEA8.3090105@panasas.com> <4FD1EC54.9070909@panasas.com> From: Peng Tao Date: Wed, 13 Jun 2012 21:53:59 +0800 Message-ID: Subject: Re: [PATCH 3/6] pnfs-obj: Fix __r4w_get_page when offset is beyond i_size To: Boaz Harrosh Cc: Trond Myklebust , NFS list , open-osd , Benny Halevy Content-Type: text/plain; charset=UTF-8 Sender: linux-nfs-owner@vger.kernel.org List-ID: On Fri, Jun 8, 2012 at 8:13 PM, Boaz Harrosh wrote: > > It is very common for the end of the file to be unaligned on > stripe size. But since we know it's beyond file's end then > the XOR should be preformed with all zeros. > > Old code used to just read zeros out of the OSD devices, which is a great > waist. But what scares me more about this situation is that, we now have > pages attached to the file's mapping that are beyond i_size. I don't > like the kind of bugs this calls for. > > Fix both birds, by returning a global zero_page, if offset is beyond > i_size. > > TODO: >        Change the API to ->__r4w_get_page() so a NULL can be >        returned without being considered as error, since XOR API >        treats NULL entries as zero_pages. > > [Bug since 3.2. Should apply the same way to all Kernels since] > CC: Stable Tree > Signed-off-by: Boaz Harrosh > --- >  fs/nfs/objlayout/objio_osd.c | 24 +++++++++++++++++++++--- >  1 file changed, 21 insertions(+), 3 deletions(-) > > diff --git a/fs/nfs/objlayout/objio_osd.c b/fs/nfs/objlayout/objio_osd.c > index b47277b..0e7c833 100644 > --- a/fs/nfs/objlayout/objio_osd.c > +++ b/fs/nfs/objlayout/objio_osd.c > @@ -480,14 +480,28 @@ static void _write_done(struct ore_io_state *ios, void *private) >        objlayout_write_done(&objios->oir, status, objios->sync); >  } > > +static struct page *g_zero_page; > + Since you don't attach g_zero_page to any file's address space, I think that you can replace it with ZERO_PAGE(0), no? Cheers, Tao >  static struct page *__r4w_get_page(void *priv, u64 offset, bool *uptodate) >  { >        struct objio_state *objios = priv; >        struct nfs_write_data *wdata = objios->oir.rpcdata; >        struct address_space *mapping = wdata->header->inode->i_mapping; >        pgoff_t index = offset / PAGE_SIZE; > -       struct page *page = find_get_page(mapping, index); > +       struct page *page; > +       loff_t i_size = i_size_read(wdata->header->inode); > + > +       if (offset >= i_size) { > +               if (!g_zero_page) { > +                       g_zero_page = alloc_page(GFP_NOFS); > +                       clear_highpage(g_zero_page); > +               } > +               *uptodate = true; > +               dprintk("%s: g_zero_page index=0x%lx\n", __func__, index); > +               return g_zero_page; > +       } > > +       page = find_get_page(mapping, index); >        if (!page) { >                page = find_or_create_page(mapping, index, GFP_NOFS); >                if (unlikely(!page)) { > @@ -507,8 +521,10 @@ static struct page *__r4w_get_page(void *priv, u64 offset, bool *uptodate) > >  static void __r4w_put_page(void *priv, struct page *page) >  { > -       dprintk("%s: index=0x%lx\n", __func__, page->index); > -       page_cache_release(page); > +       dprintk("%s: index=0x%lx\n", __func__, > +               (page == g_zero_page) ? -1UL : page->index); > +       if (g_zero_page != page) > +               page_cache_release(page); >        return; >  } > > @@ -615,6 +631,8 @@ static bool objio_pg_test(struct nfs_pageio_descriptor *pgio, >  static void __exit >  objlayout_exit(void) >  { > +       if (g_zero_page) > +               __free_page(g_zero_page); >        pnfs_unregister_layoutdriver(&objlayout_type); >        printk(KERN_INFO "NFS: %s: Unregistered OSD pNFS Layout Driver\n", >               __func__); > -- > 1.7.10.2.677.gb6bc67f > >