Re: RE: please revert the UFS HPB support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 25/10/2021 08:16, Daejun Park wrote:
>> On Thu, 2021-10-21 at 09:22 -0700, Bart Van Assche wrote:
>>> On 10/21/21 8:17 AM, Christoph Hellwig wrote:
>>>> On Thu, Oct 21, 2021 at 05:15:20PM +0200, Christoph Hellwig wrote:
>>>>>>> I just noticed the UFS HPB support landed in 5.15, and just
>>>>>>> as before it is completely broken by allocating another
>>>>>>> request on the same device and then reinserting it in the
>>>>>>> queue.  It is bad enough that we have to live with
>>>>>>> blk_insert_cloned_request for dm-mpath, but this is too big
>>>>>>> of an API abuse to make it into a release.  We need to drop
>>>>>>> this code ASAP, and I can prepare a patch for that.
>>>>>>
>>>>>> That sounds awful, do you have a link to the offending
>>>>>> commit(s)?
>>>>>
>>>>> I'll need to look for it, busy in calls right now, but just grep
>>>>> for blk_insert_cloned_request.
>>>>
>>>> Might as well finish the git blame:
>>>>
>>>> commit 41d8a9333cc96f5ad4dd7a52786585338257d9f1
>>>> Author: Daejun Park <daejun7.park@xxxxxxxxxxx>
>>>> Date:   Mon Jul 12 18:00:25 2021 +0900
>>>>
>>>>      scsi: ufs: ufshpb: Add HPB 2.0 support
>>>>          
>>>>      Version 2.0 of HBP supports reads of varying sizes from 4KB to
>>>> 1MB.
>>>>
>>>>      A read operation <= 32KB is supported as single HPB read. A
>>>> read between
>>>>      36KB and 1MB is supported by a combination of write buffer
>>>> command and HPB
>>>>      read command to deliver more PPN. The write buffer commands
>>>> may not be
>>>>      issued immediately due to busy tags. To use HPB read more
>>>> aggressively, the
>>>>      driver can requeue the write buffer command. The requeue
>>>> threshold is
>>>>      implemented as timeout and can be modified with
>>>> requeue_timeout_ms entry in
>>>>      sysfs.
>>>
>>> (+Daejun)
>>>
>>> Daejun, can the HPB code be reworked such that it does not use 
>>> blk_insert_cloned_request()? I'm concerned that if the HPB code is
>>> not reworked that it will be removed from the upstream kernel.
>>  
>> Just to give urgency to Bart's request: we have two or three weeks
>> before the kernel is due to go final.  Can the problems identified by
>> Christoph be fixed within that timeframe?
> 
> I'm checking to see if I can replace blk_execute_rq_nowait with
> blk_insert_cloned_request in the HPB code.

In case it is of any use, there is an example of sending a REQUEST_SENSE
command directly here:

	https://lore.kernel.org/all/20210930124224.114031-2-adrian.hunter@xxxxxxxxx

> 
>>  
>> Specifically, looking at the paper you reference, it only uses READ
>> BUFFER for the host cache sharing.  Since the JDEC standard appears to
>> be proprietary, I have no method of understanding why the driver now
>> uses WRITE BUFFER as well, but it appears to be a simple optimization. 
>> If you can cut out the WRITE BUFFER code, blk_insert_cloned_request()
>> will also be gone and thus the API abuse.  Can you get us a simple
>> patch doing this ASAP so we don't have to revert the driver?
> 
> If WRITE BUFFER is not used, only READs with a size of 32KB or less can be
> changed to HPB READs. This becomes a limiting factor in how READ
> performance can be improved by the HPB.
> 
> Thanks,
> Daejun
> 




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux