| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <8b4e8dc1fd91f52a191d681b7096ef48@codeaurora.org>
Date: Mon, 15 Mar 2021 14:40:56 +0800
From: Can Guo <cang@...eaurora.org>
To: Avri Altman <avri.altman@....com>
Cc: "James E . J . Bottomley" <jejb@...ux.vnet.ibm.com>,
"Martin K . Petersen" <martin.petersen@...cle.com>,
linux-scsi@...r.kernel.org, linux-kernel@...r.kernel.org,
gregkh@...uxfoundation.org, Bart Van Assche <bvanassche@....org>,
yongmyung lee <ymhungry.lee@...sung.com>,
Daejun Park <daejun7.park@...sung.com>,
alim.akhtar@...sung.com, asutoshd@...eaurora.org,
Zang Leigang <zangleigang@...ilicon.com>,
Avi Shchislowski <avi.shchislowski@....com>,
Bean Huo <beanhuo@...ron.com>, stanley.chu@...iatek.com
Subject: Re: [PATCH v5 06/10] scsi: ufshpb: Add hpb dev reset response
On 2021-03-15 09:34, Can Guo wrote:
> On 2021-03-02 21:24, Avri Altman wrote:
>> The spec does not define what is the host's recommended response when
>> the device send hpb dev reset response (oper 0x2).
>>
>> We will update all active hpb regions: mark them and do that on the
>> next
>> read.
>>
>> Signed-off-by: Avri Altman <avri.altman@....com>
>> ---
>> drivers/scsi/ufs/ufshpb.c | 47
>> ++++++++++++++++++++++++++++++++++++---
>> drivers/scsi/ufs/ufshpb.h | 2 ++
>> 2 files changed, 46 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/scsi/ufs/ufshpb.c b/drivers/scsi/ufs/ufshpb.c
>> index 0744feb4d484..0034fa03fdc6 100644
>> --- a/drivers/scsi/ufs/ufshpb.c
>> +++ b/drivers/scsi/ufs/ufshpb.c
>> @@ -642,7 +642,8 @@ int ufshpb_prep(struct ufs_hba *hba, struct
>> ufshcd_lrb *lrbp)
>> if (rgn->reads == ACTIVATION_THRESHOLD)
>> activate = true;
>> spin_unlock_irqrestore(&rgn->rgn_lock, flags);
>> - if (activate) {
>> + if (activate ||
>> + test_and_clear_bit(RGN_FLAG_UPDATE, &rgn->rgn_flags)) {
>> spin_lock_irqsave(&hpb->rsp_list_lock, flags);
>> ufshpb_update_active_info(hpb, rgn_idx, srgn_idx);
>> hpb->stats.rb_active_cnt++;
>> @@ -1480,6 +1481,20 @@ void ufshpb_rsp_upiu(struct ufs_hba *hba,
>> struct ufshcd_lrb *lrbp)
>> case HPB_RSP_DEV_RESET:
>> dev_warn(&hpb->sdev_ufs_lu->sdev_dev,
>> "UFS device lost HPB information during PM.\n");
>> +
>> + if (hpb->is_hcm) {
>> + struct scsi_device *sdev;
> bool need_reset = false;
>> +
>> + __shost_for_each_device(sdev, hba->host) {
>> + struct ufshpb_lu *h = sdev->hostdata;
>> +
>> + if (!h)
>> + continue;
>> +
>> + need_reset = true;
>> + }
>
> if (need_reset)
> schedule_work(&hpb->ufshpb_lun_reset_work);
>
> At last, scheduling only one reset work shall be enough, otherwise
> multiple
> reset work can be flying in parallel, so maybe above changes?
Forget about this one, I misunderstood it - reset work is for each
ufshpb_lu...
Regards,
Can Guo.
>
>> + }
>> +
>> break;
>> default:
>> dev_notice(&hpb->sdev_ufs_lu->sdev_dev,
>> @@ -1594,6 +1609,25 @@ static void
>> ufshpb_run_inactive_region_list(struct ufshpb_lu *hpb)
>> spin_unlock_irqrestore(&hpb->rsp_list_lock, flags);
>> }
>>
>> +static void ufshpb_reset_work_handler(struct work_struct *work)
>> +{
>> + struct ufshpb_lu *hpb;
>
> struct ufshpb_lu *hpb = container_of(work, struct ufshpb_lu,
> ufshpb_lun_reset_work);
>
>> + struct victim_select_info *lru_info;
>
> struct victim_select_info *lru_info = &hpb->lru_info;
>
> This can save some lines.
>
> Thanks,
> Can Guo.
>
>> + struct ufshpb_region *rgn;
>> + unsigned long flags;
>> +
>> + spin_lock_irqsave(&hpb->rgn_state_lock, flags);
>> +
>> + list_for_each_entry(rgn, &lru_info->lh_lru_rgn, list_lru_rgn)
>> + set_bit(RGN_FLAG_UPDATE, &rgn->rgn_flags);
>> +
>> + spin_unlock_irqrestore(&hpb->rgn_state_lock, flags);
>> +}
>> +
>> static void ufshpb_normalization_work_handler(struct work_struct
>> *work)
>> {
>> struct ufshpb_lu *hpb;
>> @@ -1798,6 +1832,8 @@ static int ufshpb_alloc_region_tbl(struct
>> ufs_hba *hba, struct ufshpb_lu *hpb)
>> } else {
>> rgn->rgn_state = HPB_RGN_INACTIVE;
>> }
>> +
>> + rgn->rgn_flags = 0;
>> }
>>
>> return 0;
>> @@ -2012,9 +2048,12 @@ static int ufshpb_lu_hpb_init(struct ufs_hba
>> *hba, struct ufshpb_lu *hpb)
>> INIT_LIST_HEAD(&hpb->list_hpb_lu);
>>
>> INIT_WORK(&hpb->map_work, ufshpb_map_work_handler);
>> - if (hpb->is_hcm)
>> + if (hpb->is_hcm) {
>> INIT_WORK(&hpb->ufshpb_normalization_work,
>> ufshpb_normalization_work_handler);
>> + INIT_WORK(&hpb->ufshpb_lun_reset_work,
>> + ufshpb_reset_work_handler);
>> + }
>>
>> hpb->map_req_cache = kmem_cache_create("ufshpb_req_cache",
>> sizeof(struct ufshpb_req), 0, 0, NULL);
>> @@ -2114,8 +2153,10 @@ static void ufshpb_discard_rsp_lists(struct
>> ufshpb_lu *hpb)
>>
>> static void ufshpb_cancel_jobs(struct ufshpb_lu *hpb)
>> {
>> - if (hpb->is_hcm)
>> + if (hpb->is_hcm) {
>> + cancel_work_sync(&hpb->ufshpb_lun_reset_work);
>> cancel_work_sync(&hpb->ufshpb_normalization_work);
>> + }
>> cancel_work_sync(&hpb->map_work);
>> }
>>
>> diff --git a/drivers/scsi/ufs/ufshpb.h b/drivers/scsi/ufs/ufshpb.h
>> index 84598a317897..37c1b0ea0c0a 100644
>> --- a/drivers/scsi/ufs/ufshpb.h
>> +++ b/drivers/scsi/ufs/ufshpb.h
>> @@ -121,6 +121,7 @@ struct ufshpb_region {
>> struct list_head list_lru_rgn;
>> unsigned long rgn_flags;
>> #define RGN_FLAG_DIRTY 0
>> +#define RGN_FLAG_UPDATE 1
>>
>> /* region reads - for host mode */
>> spinlock_t rgn_lock;
>> @@ -217,6 +218,7 @@ struct ufshpb_lu {
>> /* for selecting victim */
>> struct victim_select_info lru_info;
>> struct work_struct ufshpb_normalization_work;
>> + struct work_struct ufshpb_lun_reset_work;
>>
>> /* pinned region information */
>> u32 lu_pinned_start;
Powered by blists - more mailing lists