RE: RE: [PATCH v22 3/4] scsi: ufs: Prepare HPB read for cached sub-region
> > +static int ufshpb_fill_ppn_from_page(struct ufshpb_lu *hpb,
> > +struct ufshpb_map_ctx *mctx, int pos,
> > +int len, u64 *ppn_buf)
> > +{
> > + struct page *page;
> > + int index, offset;
> > + int copied;
> > +
> > + index = pos / (PAGE_SIZE / HPB_ENTRY_SIZE);
> > + offset = pos % (PAGE_SIZE / HPB_ENTRY_SIZE);
> Maybe cache hpb->entries_per_page in ufshpb_lu_parameter_init as well?
They are just defined constants and complier will optimize them.
Thanks,
Daejun
> > +
> > + if ((offset + len) <= (PAGE_SIZE / HPB_ENTRY_SIZE))
> > + copied = len;
> > + else
> > + copied = (PAGE_SIZE / HPB_ENTRY_SIZE) - offset;
> > +
> > + page = mctx->m_page[index];
> > + if (unlikely(!page)) {
> > + dev_err(>sdev_ufs_lu->sdev_dev,
> > + "error. cannot find page in mctx\n");
> > + return -ENOMEM;
> > + }
> > +
> > + memcpy(ppn_buf, page_address(page) + (offset * HPB_ENTRY_SIZE),
> > + copied * HPB_ENTRY_SIZE);
> > +
> > + return copied;
> > +}
>
>
>
RE: RE: RE: [PATCH v22 3/4] scsi: ufs: Prepare HPB read for cached sub-region
> > > > > + err = ufshpb_fill_ppn_from_page(hpb, srgn->mctx, srgn_offset,
> > > > > 1,
> > > );
> > > > > + spin_unlock_irqrestore(>rgn_state_lock, flags);
> > > > > + if (unlikely(err < 0)) {
> > > > > + /*
> > > > > +* In this case, the region state is active,
> > > > > +* but the ppn table is not allocated.
> > > > > +* Make sure that ppn table must be allocated on
> > > > > +* active state.
> > > > > +*/
> > > > > + WARN_ON(true);
> > > > > + dev_err(hba->dev, "get ppn failed. err %d\n", err);
> > > > Maybe just pr_warn instead of risking crashing the machine over that?
> > >
> > > Why it crashing the machine? WARN_ON will just print kernel message.
> > I think that it can be configured, but I am not sure.
> I think it can be configured via the parameter panic_on_warn
OK, I will change WARN_ON to dev_err for print message.
Thanks,
Daejun
RE: RE: [PATCH v22 3/4] scsi: ufs: Prepare HPB read for cached sub-region
> > > > + err = ufshpb_fill_ppn_from_page(hpb, srgn->mctx, srgn_offset, 1,
> > );
> > > > + spin_unlock_irqrestore(>rgn_state_lock, flags);
> > > > + if (unlikely(err < 0)) {
> > > > + /*
> > > > +* In this case, the region state is active,
> > > > +* but the ppn table is not allocated.
> > > > +* Make sure that ppn table must be allocated on
> > > > +* active state.
> > > > +*/
> > > > + WARN_ON(true);
> > > > + dev_err(hba->dev, "get ppn failed. err %d\n", err);
> > > Maybe just pr_warn instead of risking crashing the machine over that?
> >
> > Why it crashing the machine? WARN_ON will just print kernel message.
> I think that it can be configured, but I am not sure.
I think it can be configured via the parameter panic_on_warn
RE: RE: [PATCH v22 3/4] scsi: ufs: Prepare HPB read for cached sub-region
>
>
> > > + err = ufshpb_fill_ppn_from_page(hpb, srgn->mctx, srgn_offset, 1,
> );
> > > + spin_unlock_irqrestore(>rgn_state_lock, flags);
> > > + if (unlikely(err < 0)) {
> > > + /*
> > > +* In this case, the region state is active,
> > > +* but the ppn table is not allocated.
> > > +* Make sure that ppn table must be allocated on
> > > +* active state.
> > > +*/
> > > + WARN_ON(true);
> > > + dev_err(hba->dev, "get ppn failed. err %d\n", err);
> > Maybe just pr_warn instead of risking crashing the machine over that?
>
> Why it crashing the machine? WARN_ON will just print kernel message.
I think that it can be configured, but I am not sure.
>
> Thanks,
> Daejun
RE: RE: [PATCH v22 3/4] scsi: ufs: Prepare HPB read for cached sub-region
> > + err = ufshpb_fill_ppn_from_page(hpb, srgn->mctx, srgn_offset, 1,
> > );
> > + spin_unlock_irqrestore(>rgn_state_lock, flags);
> > + if (unlikely(err < 0)) {
> > + /*
> > +* In this case, the region state is active,
> > +* but the ppn table is not allocated.
> > +* Make sure that ppn table must be allocated on
> > +* active state.
> > +*/
> > + WARN_ON(true);
> > + dev_err(hba->dev, "get ppn failed. err %d\n", err);
> Maybe just pr_warn instead of risking crashing the machine over that?
Why it crashing the machine? WARN_ON will just print kernel message.
Thanks,
Daejun
