| Message ID | 20200401185652.1904777-6-trondmy@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | NFS: Fix a number of memory leaks and use-after-free | expand |
Hi Trond, On 4/1/20 2:56 PM, trondmy@kernel.org wrote: > From: Trond Myklebust <trond.myklebust@hammerspace.com> > > If we just set the mirror count to 1 without first clearing out > the mirrors, we can leak queued up requests. > > Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> > --- > fs/nfs/pagelist.c | 17 ++++++++--------- > 1 file changed, 8 insertions(+), 9 deletions(-) > > diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c > index 99eb839c5778..1fd4862217e0 100644 > --- a/fs/nfs/pagelist.c > +++ b/fs/nfs/pagelist.c > @@ -900,15 +900,6 @@ static void nfs_pageio_setup_mirroring(struct nfs_pageio_descriptor *pgio, > pgio->pg_mirror_count = mirror_count; > } > > -/* > - * nfs_pageio_stop_mirroring - stop using mirroring (set mirror count to 1) > - */ > -void nfs_pageio_stop_mirroring(struct nfs_pageio_descriptor *pgio) > -{ > - pgio->pg_mirror_count = 1; > - pgio->pg_mirror_idx = 0; > -} > - > static void nfs_pageio_cleanup_mirroring(struct nfs_pageio_descriptor *pgio) > { > pgio->pg_mirror_count = 1; > @@ -1334,6 +1325,14 @@ void nfs_pageio_cond_complete(struct nfs_pageio_descriptor *desc, pgoff_t index) > } > } > > +/* > + * nfs_pageio_stop_mirroring - stop using mirroring (set mirror count to 1) > + */ > +void nfs_pageio_stop_mirroring(struct nfs_pageio_descriptor *pgio) > +{ > + nfs_pageio_complete(pgio); > +} > + Would it make sense to replace calls to nfs_pageio_stop_mirroring() with nfs_pageio_complete() instead? Anna > int __init nfs_init_nfspagecache(void) > { > nfs_page_cachep = kmem_cache_create("nfs_page",
On Thu, 2020-04-02 at 12:14 -0400, Anna Schumaker wrote: > Hi Trond, > > On 4/1/20 2:56 PM, trondmy@kernel.org wrote: > > From: Trond Myklebust <trond.myklebust@hammerspace.com> > > > > If we just set the mirror count to 1 without first clearing out > > the mirrors, we can leak queued up requests. > > > > Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> > > --- > > fs/nfs/pagelist.c | 17 ++++++++--------- > > 1 file changed, 8 insertions(+), 9 deletions(-) > > > > diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c > > index 99eb839c5778..1fd4862217e0 100644 > > --- a/fs/nfs/pagelist.c > > +++ b/fs/nfs/pagelist.c > > @@ -900,15 +900,6 @@ static void nfs_pageio_setup_mirroring(struct > > nfs_pageio_descriptor *pgio, > > pgio->pg_mirror_count = mirror_count; > > } > > > > -/* > > - * nfs_pageio_stop_mirroring - stop using mirroring (set mirror > > count to 1) > > - */ > > -void nfs_pageio_stop_mirroring(struct nfs_pageio_descriptor *pgio) > > -{ > > - pgio->pg_mirror_count = 1; > > - pgio->pg_mirror_idx = 0; > > -} > > - > > static void nfs_pageio_cleanup_mirroring(struct > > nfs_pageio_descriptor *pgio) > > { > > pgio->pg_mirror_count = 1; > > @@ -1334,6 +1325,14 @@ void nfs_pageio_cond_complete(struct > > nfs_pageio_descriptor *desc, pgoff_t index) > > } > > } > > > > +/* > > + * nfs_pageio_stop_mirroring - stop using mirroring (set mirror > > count to 1) > > + */ > > +void nfs_pageio_stop_mirroring(struct nfs_pageio_descriptor *pgio) > > +{ > > + nfs_pageio_complete(pgio); > > +} > > + > > Would it make sense to replace calls to nfs_pageio_stop_mirroring() > with nfs_pageio_complete() instead? > No. Let's keep the wrapper, since it makes the writeback code easier to read (it expresses the intent more clearly). > Anna > > > int __init nfs_init_nfspagecache(void) > > { > > nfs_page_cachep = kmem_cache_create("nfs_page",
diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c index 99eb839c5778..1fd4862217e0 100644 --- a/fs/nfs/pagelist.c +++ b/fs/nfs/pagelist.c @@ -900,15 +900,6 @@ static void nfs_pageio_setup_mirroring(struct nfs_pageio_descriptor *pgio, pgio->pg_mirror_count = mirror_count; } -/* - * nfs_pageio_stop_mirroring - stop using mirroring (set mirror count to 1) - */ -void nfs_pageio_stop_mirroring(struct nfs_pageio_descriptor *pgio) -{ - pgio->pg_mirror_count = 1; - pgio->pg_mirror_idx = 0; -} - static void nfs_pageio_cleanup_mirroring(struct nfs_pageio_descriptor *pgio) { pgio->pg_mirror_count = 1; @@ -1334,6 +1325,14 @@ void nfs_pageio_cond_complete(struct nfs_pageio_descriptor *desc, pgoff_t index) } } +/* + * nfs_pageio_stop_mirroring - stop using mirroring (set mirror count to 1) + */ +void nfs_pageio_stop_mirroring(struct nfs_pageio_descriptor *pgio) +{ + nfs_pageio_complete(pgio); +} + int __init nfs_init_nfspagecache(void) { nfs_page_cachep = kmem_cache_create("nfs_page",