| Message ID | 20210604050324.28670-4-nanich.lee@samsung.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v12,1/3] bio: control bio max size | expand |
On 6/3/21 10:03 PM, Changheun Lee wrote: > Set max_bio_bytes same with queue max sectors. It will lead to fast bio > submit when bio size is over than queue max sectors. And it might be helpful > to align submit bio size in some case. > > Signed-off-by: Changheun Lee <nanich.lee@samsung.com> > --- > drivers/scsi/ufs/ufshcd.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > index 3eb54937f1d8..37365a726517 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -4858,6 +4858,7 @@ static int ufshcd_slave_configure(struct scsi_device *sdev) > { > struct ufs_hba *hba = shost_priv(sdev->host); > struct request_queue *q = sdev->request_queue; > + unsigned int max_bio_bytes; > > blk_queue_update_dma_pad(q, PRDT_DATA_BYTE_COUNT_PAD - 1); > if (hba->quirks & UFSHCD_QUIRK_ALIGN_SG_WITH_PAGE_SIZE) > @@ -4868,6 +4869,10 @@ static int ufshcd_slave_configure(struct scsi_device *sdev) > > ufshcd_crypto_setup_rq_keyslot_manager(hba, q); > > + if (!check_shl_overflow(queue_max_sectors(q), > + SECTOR_SHIFT, &max_bio_bytes)) > + blk_queue_max_bio_bytes(q, max_bio_bytes); > + > return 0; > } Just like previous versions of this patch series, this approach will trigger data corruption if dm-crypt is stacked on top of the UFS driver since ufs_max_sectors << SECTOR_SHIFT = 1024 * 512 is less than the size of the dm-crypt buffer (BIO_MAX_VECS << PAGE_SHIFT = 256 * 4096). I am not recommending to increase max_sectors for the UFS driver. We need a solution that is independent of the dm-crypt internals. Bart.
> On 6/3/21 10:03 PM, Changheun Lee wrote: > > Set max_bio_bytes same with queue max sectors. It will lead to fast bio > > submit when bio size is over than queue max sectors. And it might be helpful > > to align submit bio size in some case. > > > > Signed-off-by: Changheun Lee <nanich.lee@samsung.com> > > --- > > drivers/scsi/ufs/ufshcd.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > > index 3eb54937f1d8..37365a726517 100644 > > --- a/drivers/scsi/ufs/ufshcd.c > > +++ b/drivers/scsi/ufs/ufshcd.c > > @@ -4858,6 +4858,7 @@ static int ufshcd_slave_configure(struct scsi_device *sdev) > > { > > struct ufs_hba *hba = shost_priv(sdev->host); > > struct request_queue *q = sdev->request_queue; > > + unsigned int max_bio_bytes; > > > > blk_queue_update_dma_pad(q, PRDT_DATA_BYTE_COUNT_PAD - 1); > > if (hba->quirks & UFSHCD_QUIRK_ALIGN_SG_WITH_PAGE_SIZE) > > @@ -4868,6 +4869,10 @@ static int ufshcd_slave_configure(struct scsi_device *sdev) > > > > ufshcd_crypto_setup_rq_keyslot_manager(hba, q); > > > > + if (!check_shl_overflow(queue_max_sectors(q), > > + SECTOR_SHIFT, &max_bio_bytes)) > > + blk_queue_max_bio_bytes(q, max_bio_bytes); > > + > > return 0; > > } > > Just like previous versions of this patch series, this approach will > trigger data corruption if dm-crypt is stacked on top of the UFS driver > since ufs_max_sectors << SECTOR_SHIFT = 1024 * 512 is less than the size > of the dm-crypt buffer (BIO_MAX_VECS << PAGE_SHIFT = 256 * 4096). max_bio_bytes can't be smaller than "BIO_MAX_VECS * PAGE_SIZE". Large value will be selected between input parameter and "BIO_MAX_VECS * PAGE_SIZE" in blk_queue_max_bio_bytes(). I think bio max size should be larger than "BIO_MAX_VECS * PAGE_SIZE" for kernel stability. void blk_queue_max_bio_bytes(struct request_queue *q, unsigned int bytes) { struct queue_limits *limits = &q->limits; unsigned int max_bio_bytes = round_up(bytes, PAGE_SIZE); limits->max_bio_bytes = max_t(unsigned int, max_bio_bytes, BIO_MAX_VECS * PAGE_SIZE); } EXPORT_SYMBOL(blk_queue_max_bio_bytes); > > I am not recommending to increase max_sectors for the UFS driver. We > need a solution that is independent of the dm-crypt internals. > > Bart. No need to increase max_sectors in driver to set max_bio_bytes. There are no dependency with dm-crypt I think. Thank you, Changheun Lee
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 3eb54937f1d8..37365a726517 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -4858,6 +4858,7 @@ static int ufshcd_slave_configure(struct scsi_device *sdev) { struct ufs_hba *hba = shost_priv(sdev->host); struct request_queue *q = sdev->request_queue; + unsigned int max_bio_bytes; blk_queue_update_dma_pad(q, PRDT_DATA_BYTE_COUNT_PAD - 1); if (hba->quirks & UFSHCD_QUIRK_ALIGN_SG_WITH_PAGE_SIZE) @@ -4868,6 +4869,10 @@ static int ufshcd_slave_configure(struct scsi_device *sdev) ufshcd_crypto_setup_rq_keyslot_manager(hba, q); + if (!check_shl_overflow(queue_max_sectors(q), + SECTOR_SHIFT, &max_bio_bytes)) + blk_queue_max_bio_bytes(q, max_bio_bytes); + return 0; }
Set max_bio_bytes same with queue max sectors. It will lead to fast bio submit when bio size is over than queue max sectors. And it might be helpful to align submit bio size in some case. Signed-off-by: Changheun Lee <nanich.lee@samsung.com> --- drivers/scsi/ufs/ufshcd.c | 5 +++++ 1 file changed, 5 insertions(+)