Re: [PATCH] scsi: ufs: mark HPB support as BROKEN

James Bottomley
2021-10-26T18:18:19Z

On Tue, 2021-10-26 at 12:10 -0600, Jens Axboe wrote:
> On 10/26/21 12:05 PM, Bart Van Assche wrote:
[...]
> > Hi Jens and James,
> >
> > This is what I found in the HPB 2.0 specification (the spec is
> > copyrighted but I assume that I have the right to quote small parts
> > of that spec):
> >
> > <quote>
> > 6.3 HPB WRITE BUFFER Command
> >
> > HPB WRITE BUFFER command have following 3 different function
> > depending
> > on the value of BUFFER_ID field.
> > 1) Inactivating an HPB Region (supported in host control mode only)
> > 2) prefetching HPB Entries from the host to the device (supported
> > in any control mode)
> > 3) Inactivating all HPB Regions, except for Provisioning Pinned
> > Region
> > in the host (supported in device control mode only)
> > </quote>
> >
> > Reverting only the problematic code (HPB 2.0) sounds reasonable to
> > me because reworking the HPB 2.0 code will be nontrivial.
>
> Then let's please go ahead and do that. I'm assuming this is a
> smaller set than what Christoph originally posted, who's taking on
> the job of lining it up?

I was hoping the HPB guys would do this. I think I know how to do it
based on my investigations, but I'd really prefer that someone who
cares about HPB did it. It looks like a fairly simple excision, so I
think we can have this done before the end of the week and if it isn't
we could revert the whole driver.

> > Using BLK_MQ_F_BLOCKING might be a viable approach. However, I
> > don't want to see that flag enabled in the UFS driver if HPB is not
> > used because of the negative performance effects of that flag.
>
> Agree, and if we do just the problematic revert, then the decision on
> whether BLK_MQ_F_BLOCKING is useful or not belongs to whoever reworks
> the WRITE BUFFER code and reposts that support.

Agreed, that was my initial proposed solution: get rid of the write
buffer optimzation now to fix the API abuse and see if we can add it
back in a more acceptable form later.

James