public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/1] Fix unaligned queue_limit->io_opt on SAS and 4KN disk
@ 2025-11-14 10:28 Lukas Herbolt
  2025-11-14 10:28 ` [PATCH 1/1] scsi: sd: Rounddown host->opt_sectors to logical Lukas Herbolt
  0 siblings, 1 reply; 6+ messages in thread
From: Lukas Herbolt @ 2025-11-14 10:28 UTC (permalink / raw)
  To: James.Bottomley, martin.petersen; +Cc: linux-scsi, Lukas Herbolt

If there is an SAS attached disk we set the SCSI host opt_sectors in 
sas_host_setup() to either dma_opt_mapping_size() or host->max_sectors 
this value can be later assigned to the device queue_limit as io_opt, but 
there is no further check if the io_opt is aligned with the device limits.

Rounding down to device logical_sectors will keep the io_opt large enough 
to keep benefit of bigger IO but still aligned within the device limits. 

Lukas Herbolt (1):
  scsi: sd: Rounddown host->opt_sectors to logical sectors reported by
    the disk.

 drivers/scsi/sd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

-- 
2.51.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/1] scsi: sd: Rounddown host->opt_sectors to logical
  2025-11-14 10:28 [PATCH 0/1] Fix unaligned queue_limit->io_opt on SAS and 4KN disk Lukas Herbolt
@ 2025-11-14 10:28 ` Lukas Herbolt
  2025-11-20  3:26   ` Martin K. Petersen
  0 siblings, 1 reply; 6+ messages in thread
From: Lukas Herbolt @ 2025-11-14 10:28 UTC (permalink / raw)
  To: James.Bottomley, martin.petersen; +Cc: linux-scsi, Lukas Herbolt

The host->opt_sectors are by default 512 bytes aligned if we have 4KN SAS
disk reporting zero or smaller opt_xfer_block than the host->opt_sectors
we will end up with unaligned queue_limit->io_opt.

Signed-off-by: Lukas Herbolt <lukas@herbolt.com>
---
 drivers/scsi/sd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 5b8668accf8e8..382474c3555c3 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3787,7 +3787,8 @@ static int sd_revalidate_disk(struct gendisk *disk)
 	 * an impact on performance for when the size of a request exceeds this
 	 * host limit.
 	 */
-	lim.io_opt = sdp->host->opt_sectors << SECTOR_SHIFT;
+	lim.io_opt = rounddown(lim.logical_block_size,
+			sdp->host->opt_sectors << SECTOR_SHIFT);
 	if (sd_validate_opt_xfer_size(sdkp, dev_max)) {
 		lim.io_opt = min_not_zero(lim.io_opt,
 				logical_to_bytes(sdp, sdkp->opt_xfer_blocks));
-- 
2.51.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] scsi: sd: Rounddown host->opt_sectors to logical
  2025-11-14 10:28 ` [PATCH 1/1] scsi: sd: Rounddown host->opt_sectors to logical Lukas Herbolt
@ 2025-11-20  3:26   ` Martin K. Petersen
  2025-11-20 12:07     ` lukas
  2025-11-20 16:07     ` John Garry
  0 siblings, 2 replies; 6+ messages in thread
From: Martin K. Petersen @ 2025-11-20  3:26 UTC (permalink / raw)
  To: Lukas Herbolt; +Cc: James.Bottomley, martin.petersen, linux-scsi


Lukas,

> The host->opt_sectors are by default 512 bytes aligned if we have 4KN
> SAS disk reporting zero or smaller opt_xfer_block than the
> host->opt_sectors we will end up with unaligned queue_limit->io_opt.

I am intrigued wrt. which controller returns a host->opt_sectors that
would trigger this misalignment. What is the actual value reported?

> +	lim.io_opt = rounddown(lim.logical_block_size,
> +			sdp->host->opt_sectors << SECTOR_SHIFT);

Those parameters would need to be reversed, I think. You want to round
down opt_sectors and not the logical_block_size.

-- 
Martin K. Petersen

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] scsi: sd: Rounddown host->opt_sectors to logical
  2025-11-20  3:26   ` Martin K. Petersen
@ 2025-11-20 12:07     ` lukas
  2025-11-20 16:07     ` John Garry
  1 sibling, 0 replies; 6+ messages in thread
From: lukas @ 2025-11-20 12:07 UTC (permalink / raw)
  To: Martin K. Petersen; +Cc: linux-scsi

On 2025-11-20 04:26, Martin K. Petersen wrote:
> Lukas,
> 
>> The host->opt_sectors are by default 512 bytes aligned if we have 4KN
>> SAS disk reporting zero or smaller opt_xfer_block than the
>> host->opt_sectors we will end up with unaligned queue_limit->io_opt.
> 
> I am intrigued wrt. which controller returns a host->opt_sectors that
> would trigger this misalignment. What is the actual value reported?
> 

I have seen it on mpt3sas, I do not have the hardware in hand, but I do
not think is related to specific driver as it goes trough 
sas_host_setup()

static int sas_host_setup(struct transport_container *tc, struct device 
*dev,
                           struct device *cdev)
{
...
         if (dma_dev->dma_mask) {
                 shost->opt_sectors = min_t(unsigned int, 
shost->max_sectors,
                                 dma_opt_mapping_size(dma_dev) >> 
SECTOR_SHIFT);
         }

         return 0;
}

I have vmcore from the system.

crash> scsishow | grep ^host | grep -v ahci
host0     mpt3sas                ffff90b78c936000         
ffff90b83ac92d20         ffff90b78c9368d0
crash> Scsi_Host.opt_sectors ffff90b78c936000
       opt_sectors = 0x7fff
crash> pd 0x7fff<<9
$3 = 16776704

and lsblk output:

NAME          ALIGNMENT MIN-IO   OPT-IO PHY-SEC LOG-SEC ROTA SCHED       
RQ-SIZE    RA WSAME
sdc                   0   4096 16776704    4096    4096    1 mq-deadline 
     256 32764    0B


>> +	lim.io_opt = rounddown(lim.logical_block_size,
>> +			sdp->host->opt_sectors << SECTOR_SHIFT);
> 
> Those parameters would need to be reversed, I think. You want to round
> down opt_sectors and not the logical_block_size.

Ugh! My bad! Will send v2.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] scsi: sd: Rounddown host->opt_sectors to logical
  2025-11-20  3:26   ` Martin K. Petersen
  2025-11-20 12:07     ` lukas
@ 2025-11-20 16:07     ` John Garry
  2025-11-21  8:34       ` lukas
  1 sibling, 1 reply; 6+ messages in thread
From: John Garry @ 2025-11-20 16:07 UTC (permalink / raw)
  To: Martin K. Petersen, Lukas Herbolt; +Cc: James.Bottomley, linux-scsi

On 20/11/2025 03:26, Martin K. Petersen wrote:
> 
> Lukas,
> 
>> The host->opt_sectors are by default 512 bytes aligned if we have 4KN
>> SAS disk reporting zero or smaller opt_xfer_block than the
>> host->opt_sectors we will end up with unaligned queue_limit->io_opt.
> 
> I am intrigued wrt. which controller returns a host->opt_sectors that
> would trigger this misalignment. What is the actual value reported?
> 
>> +	lim.io_opt = rounddown(lim.logical_block_size,
>> +			sdp->host->opt_sectors << SECTOR_SHIFT);
> 
> Those parameters would need to be reversed, I think. You want to round
> down opt_sectors and not the logical_block_size.
> 

In blk_validate_limits(), we already round down lim.io_opt to physical 
block size, and physical block size >= logical block size (so this extra 
rounding down should not be required). What am I missing?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] scsi: sd: Rounddown host->opt_sectors to logical
  2025-11-20 16:07     ` John Garry
@ 2025-11-21  8:34       ` lukas
  0 siblings, 0 replies; 6+ messages in thread
From: lukas @ 2025-11-21  8:34 UTC (permalink / raw)
  To: John Garry; +Cc: Martin K. Petersen, James.Bottomley, linux-scsi

On 2025-11-20 17:07, John Garry wrote:
> On 20/11/2025 03:26, Martin K. Petersen wrote:
>> 
>> Lukas,
>> 
>>> The host->opt_sectors are by default 512 bytes aligned if we have 4KN
>>> SAS disk reporting zero or smaller opt_xfer_block than the
>>> host->opt_sectors we will end up with unaligned queue_limit->io_opt.
>> 
>> I am intrigued wrt. which controller returns a host->opt_sectors that
>> would trigger this misalignment. What is the actual value reported?
>> 
>>> +	lim.io_opt = rounddown(lim.logical_block_size,
>>> +			sdp->host->opt_sectors << SECTOR_SHIFT);
>> 
>> Those parameters would need to be reversed, I think. You want to round
>> down opt_sectors and not the logical_block_size.
>> 
> 
> In blk_validate_limits(), we already round down lim.io_opt to physical 
> block size, and physical block size >= logical block size (so this 
> extra rounding down should not be required). What am I missing?


Right, I overlooked it. The problematic kernel I was running is before 
this patch
9c0ba14828d64744ccd195c610594ba254a1a9ab which is setting the lim.io_opt 
to the proper value.

Thanks for the help!

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-11-21  8:34 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-14 10:28 [PATCH 0/1] Fix unaligned queue_limit->io_opt on SAS and 4KN disk Lukas Herbolt
2025-11-14 10:28 ` [PATCH 1/1] scsi: sd: Rounddown host->opt_sectors to logical Lukas Herbolt
2025-11-20  3:26   ` Martin K. Petersen
2025-11-20 12:07     ` lukas
2025-11-20 16:07     ` John Garry
2025-11-21  8:34       ` lukas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox