Looks good to me. Reviewed-by: Weston Andros Adamson <dros@xxxxxxxxxxxxxxx> -dros > On Aug 20, 2017, at 10:32 AM, Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx> wrote: > > There are a number of callers of nfs_pageio_complete() that want to > continue using the nfs_pageio_descriptor without needing to call > nfs_pageio_init() again. Examples include nfs_pageio_resend() and > nfs_pageio_cond_complete(). > > The problem is that nfs_pageio_complete() also calls > nfs_pageio_cleanup_mirroring(), which frees up the array of mirrors. > This can lead to writeback errors, in the next call to > nfs_pageio_setup_mirroring(). > > Fix by simply moving the allocation of the mirrors to > nfs_pageio_setup_mirroring(). > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=196709 > Reported-by: JianhongYin <yin-jianhong@xxxxxxx> > Cc: stable@xxxxxxxxxxxxxxx # 4.0+ > Signed-off-by: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx> > --- > fs/nfs/pagelist.c | 73 +++++++++++++++++++++++++++++-------------------------- > 1 file changed, 39 insertions(+), 34 deletions(-) > > diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c > index 548ebc7256ff..1c7e625824dc 100644 > --- a/fs/nfs/pagelist.c > +++ b/fs/nfs/pagelist.c > @@ -685,9 +685,6 @@ void nfs_pageio_init(struct nfs_pageio_descriptor *desc, > int io_flags, > gfp_t gfp_flags) > { > - struct nfs_pgio_mirror *new; > - int i; > - > desc->pg_moreio = 0; > desc->pg_inode = inode; > desc->pg_ops = pg_ops; > @@ -703,21 +700,9 @@ void nfs_pageio_init(struct nfs_pageio_descriptor *desc, > desc->pg_mirror_count = 1; > desc->pg_mirror_idx = 0; > > - if (pg_ops->pg_get_mirror_count) { > - /* until we have a request, we don't have an lseg and no > - * idea how many mirrors there will be */ > - new = kcalloc(NFS_PAGEIO_DESCRIPTOR_MIRROR_MAX, > - sizeof(struct nfs_pgio_mirror), gfp_flags); > - desc->pg_mirrors_dynamic = new; > - desc->pg_mirrors = new; > - > - for (i = 0; i < NFS_PAGEIO_DESCRIPTOR_MIRROR_MAX; i++) > - nfs_pageio_mirror_init(&desc->pg_mirrors[i], bsize); > - } else { > - desc->pg_mirrors_dynamic = NULL; > - desc->pg_mirrors = desc->pg_mirrors_static; > - nfs_pageio_mirror_init(&desc->pg_mirrors[0], bsize); > - } > + desc->pg_mirrors_dynamic = NULL; > + desc->pg_mirrors = desc->pg_mirrors_static; > + nfs_pageio_mirror_init(&desc->pg_mirrors[0], bsize); > } > EXPORT_SYMBOL_GPL(nfs_pageio_init); > > @@ -836,32 +821,52 @@ static int nfs_generic_pg_pgios(struct nfs_pageio_descriptor *desc) > return ret; > } > > +static struct nfs_pgio_mirror * > +nfs_pageio_alloc_mirrors(struct nfs_pageio_descriptor *desc, > + unsigned int mirror_count) > +{ > + struct nfs_pgio_mirror *ret; > + unsigned int i; > + > + kfree(desc->pg_mirrors_dynamic); > + desc->pg_mirrors_dynamic = NULL; > + if (mirror_count == 1) > + return desc->pg_mirrors_static; > + ret = kmalloc_array(mirror_count, sizeof(*ret), GFP_NOFS); > + if (ret != NULL) { > + for (i = 0; i < mirror_count; i++) > + nfs_pageio_mirror_init(&ret[i], desc->pg_bsize); > + desc->pg_mirrors_dynamic = ret; > + } > + return ret; > +} > + > /* > * nfs_pageio_setup_mirroring - determine if mirroring is to be used > * by calling the pg_get_mirror_count op > */ > -static int nfs_pageio_setup_mirroring(struct nfs_pageio_descriptor *pgio, > +static void nfs_pageio_setup_mirroring(struct nfs_pageio_descriptor *pgio, > struct nfs_page *req) > { > - int mirror_count = 1; > + unsigned int mirror_count = 1; > > - if (!pgio->pg_ops->pg_get_mirror_count) > - return 0; > - > - mirror_count = pgio->pg_ops->pg_get_mirror_count(pgio, req); > - > - if (pgio->pg_error < 0) > - return pgio->pg_error; > - > - if (!mirror_count || mirror_count > NFS_PAGEIO_DESCRIPTOR_MIRROR_MAX) > - return -EINVAL; > + if (pgio->pg_ops->pg_get_mirror_count) > + mirror_count = pgio->pg_ops->pg_get_mirror_count(pgio, req); > + if (mirror_count == pgio->pg_mirror_count || pgio->pg_error < 0) > + return; > > - if (WARN_ON_ONCE(!pgio->pg_mirrors_dynamic)) > - return -EINVAL; > + if (!mirror_count || mirror_count > NFS_PAGEIO_DESCRIPTOR_MIRROR_MAX) { > + pgio->pg_error = -EINVAL; > + return; > + } > > + pgio->pg_mirrors = nfs_pageio_alloc_mirrors(pgio, mirror_count); > + if (pgio->pg_mirrors == NULL) { > + pgio->pg_error = -ENOMEM; > + pgio->pg_mirrors = pgio->pg_mirrors_static; > + mirror_count = 1; > + } > pgio->pg_mirror_count = mirror_count; > - > - return 0; > } > > /* > -- > 2.13.5 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
