* Re: [PATCH] Fix a memory leak in scsi_host_dev_release() [not found] <564D0224.9040001@sandisk.com> @ 2015-11-20 11:52 ` Christoph Hellwig 2015-11-20 17:49 ` Bart Van Assche 0 siblings, 1 reply; 3+ messages in thread From: Christoph Hellwig @ 2015-11-20 11:52 UTC (permalink / raw) To: Bart Van Assche Cc: James Bottomley, Martin K. Petersen, Christoph Hellwig, Hannes Reinecke, linux-scsi@vger.kernel.org, linux-kernel, Greg Kroah-Hartman Hi Bart, the memory leak looks real, and your fix looks corret, but I still don't like it. I think it's reasonable for SCSI to assume that the final put_device fully frees the struct device including the name pointer that is assigned entirely behind the back of the caller. So I think the fix for this probably should be in the driver core. ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Fix a memory leak in scsi_host_dev_release() 2015-11-20 11:52 ` [PATCH] Fix a memory leak in scsi_host_dev_release() Christoph Hellwig @ 2015-11-20 17:49 ` Bart Van Assche 2015-11-22 15:08 ` Christoph Hellwig 0 siblings, 1 reply; 3+ messages in thread From: Bart Van Assche @ 2015-11-20 17:49 UTC (permalink / raw) To: Christoph Hellwig Cc: James Bottomley, Martin K. Petersen, Hannes Reinecke, linux-scsi@vger.kernel.org, linux-kernel, Greg Kroah-Hartman On 11/20/2015 03:52 AM, Christoph Hellwig wrote: > the memory leak looks real, and your fix looks corret, but I still > don't like it. > > I think it's reasonable for SCSI to assume that the final put_device > fully frees the struct device including the name pointer that is > assigned entirely behind the back of the caller. > > So I think the fix for this probably should be in the driver core. Hello Christoph, Thanks for the feedback. However, I'm not sure this can be fixed by modifying the driver core. If scsi_host_remove() is not called the SCSI core doesn't call put_device(&shost->shost_dev). I will post a second version of this patch that ensures that the SCSI core always calls put_device(&shost->shost_dev). Bart. ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Fix a memory leak in scsi_host_dev_release() 2015-11-20 17:49 ` Bart Van Assche @ 2015-11-22 15:08 ` Christoph Hellwig 0 siblings, 0 replies; 3+ messages in thread From: Christoph Hellwig @ 2015-11-22 15:08 UTC (permalink / raw) To: Bart Van Assche Cc: Christoph Hellwig, James Bottomley, Martin K. Petersen, Hannes Reinecke, linux-scsi@vger.kernel.org, linux-kernel, Greg Kroah-Hartman On Fri, Nov 20, 2015 at 09:49:42AM -0800, Bart Van Assche wrote: > On 11/20/2015 03:52 AM, Christoph Hellwig wrote: > >the memory leak looks real, and your fix looks corret, but I still > >don't like it. > > > >I think it's reasonable for SCSI to assume that the final put_device > >fully frees the struct device including the name pointer that is > >assigned entirely behind the back of the caller. > > > >So I think the fix for this probably should be in the driver core. > > Hello Christoph, > > Thanks for the feedback. However, I'm not sure this can be fixed by > modifying the driver core. If scsi_host_remove() is not called the SCSI core > doesn't call put_device(&shost->shost_dev). I will post a second version of > this patch that ensures that the SCSI core always calls > put_device(&shost->shost_dev). Oh, I see. The release method is called on shost_gendev, but the name that needs to be freed is in shost_dev. I take my comment on the core back. Let's get this patch in for now and see if we can do something about the creative driver model (ab-)use for struct Scsi_Host in the long run. Reviewed-by: Christoph Hellwig <hch@lst.de> ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-11-22 15:08 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <564D0224.9040001@sandisk.com>
2015-11-20 11:52 ` [PATCH] Fix a memory leak in scsi_host_dev_release() Christoph Hellwig
2015-11-20 17:49 ` Bart Van Assche
2015-11-22 15:08 ` Christoph Hellwig
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox