| Message ID | 20210602133029.2864069-5-ming.lei@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | scsi: fix failure handling of alloc/add host | expand |
On 6/2/21 6:30 AM, Ming Lei wrote: > get_device(shost->shost_gendev.parent) is called after host state is > changed to SHOST_RUNNING. So scsi_host_dev_release() shouldn't release > the parent device if host state is still SHOST_CREATED. > > Cc: Bart Van Assche <bvanassche@acm.org> > Cc: John Garry <john.garry@huawei.com> > Cc: Hannes Reinecke <hare@suse.de> > Signed-off-by: Ming Lei <ming.lei@redhat.com> > --- > drivers/scsi/hosts.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c > index 7049844adb6b..34db5be7a562 100644 > --- a/drivers/scsi/hosts.c > +++ b/drivers/scsi/hosts.c > @@ -350,7 +350,7 @@ static void scsi_host_dev_release(struct device *dev) > > ida_simple_remove(&host_index_ida, shost->host_no); > > - if (parent) > + if (shost->shost_state != SHOST_CREATED) > put_device(parent); > kfree(shost); > } Not sure what others think but I prefer that this patch would also be merged into patch 2/4. Thanks, Bart.
On 02/06/2021 14:30, Ming Lei wrote: > get_device(shost->shost_gendev.parent) is called after host state is > changed to SHOST_RUNNING. So scsi_host_dev_release() shouldn't release > the parent device if host state is still SHOST_CREATED. > > Cc: Bart Van Assche <bvanassche@acm.org> > Cc: John Garry <john.garry@huawei.com> > Cc: Hannes Reinecke <hare@suse.de> > Signed-off-by: Ming Lei <ming.lei@redhat.com> > --- > drivers/scsi/hosts.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c > index 7049844adb6b..34db5be7a562 100644 > --- a/drivers/scsi/hosts.c > +++ b/drivers/scsi/hosts.c > @@ -350,7 +350,7 @@ static void scsi_host_dev_release(struct device *de > > ida_simple_remove(&host_index_ida, shost->host_no); > > - if (parent) > + if (shost->shost_state != SHOST_CREATED) > put_device(parent); > kfree(shost); > } > Reviewed-by: John Garry <john.garry@huawei.com>
On 6/2/21 3:30 PM, Ming Lei wrote: > get_device(shost->shost_gendev.parent) is called after host state is > changed to SHOST_RUNNING. So scsi_host_dev_release() shouldn't release > the parent device if host state is still SHOST_CREATED. > > Cc: Bart Van Assche <bvanassche@acm.org> > Cc: John Garry <john.garry@huawei.com> > Cc: Hannes Reinecke <hare@suse.de> > Signed-off-by: Ming Lei <ming.lei@redhat.com> > --- > drivers/scsi/hosts.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c > index 7049844adb6b..34db5be7a562 100644 > --- a/drivers/scsi/hosts.c > +++ b/drivers/scsi/hosts.c > @@ -350,7 +350,7 @@ static void scsi_host_dev_release(struct device *dev) > > ida_simple_remove(&host_index_ida, shost->host_no); > > - if (parent) > + if (shost->shost_state != SHOST_CREATED) > put_device(parent); > kfree(shost); > } > What happened to the check 'if (parent)'? Cheers, Hannes
On Mon, Jun 07, 2021 at 01:41:36PM +0200, Hannes Reinecke wrote: > On 6/2/21 3:30 PM, Ming Lei wrote: > > get_device(shost->shost_gendev.parent) is called after host state is > > changed to SHOST_RUNNING. So scsi_host_dev_release() shouldn't release > > the parent device if host state is still SHOST_CREATED. > > > > Cc: Bart Van Assche <bvanassche@acm.org> > > Cc: John Garry <john.garry@huawei.com> > > Cc: Hannes Reinecke <hare@suse.de> > > Signed-off-by: Ming Lei <ming.lei@redhat.com> > > --- > > drivers/scsi/hosts.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c > > index 7049844adb6b..34db5be7a562 100644 > > --- a/drivers/scsi/hosts.c > > +++ b/drivers/scsi/hosts.c > > @@ -350,7 +350,7 @@ static void scsi_host_dev_release(struct device *dev) > > > > ida_simple_remove(&host_index_ida, shost->host_no); > > > > - if (parent) > > + if (shost->shost_state != SHOST_CREATED) > > put_device(parent); > > kfree(shost); > > } > > > What happened to the check 'if (parent)'? put_device() is just like kfree(), and it will handle null 'parent'. Thanks, Ming
diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c index 7049844adb6b..34db5be7a562 100644 --- a/drivers/scsi/hosts.c +++ b/drivers/scsi/hosts.c @@ -350,7 +350,7 @@ static void scsi_host_dev_release(struct device *dev) ida_simple_remove(&host_index_ida, shost->host_no); - if (parent) + if (shost->shost_state != SHOST_CREATED) put_device(parent); kfree(shost); }
get_device(shost->shost_gendev.parent) is called after host state is changed to SHOST_RUNNING. So scsi_host_dev_release() shouldn't release the parent device if host state is still SHOST_CREATED. Cc: Bart Van Assche <bvanassche@acm.org> Cc: John Garry <john.garry@huawei.com> Cc: Hannes Reinecke <hare@suse.de> Signed-off-by: Ming Lei <ming.lei@redhat.com> --- drivers/scsi/hosts.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)