The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Bart Van Assche <bvanassche@acm.org>
To: Avri Altman <Avri.Altman@wdc.com>,
	"huobean@gmail.com" <huobean@gmail.com>,
	"alim.akhtar@samsung.com" <alim.akhtar@samsung.com>,
	"asutoshd@codeaurora.org" <asutoshd@codeaurora.org>,
	"jejb@linux.ibm.com" <jejb@linux.ibm.com>,
	"martin.petersen@oracle.com" <martin.petersen@oracle.com>,
	"stanley.chu@mediatek.com" <stanley.chu@mediatek.com>,
	"beanhuo@micron.com" <beanhuo@micron.com>,
	"tomas.winkler@intel.com" <tomas.winkler@intel.com>,
	"cang@codeaurora.org" <cang@codeaurora.org>
Cc: "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 5/5] scsi: ufs: UFS Host Performance Booster(HPB) driver
Date: Mon, 27 Apr 2020 20:36:43 -0700	[thread overview]
Message-ID: <b2584ba8-3542-1aae-5802-e59d218e1553@acm.org> (raw)
In-Reply-To: <SN6PR04MB4640A33BBE0CD58107D7FC69FCAF0@SN6PR04MB4640.namprd04.prod.outlook.com>

On 2020-04-26 23:13, Avri Altman wrote:
>> On 2020-04-25 01:59, Avri Altman wrote:
> HPB support is comprised of 4 main duties:
> 1) Read the device HPB configuration
> 2) Attend the device's recommendations that are embedded in the sense buffer
> 3) L2P cache management - This entails sending 2 new scsi commands (opcodes were taken from the vendor pool):
> 	a. HPB-READ-BUFFER - read L2P physical addresses for a subregion
> 	b. HPB-WRITE-BUFFER - notify the device that a region is inactive (in host-managed mode)
> 4) Use HPB-READ: a 3rd new scsi command (again - uses the vendor pool) to perform read operation instead of READ10.  HPB-READ carries both the logical and the physical addresses.
> 
> I will let Bean defend the Samsung approach of using a single LLD to attend all 4 duties.
> 
> Another approach might be to split those duties between 2 modules:
>  - A LLD that will perform the first 2 - those can be done only using ufs privet stuff, and 
>  - another module in scsi mid-layer that will be responsible of L2P cache management,
>   and HPB-READ command setup.
> A framework to host the scsi mid-layer module can be the scsi device handler.
> 
> The scsi-device-handler infrastructure was added to the kernel mainly to facilitate multiple paths for storage devices.
> The HPB framework, although far from the original intention of the authors, might as well fit in.
> In that sense, using READs and HPB_READs intermittently, can be perceived as a multi-path.
> 
> Scsi device handlers are also attached to a specific scsi_device (lun).
> This can serve as the glue linking between the ufs LLD and the device handler which resides in the scsi level.
> 
> Device handlers comes with a rich and handy set of APIs & ops, which we can use to support HPB.
> Specifically we can use it to attach & activate the device handler,
> only after the ufs driver verified that HPB is supported by both the platform and the device. 
> 
> The 2 modules can communicate using the handler_data opaque pointer,
> and the handler’s set_params op-mode: which is an open protocol essentially,
> and we can use it to pass the sense buffer in its full or just a parsed version.
> 
> Being a scsi mid-layer module, it will not break anything while sending
> HPB-READ-BUFFER and HPB-WRITE-BUFFER as part of the L2P cache management duties.
> 
> Last but not least, the device handler is already hooked in the scsi command setup flow - scsi_setup_fs_cmnd(),
> So we get the hook into HPB-READ prep_fn for free.   
>  
> Later on, we might want to export the L2P cache management logic to user-space.
> Locating the L2P cache management in scsi mid-layer will enable us to do so, using the scsi-netlink or some other means.

Hi Avri,

I'm not sure that I agree that HPB can be perceived as multi-path.
Anyway, the above approach sounds interesting to me. A few questions though:
- The only in-tree caller of scsi_dh_attach() I am aware of exists in
  the dm-mpath driver. I think that call is triggered by multipathd.
  I don't think that it is acceptable to require that multipathd is
  running to use the UFS HPB functionality. What is the plan for
  attaching the UFS device handler to UFS devices?
- Will preparing a SCSI command involve executing a SCSI command? If so,
  how will it be prevented that execution of that internally submitted
  SCSI command triggers a deadlock due to tag exhaustion?

Thanks,

Bart.

  reply	other threads:[~2020-04-28  3:36 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-16 20:31 [PATCH v2 0/5] UFS Host Performance Booster (HPB v1.0) driver huobean
2020-04-16 20:31 ` [PATCH v2 1/5] scsi; ufs: add device descriptor for Host Performance Booster huobean
2020-04-22 23:11   ` Bart Van Assche
2020-04-23 11:01     ` [EXT] " Bean Huo (beanhuo)
2020-04-23 17:57       ` Bart Van Assche
2020-04-16 20:31 ` [PATCH v2 2/5] scsi: ufs: make ufshcd_read_unit_desc_param() non-static func huobean
2020-04-16 20:31 ` [PATCH v2 3/5] scsi: ufs: add ufs_features parameter in structure ufs_dev_info huobean
2020-04-22 23:13   ` Bart Van Assche
2020-04-16 20:31 ` [PATCH v2 4/5] scsi: ufs: add unit and geometry parameters for HPB huobean
2020-04-16 20:31 ` [PATCH v2 5/5] scsi: ufs: UFS Host Performance Booster(HPB) driver huobean
2020-04-16 21:35   ` Randy Dunlap
2020-04-23  0:00   ` Bart Van Assche
2020-04-24  9:51     ` [EXT] " Bean Huo (beanhuo)
2020-04-24 18:17     ` Avri Altman
2020-04-24 20:02       ` [EXT] " Bean Huo (beanhuo)
2020-04-25  8:59       ` Avri Altman
2020-04-25 17:51         ` Bart Van Assche
2020-04-27  6:13           ` Avri Altman
2020-04-28  3:36             ` Bart Van Assche [this message]
2020-04-28  8:14               ` Avri Altman
2020-04-28 11:59                 ` Bean Huo
2020-04-30  7:23                   ` Avri Altman
2020-04-30 12:45                     ` Bean Huo
2020-05-02 16:19                       ` Avri Altman
2020-04-28  9:12               ` Bean Huo
2020-04-25 18:07   ` Bart Van Assche
2020-04-26 22:03     ` Bean Huo
2020-04-22  6:43 ` [PATCH v2 0/5] UFS Host Performance Booster (HPB v1.0) driver Christoph Hellwig
2020-04-22 22:09   ` [EXT] " Bean Huo (beanhuo)
2020-04-23  8:26     ` Avri Altman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b2584ba8-3542-1aae-5802-e59d218e1553@acm.org \
    --to=bvanassche@acm.org \
    --cc=Avri.Altman@wdc.com \
    --cc=alim.akhtar@samsung.com \
    --cc=asutoshd@codeaurora.org \
    --cc=beanhuo@micron.com \
    --cc=cang@codeaurora.org \
    --cc=huobean@gmail.com \
    --cc=jejb@linux.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=stanley.chu@mediatek.com \
    --cc=tomas.winkler@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox