Re: [PATCH 04/23] scsi: initialize scsi midlayer limits before allocating the queue

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 5/16/24 06:08, John Garry wrote:
On 15/05/2024 17:52, Guenter Roeck wrote:
max_segment_size is 65280; PAGE_SIZE is 65536; BLK_MAX_SEGMENT_SIZE is 65536
WARNING: CPU: 0 PID: 12 at block/blk-settings.c:202 blk_validate_limits+0x2d4/0x364
...

This is with PPC_BOOK3S_64 which selects a default page size of 64k.
Looking at the old code, I think it did what you suggested above,

void blk_queue_max_segment_size(struct request_queue *q, unsigned int max_size)
{
         if (max_size < PAGE_SIZE) {
                 max_size = PAGE_SIZE;
                 printk(KERN_INFO "%s: set to minimum %d\n",
                        __func__, max_size);
         }
...

but assuming that the driver requested a lower limit on purpose that
may not be the best solution.

Right, it is relied on that PAGE_SIZE can fit into a segment.


Never mind, though - I updated my test configuration to explicitly
configure the page size to 4k to work around the problem. With that,
please consider this report a note in case someone hits the problem
on a real system (and sorry for the noise).

Other controllers do have a 4K segment limit and are broken on systems with 16/64K PAGE_SIZE, like:

https://lore.kernel.org/linux-block/20230612203314.17820-1-bvanassche@xxxxxxx/


Understood, only it isn't just 4k segment limit with 16/64k page size, but more
generally any system with segment limit < PAGE_SIZE.

It is a bit sad that support for affected configurations is [now ?] broken
because above patch series was rejected (and, no, that has nothing to do
with me working for the same company as the submitter of that patch series -
that is me testing the upstream kernel with qemu).

Given that various controllers are affected by that problem, would it be
acceptable to submit patches such as the following to avoid runtime failures ?

diff --git a/drivers/ata/pata_macio.c b/drivers/ata/pata_macio.c
index 817838e2f70e..6adf9105b5fb 100644
--- a/drivers/ata/pata_macio.c
+++ b/drivers/ata/pata_macio.c
@@ -1380,6 +1380,8 @@ static int __init pata_macio_init(void)
 {
        int rc;

+       BUILD_BUG_ON(MAX_DBDMA_SEG < PAGE_SIZE);
+
        if (!machine_is(powermac))
                return -ENODEV;

or, alternatively,

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index b595494ab9b4..d7bd64702109 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -789,6 +789,7 @@ config PATA_JMICRON
 config PATA_MACIO
        tristate "Apple PowerMac/PowerBook internal 'MacIO' IDE"
        depends on PPC_PMAC
+       depends on PAGE_SIZE_LESS_THAN_64KB
        help
          Most IDE capable PowerMacs have IDE busses driven by a variant
           of this controller which is part of the Apple chipset used on

Thanks,
Guenter





[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux