On Wednesday 22 March 2006 17:32, Douglas Gilbert wrote: > Bryan Holty wrote: > > Hello, > > There is a memory leak in sg that is hit in the case when direct_io > > is in use and the length of the data is longer than we can fit into a > > single scatter-gather list. Two scatter-gather lists are allocated for > > each io that meets the criteria, and only 1 ever gets released. > > > > The following fix releases the scatter-gather list created by the > > attempted sg_build_direct when abandoning the effort in favor of > > sg_build_indirect. sg_build_indirect creates a new scatter-gather list. > > Wow, sg leak fixes are coming thick and fast. > I agree there is a leak there but I'm not sure about > the placement of sg_remove_scat(). > > Bryan, what do you think about the variant attached to > this post? It is untested. > > Doug Gilbert > > > Signed-off-by: Bryan Holty <lgeek@xxxxxxxxxxxxxxx> > > > > --- a/drivers/scsi/sg.c 2006-03-03 13:17:23.000000000 -0600 > > +++ b/drivers/scsi/sg.c 2006-03-22 16:32:17.536091840 -0600 > > @@ -1648,6 +1648,7 @@ > > res = sg_build_direct(srp, sfp, dxfer_len); > > if (res <= 0) /* -ve -> error, 0 -> done, 1 -> try indirect */ > > return res; > > + sg_remove_scat(req_schp); > > } > > if ((!sg_res_in_use(sfp)) && (dxfer_len <= rsv_schp->bufflen)) > > sg_link_reserve(sfp, srp, dxfer_len); I think the variant solves the same problem. I had no other reason for releasing the scatter-gather list outside of sg_build_direct other than that is how the the sg_build_indirect is done and that due to the checks in sg_remove_scat it could be called from there. I agree that the variant placement makes more sense. -- Bryan Holty - : send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html