Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <f8c743d8-fcbe-4ef7-5f86-d63086552ffd@arm.com>
Date:   Mon, 22 Aug 2022 10:49:09 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:91.0) Gecko/20100101
 Thunderbird/91.12.0
Subject: Re: [PATCH v1 4/4] swiotlb: panic if nslabs is too small
Content-Language: en-GB
To:     Yu Zhao <yuzhao@google.com>, dongli.zhang@oracle.com
Cc:     ak@linux.intel.com, akpm@linux-foundation.org,
        alexander.sverdlin@nokia.com, andi.kleen@intel.com, bp@alien8.de,
        bp@suse.de, cminyard@mvista.com, corbet@lwn.net,
        damien.lemoal@opensource.wdc.com, dave.hansen@linux.intel.com,
        hch@infradead.org, iommu@lists.linux-foundation.org,
        joe.jin@oracle.com, joe@perches.com, keescook@chromium.org,
        kirill.shutemov@intel.com, kys@microsoft.com,
        linux-doc@vger.kernel.org, linux-hyperv@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-mips@vger.kernel.org,
        ltykernel@gmail.com, michael.h.kelley@microsoft.com,
        mingo@redhat.com, m.szyprowski@samsung.com, parri.andrea@gmail.com,
        paulmck@kernel.org, pmladek@suse.com, rdunlap@infradead.org,
        tglx@linutronix.de, thomas.lendacky@amd.com,
        Tianyu.Lan@microsoft.com, tsbogend@alpha.franken.de,
        vkuznets@redhat.com, wei.liu@kernel.org, x86@kernel.org
References: <20220611082514.37112-5-dongli.zhang@oracle.com>
 <20220820012031.1285979-1-yuzhao@google.com>
From:   Robin Murphy <robin.murphy@arm.com>
In-Reply-To: <20220820012031.1285979-1-yuzhao@google.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 2022-08-20 02:20, Yu Zhao wrote:
>> Panic on purpose if nslabs is too small, in order to sync with the remap
>> retry logic.
>>
>> In addition, print the number of bytes for tlb alloc failure.
>>
>> Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
>> ---
>>   kernel/dma/swiotlb.c | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
>> index fd21f4162f4b..1758b724c7a8 100644
>> --- a/kernel/dma/swiotlb.c
>> +++ b/kernel/dma/swiotlb.c
>> @@ -242,6 +242,9 @@ void __init swiotlb_init_remap(bool addressing_limit, unsigned int flags,
>>   	if (swiotlb_force_disable)
>>   		return;
>>   
>> +	if (nslabs < IO_TLB_MIN_SLABS)
>> +		panic("%s: nslabs = %lu too small\n", __func__, nslabs);
> 
> Hi,
> 
> This patch breaks MIPS. Please take a look. Thanks.

Hmm, it's possible this might be quietly fixed by 20347fca71a3, but 
either way I'm not sure why we would need to panic *before* we've even 
tried to allocate anything, when we could simply return with no harm 
done? If we've ended up calculating (or being told) a buffer size which 
is too small to be usable, that should be no different to disabling 
SWIOTLB entirely.

Historically, passing "swiotlb=1" on the command line has been used to 
save memory when the user knows SWIOTLB isn't needed. That should 
definitely not be allowed to start panicking.

(once again, another patch which was not CCed to the correct reviewers, 
sigh...)

Thanks,
Robin.

> On v5.19.0:
>    Linux version 5.19.0 (builder@buildhost) (mips64-openwrt-linux-musl-gcc (OpenWrt GCC 11.2.0 r19590-042d558536) 11.2.0, GNU ld (GNU Binutils) 2.37) #0 SMP Sun Jul 31 15:12:47 2022
>    Skipping L2 locking due to reduced L2 cache size
>    CVMSEG size: 0 cache lines (0 bytes)
>    printk: bootconsole [early0] enabled
>    CPU0 revision is: 000d9301 (Cavium Octeon II)
>    Kernel sections are not in the memory maps
>    Wasting 278528 bytes for tracking 4352 unused pages
>    Initrd not found or empty - disabling initrd
>    Using appended Device Tree.
>    software IO TLB: SWIOTLB bounce buffer size adjusted to 0MB
>    software IO TLB: mapped [mem 0x0000000004b0c000-0x0000000004b4c000] (0MB)
> 
> On v6.0-rc1, with
>    commit 0bf28fc40d89 ("swiotlb: panic if nslabs is too small")
>    commit 20347fca71a3 ("swiotlb: split up the global swiotlb lock")
>    commit 534ea58b3ceb ("Revert "MIPS: octeon: Remove vestiges of CONFIG_CAVIUM_RESERVE32"")
> 
>    Linux version 6.0.0-rc1 (builder@buildhost) (mips64-openwrt-linux-musl-gcc (OpenWrt GCC 11.2.0 r19590-042d558536) 11.2.0, GNU ld (GNU Binutils) 2.37) #0 SMP Sun Jul 31 15:12:47 2022
>    Failed to allocate CAVIUM_RESERVE32 memory area
>    Skipping L2 locking due to reduced L2 cache size
>    CVMSEG size: 0 cache lines (0 bytes)
>    printk: bootconsole [early0] enabled
>    CPU0 revision is: 000d9301 (Cavium Octeon II)
>    Kernel sections are not in the memory maps
>    Wasting 278528 bytes for tracking 4352 unused pages
>    Initrd not found or empty - disabling initrd
>    Using appended Device Tree.
>    software IO TLB: SWIOTLB bounce buffer size adjusted to 0MB
>    software IO TLB: area num 1.
>    Kernel panic - not syncing: swiotlb_init_remap: nslabs = 128 too small
> 
> On v6.0-rc1, with
>    commit 20347fca71a3 ("swiotlb: split up the global swiotlb lock")
>    commit 534ea58b3ceb ("Revert "MIPS: octeon: Remove vestiges of CONFIG_CAVIUM_RESERVE32"")
> 
>    Linux version 6.0.0-rc1+ (builder@buildhost) (mips64-openwrt-linux-musl-gcc (OpenWrt GCC 11.2.0 r19590-042d558536) 11.2.0, GNU ld (GNU Binutils) 2.37) #0 SMP Sun Jul 31 15:12:47 2022
>    Failed to allocate CAVIUM_RESERVE32 memory area
>    Skipping L2 locking due to reduced L2 cache size
>    CVMSEG size: 0 cache lines (0 bytes)
>    printk: bootconsole [early0] enabled
>    CPU0 revision is: 000d9301 (Cavium Octeon II)
>    Kernel sections are not in the memory maps
>    Wasting 278528 bytes for tracking 4352 unused pages
>    Initrd not found or empty - disabling initrd
>    Using appended Device Tree.
>    software IO TLB: SWIOTLB bounce buffer size adjusted to 0MB
>    software IO TLB: area num 1.
>    software IO TLB: mapped [mem 0x0000000004c0c000-0x0000000004c4c000] (0MB)