mbox series

[v2,0/5] UFS Host Performance Booster (HPB v1.0) driver

Message ID 20200416203126.1210-1-beanhuo@micron.com (mailing list archive)
Headers show
Series UFS Host Performance Booster (HPB v1.0) driver | expand

Message

Bean Huo April 16, 2020, 8:31 p.m. UTC
From: Bean Huo <beanhuo@micron.com>

Hi,
I disclose my development/changes on UFS HPB (UFS Host Performance Booster) to
the community, as this would enable more UFS developers to test and start an
iterative review and update process.

The HPB is defined in Jedec Standard Universal Flash (UFS) Host Performance
Booster (HPB) Extension Version 1.0, which is designed to improve  read
performance by utilizing the host side memory. Based on our testing, the HPB
can increase the random read performance by up to about 46% in the random read.

The original HPB driver is from [1]. Based on it, I did optimizations,
simplifications, fixed reliability issues, implemented HPB host control mode
and make it much more readable. which's pushed to here [2] as V1.

To avoid touching the traditional SCSI core, the HPB driver in this series HPB
patch chooses to develop under SCSI sub-system layer, and sits the same layer
with UFSHCD. At the same time, to minimize changes to UFSHCD driver, the HPB
driver submits its HPB READ BUFFER and HPB WRITE BUFFER requests to the scsi
device->request_queueu to execute, rather than that directly go through
raw UPIU request path.

v1--v2:
    1. Rebased the patch on the [3] branch 5.8/scsi-queue
    1. Optimized and simplified several functions
    2. Add parameter read_threshold in HPB sysfs, by which the user can change
       read_threshold for the HPB host control mode
    3. Add HPB memory limitation, let the user adjust its size according to the
       system memory capacity

[1] https://github.com/OpenMPDK/HPBDriver
[2] https://www.spinics.net/lists/kernel/msg3449471.html
[3] https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git

Bean Huo (5):
  scsi; ufs: add device descriptor for Host Performance Booster
  scsi: ufs: make ufshcd_read_unit_desc_param() non-static func
  scsi: ufs: add ufs_features parameter in structure ufs_dev_info
  scsi: ufs: add unit and geometry parameters for HPB
  scsi: ufs: UFS Host Performance Booster(HPB) driver

 drivers/scsi/ufs/Kconfig  |   62 +
 drivers/scsi/ufs/Makefile |    1 +
 drivers/scsi/ufs/ufs.h    |   15 +
 drivers/scsi/ufs/ufshcd.c |   66 +-
 drivers/scsi/ufs/ufshcd.h |   15 +-
 drivers/scsi/ufs/ufshpb.c | 3279 +++++++++++++++++++++++++++++++++++++
 drivers/scsi/ufs/ufshpb.h |  450 +++++
 7 files changed, 3881 insertions(+), 7 deletions(-)
 create mode 100644 drivers/scsi/ufs/ufshpb.c
 create mode 100644 drivers/scsi/ufs/ufshpb.h

Comments

Christoph Hellwig April 22, 2020, 6:43 a.m. UTC | #1
On Thu, Apr 16, 2020 at 10:31:21PM +0200, huobean@gmail.com wrote:
> From: Bean Huo <beanhuo@micron.com>
> 
> Hi,
> I disclose my development/changes on UFS HPB (UFS Host Performance Booster) to
> the community, as this would enable more UFS developers to test and start an
> iterative review and update process.
> 
> The HPB is defined in Jedec Standard Universal Flash (UFS) Host Performance
> Booster (HPB) Extension Version 1.0, which is designed to improve  read
> performance by utilizing the host side memory. Based on our testing, the HPB
> can increase the random read performance by up to about 46% in the random read.
> 
> The original HPB driver is from [1]. Based on it, I did optimizations,
> simplifications, fixed reliability issues, implemented HPB host control mode
> and make it much more readable. which's pushed to here [2] as V1.
> 
> To avoid touching the traditional SCSI core, the HPB driver in this series HPB
> patch chooses to develop under SCSI sub-system layer, and sits the same layer
> with UFSHCD. At the same time, to minimize changes to UFSHCD driver, the HPB
> driver submits its HPB READ BUFFER and HPB WRITE BUFFER requests to the scsi
> device->request_queueu to execute, rather than that directly go through
> raw UPIU request path.

This feature is completley broken, and rather dangerous due to feeding
"physical" addresses looked up by the host in.  I do not think we should
support something that broken in Linux.

Independent of that using two requests in the I/O path is not going
to fly either.  The whole thing seems like an exercise in benchmarketing.
Bean Huo April 22, 2020, 10:09 p.m. UTC | #2
Hi, Christoph
Thanks for your feedback. 

> > To avoid touching the traditional SCSI core, the HPB driver in this
> > series HPB patch chooses to develop under SCSI sub-system layer, and
> > sits the same layer with UFSHCD. At the same time, to minimize changes
> > to UFSHCD driver, the HPB driver submits its HPB READ BUFFER and HPB
> > WRITE BUFFER requests to the scsi
> > device->request_queueu to execute, rather than that directly go
> > device->through
> > raw UPIU request path.
> 
> This feature is completley broken, and rather dangerous due to feeding
> "physical" addresses looked up by the host in.  I do not think we should support
> something that broken in Linux.
>

It Is not plain physical address,  has been encrypted before loading from UFS to
HPB memory, I think we don't worry about its safety.

> Independent of that using two requests in the I/O path is not going to fly either.
> The whole thing seems like an exercise in benchmarketing.

I agree with you. This is my major concern. I have been thinking about HPB implementation in SCSI layer.
That will let SCSI layer manage HPB by calling UFS helper interface. 
If you don't consider UFS HPB is an idiot design,  I want to  change in another version.  Firstly, we really
want to hear your suggestion.
Thanks,

//Bean
Avri Altman April 23, 2020, 8:26 a.m. UTC | #3
> 
> Hi, Christoph
> Thanks for your feedback.
> 
> > > To avoid touching the traditional SCSI core, the HPB driver in this
> > > series HPB patch chooses to develop under SCSI sub-system layer, and
> > > sits the same layer with UFSHCD. At the same time, to minimize changes
> > > to UFSHCD driver, the HPB driver submits its HPB READ BUFFER and HPB
> > > WRITE BUFFER requests to the scsi
> > > device->request_queueu to execute, rather than that directly go
> > > device->through
> > > raw UPIU request path.
> >
> > This feature is completley broken, and rather dangerous due to feeding
> > "physical" addresses looked up by the host in.  I do not think we should
> support
> > something that broken in Linux.
> >
> 
> It Is not plain physical address,  has been encrypted before loading from UFS
> to
> HPB memory, I think we don't worry about its safety.
> 
> > Independent of that using two requests in the I/O path is not going to fly
> either.
> > The whole thing seems like an exercise in benchmarketing.
> 
> I agree with you. This is my major concern. I have been thinking about HPB
> implementation in SCSI layer.
> That will let SCSI layer manage HPB by calling UFS helper interface.
> If you don't consider UFS HPB is an idiot design,  I want to  change in another
> version.  Firstly, we really
> want to hear your suggestion.
> Thanks,
> 
> //Bean

If indeed this will move forward, please publish your patches as a RFC,
To allow competing approaches to be published as well.

Thanks,
Avri