| Message ID | 20240606111445.400001-1-mpe@ellerman.id.au |
|---|---|
| State | New |
| Headers | show |
| Series | ata: pata_macio: Fix max_segment_size with PAGE_SIZE == 64K | expand |
On 6/6/24 20:14, Michael Ellerman wrote: > The pata_macio driver advertises a max_segment_size of 0xff00, because > the hardware doesn't cope with requests >= 64K. > > However the SCSI core requires max_segment_size to be at least > PAGE_SIZE, which is a problem for pata_macio when the kernel is built > with 64K pages. > > In older kernels the SCSI core would just increase the segment size to > be equal to PAGE_SIZE, however since the commit tagged below it causes a > warning and the device fails to probe: > > WARNING: CPU: 0 PID: 26 at block/blk-settings.c:202 .blk_validate_limits+0x2f8/0x35c > CPU: 0 PID: 26 Comm: kworker/u4:1 Not tainted 6.10.0-rc1 #1 > Hardware name: PowerMac7,2 PPC970 0x390202 PowerMac > ... > NIP .blk_validate_limits+0x2f8/0x35c > LR .blk_alloc_queue+0xc0/0x2f8 > Call Trace: > .blk_alloc_queue+0xc0/0x2f8 > .blk_mq_alloc_queue+0x60/0xf8 > .scsi_alloc_sdev+0x208/0x3c0 > .scsi_probe_and_add_lun+0x314/0x52c > .__scsi_add_device+0x170/0x1a4 > .ata_scsi_scan_host+0x2bc/0x3e4 > .async_port_probe+0x6c/0xa0 > .async_run_entry_fn+0x60/0x1bc > .process_one_work+0x228/0x510 > .worker_thread+0x360/0x530 > .kthread+0x134/0x13c > .start_kernel_thread+0x10/0x14 > ... > scsi_alloc_sdev: Allocation failure during SCSI scanning, some SCSI devices might not be configured > > Although the hardware can't cope with a 64K segment, the driver > already deals with that internally by splitting large requests in > pata_macio_qc_prep(). That is how the driver has managed to function > until now on 64K kernels. > > So fix the driver to advertise a max_segment_size of 64K, which avoids > the warning and keeps the SCSI core happy. > > Fixes: afd53a3d8528 ("scsi: core: Initialize scsi midlayer limits before allocating the queue") > Reported-by: Guenter Roeck <linux@roeck-us.net> > Closes: https://lore.kernel.org/all/ce2bf6af-4382-4fe1-b392-cc6829f5ceb2@roeck-us.net/ > Reported-by: Doru Iorgulescu <doru.iorgulescu1@gmail.com> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218858 > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> > Reviewed-by: Christoph Hellwig <hch@lst.de> Looks good. Reviewed-by: Damien Le Moal <dlemoal@kernel.org> > --- > drivers/ata/pata_macio.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/ata/pata_macio.c b/drivers/ata/pata_macio.c > index 817838e2f70e..3cb455a32d92 100644 > --- a/drivers/ata/pata_macio.c > +++ b/drivers/ata/pata_macio.c > @@ -915,10 +915,13 @@ static const struct scsi_host_template pata_macio_sht = { > .sg_tablesize = MAX_DCMDS, > /* We may not need that strict one */ > .dma_boundary = ATA_DMA_BOUNDARY, > - /* Not sure what the real max is but we know it's less than 64K, let's > - * use 64K minus 256 > + /* > + * The SCSI core requires the segment size to cover at least a page, so > + * for 64K page size kernels this must be at least 64K. However the > + * hardware can't handle 64K, so pata_macio_qc_prep() will split large > + * requests. > */ > - .max_segment_size = MAX_DBDMA_SEG, > + .max_segment_size = SZ_64K, > .device_configure = pata_macio_device_configure, > .sdev_groups = ata_common_sdev_groups, > .can_queue = ATA_DEF_QUEUE,
On Thu, Jun 06, 2024 at 09:14:45PM +1000, Michael Ellerman wrote: > The pata_macio driver advertises a max_segment_size of 0xff00, because > the hardware doesn't cope with requests >= 64K. > > However the SCSI core requires max_segment_size to be at least > PAGE_SIZE, which is a problem for pata_macio when the kernel is built > with 64K pages. > > In older kernels the SCSI core would just increase the segment size to > be equal to PAGE_SIZE, however since the commit tagged below it causes a > warning and the device fails to probe: > > WARNING: CPU: 0 PID: 26 at block/blk-settings.c:202 .blk_validate_limits+0x2f8/0x35c > CPU: 0 PID: 26 Comm: kworker/u4:1 Not tainted 6.10.0-rc1 #1 > Hardware name: PowerMac7,2 PPC970 0x390202 PowerMac > ... > NIP .blk_validate_limits+0x2f8/0x35c > LR .blk_alloc_queue+0xc0/0x2f8 > Call Trace: > .blk_alloc_queue+0xc0/0x2f8 > .blk_mq_alloc_queue+0x60/0xf8 > .scsi_alloc_sdev+0x208/0x3c0 > .scsi_probe_and_add_lun+0x314/0x52c > .__scsi_add_device+0x170/0x1a4 > .ata_scsi_scan_host+0x2bc/0x3e4 > .async_port_probe+0x6c/0xa0 > .async_run_entry_fn+0x60/0x1bc > .process_one_work+0x228/0x510 > .worker_thread+0x360/0x530 > .kthread+0x134/0x13c > .start_kernel_thread+0x10/0x14 > ... > scsi_alloc_sdev: Allocation failure during SCSI scanning, some SCSI devices might not be configured > > Although the hardware can't cope with a 64K segment, the driver > already deals with that internally by splitting large requests in > pata_macio_qc_prep(). That is how the driver has managed to function > until now on 64K kernels. > > So fix the driver to advertise a max_segment_size of 64K, which avoids > the warning and keeps the SCSI core happy. > > Fixes: afd53a3d8528 ("scsi: core: Initialize scsi midlayer limits before allocating the queue") > Reported-by: Guenter Roeck <linux@roeck-us.net> > Closes: https://lore.kernel.org/all/ce2bf6af-4382-4fe1-b392-cc6829f5ceb2@roeck-us.net/ > Reported-by: Doru Iorgulescu <doru.iorgulescu1@gmail.com> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218858 > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> > Reviewed-by: Christoph Hellwig <hch@lst.de> > --- Applied to libata/for-6.10-fixes: https://git.kernel.org/pub/scm/linux/kernel/git/libata/linux.git/log/?h=for-6.10-fixes With John's Reviewed-by from the other thread: https://lore.kernel.org/linux-ide/171362345502.571343.9746199181827642774.b4-ty@oracle.com/T/#t Kind regards, Niklas
Niklas Cassel <cassel@kernel.org> writes: > On Thu, Jun 06, 2024 at 09:14:45PM +1000, Michael Ellerman wrote: >> The pata_macio driver advertises a max_segment_size of 0xff00, because >> the hardware doesn't cope with requests >= 64K. >> >> However the SCSI core requires max_segment_size to be at least >> PAGE_SIZE, which is a problem for pata_macio when the kernel is built >> with 64K pages. >> >> In older kernels the SCSI core would just increase the segment size to >> be equal to PAGE_SIZE, however since the commit tagged below it causes a >> warning and the device fails to probe: >> >> WARNING: CPU: 0 PID: 26 at block/blk-settings.c:202 .blk_validate_limits+0x2f8/0x35c >> CPU: 0 PID: 26 Comm: kworker/u4:1 Not tainted 6.10.0-rc1 #1 >> Hardware name: PowerMac7,2 PPC970 0x390202 PowerMac >> ... >> NIP .blk_validate_limits+0x2f8/0x35c >> LR .blk_alloc_queue+0xc0/0x2f8 >> Call Trace: >> .blk_alloc_queue+0xc0/0x2f8 >> .blk_mq_alloc_queue+0x60/0xf8 >> .scsi_alloc_sdev+0x208/0x3c0 >> .scsi_probe_and_add_lun+0x314/0x52c >> .__scsi_add_device+0x170/0x1a4 >> .ata_scsi_scan_host+0x2bc/0x3e4 >> .async_port_probe+0x6c/0xa0 >> .async_run_entry_fn+0x60/0x1bc >> .process_one_work+0x228/0x510 >> .worker_thread+0x360/0x530 >> .kthread+0x134/0x13c >> .start_kernel_thread+0x10/0x14 >> ... >> scsi_alloc_sdev: Allocation failure during SCSI scanning, some SCSI devices might not be configured >> >> Although the hardware can't cope with a 64K segment, the driver >> already deals with that internally by splitting large requests in >> pata_macio_qc_prep(). That is how the driver has managed to function >> until now on 64K kernels. >> >> So fix the driver to advertise a max_segment_size of 64K, which avoids >> the warning and keeps the SCSI core happy. >> >> Fixes: afd53a3d8528 ("scsi: core: Initialize scsi midlayer limits before allocating the queue") >> Reported-by: Guenter Roeck <linux@roeck-us.net> >> Closes: https://lore.kernel.org/all/ce2bf6af-4382-4fe1-b392-cc6829f5ceb2@roeck-us.net/ >> Reported-by: Doru Iorgulescu <doru.iorgulescu1@gmail.com> >> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218858 >> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> >> Reviewed-by: Christoph Hellwig <hch@lst.de> >> --- > > Applied to libata/for-6.10-fixes: > https://git.kernel.org/pub/scm/linux/kernel/git/libata/linux.git/log/?h=for-6.10-fixes > > With John's Reviewed-by from the other thread: > https://lore.kernel.org/linux-ide/171362345502.571343.9746199181827642774.b4-ty@oracle.com/T/#t Thanks. cheers
diff --git a/drivers/ata/pata_macio.c b/drivers/ata/pata_macio.c index 817838e2f70e..3cb455a32d92 100644 --- a/drivers/ata/pata_macio.c +++ b/drivers/ata/pata_macio.c @@ -915,10 +915,13 @@ static const struct scsi_host_template pata_macio_sht = { .sg_tablesize = MAX_DCMDS, /* We may not need that strict one */ .dma_boundary = ATA_DMA_BOUNDARY, - /* Not sure what the real max is but we know it's less than 64K, let's - * use 64K minus 256 + /* + * The SCSI core requires the segment size to cover at least a page, so + * for 64K page size kernels this must be at least 64K. However the + * hardware can't handle 64K, so pata_macio_qc_prep() will split large + * requests. */ - .max_segment_size = MAX_DBDMA_SEG, + .max_segment_size = SZ_64K, .device_configure = pata_macio_device_configure, .sdev_groups = ata_common_sdev_groups, .can_queue = ATA_DEF_QUEUE,
The pata_macio driver advertises a max_segment_size of 0xff00, because the hardware doesn't cope with requests >= 64K. However the SCSI core requires max_segment_size to be at least PAGE_SIZE, which is a problem for pata_macio when the kernel is built with 64K pages. In older kernels the SCSI core would just increase the segment size to be equal to PAGE_SIZE, however since the commit tagged below it causes a warning and the device fails to probe: WARNING: CPU: 0 PID: 26 at block/blk-settings.c:202 .blk_validate_limits+0x2f8/0x35c CPU: 0 PID: 26 Comm: kworker/u4:1 Not tainted 6.10.0-rc1 #1 Hardware name: PowerMac7,2 PPC970 0x390202 PowerMac ... NIP .blk_validate_limits+0x2f8/0x35c LR .blk_alloc_queue+0xc0/0x2f8 Call Trace: .blk_alloc_queue+0xc0/0x2f8 .blk_mq_alloc_queue+0x60/0xf8 .scsi_alloc_sdev+0x208/0x3c0 .scsi_probe_and_add_lun+0x314/0x52c .__scsi_add_device+0x170/0x1a4 .ata_scsi_scan_host+0x2bc/0x3e4 .async_port_probe+0x6c/0xa0 .async_run_entry_fn+0x60/0x1bc .process_one_work+0x228/0x510 .worker_thread+0x360/0x530 .kthread+0x134/0x13c .start_kernel_thread+0x10/0x14 ... scsi_alloc_sdev: Allocation failure during SCSI scanning, some SCSI devices might not be configured Although the hardware can't cope with a 64K segment, the driver already deals with that internally by splitting large requests in pata_macio_qc_prep(). That is how the driver has managed to function until now on 64K kernels. So fix the driver to advertise a max_segment_size of 64K, which avoids the warning and keeps the SCSI core happy. Fixes: afd53a3d8528 ("scsi: core: Initialize scsi midlayer limits before allocating the queue") Reported-by: Guenter Roeck <linux@roeck-us.net> Closes: https://lore.kernel.org/all/ce2bf6af-4382-4fe1-b392-cc6829f5ceb2@roeck-us.net/ Reported-by: Doru Iorgulescu <doru.iorgulescu1@gmail.com> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218858 Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> Reviewed-by: Christoph Hellwig <hch@lst.de> --- drivers/ata/pata_macio.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)