RE: [RESENT PATCH RFC v3 5/5] scsi: ufs: UFS Host Performance Booster(HPB) driver

[Alim Akhtar]

+ seunguk.shin (who is one of the original author of the HPB, in case he has some more improvement inputs)

Hi Bean,
I second Avri input on splitting this patch series into logical smaller patches, that will helps reviewers.
Also if you can add support to build HPB as kernel module that will be useful. 
I am looking into the HPB 1.0 spec, will review your patches soon.

Regards,
Alim

> -----Original Message-----
> From: Avri Altman <Avri.Altman@xxxxxxx>
> Sent: 08 May 2020 17:09
> To: huobean@xxxxxxxxx; alim.akhtar@xxxxxxxxxxx;
> asutoshd@xxxxxxxxxxxxxx; jejb@xxxxxxxxxxxxx; martin.petersen@xxxxxxxxxx;
> stanley.chu@xxxxxxxxxxxx; beanhuo@xxxxxxxxxx; bvanassche@xxxxxxx;
> tomas.winkler@xxxxxxxxx; cang@xxxxxxxxxxxxxx; rdunlap@xxxxxxxxxxxxx
> Cc: linux-scsi@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> hch@xxxxxxxxxxxxx
> Subject: RE: [RESENT PATCH RFC v3 5/5] scsi: ufs: UFS Host Performance
> Booster(HPB) driver
> 
> Hi Bean,
> This patch is ~3,000 lines long.
> Is it possible to divide it into a series of a smaller, more reviewable patches?
> E.g. it can be something like:
> 1) Init part I - Read HPB config
> 2) Init part II - prepare for L2P cache management (introduce here all the new
> data structures, memory allocation, etc.)
> 3) L2P cache management - activation / inactivation / eviction handlers
> 4) Add response API
> 5) Add prep_fn handler - the flows that contruct HPB-READ command.
> Or any other division that makes sense to you.
> 
> Also, Is there a way to avoid all those #ifdefs everywhere?
> 
> Thanks,
> Avri
> 
> 
> >
> > From: Bean Huo <beanhuo@xxxxxxxxxx>
> >
> > This patch is to add support for the UFS Host Performance Booster (HPB
> > v1.0), which is used to improve UFS read performance, especially for
> > the random read.
> >
> > NAND flash-based storage devices, including UFS, have mechanisms to
> > translate logical addresses of requests to the corresponding physical
> > addresses of the flash storage. Traditionally this L2P mapping data is
> > loaded to the internal SRAM in the storage controller. When the
> > capacity of storage is larger, a larger size of SRAM for the L2P map
> > data is required. Since increased SRAM size affects the manufacturing
> > cost significantly, it is not cost-effective to allocate all the
> > amount of SRAM needed to keep all the Logical-address to
> > Physical-address (L2P) map data. Therefore, L2P map data, which is
> > required to identify the physical address for the requested IOs, can
> > only be partially stored in SRAM from NAND flash. Due to this partial
> > loading, accessing the flash address area where the L2P information
> > for that address is not loaded in the SRAM can result in serious performance
> degradation.
> >
> > The HPB is a software solution for the above problem, which uses the
> > hostâs system memory as a cache for the FTL L2P mapping table. It does
> > not need additional hardware support from the host side. By using HPB,
> > the L2P mapping table can be read from host memory and stored in
> > host-side memory. while reading the operation, the corresponding L2P
> > information will be sent to the UFS device along with the reading
> > request. Since the L2P entry is provided in the read request, UFS
> > device does not have to load L2P entry from flash memory.
> > This will significantly improve random read performance.
> >
> > Signed-off-by: Bean Huo <beanhuo@xxxxxxxxxx>
> > ---
> >  drivers/scsi/ufs/Kconfig  |   62 +
> >  drivers/scsi/ufs/Makefile |    1 +
> >  drivers/scsi/ufs/ufshcd.c |  234 +++-
> >  drivers/scsi/ufs/ufshcd.h |   23 +
> >  drivers/scsi/ufs/ufshpb.c | 2767
> > +++++++++++++++++++++++++++++++++++++
> >  drivers/scsi/ufs/ufshpb.h |  423 ++++++
> >  6 files changed, 3504 insertions(+), 6 deletions(-)  create mode
> > 100644 drivers/scsi/ufs/ufshpb.c  create mode 100644
> > drivers/scsi/ufs/ufshpb.h
> >