Re: [Xen-devel] [PATCH] xen/blkfront: Adjust indentation in xlvbd_alloc_gendisk
On Mon, Dec 09, 2019 at 01:07:41PM -0800, Nick Desaulniers wrote: > On Mon, Dec 9, 2019 at 12:14 PM Nathan Chancellor > wrote: > > > > Clang warns: > > > > ../drivers/block/xen-blkfront.c:1117:4: warning: misleading indentation; > > statement is not part of the previous 'if' [-Wmisleading-indentation] > > nr_parts = PARTS_PER_DISK; > > ^ > > ../drivers/block/xen-blkfront.c:1115:3: note: previous statement is here > > if (err) > > ^ > > > > This is because there is a space at the beginning of this line; remove > > it so that the indentation is consistent according to the Linux kernel > > coding style and clang no longer warns. > > > > While we are here, the previous line has some trailing whitespace; clean > > that up as well. > > > > Fixes: c80a420995e7 ("xen-blkfront: handle Xen major numbers other than > > XENVBD") > > Link: https://github.com/ClangBuiltLinux/linux/issues/791 > > Signed-off-by: Nathan Chancellor > > --- > > drivers/block/xen-blkfront.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c > > index a74d03913822..c02be06c5299 100644 > > --- a/drivers/block/xen-blkfront.c > > +++ b/drivers/block/xen-blkfront.c > > @@ -1113,8 +1113,8 @@ static int xlvbd_alloc_gendisk(blkif_sector_t > > capacity, > > While you're here, would you please also removing the single space > before the labels in this function? > > In vim: > > /^ [a-zA-Z] > > turns up 5 labels with this. That should probably be a separate patch since there are only two labels in the function I am touching here. I'll whip up a v2 if the maintainers want it though or I'll just draft a separate patch when I am done addressing all of the misleading indentation warnings. Thanks for the reply! Nathan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] xen/blkfront: Adjust indentation in xlvbd_alloc_gendisk
Clang warns: ../drivers/block/xen-blkfront.c:1117:4: warning: misleading indentation; statement is not part of the previous 'if' [-Wmisleading-indentation] nr_parts = PARTS_PER_DISK; ^ ../drivers/block/xen-blkfront.c:1115:3: note: previous statement is here if (err) ^ This is because there is a space at the beginning of this line; remove it so that the indentation is consistent according to the Linux kernel coding style and clang no longer warns. While we are here, the previous line has some trailing whitespace; clean that up as well. Fixes: c80a420995e7 ("xen-blkfront: handle Xen major numbers other than XENVBD") Link: https://github.com/ClangBuiltLinux/linux/issues/791 Signed-off-by: Nathan Chancellor --- drivers/block/xen-blkfront.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c index a74d03913822..c02be06c5299 100644 --- a/drivers/block/xen-blkfront.c +++ b/drivers/block/xen-blkfront.c @@ -1113,8 +1113,8 @@ static int xlvbd_alloc_gendisk(blkif_sector_t capacity, if (!VDEV_IS_EXTENDED(info->vdevice)) { err = xen_translate_vdev(info->vdevice, &minor, &offset); if (err) - return err; - nr_parts = PARTS_PER_DISK; + return err; + nr_parts = PARTS_PER_DISK; } else { minor = BLKIF_MINOR_EXT(info->vdevice); nr_parts = PARTS_PER_EXT_DISK; -- 2.24.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[PATCH] kernel/resource: Fix use of ternary condition in release_mem_region_adjustable
Clang warns: kernel/resource.c:1281:53: warning: operator '?:' has lower precedence than '|'; '|' will be evaluated first [-Wbitwise-conditional-parentheses] new_res = alloc_resource(GFP_KERNEL | alloc_nofail ? __GFP_NOFAIL : 0); ~ ^ kernel/resource.c:1281:53: note: place parentheses around the '|' expression to silence this warning new_res = alloc_resource(GFP_KERNEL | alloc_nofail ? __GFP_NOFAIL : 0); ~ ^ kernel/resource.c:1281:53: note: place parentheses around the '?:' expression to evaluate it first new_res = alloc_resource(GFP_KERNEL | alloc_nofail ? __GFP_NOFAIL : 0); ^ ( ) 1 warning generated. Add the parentheses as it was clearly intended for the ternary condition to be evaluated first. Fixes: 5fd23bd0d739 ("kernel/resource: make release_mem_region_adjustable() never fail") Link: https://github.com/ClangBuiltLinux/linux/issues/1159 Signed-off-by: Nathan Chancellor --- Presumably, this will be squashed but I included a fixes tag nonetheless. Apologies if this has already been noticed and fixed already, I did not find anything on LKML. kernel/resource.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/resource.c b/kernel/resource.c index ca2a666e4317..3ae2f56cc79d 100644 --- a/kernel/resource.c +++ b/kernel/resource.c @@ -1278,7 +1278,7 @@ void release_mem_region_adjustable(resource_size_t start, resource_size_t size) * similarly). */ retry: - new_res = alloc_resource(GFP_KERNEL | alloc_nofail ? __GFP_NOFAIL : 0); + new_res = alloc_resource(GFP_KERNEL | (alloc_nofail ? __GFP_NOFAIL : 0)); p = &parent->child; write_lock(&resource_lock); base-commit: 40ee82f47bf297e31d0c47547cd8f24ede52415a -- 2.28.0
Re: [PATCH v15 06/12] swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing
Hi Will and Robin, On Fri, Jul 02, 2021 at 04:13:50PM +0100, Robin Murphy wrote: > On 2021-07-02 14:58, Will Deacon wrote: > > Hi Nathan, > > > > On Thu, Jul 01, 2021 at 12:52:20AM -0700, Nathan Chancellor wrote: > > > On 7/1/2021 12:40 AM, Will Deacon wrote: > > > > On Wed, Jun 30, 2021 at 08:56:51AM -0700, Nathan Chancellor wrote: > > > > > On Wed, Jun 30, 2021 at 12:43:48PM +0100, Will Deacon wrote: > > > > > > On Wed, Jun 30, 2021 at 05:17:27PM +0800, Claire Chang wrote: > > > > > > > `BUG: unable to handle page fault for address: 003a8290` > > > > > > > and > > > > > > > the fact it crashed at `_raw_spin_lock_irqsave` look like the > > > > > > > memory > > > > > > > (maybe dev->dma_io_tlb_mem) was corrupted? > > > > > > > The dev->dma_io_tlb_mem should be set here > > > > > > > (https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/pci/probe.c#n2528) > > > > > > > through device_initialize. > > > > > > > > > > > > I'm less sure about this. 'dma_io_tlb_mem' should be pointing at > > > > > > 'io_tlb_default_mem', which is a page-aligned allocation from > > > > > > memblock. > > > > > > The spinlock is at offset 0x24 in that structure, and looking at the > > > > > > register dump from the crash: > > > > > > > > > > > > Jun 29 18:28:42 hp-4300G kernel: RSP: 0018:adb4013db9e8 EFLAGS: > > > > > > 00010006 > > > > > > Jun 29 18:28:42 hp-4300G kernel: RAX: 003a8290 RBX: > > > > > > RCX: 8900572ad580 > > > > > > Jun 29 18:28:42 hp-4300G kernel: RDX: 89005653f024 RSI: > > > > > > 000c RDI: 1d17 > > > > > > Jun 29 18:28:42 hp-4300G kernel: RBP: 0a20d000 R08: > > > > > > 000c R09: > > > > > > Jun 29 18:28:42 hp-4300G kernel: R10: 0a20d000 R11: > > > > > > 89005653f000 R12: 0212 > > > > > > Jun 29 18:28:42 hp-4300G kernel: R13: 1000 R14: > > > > > > 0002 R15: 0020 > > > > > > Jun 29 18:28:42 hp-4300G kernel: FS: 7f1f8898ea40() > > > > > > GS:89005728() knlGS: > > > > > > Jun 29 18:28:42 hp-4300G kernel: CS: 0010 DS: ES: CR0: > > > > > > 80050033 > > > > > > Jun 29 18:28:42 hp-4300G kernel: CR2: 003a8290 CR3: > > > > > > 0001020d CR4: 00350ee0 > > > > > > Jun 29 18:28:42 hp-4300G kernel: Call Trace: > > > > > > Jun 29 18:28:42 hp-4300G kernel: _raw_spin_lock_irqsave+0x39/0x50 > > > > > > Jun 29 18:28:42 hp-4300G kernel: swiotlb_tbl_map_single+0x12b/0x4c0 > > > > > > > > > > > > Then that correlates with R11 holding the 'dma_io_tlb_mem' pointer > > > > > > and > > > > > > RDX pointing at the spinlock. Yet RAX is holding junk :/ > > > > > > > > > > > > I agree that enabling KASAN would be a good idea, but I also think > > > > > > we > > > > > > probably need to get some more information out of > > > > > > swiotlb_tbl_map_single() > > > > > > to see see what exactly is going wrong in there. > > > > > > > > > > I can certainly enable KASAN and if there is any debug print I can add > > > > > or dump anything, let me know! > > > > > > > > I bit the bullet and took v5.13 with swiotlb/for-linus-5.14 merged in, > > > > built > > > > x86 defconfig and ran it on my laptop. However, it seems to work fine! > > > > > > > > Please can you share your .config? > > > > > > Sure thing, it is attached. It is just Arch Linux's config run through > > > olddefconfig. The original is below in case you need to diff it. > > > > > > https://raw.githubusercontent.com/archlinux/svntogit-packages/9045405dc835527164f3034b3ceb9a67c7a53cd4/trunk/config > > > > > > If there is anything more that I can provide, please let me know. > > > > I eventually got this booting (for some reason it was causing LD to
Re: [PATCH v15 06/12] swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing
Hi Will and Robin, On 7/6/2021 10:06 AM, Will Deacon wrote: On Tue, Jul 06, 2021 at 04:39:11PM +0100, Robin Murphy wrote: On 2021-07-06 15:05, Christoph Hellwig wrote: On Tue, Jul 06, 2021 at 03:01:04PM +0100, Robin Murphy wrote: FWIW I was pondering the question of whether to do something along those lines or just scrap the default assignment entirely, so since I hadn't got round to saying that I've gone ahead and hacked up the alternative (similarly untested) for comparison :) TBH I'm still not sure which one I prefer... Claire did implement something like your suggestion originally, but I don't really like it as it doesn't scale for adding multiple global pools, e.g. for the 64-bit addressable one for the various encrypted secure guest schemes. Ah yes, that had slipped my mind, and it's a fair point indeed. Since we're not concerned with a minimal fix for backports anyway I'm more than happy to focus on Will's approach. Another thing is that that looks to take us a quiet step closer to the possibility of dynamically resizing a SWIOTLB pool, which is something that some of the hypervisor protection schemes looking to build on top of this series may want to explore at some point. Ok, I'll split that nasty diff I posted up into a reviewable series and we can take it from there. For what it's worth, I attempted to boot Will's diff on top of Konrad's devel/for-linus-5.14 and it did not work; in fact, I got no output on my monitor period, even with earlyprintk=, and I do not think this machine has a serial console. Robin's fix does work, it survived ten reboots with no issues getting to X and I do not see the KASAN and slub debug messages anymore but I understand that this is not the preferred solution it seems (although Konrad did want to know if it works). I am happy to test any further patches or follow ups as needed, just keep me on CC. Cheers, Nathan
Re: [PATCH v15 06/12] swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing
Hi Will and Claire, On Wed, Jun 30, 2021 at 12:43:48PM +0100, Will Deacon wrote: > On Wed, Jun 30, 2021 at 05:17:27PM +0800, Claire Chang wrote: > > On Wed, Jun 30, 2021 at 9:43 AM Nathan Chancellor wrote: > > > > > > On Thu, Jun 24, 2021 at 11:55:20PM +0800, Claire Chang wrote: > > > > Propagate the swiotlb_force into io_tlb_default_mem->force_bounce and > > > > use it to determine whether to bounce the data or not. This will be > > > > useful later to allow for different pools. > > > > > > > > Signed-off-by: Claire Chang > > > > Reviewed-by: Christoph Hellwig > > > > Tested-by: Stefano Stabellini > > > > Tested-by: Will Deacon > > > > Acked-by: Stefano Stabellini > > > > > > This patch as commit af452ec1b1a3 ("swiotlb: Use is_swiotlb_force_bounce > > > for swiotlb data bouncing") causes my Ryzen 3 4300G system to fail to > > > get to an X session consistently (although not every single time), > > > presumably due to a crash in the AMDGPU driver that I see in dmesg. > > > > > > I have attached logs at af452ec1b1a3 and f127c9556a8e and I am happy > > > to provide any further information, debug, or test patches as necessary. > > > > Are you using swiotlb=force? or the swiotlb_map is called because of > > !dma_capable? > > (https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/kernel/dma/direct.h#n93) > > The command line is in the dmesg: > > | Kernel command line: initrd=\amd-ucode.img > initrd=\initramfs-linux-next-llvm.img > root=PARTUUID=8680aa0c-cf09-4a69-8cf3-970478040ee7 rw intel_pstate=no_hwp > irqpoll > > but I worry that this looks _very_ similar to the issue reported by Qian > Cai which we thought we had fixed. Nathan -- is the failure deterministic? Yes, for the most part. It does not happen every single boot so when I was bisecting, I did a series of seven boots and only considered the revision good when all seven of them made it to LightDM's greeter. My results that I notated show most bad revisions failed anywhere from four to six times. > > `BUG: unable to handle page fault for address: 003a8290` and > > the fact it crashed at `_raw_spin_lock_irqsave` look like the memory > > (maybe dev->dma_io_tlb_mem) was corrupted? > > The dev->dma_io_tlb_mem should be set here > > (https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/pci/probe.c#n2528) > > through device_initialize. > > I'm less sure about this. 'dma_io_tlb_mem' should be pointing at > 'io_tlb_default_mem', which is a page-aligned allocation from memblock. > The spinlock is at offset 0x24 in that structure, and looking at the > register dump from the crash: > > Jun 29 18:28:42 hp-4300G kernel: RSP: 0018:adb4013db9e8 EFLAGS: 00010006 > Jun 29 18:28:42 hp-4300G kernel: RAX: 003a8290 RBX: > RCX: 8900572ad580 > Jun 29 18:28:42 hp-4300G kernel: RDX: 89005653f024 RSI: 000c > RDI: 1d17 > Jun 29 18:28:42 hp-4300G kernel: RBP: 0a20d000 R08: 000c > R09: > Jun 29 18:28:42 hp-4300G kernel: R10: 0a20d000 R11: 89005653f000 > R12: 0212 > Jun 29 18:28:42 hp-4300G kernel: R13: 1000 R14: 0002 > R15: 0020 > Jun 29 18:28:42 hp-4300G kernel: FS: 7f1f8898ea40() > GS:89005728() knlGS: > Jun 29 18:28:42 hp-4300G kernel: CS: 0010 DS: ES: CR0: > 80050033 > Jun 29 18:28:42 hp-4300G kernel: CR2: 003a8290 CR3: 0001020d > CR4: 00350ee0 > Jun 29 18:28:42 hp-4300G kernel: Call Trace: > Jun 29 18:28:42 hp-4300G kernel: _raw_spin_lock_irqsave+0x39/0x50 > Jun 29 18:28:42 hp-4300G kernel: swiotlb_tbl_map_single+0x12b/0x4c0 > > Then that correlates with R11 holding the 'dma_io_tlb_mem' pointer and > RDX pointing at the spinlock. Yet RAX is holding junk :/ > > I agree that enabling KASAN would be a good idea, but I also think we > probably need to get some more information out of swiotlb_tbl_map_single() > to see see what exactly is going wrong in there. I can certainly enable KASAN and if there is any debug print I can add or dump anything, let me know! Cheers, Nathan
Re: Fwd: UBSAN: index 1 is out of range for type 'xen_netif_rx_sring_entry [1]'
On Sat, Jul 22, 2023 at 07:21:05AM +0700, Bagas Sanjaya wrote: > Hi, > > I notice a regression report on Bugzilla [1]. Quoting from it: > > > Hi Kernel Team, > > > > I rebuild today latest version from mainline repo. > > And i notice issue regarding xen-netfront.c. > > > > Error: > > [3.477400] > > > > [3.477633] UBSAN: array-index-out-of-bounds in > > drivers/net/xen-netfront.c:1291:3 > > [3.477858] index 1 is out of range for type 'xen_netif_rx_sring_entry > > [1]' > > [3.478085] CPU: 0 PID: 700 Comm: NetworkManager Not tainted > > 6.5.0-rc2-1-generation1 #3 > > [3.478088] Hardware name: Intel Corporation W2600CR/W2600CR, BIOS > > SE5C600.86B.02.06.0007.082420181029 01/13/2022 > > [3.478090] Call Trace: > > [3.478092] > > [3.478097] dump_stack_lvl+0x48/0x70 > > [3.478105] dump_stack+0x10/0x20 > > [3.478107] __ubsan_handle_out_of_bounds+0xc6/0x110 > > [3.478114] xennet_poll+0xa94/0xac0 > > [3.478118] ? generic_smp_call_function_single_interrupt+0x13/0x20 > > [3.478125] __napi_poll+0x33/0x200 > > [3.478131] net_rx_action+0x181/0x2e0 > > [3.478135] __do_softirq+0xd9/0x346 > > [3.478139] do_softirq.part.0+0x41/0x80 > > [3.478144] > > [3.478145] > > [3.478146] __local_bh_enable_ip+0x72/0x80 > > [3.478149] _raw_spin_unlock_bh+0x1d/0x30 > > [3.478151] xennet_open+0x75/0x160 > > [3.478154] __dev_open+0x105/0x1d0 > > [3.478156] __dev_change_flags+0x1b5/0x230 > > [3.478158] dev_change_flags+0x27/0x80 > > [3.478160] do_setlink+0x3d2/0x12b0 > > [3.478164] ? __nla_validate_parse+0x5b/0xdb0 > > [3.478169] __rtnl_newlink+0x6f6/0xb10 > > [3.478173] ? rtnl_newlink+0x2f/0x80 > > [3.478177] rtnl_newlink+0x48/0x80 > > [3.478180] rtnetlink_rcv_msg+0x170/0x430 > > [3.478183] ? fib6_clean_node+0xad/0x190 > > [3.478188] ? __pfx_rtnetlink_rcv_msg+0x10/0x10 > > [3.478191] netlink_rcv_skb+0x5d/0x110 > > [3.478195] rtnetlink_rcv+0x15/0x30 > > [3.478198] netlink_unicast+0x247/0x390 > > [3.478200] netlink_sendmsg+0x25e/0x4e0 > > [3.478202] sock_sendmsg+0xaf/0xc0 > > [3.478204] sys_sendmsg+0x2a9/0x350 > > [3.478206] ___sys_sendmsg+0x9a/0xf0 > > [3.478212] ? _copy_from_iter+0x80/0x4a0 > > [3.478217] __sys_sendmsg+0x89/0xf0 > > [3.478220] __x64_sys_sendmsg+0x1d/0x30 > > [3.478222] do_syscall_64+0x5c/0x90 > > [3.478226] ? do_syscall_64+0x68/0x90 > > [3.478228] ? ksys_write+0xe6/0x100 > > [3.478232] ? exit_to_user_mode_prepare+0x49/0x220 > > [3.478236] ? syscall_exit_to_user_mode+0x1b/0x50 > > [3.478240] ? do_syscall_64+0x68/0x90 > > [3.478242] ? do_syscall_64+0x68/0x90 > > [3.478243] ? irqentry_exit_to_user_mode+0x9/0x30 > > [3.478246] ? irqentry_exit+0x43/0x50 > > [3.478248] ? sysvec_xen_hvm_callback+0x4b/0xd0 > > [3.478250] entry_SYSCALL_64_after_hwframe+0x6e/0xd8 > > [3.478253] RIP: 0033:0x7f973c244e4d > > [3.478268] Code: 28 89 54 24 1c 48 89 74 24 10 89 7c 24 08 e8 ca ee ff > > ff 8b 54 24 1c 48 8b 74 24 10 41 89 c0 8b 7c 24 08 b8 2e 00 00 00 0f 05 > > <48> 3d 00 f0 ff ff 77 33 44 89 c7 48 89 44 24 08 e8 fe ee ff ff 48 > > [3.478270] RSP: 002b:7fff4777f470 EFLAGS: 0293 ORIG_RAX: > > 002e > > [3.478273] RAX: ffda RBX: 5583087c6480 RCX: > > 7f973c244e4d > > [3.478274] RDX: RSI: 7fff4777f4c0 RDI: > > 000c > > [3.478276] RBP: 7fff4777f4c0 R08: R09: > > > > [3.478277] R10: R11: 0293 R12: > > 5583087c6480 > > [3.478279] R13: 7fff4777f668 R14: 7fff4777f65c R15: > > > > [3.478283] > > [3.478284] > > > > [3.685513] > > > > [3.685751] UBSAN: array-index-out-of-bounds in > > drivers/net/xen-netfront.c:485:7 > > [3.686111] index 1 is out of range for type 'xen_netif_tx_sring_entry > > [1]' > > [3.686379] CPU: 1 PID: 697 Comm: avahi-daemon Not tainted > > 6.5.0-rc2-1-generation1 #3 > > [3.686381] Hardware name: Intel Corporation W2600CR/W2600CR, BIOS > > SE5C600.86B.02.06.0007.082420181029 01/13/2022 > > [3.686385] Call Trace: > > [3.686388] > > [3.686391] dump_stack_lvl+0x48/0x70 > > [3.686399] dump_stack+0x10/0x20 > > [3.686399] __ubsan_handle_out_of_bounds+0xc6/0x110 > > [3.686403] xennet_tx_setup_grant+0x1f7/0x230 > > [3.686403] ? __pfx_xennet_tx_setup_grant+0x10/0x10 > > [3.686403] gnttab_foreach_grant_in_range+0x5c/0x100 > > [3.686415] xennet_start_xmit+0x428/0x990 > > [3.686415] ? kmem_cache_alloc_node+0x1b1/0x3b0 > > [3.686415] dev_har
Re: [PATCH 09/15] swiotlb: make the swiotlb_init interface more useful
Hi Christoph, On Mon, Apr 04, 2022 at 07:05:53AM +0200, Christoph Hellwig wrote: > Pass a bool to pass if swiotlb needs to be enabled based on the > addressing needs and replace the verbose argument with a set of > flags, including one to force enable bounce buffering. > > Note that this patch removes the possibility to force xen-swiotlb > use using swiotlb=force on the command line on x86 (arm and arm64 > never supported that), but this interface will be restored shortly. > > Signed-off-by: Christoph Hellwig I bisected a performance regression in WSL2 to this change as commit c6af2aa9ffc9 ("swiotlb: make the swiotlb_init interface more useful") in mainline (bisect log below). I initially noticed it because accessing the Windows filesystem through the /mnt/c mount is about 40x slower if I am doing my math right based on the benchmarks below. Before: $ uname -r; and hyperfine "ls -l /mnt/c/Users/natec/Downloads" 5.18.0-rc3-microsoft-standard-WSL2-8-ga3e230926708 Benchmark 1: ls -l /mnt/c/Users/natec/Downloads Time (mean ± σ): 564.5 ms ± 24.1 ms[User: 2.5 ms, System: 130.3 ms] Range (min … max): 510.2 ms … 588.0 ms10 runs After $ uname -r; and hyperfine "ls -l /mnt/c/Users/natec/Downloads" 5.18.0-rc3-microsoft-standard-WSL2-9-gc6af2aa9ffc9 Benchmark 1: ls -l /mnt/c/Users/natec/Downloads Time (mean ± σ): 23.282 s ± 1.220 s[User: 0.013 s, System: 0.101 s] Range (min … max): 21.793 s … 25.317 s10 runs I do see 'swiotlb=force' on the cmdline: $ cat /proc/cmdline initrd=\initrd.img panic=-1 nr_cpus=8 swiotlb=force earlycon=uart8250,io,0x3f8,115200 console=hvc0 debug pty.legacy_count=0 /mnt/c appears to be a 9p mount, not sure if that is relevant here: $ mount &| grep /mnt/c drvfs on /mnt/c type 9p (rw,noatime,dirsync,aname=drvfs;path=C:\;uid=1000;gid=1000;symlinkroot=/mnt/,mmap,access=client,msize=262144,trans=virtio) If there is any other information I can provide, please let me know. Cheers, Nathan # bad: [700170bf6b4d773e328fa54ebb70ba444007c702] Merge tag 'nfs-for-5.19-1' of git://git.linux-nfs.org/projects/anna/linux-nfs # good: [4b0986a3613c92f4ec1bdc7f60ec66fea135991f] Linux 5.18 git bisect start '700170bf6b4d773e328fa54ebb70ba444007c702' 'v5.18' # good: [86c87bea6b42100c67418af690919c44de6ede6e] Merge tag 'devicetree-for-5.19' of git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux git bisect good 86c87bea6b42100c67418af690919c44de6ede6e # bad: [ae862183285cbb2ef9032770d98ffa9becffe9d5] Merge tag 'arm-dt-5.19' of git://git.kernel.org/pub/scm/linux/kernel/git/soc/soc git bisect bad ae862183285cbb2ef9032770d98ffa9becffe9d5 # good: [2518f226c60d8e04d18ba4295500a5b0b8ac7659] Merge tag 'drm-next-2022-05-25' of git://anongit.freedesktop.org/drm/drm git bisect good 2518f226c60d8e04d18ba4295500a5b0b8ac7659 # bad: [babf0bb978e3c9fce6c4eba6b744c8754fd43d8e] Merge tag 'xfs-5.19-for-linus' of git://git.kernel.org/pub/scm/fs/xfs/xfs-linux git bisect bad babf0bb978e3c9fce6c4eba6b744c8754fd43d8e # good: [beed983621fbdfd291e6e3a0cdc4d10517e60af8] ASoC: Intel: avs: Machine board registration git bisect good beed983621fbdfd291e6e3a0cdc4d10517e60af8 # good: [fbe86daca0ba878b04fa241b85e26e54d17d4229] Merge tag 'scsi-misc' of git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi git bisect good fbe86daca0ba878b04fa241b85e26e54d17d4229 # good: [166afc45ed5523298541fd0297f9ad585cc2708c] Merge tag 'reflink-speedups-5.19_2022-04-28' of git://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux into xfs-5.19-for-next git bisect good 166afc45ed5523298541fd0297f9ad585cc2708c # bad: [e375780b631a5fc2a61a3b4fa12429255361a31e] Merge tag 'fsnotify_for_v5.19-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs git bisect bad e375780b631a5fc2a61a3b4fa12429255361a31e # bad: [4a37f3dd9a83186cb88d44808ab35b78375082c9] dma-direct: don't over-decrypt memory git bisect bad 4a37f3dd9a83186cb88d44808ab35b78375082c9 # bad: [742519538e6b07250c8085bbff4bd358bc03bf16] swiotlb: pass a gfp_mask argument to swiotlb_init_late git bisect bad 742519538e6b07250c8085bbff4bd358bc03bf16 # good: [9bbe7a7fc126e3d14fefa4b035854aba080926d9] arm/xen: don't check for xen_initial_domain() in xen_create_contiguous_region git bisect good 9bbe7a7fc126e3d14fefa4b035854aba080926d9 # good: [a3e230926708125205ffd06d3dc2175a8263ae7e] x86: centralize setting SWIOTLB_FORCE when guest memory encryption is enabled git bisect good a3e230926708125205ffd06d3dc2175a8263ae7e # bad: [8ba2ed1be90fc210126f68186564707478552c95] swiotlb: add a SWIOTLB_ANY flag to lift the low memory restriction git bisect bad 8ba2ed1be90fc210126f68186564707478552c95 # bad: [c6af2aa9ffc9763826607bc2664ef3ea4475ed18] swiotlb: make the swiotlb_init interface more useful git bisect bad c6af2aa9ffc9763826607bc2664ef3ea4475ed18 # first bad commit: [c6af2aa9ffc9763826607bc2664ef3ea4475ed18] swiotlb: make the swiotlb_init interface more useful
Re: [PATCH 09/15] swiotlb: make the swiotlb_init interface more useful
On Wed, Jun 01, 2022 at 07:34:41PM +0200, Christoph Hellwig wrote: > Can you send me the full dmesg and the content of > /sys/kernel/debug/swiotlb/io_tlb_nslabs for a good and a bad boot? Sure thing, they are attached! If there is anything else I can provide or test, I am more than happy to do so. Cheers, Nathan # cat /sys/kernel/debug/swiotlb/io_tlb_nslabs 32768 # dmesg [0.00] Linux version 5.18.0-rc3-microsoft-standard-WSL2-8-ga3e230926708 (nathan@dev-arch.thelio-3990X) (gcc (GCC) 12.1.0, GNU ld (GNU Binutils) 2.38) #1 SMP PREEMPT_DYNAMIC Wed Jun 1 10:38:34 MST 2022 [0.00] Command line: initrd=\initrd.img panic=-1 nr_cpus=8 swiotlb=force earlycon=uart8250,io,0x3f8,115200 console=hvc0 debug pty.legacy_count=0 [0.00] KERNEL supported cpus: [0.00] Intel GenuineIntel [0.00] AMD AuthenticAMD [0.00] Centaur CentaurHauls [0.00] x86/fpu: Supporting XSAVE feature 0x001: 'x87 floating point registers' [0.00] x86/fpu: Supporting XSAVE feature 0x002: 'SSE registers' [0.00] x86/fpu: Supporting XSAVE feature 0x004: 'AVX registers' [0.00] x86/fpu: xstate_offset[2]: 576, xstate_sizes[2]: 256 [0.00] x86/fpu: Enabled xstate features 0x7, context size is 832 bytes, using 'compacted' format. [0.00] signal: max sigframe size: 1776 [0.00] BIOS-provided physical RAM map: [0.00] BIOS-e820: [mem 0x-0x0009] usable [0.00] BIOS-e820: [mem 0x000e-0x000e0fff] reserved [0.00] BIOS-e820: [mem 0x0010-0x001f] ACPI data [0.00] BIOS-e820: [mem 0x0020-0xf7ff] usable [0.00] BIOS-e820: [mem 0x0001-0x000407ff] usable [0.00] earlycon: uart8250 at I/O port 0x3f8 (options '115200') [0.00] printk: bootconsole [uart8250] enabled [0.00] NX (Execute Disable) protection: active [0.00] DMI not present or invalid. [0.00] Hypervisor detected: Microsoft Hyper-V [0.00] Hyper-V: privilege flags low 0xae7f, high 0x3b8030, hints 0xc2c, misc 0xe0bed7b6 [0.00] Hyper-V: Host Build 10.0.22000.708-0-0 [0.00] Hyper-V: Nested features: 0x4a [0.00] Hyper-V: LAPIC Timer Frequency: 0x1e8480 [0.00] Hyper-V: Using hypercall for remote TLB flush [0.00] clocksource: hyperv_clocksource_tsc_page: mask: 0x max_cycles: 0x24e6a1710, max_idle_ns: 440795202120 ns [0.05] tsc: Detected 3800.008 MHz processor [0.001901] e820: update [mem 0x-0x0fff] usable ==> reserved [0.004593] e820: remove [mem 0x000a-0x000f] usable [0.006806] last_pfn = 0x408000 max_arch_pfn = 0x4 [0.009042] x86/PAT: Configuration [0-7]: WB WC UC- UC WB WP UC- WT [0.011760] last_pfn = 0xf8000 max_arch_pfn = 0x4 [0.013959] Using GB pages for direct mapping [0.015749] RAMDISK: [mem 0x0371f000-0x03779fff] [0.017616] ACPI: Early table checksum verification disabled [0.019854] ACPI: RSDP 0x000E 24 (v02 VRTUAL) [0.022162] ACPI: XSDT 0x0010 44 (v01 VRTUAL MICROSFT 0001 MSFT 0001) [0.025624] ACPI: FACP 0x00101000 000114 (v06 VRTUAL MICROSFT 0001 MSFT 0001) [0.029022] ACPI: DSDT 0x001011B8 01E184 (v02 MSFTVM DSDT01 0001 MSFT 0500) [0.032413] ACPI: FACS 0x00101114 40 [0.034280] ACPI: OEM0 0x00101154 64 (v01 VRTUAL MICROSFT 0001 MSFT 0001) [0.037699] ACPI: SRAT 0x0011F33C 000330 (v02 VRTUAL MICROSFT 0001 MSFT 0001) [0.041089] ACPI: APIC 0x0011F66C 88 (v04 VRTUAL MICROSFT 0001 MSFT 0001) [0.044475] ACPI: Reserving FACP table memory at [mem 0x101000-0x101113] [0.047159] ACPI: Reserving DSDT table memory at [mem 0x1011b8-0x11f33b] [0.049905] ACPI: Reserving FACS table memory at [mem 0x101114-0x101153] [0.052693] ACPI: Reserving OEM0 table memory at [mem 0x101154-0x1011b7] [0.055404] ACPI: Reserving SRAT table memory at [mem 0x11f33c-0x11f66b] [0.058040] ACPI: Reserving APIC table memory at [mem 0x11f66c-0x11f6f3] [0.061078] Zone ranges: [0.062074] DMA [mem 0x1000-0x00ff] [0.066106] DMA32[mem 0x0100-0x] [0.068763] Normal [mem 0x0001-0x000407ff] [0.071235] Device empty [0.072384] Movable zone start for each node [0.074058] Early memory node ranges [0.075515] node 0: [mem 0x1000-0x0009] [0.077979] node 0: [mem 0x0020-0xf7ff] [0.080483] node 0: [mem 0x0001-0x000407ff] [0.082980] Initmem setup node 0 [mem 0x1000-0x000407ff] [0.085954] On node 0, zone DMA: 1 pages in unavailable ranges [0.085972] On node 0, zone DMA: 352 pages
Re: [PATCH 09/15] swiotlb: make the swiotlb_init interface more useful
On Wed, Jun 01, 2022 at 07:57:43PM +0200, Christoph Hellwig wrote: > On Wed, Jun 01, 2022 at 10:46:54AM -0700, Nathan Chancellor wrote: > > On Wed, Jun 01, 2022 at 07:34:41PM +0200, Christoph Hellwig wrote: > > > Can you send me the full dmesg and the content of > > > /sys/kernel/debug/swiotlb/io_tlb_nslabs for a good and a bad boot? > > > > Sure thing, they are attached! If there is anything else I can provide > > or test, I am more than happy to do so. > > Nothing interesting. But the performance numbers almost look like > swiotlb=force got ignored before (even if I can't explain why). I was able to get my performance back with this diff but I don't know if this is a hack or a proper fix in the context of the series. diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c index dfa1de89dc94..0bfb2fe3d8c5 100644 --- a/kernel/dma/swiotlb.c +++ b/kernel/dma/swiotlb.c @@ -276,7 +276,7 @@ void __init swiotlb_init_remap(bool addressing_limit, unsigned int flags, __func__, alloc_size, PAGE_SIZE); swiotlb_init_io_tlb_mem(mem, __pa(tlb), nslabs, false); - mem->force_bounce = flags & SWIOTLB_FORCE; + mem->force_bounce = swiotlb_force_bounce || (flags & SWIOTLB_FORCE); if (flags & SWIOTLB_VERBOSE) swiotlb_print_info(); > Do you get a similar performance with the new kernel without > swiotlb=force as the old one with that argument by any chance? I'll see if I can test that, as I am not sure I have control over those cmdline arguments. Cheers, Nathan
Re: [PATCH 09/15] swiotlb: make the swiotlb_init interface more useful
On Wed, Jun 01, 2022 at 08:21:41PM +0200, Christoph Hellwig wrote: > On Wed, Jun 01, 2022 at 11:11:57AM -0700, Nathan Chancellor wrote: > > On Wed, Jun 01, 2022 at 07:57:43PM +0200, Christoph Hellwig wrote: > > > On Wed, Jun 01, 2022 at 10:46:54AM -0700, Nathan Chancellor wrote: > > > > On Wed, Jun 01, 2022 at 07:34:41PM +0200, Christoph Hellwig wrote: > > > > > Can you send me the full dmesg and the content of > > > > > /sys/kernel/debug/swiotlb/io_tlb_nslabs for a good and a bad boot? > > > > > > > > Sure thing, they are attached! If there is anything else I can provide > > > > or test, I am more than happy to do so. > > > > > > Nothing interesting. But the performance numbers almost look like > > > swiotlb=force got ignored before (even if I can't explain why). > > > > I was able to get my performance back with this diff but I don't know if > > this is a hack or a proper fix in the context of the series. > > This looks good, but needs a little tweak. I'd go for this variant of > it: Tested-by: Nathan Chancellor Thanks a lot for the quick fix! > diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c > index dfa1de89dc944..cb50f8d383606 100644 > --- a/kernel/dma/swiotlb.c > +++ b/kernel/dma/swiotlb.c > @@ -192,7 +192,7 @@ void __init swiotlb_update_mem_attributes(void) > } > > static void swiotlb_init_io_tlb_mem(struct io_tlb_mem *mem, phys_addr_t > start, > - unsigned long nslabs, bool late_alloc) > + unsigned long nslabs, unsigned int flags, bool late_alloc) > { > void *vaddr = phys_to_virt(start); > unsigned long bytes = nslabs << IO_TLB_SHIFT, i; > @@ -203,8 +203,7 @@ static void swiotlb_init_io_tlb_mem(struct io_tlb_mem > *mem, phys_addr_t start, > mem->index = 0; > mem->late_alloc = late_alloc; > > - if (swiotlb_force_bounce) > - mem->force_bounce = true; > + mem->force_bounce = swiotlb_force_bounce || (flags & SWIOTLB_FORCE); > > spin_lock_init(&mem->lock); > for (i = 0; i < mem->nslabs; i++) { > @@ -275,8 +274,7 @@ void __init swiotlb_init_remap(bool addressing_limit, > unsigned int flags, > panic("%s: Failed to allocate %zu bytes align=0x%lx\n", > __func__, alloc_size, PAGE_SIZE); > > - swiotlb_init_io_tlb_mem(mem, __pa(tlb), nslabs, false); > - mem->force_bounce = flags & SWIOTLB_FORCE; > + swiotlb_init_io_tlb_mem(mem, __pa(tlb), nslabs, flags, false); > > if (flags & SWIOTLB_VERBOSE) > swiotlb_print_info(); > @@ -348,7 +346,7 @@ int swiotlb_init_late(size_t size, gfp_t gfp_mask, > > set_memory_decrypted((unsigned long)vstart, >(nslabs << IO_TLB_SHIFT) >> PAGE_SHIFT); > - swiotlb_init_io_tlb_mem(mem, virt_to_phys(vstart), nslabs, true); > + swiotlb_init_io_tlb_mem(mem, virt_to_phys(vstart), nslabs, 0, true); > > swiotlb_print_info(); > return 0; > @@ -835,8 +833,8 @@ static int rmem_swiotlb_device_init(struct reserved_mem > *rmem, > > set_memory_decrypted((unsigned long)phys_to_virt(rmem->base), >rmem->size >> PAGE_SHIFT); > - swiotlb_init_io_tlb_mem(mem, rmem->base, nslabs, false); > - mem->force_bounce = true; > + swiotlb_init_io_tlb_mem(mem, rmem->base, nslabs, SWIOTLB_FORCE, > + false); > mem->for_alloc = true; > > rmem->priv = mem; > Cheers, Nathan
Re: [patch V3 28/35] PCI/MSI: Simplify pci_irq_get_affinity()
Hi Thomas, On Fri, Dec 10, 2021 at 11:19:26PM +0100, Thomas Gleixner wrote: > From: Thomas Gleixner > > Replace open coded MSI descriptor chasing and use the proper accessor > functions instead. > > Signed-off-by: Thomas Gleixner > Reviewed-by: Greg Kroah-Hartman > Reviewed-by: Jason Gunthorpe Apologies if this has already been reported somewhere else or already fixed, I did a search of all of lore and did not see anything similar to it and I did not see any new commits in -tip around this. I just bisected a boot failure on my AMD test desktop to this patch as commit f48235900182 ("PCI/MSI: Simplify pci_irq_get_affinity()") in -next. It looks like there is a problem with the NVMe drive after this change according to the logs. Given that the hard drive is not getting mounted for journald to write logs to, I am not really sure how to get them from the machine so I have at least taken a picture of what I see on my screen; open to ideas on that front! https://github.com/nathanchance/bug-files/blob/0d25d78b5bc1d5e9c15192b3bc80676364de8287/f48235900182/crash.jpg Please let me know what information I can provide to make debugging this easier and I am more than happy to apply and test patches as needed. Cheers, Nathan
Re: [patch V3 28/35] PCI/MSI: Simplify pci_irq_get_affinity()
On Sat, Dec 18, 2021 at 11:25:14AM +0100, Thomas Gleixner wrote: > On Fri, Dec 17 2021 at 15:30, Nathan Chancellor wrote: > > On Fri, Dec 10, 2021 at 11:19:26PM +0100, Thomas Gleixner wrote: > > I just bisected a boot failure on my AMD test desktop to this patch as > > commit f48235900182 ("PCI/MSI: Simplify pci_irq_get_affinity()") in > > -next. It looks like there is a problem with the NVMe drive after this > > change according to the logs. Given that the hard drive is not getting > > mounted for journald to write logs to, I am not really sure how to get > > them from the machine so I have at least taken a picture of what I see > > on my screen; open to ideas on that front! > > Bah. Fix below. Tested-by: Nathan Chancellor > Thanks, > > tglx > --- > diff --git a/drivers/pci/msi/msi.c b/drivers/pci/msi/msi.c > index 71802410e2ab..9b4910befeda 100644 > --- a/drivers/pci/msi/msi.c > +++ b/drivers/pci/msi/msi.c > @@ -1100,7 +1100,7 @@ EXPORT_SYMBOL(pci_irq_vector); > */ > const struct cpumask *pci_irq_get_affinity(struct pci_dev *dev, int nr) > { > - int irq = pci_irq_vector(dev, nr); > + int idx, irq = pci_irq_vector(dev, nr); > struct msi_desc *desc; > > if (WARN_ON_ONCE(irq <= 0)) > @@ -1113,7 +1113,10 @@ const struct cpumask *pci_irq_get_affinity(struct > pci_dev *dev, int nr) > > if (WARN_ON_ONCE(!desc->affinity)) > return NULL; > - return &desc->affinity[nr].mask; > + > + /* MSI has a mask array in the descriptor. */ > + idx = dev->msi_enabled ? nr : 0; > + return &desc->affinity[idx].mask; > } > EXPORT_SYMBOL(pci_irq_get_affinity); > >
Re: [PATCH v2] x86/setup: call early_reserve_memory() earlier
On Mon, Sep 20, 2021 at 02:04:21PM +0200, Juergen Gross wrote: > Commit a799c2bd29d19c565 ("x86/setup: Consolidate early memory > reservations") introduced early_reserve_memory() to do all needed > initial memblock_reserve() calls in one function. Unfortunately the > call of early_reserve_memory() is done too late for Xen dom0, as in > some cases a Xen hook called by e820__memory_setup() will need those > memory reservations to have happened already. > > Move the call of early_reserve_memory() before the call of > e820__memory_setup() in order to avoid such problems. > > Cc: sta...@vger.kernel.org > Fixes: a799c2bd29d19c565 ("x86/setup: Consolidate early memory reservations") > Reported-by: Marek Marczykowski-Górecki > Signed-off-by: Juergen Gross I had issues on an AMD Ryzen 3 4300G based system with v1. v2 does not trigger any boot issues on that same machine or an Intel i5-4210U based system that I also test with. Tested-by: Nathan Chancellor > --- > V2: > - update comment (Jan Beulich, Boris Petkov) > - move call down in setup_arch() (Mike Galbraith) > --- > arch/x86/kernel/setup.c | 26 ++ > 1 file changed, 14 insertions(+), 12 deletions(-) > > diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c > index 79f164141116..40ed44ead063 100644 > --- a/arch/x86/kernel/setup.c > +++ b/arch/x86/kernel/setup.c > @@ -830,6 +830,20 @@ void __init setup_arch(char **cmdline_p) > > x86_init.oem.arch_setup(); > > + /* > + * Do some memory reservations *before* memory is added to memblock, so > + * memblock allocations won't overwrite it. > + * > + * After this point, everything still needed from the boot loader or > + * firmware or kernel text should be early reserved or marked not RAM in > + * e820. All other memory is free game. > + * > + * This call needs to happen before e820__memory_setup() which calls the > + * xen_memory_setup() on Xen dom0 which relies on the fact that those > + * early reservations have happened already. > + */ > + early_reserve_memory(); > + > iomem_resource.end = (1ULL << boot_cpu_data.x86_phys_bits) - 1; > e820__memory_setup(); > parse_setup_data(); > @@ -876,18 +890,6 @@ void __init setup_arch(char **cmdline_p) > > parse_early_param(); > > - /* > - * Do some memory reservations *before* memory is added to > - * memblock, so memblock allocations won't overwrite it. > - * Do it after early param, so we could get (unlikely) panic from > - * serial. > - * > - * After this point everything still needed from the boot loader or > - * firmware or kernel text should be early reserved or marked not > - * RAM in e820. All other memory is free game. > - */ > - early_reserve_memory(); > - > #ifdef CONFIG_MEMORY_HOTPLUG > /* >* Memory used by the kernel cannot be hot-removed because Linux > -- > 2.26.2 > >