Re: [RFC PATCH 4/5] scsi: ufs: L2P map management for HPB read

2020-06-13 Thread Bart Van Assche
On 2020-06-11 20:37, Daejun Park wrote: >>> +static int ufshpb_execute_map_req(struct ufshpb_lu *hpb, >>> + struct ufshpb_req *map_req) >>> +{ >>> + struct request_queue *q; >>> + struct request *req; >>> + struct scsi_request *rq; >>> + int ret = 0; >>> + >>> +

Re: [RFC PATCH 4/5] scsi: ufs: L2P map management for HPB read

2020-06-11 Thread Daejun Park
> > +static struct ufshpb_req *ufshpb_get_map_req(struct ufshpb_lu *hpb, > > +struct ufshpb_subregion *srgn) > > +{ > > + struct ufshpb_req *map_req; > > + struct request *req; > > + struct bio *bio; > > + > > + map_req =

Re: [RFC PATCH 4/5] scsi: ufs: L2P map management for HPB read

2020-06-10 Thread Bart Van Assche
On 2020-06-04 18:56, Daejun Park wrote: > +static struct ufshpb_req *ufshpb_get_map_req(struct ufshpb_lu *hpb, > + struct ufshpb_subregion *srgn) > +{ > + struct ufshpb_req *map_req; > + struct request *req; > + struct bio *bio; > + > +

RE: [RFC PATCH 4/5] scsi: ufs: L2P map management for HPB read

2020-06-09 Thread Daejun Park
> The spec does not define what the host should do in this case, > e.g. when the device informs it that the entire db is no longer valid. > What are you planning to do? In Jedec spec, there is no decription about what the driver should do. So, I will just inform to user about the "HPB reset"

RE: RE: [RFC PATCH 4/5] scsi: ufs: L2P map management for HPB read

2020-06-09 Thread Daejun Park
> This is not a concern, just a question. > If a map request started while runtime/system suspend, can you share its flow? When suspended, the worker is cancled. And it can just process pending active/inactive list after resume. Thanks, Daejun

RE: [RFC PATCH 4/5] scsi: ufs: L2P map management for HPB read

2020-06-09 Thread Avri Altman
> > > dev_info(>hpb_lu_dev, "ufshpb resume"); > > > ufshpb_set_state(hpb, HPB_PRESENT); > > > + if (!ufshpb_is_empty_rsp_lists(hpb)) > > > + queue_work(ufshpb_drv.ufshpb_wq, >map_work); > > Ahha - so you are using the ufs driver

RE: [RFC PATCH 4/5] scsi: ufs: L2P map management for HPB read

2020-06-09 Thread Avri Altman
> > > + switch (rsp_field->hpb_type) { > > > + case HPB_RSP_REQ_REGION_UPDATE: > > > + WARN_ON(data_seg_len != DEV_DATA_SEG_LEN); > > > + ufshpb_rsp_req_region_update(hpb, rsp_field); > > > + break; > > What about hpb dev reset - oper 0x2? >

Re: [RFC PATCH 4/5] scsi: ufs: L2P map management for HPB read

2020-06-08 Thread Bart Van Assche
On 2020-06-06 11:26, Avri Altman wrote: >> + data_seg_len = be32_to_cpu(lrbp->ucd_rsp_ptr->header.dword_2) >> + & MASK_RSP_UPIU_DATA_SEG_LEN; > get_unaligned instead of be32_to_cpu ? Since sparse checks that the argument of be32_to_cpu() has type __be32 and since no such check

RE: [RFC PATCH 4/5] scsi: ufs: L2P map management for HPB read

2020-06-08 Thread Daejun Park
> > The data structure for map data request and L2P map uses mempool API, > > minimizing allocation overhead while avoiding static allocation. > Maybe one or two more sentences to explain the L2P framework: > Each hpb lun maintains 2 "to-do" lists: > - hpb->lh_inact_rgn - regions to be

RE: [RFC PATCH 4/5] scsi: ufs: L2P map management for HPB read

2020-06-06 Thread Avri Altman
> > A pinned region is a pre-set regions on the UFS device that is always > activate-state and This sentence got cut off > > The data structure for map data request and L2P map uses mempool API, > minimizing allocation overhead while avoiding static allocation. Maybe one or two more sentences