| Message ID | 0101016ef4259a7c-76bdf010-88b1-4309-ba24-8c874734184e-000000@us-west-2.amazonses.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Modulize ufs-bsg | expand |
> > In ufshcd_remove(), after SCSI host is removed, put it once so that its resources > can be released. > > Signed-off-by: Can Guo <cang@codeaurora.org> This is not really part of this patchset, is it? > --- > drivers/scsi/ufs/ufshcd.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index > b5966fa..a86b0fd 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -8251,6 +8251,7 @@ void ufshcd_remove(struct ufs_hba *hba) > ufs_bsg_remove(hba); > ufs_sysfs_remove_nodes(hba->dev); > scsi_remove_host(hba->host); > + scsi_host_put(hba->host); > /* disable interrupts */ > ufshcd_disable_intr(hba, hba->intr_mask); > ufshcd_hba_stop(hba, true); > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project
On 2019-12-11 18:37, Avri Altman wrote: >> >> In ufshcd_remove(), after SCSI host is removed, put it once so that >> its resources >> can be released. >> >> Signed-off-by: Can Guo <cang@codeaurora.org> > > This is not really part of this patchset, is it? > Hi Avri, I put this change in the same patchset due to #1. The main patch has dependency on it #2. Consider a scenario where platform driver is also compiled as a module, say ufs_qcom.ko. In this case, we have two modules, ufs-qcom.ko and ufs-bsg.ko. If do insmod ufs-qcom.ko then rmmod ufs-qcom.ko and do insmod ufs-qcom.ko again, without this change, because scsi host was not release, the new scsi host will have a different host number, meaning hba->host->host_no will be 1, but not 0. Now if do insmod ufs-bsg.ko now, the ufs-bsg dev created in /dev/bsg/ will be ufs-bsg1, but not ufs-bsg0. If keep trying these operations, the ufs-bsg device's name will be messed up. This change make sure after ufs-qcom.ko is removed and installed back, its hba->host->host_no stays 0, so that ufs-bsg device name stays same. Thanks, Can Guo. >> --- >> drivers/scsi/ufs/ufshcd.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c >> index >> b5966fa..a86b0fd 100644 >> --- a/drivers/scsi/ufs/ufshcd.c >> +++ b/drivers/scsi/ufs/ufshcd.c >> @@ -8251,6 +8251,7 @@ void ufshcd_remove(struct ufs_hba *hba) >> ufs_bsg_remove(hba); >> ufs_sysfs_remove_nodes(hba->dev); >> scsi_remove_host(hba->host); >> + scsi_host_put(hba->host); >> /* disable interrupts */ >> ufshcd_disable_intr(hba, hba->intr_mask); >> ufshcd_hba_stop(hba, true); >> -- >> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora >> Forum, >> a Linux Foundation Collaborative Project
> > > On 2019-12-11 18:37, Avri Altman wrote: > >> > >> In ufshcd_remove(), after SCSI host is removed, put it once so that > >> its resources can be released. > >> > >> Signed-off-by: Can Guo <cang@codeaurora.org> > > > > This is not really part of this patchset, is it? > > > > Hi Avri, > > I put this change in the same patchset due to #1. The main patch has > dependency on it #2. Consider a scenario where platform driver is also compiled > as a module, say ufs_qcom.ko. > In this case, we have two modules, ufs-qcom.ko and ufs-bsg.ko. If do insmod > ufs-qcom.ko > then rmmod ufs-qcom.ko and do insmod ufs-qcom.ko again, without this > change, because scsi > host was not release, the new scsi host will have a different host number, > meaning > hba->host->host_no will be 1, but not 0. Now if do insmod ufs-bsg.ko now, > the ufs-bsg dev > created in /dev/bsg/ will be ufs-bsg1, but not ufs-bsg0. If keep trying these > operations, > the ufs-bsg device's name will be messed up. This change make sure after ufs- > qcom.ko is removed > and installed back, its hba->host->host_no stays 0, so that ufs-bsg device > name stays same. Looks like we needed to manage the ufs-bsg nodes using an IDA, instead of host_no? > > Thanks, > > Can Guo. > > >> --- > >> drivers/scsi/ufs/ufshcd.c | 1 + > >> 1 file changed, 1 insertion(+) > >> > >> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > >> index b5966fa..a86b0fd 100644 > >> --- a/drivers/scsi/ufs/ufshcd.c > >> +++ b/drivers/scsi/ufs/ufshcd.c > >> @@ -8251,6 +8251,7 @@ void ufshcd_remove(struct ufs_hba *hba) > >> ufs_bsg_remove(hba); > >> ufs_sysfs_remove_nodes(hba->dev); > >> scsi_remove_host(hba->host); > >> + scsi_host_put(hba->host); > >> /* disable interrupts */ > >> ufshcd_disable_intr(hba, hba->intr_mask); > >> ufshcd_hba_stop(hba, true); > >> -- > >> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora > >> Forum, a Linux Foundation Collaborative Project
On 2019-12-11 19:22, Avri Altman wrote: >> >> >> On 2019-12-11 18:37, Avri Altman wrote: >> >> >> >> In ufshcd_remove(), after SCSI host is removed, put it once so that >> >> its resources can be released. >> >> >> >> Signed-off-by: Can Guo <cang@codeaurora.org> >> > >> > This is not really part of this patchset, is it? >> > >> >> Hi Avri, >> >> I put this change in the same patchset due to #1. The main patch has >> dependency on it #2. Consider a scenario where platform driver is also >> compiled >> as a module, say ufs_qcom.ko. >> In this case, we have two modules, ufs-qcom.ko and ufs-bsg.ko. If >> do insmod >> ufs-qcom.ko >> then rmmod ufs-qcom.ko and do insmod ufs-qcom.ko again, without >> this >> change, because scsi >> host was not release, the new scsi host will have a different >> host number, >> meaning >> hba->host->host_no will be 1, but not 0. Now if do insmod >> ufs-bsg.ko now, >> the ufs-bsg dev >> created in /dev/bsg/ will be ufs-bsg1, but not ufs-bsg0. If keep >> trying these >> operations, >> the ufs-bsg device's name will be messed up. This change make >> sure after ufs- >> qcom.ko is removed >> and installed back, its hba->host->host_no stays 0, so that >> ufs-bsg device >> name stays same. > Looks like we needed to manage the ufs-bsg nodes using an IDA, instead > of host_no? > > Marking one ufs-bsg dev with host_no still has its advantage. If we have more than one hba host, we can find the right ufs-bsgX dev by reading the sg/sd/bsg device's host->host_unique_id (through SCSI_IOCTL_GET_IDLUN for example). But If ufs-bsg has its own ID, we will lost this "mapping". Thanks, Can Guo. >> >> Thanks, >> >> Can Guo. >> >> >> --- >> >> drivers/scsi/ufs/ufshcd.c | 1 + >> >> 1 file changed, 1 insertion(+) >> >> >> >> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c >> >> index b5966fa..a86b0fd 100644 >> >> --- a/drivers/scsi/ufs/ufshcd.c >> >> +++ b/drivers/scsi/ufs/ufshcd.c >> >> @@ -8251,6 +8251,7 @@ void ufshcd_remove(struct ufs_hba *hba) >> >> ufs_bsg_remove(hba); >> >> ufs_sysfs_remove_nodes(hba->dev); >> >> scsi_remove_host(hba->host); >> >> + scsi_host_put(hba->host); >> >> /* disable interrupts */ >> >> ufshcd_disable_intr(hba, hba->intr_mask); >> >> ufshcd_hba_stop(hba, true); >> >> -- >> >> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora >> >> Forum, a Linux Foundation Collaborative Project
> On 2019-12-11 19:22, Avri Altman wrote: > >> > >> > >> On 2019-12-11 18:37, Avri Altman wrote: > >> >> > >> >> In ufshcd_remove(), after SCSI host is removed, put it once so > >> >> that its resources can be released. > >> >> > >> >> Signed-off-by: Can Guo <cang@codeaurora.org> > >> > > >> > This is not really part of this patchset, is it? > >> > > >> > >> Hi Avri, > >> > >> I put this change in the same patchset due to #1. The main patch has > >> dependency on it #2. Consider a scenario where platform driver is > >> also compiled as a module, say ufs_qcom.ko. > >> In this case, we have two modules, ufs-qcom.ko and ufs-bsg.ko. > >> If do insmod ufs-qcom.ko > >> then rmmod ufs-qcom.ko and do insmod ufs-qcom.ko again, without > >> this change, because scsi > >> host was not release, the new scsi host will have a different > >> host number, meaning > >> hba->host->host_no will be 1, but not 0. Now if do insmod > >> ufs-bsg.ko now, the ufs-bsg dev > >> created in /dev/bsg/ will be ufs-bsg1, but not ufs-bsg0. If keep > >> trying these operations, > >> the ufs-bsg device's name will be messed up. This change make > >> sure after ufs- qcom.ko is removed > >> and installed back, its hba->host->host_no stays 0, so that > >> ufs-bsg device name stays same. > > Looks like we needed to manage the ufs-bsg nodes using an IDA, instead > > of host_no? > > > > > > Marking one ufs-bsg dev with host_no still has its advantage. If we have more > than one hba host, we can find the right ufs-bsgX dev by reading the sg/sd/bsg > device's host->host_unique_id (through SCSI_IOCTL_GET_IDLUN for example). > But If ufs-bsg has its own ID, we will lost this "mapping". OK.
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index b5966fa..a86b0fd 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -8251,6 +8251,7 @@ void ufshcd_remove(struct ufs_hba *hba) ufs_bsg_remove(hba); ufs_sysfs_remove_nodes(hba->dev); scsi_remove_host(hba->host); + scsi_host_put(hba->host); /* disable interrupts */ ufshcd_disable_intr(hba, hba->intr_mask); ufshcd_hba_stop(hba, true);
In ufshcd_remove(), after SCSI host is removed, put it once so that its resources can be released. Signed-off-by: Can Guo <cang@codeaurora.org> --- drivers/scsi/ufs/ufshcd.c | 1 + 1 file changed, 1 insertion(+)