* a few more pnfs/blocklayout fixes
@ 2014-09-11 0:36 Christoph Hellwig
2014-09-11 0:36 ` [PATCH 1/4] pnfs/blocklayout: remove some debugging Christoph Hellwig
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Christoph Hellwig @ 2014-09-11 0:36 UTC (permalink / raw)
To: linux-nfs
Thanks for merging the first two sets of blocklayout patches!
Here are a few more patches to sort out the fallout.
The first two fix up warnings reported from the kbuild robot.
The third patch didn't make it in for some reason, but is needed
for reliable operation. The fourth removes the GETDEVICELIST
implementation, which was slightly buggy and unessecary.
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 1/4] pnfs/blocklayout: remove some debugging 2014-09-11 0:36 a few more pnfs/blocklayout fixes Christoph Hellwig @ 2014-09-11 0:36 ` Christoph Hellwig 2014-09-11 0:36 ` [PATCH 2/4] pnfs/objlayout: fix endianess annotation in objio_alloc_deviceid_node Christoph Hellwig ` (2 subsequent siblings) 3 siblings, 0 replies; 11+ messages in thread From: Christoph Hellwig @ 2014-09-11 0:36 UTC (permalink / raw) To: linux-nfs The kbuild test robot complained that we got the printk format wrong. Let's just kill these printks instead of fixing them as there is not point after the initial tree algorithm debugging. Reported-by: kbuild test robot <fengguang.wu@intel.com> Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/nfs/blocklayout/extent_tree.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/fs/nfs/blocklayout/extent_tree.c b/fs/nfs/blocklayout/extent_tree.c index f34f61d..43e891b 100644 --- a/fs/nfs/blocklayout/extent_tree.c +++ b/fs/nfs/blocklayout/extent_tree.c @@ -370,9 +370,6 @@ ext_tree_split(struct rb_root *root, struct pnfs_block_extent *be, struct pnfs_block_extent *new; sector_t orig_len = be->be_length; - dprintk("%s: need split for 0x%lx:0x%lx at 0x%lx\n", - __func__, be->be_f_offset, ext_f_end(be), split); - new = kzalloc(sizeof(*new), GFP_ATOMIC); if (!new) return -ENOMEM; @@ -387,11 +384,6 @@ ext_tree_split(struct rb_root *root, struct pnfs_block_extent *be, new->be_tag = be->be_tag; new->be_device = nfs4_get_deviceid(be->be_device); - dprintk("%s: got 0x%lx:0x%lx!\n", - __func__, be->be_f_offset, ext_f_end(be)); - dprintk("%s: got 0x%lx:0x%lx!\n", - __func__, new->be_f_offset, ext_f_end(new)); - __ext_tree_insert(root, new, false); return 0; } -- 1.9.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/4] pnfs/objlayout: fix endianess annotation in objio_alloc_deviceid_node 2014-09-11 0:36 a few more pnfs/blocklayout fixes Christoph Hellwig 2014-09-11 0:36 ` [PATCH 1/4] pnfs/blocklayout: remove some debugging Christoph Hellwig @ 2014-09-11 0:36 ` Christoph Hellwig 2014-09-11 0:36 ` [PATCH 3/4] pnfs/blocklayout: allocate separate pages for the layoutcommit payload Christoph Hellwig 2014-09-11 0:36 ` [PATCH 4/4] pnfs: remove GETDEVICELIST implementation Christoph Hellwig 3 siblings, 0 replies; 11+ messages in thread From: Christoph Hellwig @ 2014-09-11 0:36 UTC (permalink / raw) To: linux-nfs The kbuild test robot complained about a new sparse warning in objio_alloc_deviceid_node, but it turns out that this was just a moved reference to an existing variable. Fix it to have the right big endian annotated type. Note that there are some other endianess issues in this file that I didn't bother to sort out as they involve global headers. Reported-by: kbuild test robot <fengguang.wu@intel.com> Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/nfs/objlayout/objio_osd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/nfs/objlayout/objio_osd.c b/fs/nfs/objlayout/objio_osd.c index c502f64..c6e4bda 100644 --- a/fs/nfs/objlayout/objio_osd.c +++ b/fs/nfs/objlayout/objio_osd.c @@ -93,7 +93,7 @@ objio_alloc_deviceid_node(struct nfs_server *server, struct pnfs_device *pdev, struct osd_dev *od; struct osd_dev_info odi; bool retry_flag = true; - u32 *p; + __be32 *p; int err; deviceaddr = kzalloc(sizeof(*deviceaddr), gfp_flags); -- 1.9.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/4] pnfs/blocklayout: allocate separate pages for the layoutcommit payload 2014-09-11 0:36 a few more pnfs/blocklayout fixes Christoph Hellwig 2014-09-11 0:36 ` [PATCH 1/4] pnfs/blocklayout: remove some debugging Christoph Hellwig 2014-09-11 0:36 ` [PATCH 2/4] pnfs/objlayout: fix endianess annotation in objio_alloc_deviceid_node Christoph Hellwig @ 2014-09-11 0:36 ` Christoph Hellwig 2014-09-11 14:16 ` Anna Schumaker 2014-09-11 0:36 ` [PATCH 4/4] pnfs: remove GETDEVICELIST implementation Christoph Hellwig 3 siblings, 1 reply; 11+ messages in thread From: Christoph Hellwig @ 2014-09-11 0:36 UTC (permalink / raw) To: linux-nfs Instead of overflowing the XDR send buffer with our extent list allocate pages and pre-encode the layoutupdate payload into them. We optimistically allocate a single page use alloc_page and only switch to vmalloc when we have more extents outstanding. Currently there is only a single testcase (xfstests generic/113) which can reproduce large enough extent lists for this to occur. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/nfs/blocklayout/blocklayout.c | 15 ++---- fs/nfs/blocklayout/blocklayout.h | 8 ++-- fs/nfs/blocklayout/extent_tree.c | 100 +++++++++++++++++++++++++++++++-------- 3 files changed, 90 insertions(+), 33 deletions(-) diff --git a/fs/nfs/blocklayout/blocklayout.c b/fs/nfs/blocklayout/blocklayout.c index 25ba9e0..900eb09 100644 --- a/fs/nfs/blocklayout/blocklayout.c +++ b/fs/nfs/blocklayout/blocklayout.c @@ -500,21 +500,16 @@ bl_return_range(struct pnfs_layout_hdr *lo, err = ext_tree_remove(bl, range->iomode & IOMODE_RW, offset, end); } -static void -bl_encode_layoutcommit(struct pnfs_layout_hdr *lo, struct xdr_stream *xdr, - const struct nfs4_layoutcommit_args *arg) +static int +bl_prepare_layoutcommit(struct nfs4_layoutcommit_args *arg) { - dprintk("%s enter\n", __func__); - ext_tree_encode_commit(BLK_LO2EXT(lo), xdr); + return ext_tree_prepare_commit(arg); } static void bl_cleanup_layoutcommit(struct nfs4_layoutcommit_data *lcdata) { - struct pnfs_layout_hdr *lo = NFS_I(lcdata->args.inode)->layout; - - dprintk("%s enter\n", __func__); - ext_tree_mark_committed(BLK_LO2EXT(lo), lcdata->res.status); + ext_tree_mark_committed(&lcdata->args, lcdata->res.status); } static int @@ -670,7 +665,7 @@ static struct pnfs_layoutdriver_type blocklayout_type = { .alloc_lseg = bl_alloc_lseg, .free_lseg = bl_free_lseg, .return_range = bl_return_range, - .encode_layoutcommit = bl_encode_layoutcommit, + .prepare_layoutcommit = bl_prepare_layoutcommit, .cleanup_layoutcommit = bl_cleanup_layoutcommit, .set_layoutdriver = bl_set_layoutdriver, .alloc_deviceid_node = bl_alloc_deviceid_node, diff --git a/fs/nfs/blocklayout/blocklayout.h b/fs/nfs/blocklayout/blocklayout.h index 19fae5e..9757f3e 100644 --- a/fs/nfs/blocklayout/blocklayout.h +++ b/fs/nfs/blocklayout/blocklayout.h @@ -72,6 +72,9 @@ struct pnfs_block_extent { unsigned int be_tag; }; +/* on the wire size of the extent */ +#define BL_EXTENT_SIZE (7 * sizeof(__be32) + NFS4_DEVICEID4_SIZE) + struct pnfs_block_layout { struct pnfs_layout_hdr bl_layout; struct rb_root bl_ext_rw; @@ -129,8 +132,7 @@ int ext_tree_mark_written(struct pnfs_block_layout *bl, sector_t start, sector_t len); bool ext_tree_lookup(struct pnfs_block_layout *bl, sector_t isect, struct pnfs_block_extent *ret, bool rw); -int ext_tree_encode_commit(struct pnfs_block_layout *bl, - struct xdr_stream *xdr); -void ext_tree_mark_committed(struct pnfs_block_layout *bl, int status); +int ext_tree_prepare_commit(struct nfs4_layoutcommit_args *arg); +void ext_tree_mark_committed(struct nfs4_layoutcommit_args *arg, int status); #endif /* FS_NFS_NFS4BLOCKLAYOUT_H */ diff --git a/fs/nfs/blocklayout/extent_tree.c b/fs/nfs/blocklayout/extent_tree.c index 43e891b..2a2c261 100644 --- a/fs/nfs/blocklayout/extent_tree.c +++ b/fs/nfs/blocklayout/extent_tree.c @@ -462,19 +462,25 @@ out: return err; } -int -ext_tree_encode_commit(struct pnfs_block_layout *bl, struct xdr_stream *xdr) +static void ext_tree_free_commitdata(struct nfs4_layoutcommit_args *arg, + size_t buffer_size) { - struct pnfs_block_extent *be; - unsigned int count = 0; - __be32 *p, *xdr_start; - int ret = 0; + if (arg->layoutupdate_pages != &arg->layoutupdate_page) { + int nr_pages = DIV_ROUND_UP(buffer_size, PAGE_SIZE), i; - dprintk("%s enter\n", __func__); + for (i = 0; i < nr_pages; i++) + put_page(arg->layoutupdate_pages[i]); + kfree(arg->layoutupdate_pages); + } else { + put_page(arg->layoutupdate_page); + } +} - xdr_start = xdr_reserve_space(xdr, 8); - if (!xdr_start) - return -ENOSPC; +static int ext_tree_encode_commit(struct pnfs_block_layout *bl, __be32 *p, + size_t buffer_size, size_t *count) +{ + struct pnfs_block_extent *be; + int ret = 0; spin_lock(&bl->bl_ext_lock); for (be = ext_tree_first(&bl->bl_ext_rw); be; be = ext_tree_next(be)) { @@ -482,12 +488,11 @@ ext_tree_encode_commit(struct pnfs_block_layout *bl, struct xdr_stream *xdr) be->be_tag != EXTENT_WRITTEN) continue; - p = xdr_reserve_space(xdr, 7 * sizeof(__be32) + - NFS4_DEVICEID4_SIZE); - if (!p) { - printk("%s: out of space for extent list\n", __func__); + (*count)++; + if (*count * BL_EXTENT_SIZE > buffer_size) { + /* keep counting.. */ ret = -ENOSPC; - break; + continue; } p = xdr_encode_opaque_fixed(p, be->be_device->deviceid.data, @@ -498,25 +503,80 @@ ext_tree_encode_commit(struct pnfs_block_layout *bl, struct xdr_stream *xdr) *p++ = cpu_to_be32(PNFS_BLOCK_READWRITE_DATA); be->be_tag = EXTENT_COMMITTING; - count++; } spin_unlock(&bl->bl_ext_lock); - xdr_start[0] = cpu_to_be32((xdr->p - xdr_start - 1) * 4); - xdr_start[1] = cpu_to_be32(count); + return ret; +} + +int +ext_tree_prepare_commit(struct nfs4_layoutcommit_args *arg) +{ + struct pnfs_block_layout *bl = BLK_LO2EXT(NFS_I(arg->inode)->layout); + size_t count = 0, buffer_size = PAGE_SIZE; + __be32 *start_p; + int ret; + + dprintk("%s enter\n", __func__); + + arg->layoutupdate_page = alloc_page(GFP_NOFS); + if (!arg->layoutupdate_page) + return -ENOMEM; + start_p = page_address(arg->layoutupdate_page); + arg->layoutupdate_pages = &arg->layoutupdate_page; + +retry: + ret = ext_tree_encode_commit(bl, start_p + 1, buffer_size, &count); + if (unlikely(ret)) { + ext_tree_free_commitdata(arg, buffer_size); + + buffer_size = sizeof(__be32) + BL_EXTENT_SIZE * count; + count = 0; + + arg->layoutupdate_pages = + kcalloc(DIV_ROUND_UP(buffer_size, PAGE_SIZE), + sizeof(struct page *), GFP_NOFS); + if (!arg->layoutupdate_pages) + return -ENOMEM; + + start_p = __vmalloc(buffer_size, GFP_NOFS, PAGE_KERNEL); + if (!start_p) { + kfree(arg->layoutupdate_pages); + return -ENOMEM; + } + + goto retry; + } + + *start_p = cpu_to_be32(count); + arg->layoutupdate_len = sizeof(__be32) + BL_EXTENT_SIZE * count; + + if (unlikely(arg->layoutupdate_pages != &arg->layoutupdate_page)) { + __be32 *p = start_p; + int i = 0; + + for (p = start_p; + p < start_p + arg->layoutupdate_len; + p += PAGE_SIZE) { + arg->layoutupdate_pages[i++] = vmalloc_to_page(p); + } + } dprintk("%s found %i ranges\n", __func__, count); - return ret; + return 0; } void -ext_tree_mark_committed(struct pnfs_block_layout *bl, int status) +ext_tree_mark_committed(struct nfs4_layoutcommit_args *arg, int status) { + struct pnfs_block_layout *bl = BLK_LO2EXT(NFS_I(arg->inode)->layout); struct rb_root *root = &bl->bl_ext_rw; struct pnfs_block_extent *be; dprintk("%s status %d\n", __func__, status); + ext_tree_free_commitdata(arg, arg->layoutupdate_len); + spin_lock(&bl->bl_ext_lock); for (be = ext_tree_first(root); be; be = ext_tree_next(be)) { if (be->be_state != PNFS_BLOCK_INVALID_DATA || -- 1.9.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 3/4] pnfs/blocklayout: allocate separate pages for the layoutcommit payload 2014-09-11 0:36 ` [PATCH 3/4] pnfs/blocklayout: allocate separate pages for the layoutcommit payload Christoph Hellwig @ 2014-09-11 14:16 ` Anna Schumaker 2014-09-11 15:27 ` Christoph Hellwig 0 siblings, 1 reply; 11+ messages in thread From: Anna Schumaker @ 2014-09-11 14:16 UTC (permalink / raw) To: Christoph Hellwig, linux-nfs On 09/10/2014 08:36 PM, Christoph Hellwig wrote: > Instead of overflowing the XDR send buffer with our extent list allocate > pages and pre-encode the layoutupdate payload into them. We optimistically > allocate a single page use alloc_page and only switch to vmalloc when we > have more extents outstanding. Currently there is only a single testcase > (xfstests generic/113) which can reproduce large enough extent lists for > this to occur. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > fs/nfs/blocklayout/blocklayout.c | 15 ++---- > fs/nfs/blocklayout/blocklayout.h | 8 ++-- > fs/nfs/blocklayout/extent_tree.c | 100 +++++++++++++++++++++++++++++++-------- > 3 files changed, 90 insertions(+), 33 deletions(-) > > diff --git a/fs/nfs/blocklayout/blocklayout.c b/fs/nfs/blocklayout/blocklayout.c > index 25ba9e0..900eb09 100644 > --- a/fs/nfs/blocklayout/blocklayout.c > +++ b/fs/nfs/blocklayout/blocklayout.c > @@ -500,21 +500,16 @@ bl_return_range(struct pnfs_layout_hdr *lo, > err = ext_tree_remove(bl, range->iomode & IOMODE_RW, offset, end); > } > > -static void > -bl_encode_layoutcommit(struct pnfs_layout_hdr *lo, struct xdr_stream *xdr, > - const struct nfs4_layoutcommit_args *arg) > +static int > +bl_prepare_layoutcommit(struct nfs4_layoutcommit_args *arg) > { > - dprintk("%s enter\n", __func__); > - ext_tree_encode_commit(BLK_LO2EXT(lo), xdr); > + return ext_tree_prepare_commit(arg); > } > > static void > bl_cleanup_layoutcommit(struct nfs4_layoutcommit_data *lcdata) > { > - struct pnfs_layout_hdr *lo = NFS_I(lcdata->args.inode)->layout; > - > - dprintk("%s enter\n", __func__); > - ext_tree_mark_committed(BLK_LO2EXT(lo), lcdata->res.status); > + ext_tree_mark_committed(&lcdata->args, lcdata->res.status); > } > > static int > @@ -670,7 +665,7 @@ static struct pnfs_layoutdriver_type blocklayout_type = { > .alloc_lseg = bl_alloc_lseg, > .free_lseg = bl_free_lseg, > .return_range = bl_return_range, > - .encode_layoutcommit = bl_encode_layoutcommit, > + .prepare_layoutcommit = bl_prepare_layoutcommit, > .cleanup_layoutcommit = bl_cleanup_layoutcommit, > .set_layoutdriver = bl_set_layoutdriver, > .alloc_deviceid_node = bl_alloc_deviceid_node, > diff --git a/fs/nfs/blocklayout/blocklayout.h b/fs/nfs/blocklayout/blocklayout.h > index 19fae5e..9757f3e 100644 > --- a/fs/nfs/blocklayout/blocklayout.h > +++ b/fs/nfs/blocklayout/blocklayout.h > @@ -72,6 +72,9 @@ struct pnfs_block_extent { > unsigned int be_tag; > }; > > +/* on the wire size of the extent */ > +#define BL_EXTENT_SIZE (7 * sizeof(__be32) + NFS4_DEVICEID4_SIZE) > + > struct pnfs_block_layout { > struct pnfs_layout_hdr bl_layout; > struct rb_root bl_ext_rw; > @@ -129,8 +132,7 @@ int ext_tree_mark_written(struct pnfs_block_layout *bl, sector_t start, > sector_t len); > bool ext_tree_lookup(struct pnfs_block_layout *bl, sector_t isect, > struct pnfs_block_extent *ret, bool rw); > -int ext_tree_encode_commit(struct pnfs_block_layout *bl, > - struct xdr_stream *xdr); > -void ext_tree_mark_committed(struct pnfs_block_layout *bl, int status); > +int ext_tree_prepare_commit(struct nfs4_layoutcommit_args *arg); > +void ext_tree_mark_committed(struct nfs4_layoutcommit_args *arg, int status); > > #endif /* FS_NFS_NFS4BLOCKLAYOUT_H */ > diff --git a/fs/nfs/blocklayout/extent_tree.c b/fs/nfs/blocklayout/extent_tree.c > index 43e891b..2a2c261 100644 > --- a/fs/nfs/blocklayout/extent_tree.c > +++ b/fs/nfs/blocklayout/extent_tree.c > @@ -462,19 +462,25 @@ out: > return err; > } > > -int > -ext_tree_encode_commit(struct pnfs_block_layout *bl, struct xdr_stream *xdr) > +static void ext_tree_free_commitdata(struct nfs4_layoutcommit_args *arg, > + size_t buffer_size) > { > - struct pnfs_block_extent *be; > - unsigned int count = 0; > - __be32 *p, *xdr_start; > - int ret = 0; > + if (arg->layoutupdate_pages != &arg->layoutupdate_page) { > + int nr_pages = DIV_ROUND_UP(buffer_size, PAGE_SIZE), i; > > - dprintk("%s enter\n", __func__); > + for (i = 0; i < nr_pages; i++) > + put_page(arg->layoutupdate_pages[i]); > + kfree(arg->layoutupdate_pages); > + } else { > + put_page(arg->layoutupdate_page); > + } > +} > > - xdr_start = xdr_reserve_space(xdr, 8); > - if (!xdr_start) > - return -ENOSPC; > +static int ext_tree_encode_commit(struct pnfs_block_layout *bl, __be32 *p, > + size_t buffer_size, size_t *count) > +{ > + struct pnfs_block_extent *be; > + int ret = 0; > > spin_lock(&bl->bl_ext_lock); > for (be = ext_tree_first(&bl->bl_ext_rw); be; be = ext_tree_next(be)) { > @@ -482,12 +488,11 @@ ext_tree_encode_commit(struct pnfs_block_layout *bl, struct xdr_stream *xdr) > be->be_tag != EXTENT_WRITTEN) > continue; > > - p = xdr_reserve_space(xdr, 7 * sizeof(__be32) + > - NFS4_DEVICEID4_SIZE); > - if (!p) { > - printk("%s: out of space for extent list\n", __func__); > + (*count)++; > + if (*count * BL_EXTENT_SIZE > buffer_size) { > + /* keep counting.. */ > ret = -ENOSPC; > - break; > + continue; > } > > p = xdr_encode_opaque_fixed(p, be->be_device->deviceid.data, > @@ -498,25 +503,80 @@ ext_tree_encode_commit(struct pnfs_block_layout *bl, struct xdr_stream *xdr) > *p++ = cpu_to_be32(PNFS_BLOCK_READWRITE_DATA); > > be->be_tag = EXTENT_COMMITTING; > - count++; > } > spin_unlock(&bl->bl_ext_lock); > > - xdr_start[0] = cpu_to_be32((xdr->p - xdr_start - 1) * 4); > - xdr_start[1] = cpu_to_be32(count); > + return ret; > +} > + > +int > +ext_tree_prepare_commit(struct nfs4_layoutcommit_args *arg) > +{ > + struct pnfs_block_layout *bl = BLK_LO2EXT(NFS_I(arg->inode)->layout); > + size_t count = 0, buffer_size = PAGE_SIZE; > + __be32 *start_p; > + int ret; > + > + dprintk("%s enter\n", __func__); > + > + arg->layoutupdate_page = alloc_page(GFP_NOFS); > + if (!arg->layoutupdate_page) > + return -ENOMEM; > + start_p = page_address(arg->layoutupdate_page); > + arg->layoutupdate_pages = &arg->layoutupdate_page; > + > +retry: > + ret = ext_tree_encode_commit(bl, start_p + 1, buffer_size, &count); > + if (unlikely(ret)) { > + ext_tree_free_commitdata(arg, buffer_size); > + > + buffer_size = sizeof(__be32) + BL_EXTENT_SIZE * count; > + count = 0; > + > + arg->layoutupdate_pages = > + kcalloc(DIV_ROUND_UP(buffer_size, PAGE_SIZE), > + sizeof(struct page *), GFP_NOFS); I don't think I've seen kcalloc() before. What does it do? > + if (!arg->layoutupdate_pages) > + return -ENOMEM; > + > + start_p = __vmalloc(buffer_size, GFP_NOFS, PAGE_KERNEL); > + if (!start_p) { > + kfree(arg->layoutupdate_pages); > + return -ENOMEM; > + } > + > + goto retry; > + } > + > + *start_p = cpu_to_be32(count); > + arg->layoutupdate_len = sizeof(__be32) + BL_EXTENT_SIZE * count; > + > + if (unlikely(arg->layoutupdate_pages != &arg->layoutupdate_page)) { > + __be32 *p = start_p; > + int i = 0; > + > + for (p = start_p; > + p < start_p + arg->layoutupdate_len; > + p += PAGE_SIZE) { > + arg->layoutupdate_pages[i++] = vmalloc_to_page(p); > + } > + } > > dprintk("%s found %i ranges\n", __func__, count); I'm getting this: fs/nfs/blocklayout/extent_tree.c: In function ‘ext_tree_prepare_commit’: fs/nfs/blocklayout/extent_tree.c:565:2: warning: format ‘%i’ expects argument of type ‘int’, but argument 3 has type ‘size_t’ [-Wformat=] dprintk("%s found %i ranges\n", __func__, count); Anna > - return ret; > + return 0; > } > > void > -ext_tree_mark_committed(struct pnfs_block_layout *bl, int status) > +ext_tree_mark_committed(struct nfs4_layoutcommit_args *arg, int status) > { > + struct pnfs_block_layout *bl = BLK_LO2EXT(NFS_I(arg->inode)->layout); > struct rb_root *root = &bl->bl_ext_rw; > struct pnfs_block_extent *be; > > dprintk("%s status %d\n", __func__, status); > > + ext_tree_free_commitdata(arg, arg->layoutupdate_len); > + > spin_lock(&bl->bl_ext_lock); > for (be = ext_tree_first(root); be; be = ext_tree_next(be)) { > if (be->be_state != PNFS_BLOCK_INVALID_DATA || ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/4] pnfs/blocklayout: allocate separate pages for the layoutcommit payload 2014-09-11 14:16 ` Anna Schumaker @ 2014-09-11 15:27 ` Christoph Hellwig 2014-09-11 15:59 ` Anna Schumaker 2014-09-11 16:00 ` Anna Schumaker 0 siblings, 2 replies; 11+ messages in thread From: Christoph Hellwig @ 2014-09-11 15:27 UTC (permalink / raw) To: Anna Schumaker; +Cc: Christoph Hellwig, linux-nfs On Thu, Sep 11, 2014 at 10:16:56AM -0400, Anna Schumaker wrote: > > + arg->layoutupdate_pages = > > + kcalloc(DIV_ROUND_UP(buffer_size, PAGE_SIZE), > > + sizeof(struct page *), GFP_NOFS); > > I don't think I've seen kcalloc() before. What does it do? The same as calloc(3) in userspace - allocate an array of nent entries of size, and zero the result that is returned. > > dprintk("%s found %i ranges\n", __func__, count); > > I'm getting this: > > fs/nfs/blocklayout/extent_tree.c: In function ‘ext_tree_prepare_commit’: > fs/nfs/blocklayout/extent_tree.c:565:2: warning: format ‘%i’ expects argument of type ‘int’, but argument 3 has type ‘size_t’ [-Wformat=] > dprintk("%s found %i ranges\n", __func__, count); For some reason I haven't managed to trigger this warning even after turning on CONFIG_SUNRPC_DEBUG, but it should be easy to fix up.. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/4] pnfs/blocklayout: allocate separate pages for the layoutcommit payload 2014-09-11 15:27 ` Christoph Hellwig @ 2014-09-11 15:59 ` Anna Schumaker 2014-09-11 16:00 ` Anna Schumaker 1 sibling, 0 replies; 11+ messages in thread From: Anna Schumaker @ 2014-09-11 15:59 UTC (permalink / raw) To: Christoph Hellwig, Anna Schumaker; +Cc: linux-nfs On 09/11/2014 11:27 AM, Christoph Hellwig wrote: > On Thu, Sep 11, 2014 at 10:16:56AM -0400, Anna Schumaker wrote: >>> + arg->layoutupdate_pages = >>> + kcalloc(DIV_ROUND_UP(buffer_size, PAGE_SIZE), >>> + sizeof(struct page *), GFP_NOFS); >> I don't think I've seen kcalloc() before. What does it do? > The same as calloc(3) in userspace - allocate an array of nent entries > of size, and zero the result that is returned. I haven't run across it in userspace either ... thanks for answering! :) > >>> dprintk("%s found %i ranges\n", __func__, count); >> I'm getting this: >> >> fs/nfs/blocklayout/extent_tree.c: In function ‘ext_tree_prepare_commit’: >> fs/nfs/blocklayout/extent_tree.c:565:2: warning: format ‘%i’ expects argument of type ‘int’, but argument 3 has type ‘size_t’ [-Wformat=] >> dprintk("%s found %i ranges\n", __func__, count); > For some reason I haven't managed to trigger this warning even after > turning on CONFIG_SUNRPC_DEBUG, but it should be easy to fix up.. Thanks. I'm not sure what I have set differently, but it's consistently showing up in my compile logs. Anna > > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/4] pnfs/blocklayout: allocate separate pages for the layoutcommit payload 2014-09-11 15:27 ` Christoph Hellwig 2014-09-11 15:59 ` Anna Schumaker @ 2014-09-11 16:00 ` Anna Schumaker 1 sibling, 0 replies; 11+ messages in thread From: Anna Schumaker @ 2014-09-11 16:00 UTC (permalink / raw) To: Christoph Hellwig, Anna Schumaker; +Cc: linux-nfs On 09/11/2014 11:27 AM, Christoph Hellwig wrote: > On Thu, Sep 11, 2014 at 10:16:56AM -0400, Anna Schumaker wrote: >>> + arg->layoutupdate_pages = >>> + kcalloc(DIV_ROUND_UP(buffer_size, PAGE_SIZE), >>> + sizeof(struct page *), GFP_NOFS); >> I don't think I've seen kcalloc() before. What does it do? > The same as calloc(3) in userspace - allocate an array of nent entries > of size, and zero the result that is returned. I haven't run across it in userspace either ... thanks for answering! :) > >>> dprintk("%s found %i ranges\n", __func__, count); >> I'm getting this: >> >> fs/nfs/blocklayout/extent_tree.c: In function ‘ext_tree_prepare_commit’: >> fs/nfs/blocklayout/extent_tree.c:565:2: warning: format ‘%i’ expects argument of type ‘int’, but argument 3 has type ‘size_t’ [-Wformat=] >> dprintk("%s found %i ranges\n", __func__, count); > For some reason I haven't managed to trigger this warning even after > turning on CONFIG_SUNRPC_DEBUG, but it should be easy to fix up.. Thanks. I'm not sure what I have set differently, but it's consistently showing up in my compile logs. Anna > > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 4/4] pnfs: remove GETDEVICELIST implementation 2014-09-11 0:36 a few more pnfs/blocklayout fixes Christoph Hellwig ` (2 preceding siblings ...) 2014-09-11 0:36 ` [PATCH 3/4] pnfs/blocklayout: allocate separate pages for the layoutcommit payload Christoph Hellwig @ 2014-09-11 0:36 ` Christoph Hellwig 2014-09-11 14:51 ` Peng Tao 3 siblings, 1 reply; 11+ messages in thread From: Christoph Hellwig @ 2014-09-11 0:36 UTC (permalink / raw) To: linux-nfs The current GETDEVICELIST implementation is buggy in that it doesn't handle cursors correctly, and in that it returns an error if the server returns NFSERR_NOTSUPP. Given that there is no actual need for GETDEVICELIST, it has various issues and might get removed for NFSv4.2 stop using it in the blocklayout driver, and thus the Linux NFS client as whole. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/nfs/blocklayout/blocklayout.c | 2 +- fs/nfs/nfs4proc.c | 48 --------------- fs/nfs/nfs4xdr.c | 130 --------------------------------------- fs/nfs/pnfs.h | 5 -- fs/nfs/pnfs_dev.c | 30 --------- include/linux/nfs_xdr.h | 11 ---- 6 files changed, 1 insertion(+), 225 deletions(-) diff --git a/fs/nfs/blocklayout/blocklayout.c b/fs/nfs/blocklayout/blocklayout.c index 900eb09..cf10a6e 100644 --- a/fs/nfs/blocklayout/blocklayout.c +++ b/fs/nfs/blocklayout/blocklayout.c @@ -527,7 +527,7 @@ bl_set_layoutdriver(struct nfs_server *server, const struct nfs_fh *fh) return -EINVAL; } - return nfs4_deviceid_getdevicelist(server, fh); + return 0; } static bool diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 47fa67a..0b71122 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -7808,54 +7808,6 @@ int nfs4_proc_layoutreturn(struct nfs4_layoutreturn *lrp) return status; } -/* - * Retrieve the list of Data Server devices from the MDS. - */ -static int _nfs4_getdevicelist(struct nfs_server *server, - const struct nfs_fh *fh, - struct pnfs_devicelist *devlist) -{ - struct nfs4_getdevicelist_args args = { - .fh = fh, - .layoutclass = server->pnfs_curr_ld->id, - }; - struct nfs4_getdevicelist_res res = { - .devlist = devlist, - }; - struct rpc_message msg = { - .rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_GETDEVICELIST], - .rpc_argp = &args, - .rpc_resp = &res, - }; - int status; - - dprintk("--> %s\n", __func__); - status = nfs4_call_sync(server->client, server, &msg, &args.seq_args, - &res.seq_res, 0); - dprintk("<-- %s status=%d\n", __func__, status); - return status; -} - -int nfs4_proc_getdevicelist(struct nfs_server *server, - const struct nfs_fh *fh, - struct pnfs_devicelist *devlist) -{ - struct nfs4_exception exception = { }; - int err; - - do { - err = nfs4_handle_exception(server, - _nfs4_getdevicelist(server, fh, devlist), - &exception); - } while (exception.retry); - - dprintk("%s: err=%d, num_devs=%u\n", __func__, - err, devlist->num_devs); - - return err; -} -EXPORT_SYMBOL_GPL(nfs4_proc_getdevicelist); - static int _nfs4_proc_getdeviceinfo(struct nfs_server *server, struct pnfs_device *pdev, diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c index f2cd957..b8165ea 100644 --- a/fs/nfs/nfs4xdr.c +++ b/fs/nfs/nfs4xdr.c @@ -362,17 +362,6 @@ static int nfs4_stat_to_errno(int); XDR_QUADLEN(NFS4_MAX_SESSIONID_LEN) + 5) #define encode_reclaim_complete_maxsz (op_encode_hdr_maxsz + 4) #define decode_reclaim_complete_maxsz (op_decode_hdr_maxsz + 4) -#define encode_getdevicelist_maxsz (op_encode_hdr_maxsz + 4 + \ - encode_verifier_maxsz) -#define decode_getdevicelist_maxsz (op_decode_hdr_maxsz + \ - 2 /* nfs_cookie4 gdlr_cookie */ + \ - decode_verifier_maxsz \ - /* verifier4 gdlr_verifier */ + \ - 1 /* gdlr_deviceid_list count */ + \ - XDR_QUADLEN(NFS4_PNFS_GETDEVLIST_MAXNUM * \ - NFS4_DEVICEID4_SIZE) \ - /* gdlr_deviceid_list */ + \ - 1 /* bool gdlr_eof */) #define encode_getdeviceinfo_maxsz (op_encode_hdr_maxsz + 4 + \ XDR_QUADLEN(NFS4_DEVICEID4_SIZE)) #define decode_getdeviceinfo_maxsz (op_decode_hdr_maxsz + \ @@ -812,14 +801,6 @@ static int nfs4_stat_to_errno(int); #define NFS4_dec_reclaim_complete_sz (compound_decode_hdr_maxsz + \ decode_sequence_maxsz + \ decode_reclaim_complete_maxsz) -#define NFS4_enc_getdevicelist_sz (compound_encode_hdr_maxsz + \ - encode_sequence_maxsz + \ - encode_putfh_maxsz + \ - encode_getdevicelist_maxsz) -#define NFS4_dec_getdevicelist_sz (compound_decode_hdr_maxsz + \ - decode_sequence_maxsz + \ - decode_putfh_maxsz + \ - decode_getdevicelist_maxsz) #define NFS4_enc_getdeviceinfo_sz (compound_encode_hdr_maxsz + \ encode_sequence_maxsz +\ encode_getdeviceinfo_maxsz) @@ -1930,24 +1911,6 @@ static void encode_sequence(struct xdr_stream *xdr, #ifdef CONFIG_NFS_V4_1 static void -encode_getdevicelist(struct xdr_stream *xdr, - const struct nfs4_getdevicelist_args *args, - struct compound_hdr *hdr) -{ - __be32 *p; - nfs4_verifier dummy = { - .data = "dummmmmy", - }; - - encode_op_hdr(xdr, OP_GETDEVICELIST, decode_getdevicelist_maxsz, hdr); - p = reserve_space(xdr, 16); - *p++ = cpu_to_be32(args->layoutclass); - *p++ = cpu_to_be32(NFS4_PNFS_GETDEVLIST_MAXNUM); - xdr_encode_hyper(p, 0ULL); /* cookie */ - encode_nfs4_verifier(xdr, &dummy); -} - -static void encode_getdeviceinfo(struct xdr_stream *xdr, const struct nfs4_getdeviceinfo_args *args, struct compound_hdr *hdr) @@ -2901,24 +2864,6 @@ static void nfs4_xdr_enc_reclaim_complete(struct rpc_rqst *req, } /* - * Encode GETDEVICELIST request - */ -static void nfs4_xdr_enc_getdevicelist(struct rpc_rqst *req, - struct xdr_stream *xdr, - struct nfs4_getdevicelist_args *args) -{ - struct compound_hdr hdr = { - .minorversion = nfs4_xdr_minorversion(&args->seq_args), - }; - - encode_compound_hdr(xdr, req, &hdr); - encode_sequence(xdr, &args->seq_args, &hdr); - encode_putfh(xdr, args->fh, &hdr); - encode_getdevicelist(xdr, args, &hdr); - encode_nops(&hdr); -} - -/* * Encode GETDEVICEINFO request */ static void nfs4_xdr_enc_getdeviceinfo(struct rpc_rqst *req, @@ -5773,54 +5718,6 @@ out_overflow: } #if defined(CONFIG_NFS_V4_1) -/* - * TODO: Need to handle case when EOF != true; - */ -static int decode_getdevicelist(struct xdr_stream *xdr, - struct pnfs_devicelist *res) -{ - __be32 *p; - int status, i; - nfs4_verifier verftemp; - - status = decode_op_hdr(xdr, OP_GETDEVICELIST); - if (status) - return status; - - p = xdr_inline_decode(xdr, 8 + 8 + 4); - if (unlikely(!p)) - goto out_overflow; - - /* TODO: Skip cookie for now */ - p += 2; - - /* Read verifier */ - p = xdr_decode_opaque_fixed(p, verftemp.data, NFS4_VERIFIER_SIZE); - - res->num_devs = be32_to_cpup(p); - - dprintk("%s: num_dev %d\n", __func__, res->num_devs); - - if (res->num_devs > NFS4_PNFS_GETDEVLIST_MAXNUM) { - printk(KERN_ERR "NFS: %s too many result dev_num %u\n", - __func__, res->num_devs); - return -EIO; - } - - p = xdr_inline_decode(xdr, - res->num_devs * NFS4_DEVICEID4_SIZE + 4); - if (unlikely(!p)) - goto out_overflow; - for (i = 0; i < res->num_devs; i++) - p = xdr_decode_opaque_fixed(p, res->dev_id[i].data, - NFS4_DEVICEID4_SIZE); - res->eof = be32_to_cpup(p); - return 0; -out_overflow: - print_overflow_msg(__func__, xdr); - return -EIO; -} - static int decode_getdeviceinfo(struct xdr_stream *xdr, struct pnfs_device *pdev) { @@ -7105,32 +7002,6 @@ static int nfs4_xdr_dec_reclaim_complete(struct rpc_rqst *rqstp, } /* - * Decode GETDEVICELIST response - */ -static int nfs4_xdr_dec_getdevicelist(struct rpc_rqst *rqstp, - struct xdr_stream *xdr, - struct nfs4_getdevicelist_res *res) -{ - struct compound_hdr hdr; - int status; - - dprintk("encoding getdevicelist!\n"); - - status = decode_compound_hdr(xdr, &hdr); - if (status != 0) - goto out; - status = decode_sequence(xdr, &res->seq_res, rqstp); - if (status != 0) - goto out; - status = decode_putfh(xdr); - if (status != 0) - goto out; - status = decode_getdevicelist(xdr, res->devlist); -out: - return status; -} - -/* * Decode GETDEVINFO response */ static int nfs4_xdr_dec_getdeviceinfo(struct rpc_rqst *rqstp, @@ -7498,7 +7369,6 @@ struct rpc_procinfo nfs4_procedures[] = { PROC(SECINFO_NO_NAME, enc_secinfo_no_name, dec_secinfo_no_name), PROC(TEST_STATEID, enc_test_stateid, dec_test_stateid), PROC(FREE_STATEID, enc_free_stateid, dec_free_stateid), - PROC(GETDEVICELIST, enc_getdevicelist, dec_getdevicelist), PROC(BIND_CONN_TO_SESSION, enc_bind_conn_to_session, dec_bind_conn_to_session), PROC(DESTROY_CLIENTID, enc_destroy_clientid, dec_destroy_clientid), diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h index d84fe29..b10f12f 100644 --- a/fs/nfs/pnfs.h +++ b/fs/nfs/pnfs.h @@ -180,9 +180,6 @@ extern int pnfs_register_layoutdriver(struct pnfs_layoutdriver_type *); extern void pnfs_unregister_layoutdriver(struct pnfs_layoutdriver_type *); /* nfs4proc.c */ -extern int nfs4_proc_getdevicelist(struct nfs_server *server, - const struct nfs_fh *fh, - struct pnfs_devicelist *devlist); extern int nfs4_proc_getdeviceinfo(struct nfs_server *server, struct pnfs_device *dev, struct rpc_cred *cred); @@ -277,8 +274,6 @@ bool nfs4_put_deviceid_node(struct nfs4_deviceid_node *); void nfs4_mark_deviceid_unavailable(struct nfs4_deviceid_node *node); bool nfs4_test_deviceid_unavailable(struct nfs4_deviceid_node *node); void nfs4_deviceid_purge_client(const struct nfs_client *); -int nfs4_deviceid_getdevicelist(struct nfs_server *server, - const struct nfs_fh *fh); static inline struct nfs4_deviceid_node * nfs4_get_deviceid(struct nfs4_deviceid_node *d) diff --git a/fs/nfs/pnfs_dev.c b/fs/nfs/pnfs_dev.c index 82c2836..aa2ec00 100644 --- a/fs/nfs/pnfs_dev.c +++ b/fs/nfs/pnfs_dev.c @@ -358,33 +358,3 @@ nfs4_deviceid_mark_client_invalid(struct nfs_client *clp) } rcu_read_unlock(); } - -int -nfs4_deviceid_getdevicelist(struct nfs_server *server, - const struct nfs_fh *fh) -{ - struct pnfs_devicelist *dlist; - struct nfs4_deviceid_node *d; - int error = 0, i; - - dlist = kzalloc(sizeof(struct pnfs_devicelist), GFP_NOFS); - if (!dlist) - return -ENOMEM; - - while (!dlist->eof) { - error = nfs4_proc_getdevicelist(server, fh, dlist); - if (error) - break; - - for (i = 0; i < dlist->num_devs; i++) { - d = nfs4_find_get_deviceid(server, &dlist->dev_id[i], - NULL, GFP_NOFS); - if (d) - nfs4_put_deviceid_node(d); - } - } - - kfree(dlist); - return error; -} -EXPORT_SYMBOL_GPL(nfs4_deviceid_getdevicelist); diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h index f4092c6..7ae249c 100644 --- a/include/linux/nfs_xdr.h +++ b/include/linux/nfs_xdr.h @@ -252,17 +252,6 @@ struct nfs4_layoutget { gfp_t gfp_flags; }; -struct nfs4_getdevicelist_args { - struct nfs4_sequence_args seq_args; - const struct nfs_fh *fh; - u32 layoutclass; -}; - -struct nfs4_getdevicelist_res { - struct nfs4_sequence_res seq_res; - struct pnfs_devicelist *devlist; -}; - struct nfs4_getdeviceinfo_args { struct nfs4_sequence_args seq_args; struct pnfs_device *pdev; -- 1.9.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 4/4] pnfs: remove GETDEVICELIST implementation 2014-09-11 0:36 ` [PATCH 4/4] pnfs: remove GETDEVICELIST implementation Christoph Hellwig @ 2014-09-11 14:51 ` Peng Tao 2014-09-11 15:29 ` Christoph Hellwig 0 siblings, 1 reply; 11+ messages in thread From: Peng Tao @ 2014-09-11 14:51 UTC (permalink / raw) To: Christoph Hellwig; +Cc: Linux NFS Mailing List On Thu, Sep 11, 2014 at 8:36 AM, Christoph Hellwig <hch@lst.de> wrote: > The current GETDEVICELIST implementation is buggy in that it doesn't handle > cursors correctly, and in that it returns an error if the server returns > NFSERR_NOTSUPP. Given that there is no actual need for GETDEVICELIST, > it has various issues and might get removed for NFSv4.2 stop using it in > the blocklayout driver, and thus the Linux NFS client as whole. > I am missing patch 3/4, which I assume would fill in the gap where GETDEVICEINFO is sent for block layout. Can you please resent that? Cheers, Tao ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 4/4] pnfs: remove GETDEVICELIST implementation 2014-09-11 14:51 ` Peng Tao @ 2014-09-11 15:29 ` Christoph Hellwig 0 siblings, 0 replies; 11+ messages in thread From: Christoph Hellwig @ 2014-09-11 15:29 UTC (permalink / raw) To: Peng Tao; +Cc: Linux NFS Mailing List On Thu, Sep 11, 2014 at 10:51:17PM +0800, Peng Tao wrote: > I am missing patch 3/4, which I assume would fill in the gap where > GETDEVICEINFO is sent for block layout. Can you please resent that? That was part of a different series that Trond merged: pnfs: factor GETDEVICEINFO implementations V2 from September, 2nd. ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2014-09-11 16:00 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-09-11 0:36 a few more pnfs/blocklayout fixes Christoph Hellwig 2014-09-11 0:36 ` [PATCH 1/4] pnfs/blocklayout: remove some debugging Christoph Hellwig 2014-09-11 0:36 ` [PATCH 2/4] pnfs/objlayout: fix endianess annotation in objio_alloc_deviceid_node Christoph Hellwig 2014-09-11 0:36 ` [PATCH 3/4] pnfs/blocklayout: allocate separate pages for the layoutcommit payload Christoph Hellwig 2014-09-11 14:16 ` Anna Schumaker 2014-09-11 15:27 ` Christoph Hellwig 2014-09-11 15:59 ` Anna Schumaker 2014-09-11 16:00 ` Anna Schumaker 2014-09-11 0:36 ` [PATCH 4/4] pnfs: remove GETDEVICELIST implementation Christoph Hellwig 2014-09-11 14:51 ` Peng Tao 2014-09-11 15:29 ` Christoph Hellwig
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.