| Message ID | 1400512508-7530-9-git-send-email-dros@primarydata.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> -static void pnfs_writehdr_free(struct nfs_pgio_header *hdr) > +static void pnfs_pgio_header_free(struct nfs_pgio_header *hdr) > { > pnfs_put_lseg(hdr->lseg); > nfs_pgio_header_free(hdr); > } I think it should also be mossible to simply call pnfs_put_lseg from nfs_pgio_header_free. pnfs_put_lseg handles a NULL argument fine, is stubbed out for the non-pnfs case, and the other callers should never have it set. -- 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
On 05/19/2014 11:34 AM, Christoph Hellwig wrote: >> -static void pnfs_writehdr_free(struct nfs_pgio_header *hdr) >> +static void pnfs_pgio_header_free(struct nfs_pgio_header *hdr) >> { >> pnfs_put_lseg(hdr->lseg); >> nfs_pgio_header_free(hdr); >> } > I think it should also be mossible to simply call pnfs_put_lseg > from nfs_pgio_header_free. pnfs_put_lseg handles a NULL argument fine, > is stubbed out for the non-pnfs case, and the other callers should never > have it set. Every function in this area is identical to each other :). I'm already working on a patch series that combines these functions (and more!). It also needs more testing before I can submit, but I can update against these patches first to see if this unlocks other cleanups. 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 -- 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
On May 19, 2014, at 12:11 PM, Anna Schumaker <schumaker.anna@gmail.com> wrote: > On 05/19/2014 11:34 AM, Christoph Hellwig wrote: >>> -static void pnfs_writehdr_free(struct nfs_pgio_header *hdr) >>> +static void pnfs_pgio_header_free(struct nfs_pgio_header *hdr) >>> { >>> pnfs_put_lseg(hdr->lseg); >>> nfs_pgio_header_free(hdr); >>> } >> I think it should also be mossible to simply call pnfs_put_lseg >> from nfs_pgio_header_free. pnfs_put_lseg handles a NULL argument fine, >> is stubbed out for the non-pnfs case, and the other callers should never >> have it set. Great point. > > Every function in this area is identical to each other :). I'm already working on a patch series that combines these functions (and more!). It also needs more testing before I can submit, but I can update against these patches first to see if this unlocks other cleanups. > > Anna OK, so maybe I’ll just remove this patch from v2 and let you take care of it? -dros-- 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
On Mon, May 19, 2014 at 12:11:30PM -0400, Anna Schumaker wrote: > > I think it should also be mossible to simply call pnfs_put_lseg > > from nfs_pgio_header_free. pnfs_put_lseg handles a NULL argument fine, > > is stubbed out for the non-pnfs case, and the other callers should never > > have it set. > > Every function in this area is identical to each other :). I'm already working on a patch series that combines these functions (and more!). It also needs more testing before I can submit, but I can update against these patches first to see if this unlocks other cleanups. Yes, pnfs.c has lots of duplication of the classic I/O path. I don't think we need to rush out your patches, but it would be good to get them out in the not too far future. -- 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
On 05/19/2014 12:57 PM, Christoph Hellwig wrote: > On Mon, May 19, 2014 at 12:11:30PM -0400, Anna Schumaker wrote: >>> I think it should also be mossible to simply call pnfs_put_lseg >>> from nfs_pgio_header_free. pnfs_put_lseg handles a NULL argument fine, >>> is stubbed out for the non-pnfs case, and the other callers should never >>> have it set. >> Every function in this area is identical to each other :). I'm already working on a patch series that combines these functions (and more!). It also needs more testing before I can submit, but I can update against these patches first to see if this unlocks other cleanups. > Yes, pnfs.c has lots of duplication of the classic I/O path. I don't > think we need to rush out your patches, but it would be good to get them > out in the not too far future. > Sure. I won't rush anything, but I'll try not to take too long either. -- 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
On 05/19/2014 12:21 PM, Weston Andros Adamson wrote: > On May 19, 2014, at 12:11 PM, Anna Schumaker <schumaker.anna@gmail.com> wrote: > >> On 05/19/2014 11:34 AM, Christoph Hellwig wrote: >>>> -static void pnfs_writehdr_free(struct nfs_pgio_header *hdr) >>>> +static void pnfs_pgio_header_free(struct nfs_pgio_header *hdr) >>>> { >>>> pnfs_put_lseg(hdr->lseg); >>>> nfs_pgio_header_free(hdr); >>>> } >>> I think it should also be mossible to simply call pnfs_put_lseg >>> from nfs_pgio_header_free. pnfs_put_lseg handles a NULL argument fine, >>> is stubbed out for the non-pnfs case, and the other callers should never >>> have it set. > Great point. > >> Every function in this area is identical to each other :). I'm already working on a patch series that combines these functions (and more!). It also needs more testing before I can submit, but I can update against these patches first to see if this unlocks other cleanups. >> >> Anna > OK, so maybe I?ll just remove this patch from v2 and let you take care of it? Sure. > > -dros -- 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
On May 19, 2014, at 12:57 PM, Christoph Hellwig <hch@infradead.org> wrote: > On Mon, May 19, 2014 at 12:11:30PM -0400, Anna Schumaker wrote: >>> I think it should also be mossible to simply call pnfs_put_lseg >>> from nfs_pgio_header_free. pnfs_put_lseg handles a NULL argument fine, >>> is stubbed out for the non-pnfs case, and the other callers should never >>> have it set. >> >> Every function in this area is identical to each other :). I'm already working on a patch series that combines these functions (and more!). It also needs more testing before I can submit, but I can update against these patches first to see if this unlocks other cleanups. > > Yes, pnfs.c has lots of duplication of the classic I/O path. I don't > think we need to rush out your patches, but it would be good to get them > out in the not too far future. > I say we add them to the pgio branch (or some other public branch) as soon as you’re ready, so we can combine testing efforts. -dros -- 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
On 05/19/2014 11:34 AM, Christoph Hellwig wrote: >> -static void pnfs_writehdr_free(struct nfs_pgio_header *hdr) >> +static void pnfs_pgio_header_free(struct nfs_pgio_header *hdr) >> { >> pnfs_put_lseg(hdr->lseg); >> nfs_pgio_header_free(hdr); >> } > I think it should also be mossible to simply call pnfs_put_lseg > from nfs_pgio_header_free. pnfs_put_lseg handles a NULL argument fine, > is stubbed out for the non-pnfs case, and the other callers should never > have it set. pnfs_put_lseg is compiled into the NFS v4 module, so depmod would complain about recursive module dependencies if it was called from the generic client. > > -- > 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 -- 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
diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c index 79792a4..d45cca8 100644 --- a/fs/nfs/pnfs.c +++ b/fs/nfs/pnfs.c @@ -1552,12 +1552,11 @@ pnfs_do_write(struct nfs_pageio_descriptor *desc, pnfs_put_lseg(lseg); } -static void pnfs_writehdr_free(struct nfs_pgio_header *hdr) +static void pnfs_pgio_header_free(struct nfs_pgio_header *hdr) { pnfs_put_lseg(hdr->lseg); nfs_pgio_header_free(hdr); } -EXPORT_SYMBOL_GPL(pnfs_writehdr_free); int pnfs_generic_pg_writepages(struct nfs_pageio_descriptor *desc) @@ -1572,7 +1571,7 @@ pnfs_generic_pg_writepages(struct nfs_pageio_descriptor *desc) desc->pg_lseg = NULL; return -ENOMEM; } - nfs_pgheader_init(desc, hdr, pnfs_writehdr_free); + nfs_pgheader_init(desc, hdr, pnfs_pgio_header_free); hdr->lseg = pnfs_get_lseg(desc->pg_lseg); ret = nfs_generic_pgio(desc, hdr); if (ret != 0) { @@ -1671,13 +1670,6 @@ pnfs_do_read(struct nfs_pageio_descriptor *desc, struct nfs_pgio_header *hdr) pnfs_put_lseg(lseg); } -static void pnfs_readhdr_free(struct nfs_pgio_header *hdr) -{ - pnfs_put_lseg(hdr->lseg); - nfs_pgio_header_free(hdr); -} -EXPORT_SYMBOL_GPL(pnfs_readhdr_free); - int pnfs_generic_pg_readpages(struct nfs_pageio_descriptor *desc) { @@ -1692,7 +1684,7 @@ pnfs_generic_pg_readpages(struct nfs_pageio_descriptor *desc) desc->pg_lseg = NULL; return ret; } - nfs_pgheader_init(desc, hdr, pnfs_readhdr_free); + nfs_pgheader_init(desc, hdr, pnfs_pgio_header_free); hdr->lseg = pnfs_get_lseg(desc->pg_lseg); ret = nfs_generic_pgio(desc, hdr); if (ret != 0) {
pnfs_readhdr_free and pnfs_writehdr_free were identical - merge them info pnfs_pgio_header_free. Also drop EXPORT_SYMBOL_GPL of static function. Signed-off-by: Weston Andros Adamson <dros@primarydata.com> --- fs/nfs/pnfs.c | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-)