On 17 Oct 2018, at 13:34, Trond Myklebust wrote: > On Wed, 2018-10-17 at 12:05 -0400, Benjamin Coddington wrote: >> @@ -1245,7 +1246,9 @@ void nfs_pageio_complete(struct >> nfs_pageio_descriptor *desc) >> for (midx = 0; midx < desc->pg_mirror_count; midx++) >> nfs_pageio_complete_mirror(desc, midx); >> >> - if (desc->pg_ops->pg_cleanup) >> + if (desc->pg_error < 0) >> + nfs_pageio_error_cleanup(desc); >> + else if (desc->pg_ops->pg_cleanup) >> desc->pg_ops->pg_cleanup(desc); > > Nice catch, but shouldn't we be calling both nfs_pageio_error_cleanup() > and pg_cleanup()? The former would appear to be cleaning up the page > stuff, while the latter is mainly cleaning up the layout. Ah, yes .. I got pg_cleanup mixed up with pg_completion_ops->completion. Hmm, pg_cleanup seems unnecessary at the moment. They all point to pnfs_generic_pg_cleanup. I'll send a v2 after testing with some pNFS.. > >> nfs_pageio_cleanup_mirroring(desc); >> } > > Should this perhaps be a stable fix? There's a lot of churn in there so I gave up looking for a Fixes: tag. I'll take another look and see if I can figure out how far back to go. Thanks for the look, Ben
