Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Date:   Wed, 11 Dec 2019 11:44:08 +0000
From:   cang@codeaurora.org
To:     Avri Altman <Avri.Altman@wdc.com>
Cc:     asutoshd@codeaurora.org, nguyenb@codeaurora.org,
        rnayak@codeaurora.org, linux-scsi@vger.kernel.org,
        kernel-team@android.com, saravanak@google.com, salyzyn@google.com,
        Alim Akhtar <alim.akhtar@samsung.com>,
        Pedro Sousa <pedrom.sousa@synopsys.com>,
        "James E.J. Bottomley" <jejb@linux.ibm.com>,
        "Martin K. Petersen" <martin.petersen@oracle.com>,
        Stanley Chu <stanley.chu@mediatek.com>,
        Bean Huo <beanhuo@micron.com>,
        Venkat Gopalakrishnan <venkatg@codeaurora.org>,
        Tomas Winkler <tomas.winkler@intel.com>,
        open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 1/3] scsi: ufs: Put SCSI host after remove it
In-Reply-To: <MN2PR04MB6991754758E2840B6D529112FC5A0@MN2PR04MB6991.namprd04.prod.outlook.com>
References: <1576054123-16417-1-git-send-email-cang@codeaurora.org>
 <0101016ef4259a7c-76bdf010-88b1-4309-ba24-8c874734184e-000000@us-west-2.amazonses.com>
 <MN2PR04MB699184D76E7F9BABA923043AFC5A0@MN2PR04MB6991.namprd04.prod.outlook.com>
 <0101016ef4a3e5f5-915372c8-5e1e-4db5-b3da-f97f7ca963e4-000000@us-west-2.amazonses.com>
 <MN2PR04MB6991754758E2840B6D529112FC5A0@MN2PR04MB6991.namprd04.prod.outlook.com>
Message-ID: <0101016ef4c605e5-295b1cc0-19d7-41e8-be2e-5d026f72dcec-000000@us-west-2.amazonses.com>
User-Agent: Roundcube Webmail/1.3.9
Feedback-ID: 1.us-west-2.CZuq2qbDmUIuT3qdvXlRHZZCpfZqZ4GtG9v3VKgRyF0=:AmazonSES
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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