[PATCH] media: dt-bindings: qcom,sc7280-venus: Allow one IOMMU entry
Some SC7280-based boards crash when providing the "secure_non_pixel" context bank, so allow only one iommu in the bindings also. Signed-off-by: Luca Weiss --- Reference: https://lore.kernel.org/linux-arm-msm/20231201-sc7280-venus-pas-v3-2-bc132dc5f...@fairphone.com/ --- Documentation/devicetree/bindings/media/qcom,sc7280-venus.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/media/qcom,sc7280-venus.yaml b/Documentation/devicetree/bindings/media/qcom,sc7280-venus.yaml index 8f9b6433aeb8..10c334e6b3dc 100644 --- a/Documentation/devicetree/bindings/media/qcom,sc7280-venus.yaml +++ b/Documentation/devicetree/bindings/media/qcom,sc7280-venus.yaml @@ -43,6 +43,7 @@ properties: - const: vcodec_bus iommus: +minItems: 1 maxItems: 2 interconnects: --- base-commit: 596764183be8ebb13352b281a442a1f1151c9b06 change-id: 20240129-sc7280-venus-bindings-6e62a99620de Best regards, -- Luca Weiss
Re: [PATCH RFC v3 01/35] mm: page_alloc: Add gfp_flags parameter to arch_alloc_page()
On 1/25/24 22:12, Alexandru Elisei wrote: > Extend the usefulness of arch_alloc_page() by adding the gfp_flags > parameter. Although the change here is harmless in itself, it will definitely benefit from some additional context explaining the rationale, taking into account why-how arch_alloc_page() got added particularly for s390 platform and how it's going to be used in the present proposal. > > Signed-off-by: Alexandru Elisei > --- > > Changes since rfc v2: > > * New patch. > > arch/s390/include/asm/page.h | 2 +- > arch/s390/mm/page-states.c | 2 +- > include/linux/gfp.h | 2 +- > mm/page_alloc.c | 2 +- > 4 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/arch/s390/include/asm/page.h b/arch/s390/include/asm/page.h > index 73b9c3bf377f..859f0958c574 100644 > --- a/arch/s390/include/asm/page.h > +++ b/arch/s390/include/asm/page.h > @@ -163,7 +163,7 @@ static inline int page_reset_referenced(unsigned long > addr) > > struct page; > void arch_free_page(struct page *page, int order); > -void arch_alloc_page(struct page *page, int order); > +void arch_alloc_page(struct page *page, int order, gfp_t gfp_flags); > > static inline int devmem_is_allowed(unsigned long pfn) > { > diff --git a/arch/s390/mm/page-states.c b/arch/s390/mm/page-states.c > index 01f9b39e65f5..b986c8b158e3 100644 > --- a/arch/s390/mm/page-states.c > +++ b/arch/s390/mm/page-states.c > @@ -21,7 +21,7 @@ void arch_free_page(struct page *page, int order) > __set_page_unused(page_to_virt(page), 1UL << order); > } > > -void arch_alloc_page(struct page *page, int order) > +void arch_alloc_page(struct page *page, int order, gfp_t gfp_flags) > { > if (!cmma_flag) > return; > diff --git a/include/linux/gfp.h b/include/linux/gfp.h > index de292a007138..9e8aa3d144db 100644 > --- a/include/linux/gfp.h > +++ b/include/linux/gfp.h > @@ -172,7 +172,7 @@ static inline struct zonelist *node_zonelist(int nid, > gfp_t flags) > static inline void arch_free_page(struct page *page, int order) { } > #endif > #ifndef HAVE_ARCH_ALLOC_PAGE > -static inline void arch_alloc_page(struct page *page, int order) { } > +static inline void arch_alloc_page(struct page *page, int order, gfp_t > gfp_flags) { } > #endif > > struct page *__alloc_pages(gfp_t gfp, unsigned int order, int preferred_nid, > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index 150d4f23b010..2c140abe5ee6 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -1485,7 +1485,7 @@ inline void post_alloc_hook(struct page *page, unsigned > int order, > set_page_private(page, 0); > set_page_refcounted(page); > > - arch_alloc_page(page, order); > + arch_alloc_page(page, order, gfp_flags); > debug_pagealloc_map_pages(page, 1 << order); > > /* Otherwise LGTM.
Re: [linus:master] [eventfs] 852e46e239: BUG:unable_to_handle_page_fault_for_address
On Sun, 28 Jan 2024 at 18:59, kernel test robot wrote: > > 21: 48 8b 47 f8 mov-0x8(%rdi),%rax > 25: 48 85 c0test %rax,%rax > 28: 74 11 je 0x3b > 2a:* f6 40 78 02 testb $0x2,0x78(%rax) <-- trapping > instruction So this is struct tracefs_inode *ti = get_tracefs(inode); struct eventfs_inode *ei = ti->private; if (!ei || !ei->is_events || .. in set_top_events_ownership(), and it's that 'ei->is_events' test that oopses. The 'inode' is the incoming argument (in %rdi), and you don't see code generation for the "get_tracefs(inode)" because it's just an offset off the inode. So the "ti->private" read is that read off "-8(%rdi)", because struct tracefs_inode { unsigned long flags; void*private; struct inodevfs_inode; }; so 'private' is 8 bytes below the 'struct inode' pointer. So 'ei' isn't NULL, but it's a bad pointer, and 'ei->is_events' is that "testb $0x2,0x78(%rax)" and it oopses as a result. I don't think this is directly related to that commit 852e46e239ee ("eventfs: Do not create dentries nor inodes in iterate_shared") that the kernel test robot talks about. It looks like some inode creation never filled in the ->private field at all, and it's just garbage. I note that we have code like this: create_dir_dentry(): ... mutex_unlock(&eventfs_mutex); dentry = create_dir(ei, parent); mutex_lock(&eventfs_mutex); ... if (!ei->dentry && !ei->is_freed) { ei->dentry = dentry; eventfs_post_create_dir(ei); dentry->d_fsdata = ei; } else { and that eventfs_post_create_dir() seems to be where it fills in that ->private pointer: ti = get_tracefs(ei->dentry->d_inode); ti->private = ei; but notice how create_dir() has done that d_instantiate(dentry, inode); which exposes the inode to lookup - long before it's actually then filled in. IOW, what I think is going on is a very basic race, where create_dir_dentry() will allocate the inode and attach it to the dentry long before the inode has been fully initialized. So if somebody comes in *immediately* after that, and does a 'stat()' on that name that now is exposed, it will see an inode that has not yet made it to that eventfs_post_create_dir() phase, and that in turn explains why struct eventfs_inode *ei = ti->private; just reads random garbage values. So if I'm right (big "if" - it looks likely, but who knows) this bug is entirely unrelated to any dentry caching or any reference counting. It just needs just the right timing, where one CPU happens to do a 'stat()' just as another one creates the directory. It's not a big window, so you do need some timing "luck". End result: the d_instantiate() needs to be done *after* the inode has been fully filled in. Alternatively, you could (a) not drop the eventfs_mutex around the create_dir() call (b) take the eventfs_mutex around all of set_top_events_ownership() and just fix it by having the lock protect the lack of ordering. Linus
Re: [PATCH net-next 2/2] tun: AF_XDP Rx zero-copy support
On Sat, Jan 27, 2024 at 5:34 PM wangyunjian wrote: > > > > -Original Message- > > > From: Jason Wang [mailto:jasow...@redhat.com] > > > Sent: Thursday, January 25, 2024 12:49 PM > > > To: wangyunjian > > > Cc: m...@redhat.com; willemdebruijn.ker...@gmail.com; k...@kernel.org; > > > da...@davemloft.net; magnus.karls...@intel.com; > > > net...@vger.kernel.org; linux-kernel@vger.kernel.org; > > > k...@vger.kernel.org; virtualizat...@lists.linux.dev; xudingke > > > > > > Subject: Re: [PATCH net-next 2/2] tun: AF_XDP Rx zero-copy support > > > > > > On Wed, Jan 24, 2024 at 5:38 PM Yunjian Wang > > > > > wrote: > > > > > > > > Now the zero-copy feature of AF_XDP socket is supported by some > > > > drivers, which can reduce CPU utilization on the xdp program. > > > > This patch set allows tun to support AF_XDP Rx zero-copy feature. > > > > > > > > This patch tries to address this by: > > > > - Use peek_len to consume a xsk->desc and get xsk->desc length. > > > > - When the tun support AF_XDP Rx zero-copy, the vq's array maybe empty. > > > > So add a check for empty vq's array in vhost_net_buf_produce(). > > > > - add XDP_SETUP_XSK_POOL and ndo_xsk_wakeup callback support > > > > - add tun_put_user_desc function to copy the Rx data to VM > > > > > > Code explains themselves, let's explain why you need to do this. > > > > > > 1) why you want to use peek_len > > > 2) for "vq's array", what does it mean? > > > 3) from the view of TUN/TAP tun_put_user_desc() is the TX path, so I > > > guess you meant TX zerocopy instead of RX (as I don't see codes for > > > RX?) > > > > OK, I agree and use TX zerocopy instead of RX zerocopy. I meant RX zerocopy > > from the view of vhost-net. > > > > > > > > A big question is how could you handle GSO packets from userspace/guests? > > > > Now by disabling VM's TSO and csum feature. XDP does not support GSO > > packets. > > However, this feature can be added once XDP supports it in the future. > > > > > > > > > > > > > Signed-off-by: Yunjian Wang > > > > --- > > > > drivers/net/tun.c | 165 > > > +++- > > > > drivers/vhost/net.c | 18 +++-- > > > > 2 files changed, 176 insertions(+), 7 deletions(-) > > [...] > > > > > > > > > static int peek_head_len(struct vhost_net_virtqueue *rvq, struct > > > > sock > > > > *sk) { > > > > + struct socket *sock = sk->sk_socket; > > > > struct sk_buff *head; > > > > int len = 0; > > > > unsigned long flags; > > > > > > > > - if (rvq->rx_ring) > > > > - return vhost_net_buf_peek(rvq); > > > > + if (rvq->rx_ring) { > > > > + len = vhost_net_buf_peek(rvq); > > > > + if (likely(len)) > > > > + return len; > > > > + } > > > > + > > > > + if (sock->ops->peek_len) > > > > + return sock->ops->peek_len(sock); > > > > > > What prevents you from reusing the ptr_ring here? Then you don't need > > > the above tricks. > > > > Thank you for your suggestion. I will consider how to reuse the ptr_ring. > > If ptr_ring is used to transfer xdp_descs, there is a problem: After some > xdp_descs are obtained through xsk_tx_peek_desc(), the descs may fail > to be added to ptr_ring. However, no API is available to implement the > rollback function. I don't understand, this issue seems to exist in the physical NIC as well? We get more descriptors than the free slots in the NIC ring. How did other NIC solve this issue? Thanks > > Thanks > > > > > > > > > Thanks > > > > > > > > > > > > > > spin_lock_irqsave(&sk->sk_receive_queue.lock, flags); > > > > head = skb_peek(&sk->sk_receive_queue); > > > > -- > > > > 2.33.0 > > > > >
Re: [PATCH net-next 2/2] tun: AF_XDP Rx zero-copy support
On Thu, Jan 25, 2024 at 8:54 PM wangyunjian wrote: > > > > > -Original Message- > > From: Jason Wang [mailto:jasow...@redhat.com] > > Sent: Thursday, January 25, 2024 12:49 PM > > To: wangyunjian > > Cc: m...@redhat.com; willemdebruijn.ker...@gmail.com; k...@kernel.org; > > da...@davemloft.net; magnus.karls...@intel.com; net...@vger.kernel.org; > > linux-kernel@vger.kernel.org; k...@vger.kernel.org; > > virtualizat...@lists.linux.dev; xudingke > > Subject: Re: [PATCH net-next 2/2] tun: AF_XDP Rx zero-copy support > > > > On Wed, Jan 24, 2024 at 5:38 PM Yunjian Wang > > wrote: > > > > > > Now the zero-copy feature of AF_XDP socket is supported by some > > > drivers, which can reduce CPU utilization on the xdp program. > > > This patch set allows tun to support AF_XDP Rx zero-copy feature. > > > > > > This patch tries to address this by: > > > - Use peek_len to consume a xsk->desc and get xsk->desc length. > > > - When the tun support AF_XDP Rx zero-copy, the vq's array maybe empty. > > > So add a check for empty vq's array in vhost_net_buf_produce(). > > > - add XDP_SETUP_XSK_POOL and ndo_xsk_wakeup callback support > > > - add tun_put_user_desc function to copy the Rx data to VM > > > > Code explains themselves, let's explain why you need to do this. > > > > 1) why you want to use peek_len > > 2) for "vq's array", what does it mean? > > 3) from the view of TUN/TAP tun_put_user_desc() is the TX path, so I guess > > you > > meant TX zerocopy instead of RX (as I don't see codes for > > RX?) > > OK, I agree and use TX zerocopy instead of RX zerocopy. I meant RX zerocopy > from the view of vhost-net. Ok. > > > > > A big question is how could you handle GSO packets from userspace/guests? > > Now by disabling VM's TSO and csum feature. Btw, how could you do that? Thanks
[linus:master] [eventfs] 852e46e239: BUG:unable_to_handle_page_fault_for_address
Hello, kernel test robot noticed "BUG:unable_to_handle_page_fault_for_address" on: commit: 852e46e239ee6db3cd220614cf8bce96e79227c2 ("eventfs: Do not create dentries nor inodes in iterate_shared") https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git master [test failed on linus/master 7ed2632ec7d72e926b9e8bcc9ad1bb0cd37274bf] [test failed on linux-next/master 774551425799cb5bbac94e1768fd69eec4f78dd4] in testcase: stress-ng version: stress-ng-x86_64-1744999cb-1_20240112 with following parameters: nr_threads: 10% disk: 1HDD testtime: 60s fs: xfs class: filesystem test: getdent cpufreq_governor: performance compiler: gcc-12 test machine: 64 threads 2 sockets Intel(R) Xeon(R) Gold 6346 CPU @ 3.10GHz (Ice Lake) with 256G memory (please refer to attached dmesg/kmsg for entire log/backtrace) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot | Closes: https://lore.kernel.org/oe-lkp/202401291043.e62e89dc-oliver.s...@intel.com [ 52.318955][ T4398] BUG: unable to handle page fault for address: 042c04db [ 52.326780][ T4398] #PF: supervisor read access in kernel mode [ 52.332830][ T4398] #PF: error_code(0x) - not-present page [ 52.338864][ T4398] PGD 186970067 P4D 0 [ 52.342993][ T4398] Oops: [#1] SMP NOPTI [ 52.347552][ T4398] CPU: 32 PID: 4398 Comm: stress-ng-getde Not tainted 6.7.0-rc2-00049-g852e46e239ee #1 [ 52.357235][ T4398] Hardware name: Inspur NF5180M6/NF5180M6, BIOS 06.00.04 04/12/2022 [ 52.365278][ T4398] RIP: 0010:set_top_events_ownership (fs/tracefs/event_inode.c:172 (discriminator 1)) [ 52.371424][ T4398] Code: 00 00 66 2e 0f 1f 84 00 00 00 00 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 0f 1f 44 00 00 48 8b 47 f8 48 85 c0 74 11 40 78 02 74 0b 8b 50 50 f7 c2 00 00 08 00 75 05 c3 cc cc cc cc All code 0: 00 00 add%al,(%rax) 2: 66 2e 0f 1f 84 00 00nopw %cs:0x0(%rax,%rax,1) 9: 00 00 00 c: 90 nop d: 90 nop e: 90 nop f: 90 nop 10: 90 nop 11: 90 nop 12: 90 nop 13: 90 nop 14: 90 nop 15: 90 nop 16: 90 nop 17: 90 nop 18: 90 nop 19: 90 nop 1a: 90 nop 1b: 90 nop 1c: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1) 21: 48 8b 47 f8 mov-0x8(%rdi),%rax 25: 48 85 c0test %rax,%rax 28: 74 11 je 0x3b 2a:* f6 40 78 02 testb $0x2,0x78(%rax) <-- trapping instruction 2e: 74 0b je 0x3b 30: 8b 50 50mov0x50(%rax),%edx 33: f7 c2 00 00 08 00 test $0x8,%edx 39: 75 05 jne0x40 3b: c3 retq 3c: cc int3 3d: cc int3 3e: cc int3 3f: cc int3 Code starting with the faulting instruction === 0: f6 40 78 02 testb $0x2,0x78(%rax) 4: 74 0b je 0x11 6: 8b 50 50mov0x50(%rax),%edx 9: f7 c2 00 00 08 00 test $0x8,%edx f: 75 05 jne0x16 11: c3 retq 12: cc int3 13: cc int3 14: cc int3 15: cc int3 [ 52.391324][ T4398] RSP: 0018:ffa00f2efc90 EFLAGS: 00010206 [ 52.397481][ T4398] RAX: 042c0463 RBX: ff110002452ae2c8 RCX: 4000 [ 52.405547][ T4398] RDX: 0024 RSI: ff110002452ae2c8 RDI: ff110002452ae2c8 [ 52.413616][ T4398] RBP: 83080320 R08: 0064 R09: ff1100018a25dd40 [ 52.421686][ T4398] R10: 8c98 R11: R12: 0024 [ 52.429755][ T4398] R13: 83080320 R14: ffa00f2efedc R15: 00018000 [ 52.437828][ T4398] FS: 7f3b60e3d740() GS:ff11001fffe0() knlGS: [ 52.446854][ T4398] CS: 0010 DS: ES: CR0: 80050033 [ 52.453547][ T4398] CR2: 042c04db CR3: 000113750004 CR4: 00771ef0 [ 52.461629][ T4398] DR0: DR1: DR2: [ 52.469707][ T4398] DR3: DR6: fffe0ff0 DR7: 0400 [ 52.477783][ T4398] PKRU: 5554 [ 52.481444][ T4398] Call Trace: [ 52.484848][ T4398] [ 52.487894][ T4398]
[RESEND PATCH v2] modules: wait do_free_init correctly
The commit 1a7b7d922081 ("modules: Use vmalloc special flag") moves do_free_init() into a global workqueue instead of call_rcu(). So now rcu_barrier() can not ensure that do_free_init has completed. We should wait it via flush_work(). Without this fix, we still could encounter false positive reports in W+X checking, and rcu synchronization is unnecessary. Fixes: 1a7b7d922081 ("modules: Use vmalloc special flag") Signed-off-by: Changbin Du Cc: Xiaoyi Su --- v2: fix compilation issue for no CONFIG_MODULES found by 0-DAY. --- include/linux/moduleloader.h | 8 init/main.c | 5 +++-- kernel/module/main.c | 5 + 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/include/linux/moduleloader.h b/include/linux/moduleloader.h index 001b2ce83832..89b1e0ed9811 100644 --- a/include/linux/moduleloader.h +++ b/include/linux/moduleloader.h @@ -115,6 +115,14 @@ int module_finalize(const Elf_Ehdr *hdr, const Elf_Shdr *sechdrs, struct module *mod); +#ifdef CONFIG_MODULES +void flush_module_init_free_work(void); +#else +static inline void flush_module_init_free_work(void) +{ +} +#endif + /* Any cleanup needed when module leaves. */ void module_arch_cleanup(struct module *mod); diff --git a/init/main.c b/init/main.c index e24b0780fdff..f0b7e21ac67f 100644 --- a/init/main.c +++ b/init/main.c @@ -99,6 +99,7 @@ #include #include #include +#include #include #include @@ -1402,11 +1403,11 @@ static void mark_readonly(void) if (rodata_enabled) { /* * load_module() results in W+X mappings, which are cleaned -* up with call_rcu(). Let's make sure that queued work is +* up with init_free_wq. Let's make sure that queued work is * flushed so that we don't hit false positives looking for * insecure pages which are W+X. */ - rcu_barrier(); + flush_module_init_free_work(); mark_rodata_ro(); rodata_test(); } else diff --git a/kernel/module/main.c b/kernel/module/main.c index 36681911c05a..ea66b5c2a2a1 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -2489,6 +2489,11 @@ static void do_free_init(struct work_struct *w) } } +void flush_module_init_free_work(void) +{ + flush_work(&init_free_wq); +} + #undef MODULE_PARAM_PREFIX #define MODULE_PARAM_PREFIX "module." /* Default value for module->async_probe_requested */ -- 2.25.1
Re: [RFC PATCH] kprobes: Use synchronize_rcu_tasks_rude in kprobe_optimizer
Hello, On 2024/1/28 3:22, Paul E. McKenney wrote: On Tue, Jan 09, 2024 at 07:28:29PM +0800, Yang Jihong wrote: Hello, PING. I had a similar problem. Is this solution feasible? Sadly, no. It fails on CONFIG_PREEMPT=y kernels because synchronize_rcu_tasks_rude() will not wait on tasks that have been preempted while executing in a trampoline. But could you please try out the patch shown at the end of this email? Thanks for the patch, yes, I've tested and it solves the problem. Thanx, Paul Thanks, Yang On 2024/1/2 11:40, Chen Zhongjin wrote: There is a deadlock scenario in kprobe_optimizer(): pid A pid B pid C kprobe_optimizer() do_exit() perf_kprobe_init() mutex_lock(&kprobe_mutex) exit_tasks_rcu_start() mutex_lock(&kprobe_mutex) synchronize_rcu_tasks() zap_pid_ns_processes() // waiting kprobe_mutex // waiting tasks_rcu_exit_srcu kernel_wait4() // waiting pid C exit To avoid this deadlock loop, use synchronize_rcu_tasks_rude() in kprobe_optimizer() rather than synchronize_rcu_tasks(). synchronize_rcu_tasks_rude() can also promise that all preempted tasks have scheduled, but it will not wait tasks_rcu_exit_srcu. Signed-off-by: Chen Zhongjin --- arch/Kconfig | 2 +- kernel/kprobes.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/Kconfig b/arch/Kconfig index f4b210ab0612..dc6a18854017 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -104,7 +104,7 @@ config STATIC_CALL_SELFTEST config OPTPROBES def_bool y depends on KPROBES && HAVE_OPTPROBES - select TASKS_RCU if PREEMPTION + select TASKS_RUDE_RCU config KPROBES_ON_FTRACE def_bool y diff --git a/kernel/kprobes.c b/kernel/kprobes.c index d5a0ee40bf66..09056ae50c58 100644 --- a/kernel/kprobes.c +++ b/kernel/kprobes.c @@ -623,7 +623,7 @@ static void kprobe_optimizer(struct work_struct *work) * Note that on non-preemptive kernel, this is transparently converted * to synchronoze_sched() to wait for all interrupts to have completed. */ - synchronize_rcu_tasks(); + synchronize_rcu_tasks_rude(); /* Step 3: Optimize kprobes after quiesence period */ do_optimize_kprobes(); commit bc31e6cb27a9334140ff2f0a209d59b08bc0bc8c Author: Paul E. McKenney Date: Sat Jan 20 07:07:08 2024 -0800 rcu-tasks: Eliminate deadlocks involving do_exit() and RCU tasks Holding a mutex across synchronize_rcu_tasks() and acquiring that same mutex in code called from do_exit() after its call to exit_tasks_rcu_start() but before its call to exit_tasks_rcu_stop() results in deadlock. This is by design, because tasks that are far enough into do_exit() are no longer present on the tasks list, making it a bit difficult for RCU Tasks to find them, let alone wait on them to do a voluntary context switch. However, such deadlocks are becoming more frequent. In addition, lockdep currently does not detect such deadlocks and they can be difficult to reproduce. In addition, if a task voluntarily context switches during that time (for example, if it blocks acquiring a mutex), then this task is in an RCU Tasks quiescent state. And with some adjustments, RCU Tasks could just as well take advantage of that fact. This commit therefore eliminates these deadlock by replacing the SRCU-based wait for do_exit() completion with per-CPU lists of tasks currently exiting. A given task will be on one of these per-CPU lists for the same period of time that this task would previously have been in the previous SRCU read-side critical section. These lists enable RCU Tasks to find the tasks that have already been removed from the tasks list, but that must nevertheless be waited upon. The RCU Tasks grace period gathers any of these do_exit() tasks that it must wait on, and adds them to the list of holdouts. Per-CPU locking and get_task_struct() are used to synchronize addition to and removal from these lists. Link: https://lore.kernel.org/all/20240118021842.290665-1-chenzhong...@huawei.com/ Reported-by: Chen Zhongjin Signed-off-by: Paul E. McKenney diff --git a/include/linux/sched.h b/include/linux/sched.h index cdb8ea53c365..4f0e9274da2d 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -858,6 +858,8 @@ struct task_struct { u8 rcu_tasks_idx; int rcu_tasks_idle_cpu; struct list_headrcu_tasks_holdout_list; + int rcu_tasks_exit_cpu; + struct list_headrcu_tasks_exit_list; #en
Re: [RFC PATCH 2/2] x86/kprobes: boost more instructions from grp2/3/4/5
On 1/27/24 20:22, Masami Hiramatsu (Google) wrote: > On Fri, 26 Jan 2024 22:41:24 -0600 > Jinghao Jia wrote: > >> With the instruction decoder, we are now able to decode and recognize >> instructions with opcode extensions. There are more instructions in >> these groups that can be boosted: >> >> Group 2: ROL, ROR, RCL, RCR, SHL/SAL, SHR, SAR >> Group 3: TEST, NOT, NEG, MUL, IMUL, DIV, IDIV >> Group 4: INC, DEC (byte operation) >> Group 5: INC, DEC (word/doubleword/quadword operation) >> >> These instructions are not boosted previously because there are reserved >> opcodes within the groups, e.g., group 2 with ModR/M.nnn == 110 is >> unmapped. As a result, kprobes attached to them requires two int3 traps >> as being non-boostable also prevents jump-optimization. >> >> Some simple tests on QEMU show that after boosting and jump-optimization >> a single kprobe on these instructions with an empty pre-handler runs 10x >> faster (~1000 cycles vs. ~100 cycles). >> >> Since these instructions are mostly ALU operations and do not touch >> special registers like RIP, let's boost them so that we get the >> performance benefit. >> > > As far as we check the ModR/M byte, I think we can safely run these > instructions on trampoline buffer without adjusting results (this > means it can be "boosted"). > I just have a minor comment, but basically this looks good to me. > > Reviewed-by: Masami Hiramatsu (Google) > >> Signed-off-by: Jinghao Jia >> --- >> arch/x86/kernel/kprobes/core.c | 21 +++-- >> 1 file changed, 15 insertions(+), 6 deletions(-) >> >> diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c >> index 792b38d22126..f847bd9cc91b 100644 >> --- a/arch/x86/kernel/kprobes/core.c >> +++ b/arch/x86/kernel/kprobes/core.c >> @@ -169,22 +169,31 @@ int can_boost(struct insn *insn, void *addr) >> case 0x62: /* bound */ >> case 0x70 ... 0x7f: /* Conditional jumps */ >> case 0x9a: /* Call far */ >> -case 0xc0 ... 0xc1: /* Grp2 */ >> case 0xcc ... 0xce: /* software exceptions */ >> -case 0xd0 ... 0xd3: /* Grp2 */ >> case 0xd6: /* (UD) */ >> case 0xd8 ... 0xdf: /* ESC */ >> case 0xe0 ... 0xe3: /* LOOP*, JCXZ */ >> case 0xe8 ... 0xe9: /* near Call, JMP */ >> case 0xeb: /* Short JMP */ >> case 0xf0 ... 0xf4: /* LOCK/REP, HLT */ >> -case 0xf6 ... 0xf7: /* Grp3 */ >> -case 0xfe: /* Grp4 */ >> /* ... are not boostable */ >> return 0; >> +case 0xc0 ... 0xc1: /* Grp2 */ >> +case 0xd0 ... 0xd3: /* Grp2 */ >> +/* ModR/M nnn == 110 is reserved */ >> +return X86_MODRM_REG(insn->modrm.bytes[0]) != 6; >> +case 0xf6 ... 0xf7: /* Grp3 */ >> +/* ModR/M nnn == 001 is reserved */ > > /* AMD uses nnn == 001 as TEST, but Intel makes it reserved. */ > I will incorporate this into the v2. Since nnn == 001 is still considered reserved by Intel, we still need to prevent it from being boosted, don't we? --Jinghao >> +return X86_MODRM_REG(insn->modrm.bytes[0]) != 1; >> +case 0xfe: /* Grp4 */ >> +/* Only inc and dec are boostable */ >> +return X86_MODRM_REG(insn->modrm.bytes[0]) == 0 || >> + X86_MODRM_REG(insn->modrm.bytes[0]) == 1; >> case 0xff: /* Grp5 */ >> -/* Only indirect jmp is boostable */ >> -return X86_MODRM_REG(insn->modrm.bytes[0]) == 4; >> +/* Only inc, dec, and indirect jmp are boostable */ >> +return X86_MODRM_REG(insn->modrm.bytes[0]) == 0 || >> + X86_MODRM_REG(insn->modrm.bytes[0]) == 1 || >> + X86_MODRM_REG(insn->modrm.bytes[0]) == 4; >> default: >> return 1; >> } >> -- >> 2.43.0 >> > > Thamnk you, > OpenPGP_signature.asc Description: OpenPGP digital signature
Re: [RFC PATCH 1/2] x86/kprobes: Prohibit kprobing on INT and UD
On 1/27/24 19:19, Masami Hiramatsu (Google) wrote: > On Fri, 26 Jan 2024 22:41:23 -0600 > Jinghao Jia wrote: > >> Both INTs (INT n, INT1, INT3, INTO) and UDs (UD0, UD1, UD2) serve >> special purposes in the kernel, e.g., INT3 is used by KGDB and UD2 is >> involved in LLVM-KCFI instrumentation. At the same time, attaching >> kprobes on these instructions (particularly UDs) will pollute the stack >> trace dumped in the kernel ring buffer, since the exception is triggered >> in the copy buffer rather than the original location. >> >> Check for INTs and UDs in can_probe and reject any kprobes trying to >> attach to these instructions. >> > > Thanks for implement this check! > You are very welcome :) > >> Suggested-by: Masami Hiramatsu (Google) >> Signed-off-by: Jinghao Jia >> --- >> arch/x86/kernel/kprobes/core.c | 33 ++--- >> 1 file changed, 26 insertions(+), 7 deletions(-) >> >> diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c >> index e8babebad7b8..792b38d22126 100644 >> --- a/arch/x86/kernel/kprobes/core.c >> +++ b/arch/x86/kernel/kprobes/core.c >> @@ -252,6 +252,22 @@ unsigned long >> recover_probed_instruction(kprobe_opcode_t *buf, unsigned long add >> return __recover_probed_insn(buf, addr); >> } >> >> +static inline int is_exception_insn(struct insn *insn) >> +{ >> +if (insn->opcode.bytes[0] == 0x0f) { >> +/* UD0 / UD1 / UD2 */ >> +return insn->opcode.bytes[1] == 0xff || >> + insn->opcode.bytes[1] == 0xb9 || >> + insn->opcode.bytes[1] == 0x0b; >> +} else { > > If "else" block just return, you don't need this "else". > > bool func() > { > if (cond) > return ... > > return ... > } > > Is preferrable because this puts "return val" always at the end of non-void > function. > I will fix this in the v2. >> +/* INT3 / INT n / INTO / INT1 */ >> +return insn->opcode.bytes[0] == 0xcc || >> + insn->opcode.bytes[0] == 0xcd || >> + insn->opcode.bytes[0] == 0xce || >> + insn->opcode.bytes[0] == 0xf1; >> +} >> +} >> + >> /* Check if paddr is at an instruction boundary */ >> static int can_probe(unsigned long paddr) >> { >> @@ -294,6 +310,16 @@ static int can_probe(unsigned long paddr) >> #endif >> addr += insn.length; >> } >> +__addr = recover_probed_instruction(buf, addr); >> +if (!__addr) >> +return 0; >> + >> +if (insn_decode_kernel(&insn, (void *)__addr) < 0) >> +return 0; >> + >> +if (is_exception_insn(&insn)) >> +return 0; >> + > > Please don't put this outside of decoding loop. You should put these in > the loop which decodes the instruction from the beginning of the function. > Since the x86 instrcution is variable length, can_probe() needs to check > whether that the address is instruction boundary and decodable. > > Thank you, If my understanding is correct then this is trying to decode the kprobe target instruction, given that it is after the main decoding loop. Here I hoisted the decoding logic out of the if(IS_ENABLED(CONFIG_CFI_CLANG)) block so that we do not need to decode the same instruction twice. I left the main decoding loop unchanged so it is still decoding the function from the start and should handle instruction boundaries. Are there any caveats that I missed? --Jinghao > >> if (IS_ENABLED(CONFIG_CFI_CLANG)) { >> /* >> * The compiler generates the following instruction sequence >> @@ -308,13 +334,6 @@ static int can_probe(unsigned long paddr) >> * Also, these movl and addl are used for showing expected >> * type. So those must not be touched. >> */ >> -__addr = recover_probed_instruction(buf, addr); >> -if (!__addr) >> -return 0; >> - >> -if (insn_decode_kernel(&insn, (void *)__addr) < 0) >> -return 0; >> - >> if (insn.opcode.value == 0xBA) >> offset = 12; >> else if (insn.opcode.value == 0x3) >> -- >> 2.43.0 >> > > OpenPGP_signature.asc Description: OpenPGP digital signature
Re: [RFC PATCH 1/2] x86/kprobes: Prohibit kprobing on INT and UD
On 1/27/24 13:47, Xin Li wrote: > On 1/26/2024 8:41 PM, Jinghao Jia wrote: >> Both INTs (INT n, INT1, INT3, INTO) and UDs (UD0, UD1, UD2) serve >> special purposes in the kernel, e.g., INT3 is used by KGDB and UD2 is >> involved in LLVM-KCFI instrumentation. At the same time, attaching >> kprobes on these instructions (particularly UDs) will pollute the stack >> trace dumped in the kernel ring buffer, since the exception is triggered >> in the copy buffer rather than the original location. >> >> Check for INTs and UDs in can_probe and reject any kprobes trying to >> attach to these instructions. >> >> Suggested-by: Masami Hiramatsu (Google) >> Signed-off-by: Jinghao Jia >> --- >> arch/x86/kernel/kprobes/core.c | 33 ++--- >> 1 file changed, 26 insertions(+), 7 deletions(-) >> >> diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c >> index e8babebad7b8..792b38d22126 100644 >> --- a/arch/x86/kernel/kprobes/core.c >> +++ b/arch/x86/kernel/kprobes/core.c >> @@ -252,6 +252,22 @@ unsigned long >> recover_probed_instruction(kprobe_opcode_t *buf, unsigned long add >> return __recover_probed_insn(buf, addr); >> } >> +static inline int is_exception_insn(struct insn *insn) > > s/int/bool > Oh yes, the return type should be bool. Thanks for pointing out! --Jinghao >> +{ >> + if (insn->opcode.bytes[0] == 0x0f) { >> + /* UD0 / UD1 / UD2 */ >> + return insn->opcode.bytes[1] == 0xff || >> + insn->opcode.bytes[1] == 0xb9 || >> + insn->opcode.bytes[1] == 0x0b; >> + } else { >> + /* INT3 / INT n / INTO / INT1 */ >> + return insn->opcode.bytes[0] == 0xcc || >> + insn->opcode.bytes[0] == 0xcd || >> + insn->opcode.bytes[0] == 0xce || >> + insn->opcode.bytes[0] == 0xf1; >> + } >> +} >> + >> /* Check if paddr is at an instruction boundary */ >> static int can_probe(unsigned long paddr) >> { >> @@ -294,6 +310,16 @@ static int can_probe(unsigned long paddr) >> #endif >> addr += insn.length; >> } >> + __addr = recover_probed_instruction(buf, addr); >> + if (!__addr) >> + return 0; >> + >> + if (insn_decode_kernel(&insn, (void *)__addr) < 0) >> + return 0; >> + >> + if (is_exception_insn(&insn)) >> + return 0; >> + >> if (IS_ENABLED(CONFIG_CFI_CLANG)) { >> /* >> * The compiler generates the following instruction sequence >> @@ -308,13 +334,6 @@ static int can_probe(unsigned long paddr) >> * Also, these movl and addl are used for showing expected >> * type. So those must not be touched. >> */ >> - __addr = recover_probed_instruction(buf, addr); >> - if (!__addr) >> - return 0; >> - >> - if (insn_decode_kernel(&insn, (void *)__addr) < 0) >> - return 0; >> - >> if (insn.opcode.value == 0xBA) >> offset = 12; >> else if (insn.opcode.value == 0x3) > OpenPGP_signature.asc Description: OpenPGP digital signature
Re: [PATCH] arm64: dts: qcom: sm7225-fairphone-fp4: Switch firmware ext to .mbn
On Wed, 10 Jan 2024 16:21:19 +0100, Luca Weiss wrote: > Specify the file name for the squashed/non-split firmware with the .mbn > extension instead of the split .mdt. The kernel can load both but the > squashed version is preferred in dts nowadays. > > Applied, thanks! [1/1] arm64: dts: qcom: sm7225-fairphone-fp4: Switch firmware ext to .mbn commit: 410dd97e3f394a1bac444f1964754968557f844d Best regards, -- Bjorn Andersson
Re: [PATCH] ARM: dts: qcom: msm8926-htc-memul: Add rmtfs memory node
On Sun, 21 Jan 2024 11:21:54 +0100, Luca Weiss wrote: > Add the rmtfs-mem node which was part of one of the "unknown" memory > reservation. Split that one, make sure the reserved-memory in total > still covers the same space. > > Applied, thanks! [1/1] ARM: dts: qcom: msm8926-htc-memul: Add rmtfs memory node commit: 713bc594c6334a36d0caf4b98510ba3b6795616a Best regards, -- Bjorn Andersson
Re: [PATCH] arm64: dts: qcom: sc7280: Add static properties to cryptobam
On Fri, 29 Dec 2023 09:51:37 +0100, Luca Weiss wrote: > When the properties num-channels & qcom,num-ees are not specified, the > driver tries to read the values from registers, but this read fails and > resets the device if the interconnect from the qcom,qce node is not > already active when that happens. > > Add the static properties to not touch any registers during probe, the > rest of the time when the BAM is used by QCE then the interconnect will > be active already. > > [...] Applied, thanks! [1/1] arm64: dts: qcom: sc7280: Add static properties to cryptobam commit: 521cb01e12750fe290a3819cfe9334c8ac0d1fb0 Best regards, -- Bjorn Andersson
Re: [PATCH] arm64: dts: qcom: qcm6490-fairphone-fp5: Add missing reserved-memory
On Fri, 29 Dec 2023 13:53:17 +0100, Luca Weiss wrote: > It seems we also need to reserve a region of 81 MiB called "removed_mem" > otherwise we can easily hit the following error with higher RAM usage: > > [ 1467.809274] Internal error: synchronous external abort: 9610 > [#2] SMP > > Applied, thanks! [1/1] arm64: dts: qcom: qcm6490-fairphone-fp5: Add missing reserved-memory commit: acc38e600c52c6d423b374ec6642e5acfaea4710 Best regards, -- Bjorn Andersson
Re: (subset) [PATCH 0/3] Fairphone 5 PMIC-GLINK support (USB-C, charger, fuel gauge)
On Wed, 20 Dec 2023 11:02:55 +0100, Luca Weiss wrote: > This series adds all the necessary bits to enable USB-C role switching, > charger and fuel gauge (all via pmic-glink) on Fairphone 5. > > One thing that could be made different is the pmic-glink compatible. > I've chosen to use qcm6490 compatible for it and not sc7280 since > there's plenty of firmware variety on sc7280-based platforms and they > might require different quirks in the future, so limit this PDOS quirk > to just qcm6490 for now. > > [...] Applied, thanks! [3/3] arm64: dts: qcom: qcm6490-fairphone-fp5: Add PMIC GLINK commit: 4cc920ed7899de91ea39b6c9bdb0ebb6860e8b47 Best regards, -- Bjorn Andersson
[PATCH AUTOSEL 4.19 5/8] virtio_net: Fix "‘%d’ directive writing between 1 and 11 bytes into a region of size 10" warnings
From: Zhu Yanjun [ Upstream commit e3fe8d28c67bf6c291e920c6d04fa22afa14e6e4 ] Fix the warnings when building virtio_net driver. " drivers/net/virtio_net.c: In function ‘init_vqs’: drivers/net/virtio_net.c:4551:48: warning: ‘%d’ directive writing between 1 and 11 bytes into a region of size 10 [-Wformat-overflow=] 4551 | sprintf(vi->rq[i].name, "input.%d", i); |^~ In function ‘virtnet_find_vqs’, inlined from ‘init_vqs’ at drivers/net/virtio_net.c:4645:8: drivers/net/virtio_net.c:4551:41: note: directive argument in the range [-2147483643, 65534] 4551 | sprintf(vi->rq[i].name, "input.%d", i); | ^~ drivers/net/virtio_net.c:4551:17: note: ‘sprintf’ output between 8 and 18 bytes into a destination of size 16 4551 | sprintf(vi->rq[i].name, "input.%d", i); | ^~ drivers/net/virtio_net.c: In function ‘init_vqs’: drivers/net/virtio_net.c:4552:49: warning: ‘%d’ directive writing between 1 and 11 bytes into a region of size 9 [-Wformat-overflow=] 4552 | sprintf(vi->sq[i].name, "output.%d", i); | ^~ In function ‘virtnet_find_vqs’, inlined from ‘init_vqs’ at drivers/net/virtio_net.c:4645:8: drivers/net/virtio_net.c:4552:41: note: directive argument in the range [-2147483643, 65534] 4552 | sprintf(vi->sq[i].name, "output.%d", i); | ^~~ drivers/net/virtio_net.c:4552:17: note: ‘sprintf’ output between 9 and 19 bytes into a destination of size 16 4552 | sprintf(vi->sq[i].name, "output.%d", i); " Reviewed-by: Xuan Zhuo Signed-off-by: Zhu Yanjun Link: https://lore.kernel.org/r/20240104020902.2753599-1-yanjun@intel.com Signed-off-by: Jakub Kicinski Signed-off-by: Sasha Levin --- drivers/net/virtio_net.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 331d74f9281b..2b012d7165cd 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -2727,10 +2727,11 @@ static int virtnet_find_vqs(struct virtnet_info *vi) { vq_callback_t **callbacks; struct virtqueue **vqs; - int ret = -ENOMEM; - int i, total_vqs; const char **names; + int ret = -ENOMEM; + int total_vqs; bool *ctx; + u16 i; /* We expect 1 RX virtqueue followed by 1 TX virtqueue, followed by * possible N-1 RX/TX queue pairs used in multiqueue mode, followed by @@ -2767,8 +2768,8 @@ static int virtnet_find_vqs(struct virtnet_info *vi) for (i = 0; i < vi->max_queue_pairs; i++) { callbacks[rxq2vq(i)] = skb_recv_done; callbacks[txq2vq(i)] = skb_xmit_done; - sprintf(vi->rq[i].name, "input.%d", i); - sprintf(vi->sq[i].name, "output.%d", i); + sprintf(vi->rq[i].name, "input.%u", i); + sprintf(vi->sq[i].name, "output.%u", i); names[rxq2vq(i)] = vi->rq[i].name; names[txq2vq(i)] = vi->sq[i].name; if (ctx) -- 2.43.0
[PATCH AUTOSEL 5.4 08/11] virtio_net: Fix "‘%d’ directive writing between 1 and 11 bytes into a region of size 10" warnings
From: Zhu Yanjun [ Upstream commit e3fe8d28c67bf6c291e920c6d04fa22afa14e6e4 ] Fix the warnings when building virtio_net driver. " drivers/net/virtio_net.c: In function ‘init_vqs’: drivers/net/virtio_net.c:4551:48: warning: ‘%d’ directive writing between 1 and 11 bytes into a region of size 10 [-Wformat-overflow=] 4551 | sprintf(vi->rq[i].name, "input.%d", i); |^~ In function ‘virtnet_find_vqs’, inlined from ‘init_vqs’ at drivers/net/virtio_net.c:4645:8: drivers/net/virtio_net.c:4551:41: note: directive argument in the range [-2147483643, 65534] 4551 | sprintf(vi->rq[i].name, "input.%d", i); | ^~ drivers/net/virtio_net.c:4551:17: note: ‘sprintf’ output between 8 and 18 bytes into a destination of size 16 4551 | sprintf(vi->rq[i].name, "input.%d", i); | ^~ drivers/net/virtio_net.c: In function ‘init_vqs’: drivers/net/virtio_net.c:4552:49: warning: ‘%d’ directive writing between 1 and 11 bytes into a region of size 9 [-Wformat-overflow=] 4552 | sprintf(vi->sq[i].name, "output.%d", i); | ^~ In function ‘virtnet_find_vqs’, inlined from ‘init_vqs’ at drivers/net/virtio_net.c:4645:8: drivers/net/virtio_net.c:4552:41: note: directive argument in the range [-2147483643, 65534] 4552 | sprintf(vi->sq[i].name, "output.%d", i); | ^~~ drivers/net/virtio_net.c:4552:17: note: ‘sprintf’ output between 9 and 19 bytes into a destination of size 16 4552 | sprintf(vi->sq[i].name, "output.%d", i); " Reviewed-by: Xuan Zhuo Signed-off-by: Zhu Yanjun Link: https://lore.kernel.org/r/20240104020902.2753599-1-yanjun@intel.com Signed-off-by: Jakub Kicinski Signed-off-by: Sasha Levin --- drivers/net/virtio_net.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index f6a6678f43b9..4faf3275b1f6 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -2864,10 +2864,11 @@ static int virtnet_find_vqs(struct virtnet_info *vi) { vq_callback_t **callbacks; struct virtqueue **vqs; - int ret = -ENOMEM; - int i, total_vqs; const char **names; + int ret = -ENOMEM; + int total_vqs; bool *ctx; + u16 i; /* We expect 1 RX virtqueue followed by 1 TX virtqueue, followed by * possible N-1 RX/TX queue pairs used in multiqueue mode, followed by @@ -2904,8 +2905,8 @@ static int virtnet_find_vqs(struct virtnet_info *vi) for (i = 0; i < vi->max_queue_pairs; i++) { callbacks[rxq2vq(i)] = skb_recv_done; callbacks[txq2vq(i)] = skb_xmit_done; - sprintf(vi->rq[i].name, "input.%d", i); - sprintf(vi->sq[i].name, "output.%d", i); + sprintf(vi->rq[i].name, "input.%u", i); + sprintf(vi->sq[i].name, "output.%u", i); names[rxq2vq(i)] = vi->rq[i].name; names[txq2vq(i)] = vi->sq[i].name; if (ctx) -- 2.43.0
[PATCH AUTOSEL 5.10 09/13] virtio_net: Fix "‘%d’ directive writing between 1 and 11 bytes into a region of size 10" warnings
From: Zhu Yanjun [ Upstream commit e3fe8d28c67bf6c291e920c6d04fa22afa14e6e4 ] Fix the warnings when building virtio_net driver. " drivers/net/virtio_net.c: In function ‘init_vqs’: drivers/net/virtio_net.c:4551:48: warning: ‘%d’ directive writing between 1 and 11 bytes into a region of size 10 [-Wformat-overflow=] 4551 | sprintf(vi->rq[i].name, "input.%d", i); |^~ In function ‘virtnet_find_vqs’, inlined from ‘init_vqs’ at drivers/net/virtio_net.c:4645:8: drivers/net/virtio_net.c:4551:41: note: directive argument in the range [-2147483643, 65534] 4551 | sprintf(vi->rq[i].name, "input.%d", i); | ^~ drivers/net/virtio_net.c:4551:17: note: ‘sprintf’ output between 8 and 18 bytes into a destination of size 16 4551 | sprintf(vi->rq[i].name, "input.%d", i); | ^~ drivers/net/virtio_net.c: In function ‘init_vqs’: drivers/net/virtio_net.c:4552:49: warning: ‘%d’ directive writing between 1 and 11 bytes into a region of size 9 [-Wformat-overflow=] 4552 | sprintf(vi->sq[i].name, "output.%d", i); | ^~ In function ‘virtnet_find_vqs’, inlined from ‘init_vqs’ at drivers/net/virtio_net.c:4645:8: drivers/net/virtio_net.c:4552:41: note: directive argument in the range [-2147483643, 65534] 4552 | sprintf(vi->sq[i].name, "output.%d", i); | ^~~ drivers/net/virtio_net.c:4552:17: note: ‘sprintf’ output between 9 and 19 bytes into a destination of size 16 4552 | sprintf(vi->sq[i].name, "output.%d", i); " Reviewed-by: Xuan Zhuo Signed-off-by: Zhu Yanjun Link: https://lore.kernel.org/r/20240104020902.2753599-1-yanjun@intel.com Signed-off-by: Jakub Kicinski Signed-off-by: Sasha Levin --- drivers/net/virtio_net.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 2fd5d2b7a209..4029c56dfcf0 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -2819,10 +2819,11 @@ static int virtnet_find_vqs(struct virtnet_info *vi) { vq_callback_t **callbacks; struct virtqueue **vqs; - int ret = -ENOMEM; - int i, total_vqs; const char **names; + int ret = -ENOMEM; + int total_vqs; bool *ctx; + u16 i; /* We expect 1 RX virtqueue followed by 1 TX virtqueue, followed by * possible N-1 RX/TX queue pairs used in multiqueue mode, followed by @@ -2859,8 +2860,8 @@ static int virtnet_find_vqs(struct virtnet_info *vi) for (i = 0; i < vi->max_queue_pairs; i++) { callbacks[rxq2vq(i)] = skb_recv_done; callbacks[txq2vq(i)] = skb_xmit_done; - sprintf(vi->rq[i].name, "input.%d", i); - sprintf(vi->sq[i].name, "output.%d", i); + sprintf(vi->rq[i].name, "input.%u", i); + sprintf(vi->sq[i].name, "output.%u", i); names[rxq2vq(i)] = vi->rq[i].name; names[txq2vq(i)] = vi->sq[i].name; if (ctx) -- 2.43.0
[PATCH AUTOSEL 5.15 14/19] virtio_net: Fix "‘%d’ directive writing between 1 and 11 bytes into a region of size 10" warnings
From: Zhu Yanjun [ Upstream commit e3fe8d28c67bf6c291e920c6d04fa22afa14e6e4 ] Fix the warnings when building virtio_net driver. " drivers/net/virtio_net.c: In function ‘init_vqs’: drivers/net/virtio_net.c:4551:48: warning: ‘%d’ directive writing between 1 and 11 bytes into a region of size 10 [-Wformat-overflow=] 4551 | sprintf(vi->rq[i].name, "input.%d", i); |^~ In function ‘virtnet_find_vqs’, inlined from ‘init_vqs’ at drivers/net/virtio_net.c:4645:8: drivers/net/virtio_net.c:4551:41: note: directive argument in the range [-2147483643, 65534] 4551 | sprintf(vi->rq[i].name, "input.%d", i); | ^~ drivers/net/virtio_net.c:4551:17: note: ‘sprintf’ output between 8 and 18 bytes into a destination of size 16 4551 | sprintf(vi->rq[i].name, "input.%d", i); | ^~ drivers/net/virtio_net.c: In function ‘init_vqs’: drivers/net/virtio_net.c:4552:49: warning: ‘%d’ directive writing between 1 and 11 bytes into a region of size 9 [-Wformat-overflow=] 4552 | sprintf(vi->sq[i].name, "output.%d", i); | ^~ In function ‘virtnet_find_vqs’, inlined from ‘init_vqs’ at drivers/net/virtio_net.c:4645:8: drivers/net/virtio_net.c:4552:41: note: directive argument in the range [-2147483643, 65534] 4552 | sprintf(vi->sq[i].name, "output.%d", i); | ^~~ drivers/net/virtio_net.c:4552:17: note: ‘sprintf’ output between 9 and 19 bytes into a destination of size 16 4552 | sprintf(vi->sq[i].name, "output.%d", i); " Reviewed-by: Xuan Zhuo Signed-off-by: Zhu Yanjun Link: https://lore.kernel.org/r/20240104020902.2753599-1-yanjun@intel.com Signed-off-by: Jakub Kicinski Signed-off-by: Sasha Levin --- drivers/net/virtio_net.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 3eefe8171925..6a655bd442fe 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -2913,10 +2913,11 @@ static int virtnet_find_vqs(struct virtnet_info *vi) { vq_callback_t **callbacks; struct virtqueue **vqs; - int ret = -ENOMEM; - int i, total_vqs; const char **names; + int ret = -ENOMEM; + int total_vqs; bool *ctx; + u16 i; /* We expect 1 RX virtqueue followed by 1 TX virtqueue, followed by * possible N-1 RX/TX queue pairs used in multiqueue mode, followed by @@ -2953,8 +2954,8 @@ static int virtnet_find_vqs(struct virtnet_info *vi) for (i = 0; i < vi->max_queue_pairs; i++) { callbacks[rxq2vq(i)] = skb_recv_done; callbacks[txq2vq(i)] = skb_xmit_done; - sprintf(vi->rq[i].name, "input.%d", i); - sprintf(vi->sq[i].name, "output.%d", i); + sprintf(vi->rq[i].name, "input.%u", i); + sprintf(vi->sq[i].name, "output.%u", i); names[rxq2vq(i)] = vi->rq[i].name; names[txq2vq(i)] = vi->sq[i].name; if (ctx) -- 2.43.0
[PATCH AUTOSEL 6.1 18/27] virtio_net: Fix "‘%d’ directive writing between 1 and 11 bytes into a region of size 10" warnings
From: Zhu Yanjun [ Upstream commit e3fe8d28c67bf6c291e920c6d04fa22afa14e6e4 ] Fix the warnings when building virtio_net driver. " drivers/net/virtio_net.c: In function ‘init_vqs’: drivers/net/virtio_net.c:4551:48: warning: ‘%d’ directive writing between 1 and 11 bytes into a region of size 10 [-Wformat-overflow=] 4551 | sprintf(vi->rq[i].name, "input.%d", i); |^~ In function ‘virtnet_find_vqs’, inlined from ‘init_vqs’ at drivers/net/virtio_net.c:4645:8: drivers/net/virtio_net.c:4551:41: note: directive argument in the range [-2147483643, 65534] 4551 | sprintf(vi->rq[i].name, "input.%d", i); | ^~ drivers/net/virtio_net.c:4551:17: note: ‘sprintf’ output between 8 and 18 bytes into a destination of size 16 4551 | sprintf(vi->rq[i].name, "input.%d", i); | ^~ drivers/net/virtio_net.c: In function ‘init_vqs’: drivers/net/virtio_net.c:4552:49: warning: ‘%d’ directive writing between 1 and 11 bytes into a region of size 9 [-Wformat-overflow=] 4552 | sprintf(vi->sq[i].name, "output.%d", i); | ^~ In function ‘virtnet_find_vqs’, inlined from ‘init_vqs’ at drivers/net/virtio_net.c:4645:8: drivers/net/virtio_net.c:4552:41: note: directive argument in the range [-2147483643, 65534] 4552 | sprintf(vi->sq[i].name, "output.%d", i); | ^~~ drivers/net/virtio_net.c:4552:17: note: ‘sprintf’ output between 9 and 19 bytes into a destination of size 16 4552 | sprintf(vi->sq[i].name, "output.%d", i); " Reviewed-by: Xuan Zhuo Signed-off-by: Zhu Yanjun Link: https://lore.kernel.org/r/20240104020902.2753599-1-yanjun@intel.com Signed-off-by: Jakub Kicinski Signed-off-by: Sasha Levin --- drivers/net/virtio_net.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 21d3461fb5d1..45f1a871b7da 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -3474,10 +3474,11 @@ static int virtnet_find_vqs(struct virtnet_info *vi) { vq_callback_t **callbacks; struct virtqueue **vqs; - int ret = -ENOMEM; - int i, total_vqs; const char **names; + int ret = -ENOMEM; + int total_vqs; bool *ctx; + u16 i; /* We expect 1 RX virtqueue followed by 1 TX virtqueue, followed by * possible N-1 RX/TX queue pairs used in multiqueue mode, followed by @@ -3514,8 +3515,8 @@ static int virtnet_find_vqs(struct virtnet_info *vi) for (i = 0; i < vi->max_queue_pairs; i++) { callbacks[rxq2vq(i)] = skb_recv_done; callbacks[txq2vq(i)] = skb_xmit_done; - sprintf(vi->rq[i].name, "input.%d", i); - sprintf(vi->sq[i].name, "output.%d", i); + sprintf(vi->rq[i].name, "input.%u", i); + sprintf(vi->sq[i].name, "output.%u", i); names[rxq2vq(i)] = vi->rq[i].name; names[txq2vq(i)] = vi->sq[i].name; if (ctx) -- 2.43.0
[PATCH AUTOSEL 6.6 21/31] virtio_net: Fix "‘%d’ directive writing between 1 and 11 bytes into a region of size 10" warnings
From: Zhu Yanjun [ Upstream commit e3fe8d28c67bf6c291e920c6d04fa22afa14e6e4 ] Fix the warnings when building virtio_net driver. " drivers/net/virtio_net.c: In function ‘init_vqs’: drivers/net/virtio_net.c:4551:48: warning: ‘%d’ directive writing between 1 and 11 bytes into a region of size 10 [-Wformat-overflow=] 4551 | sprintf(vi->rq[i].name, "input.%d", i); |^~ In function ‘virtnet_find_vqs’, inlined from ‘init_vqs’ at drivers/net/virtio_net.c:4645:8: drivers/net/virtio_net.c:4551:41: note: directive argument in the range [-2147483643, 65534] 4551 | sprintf(vi->rq[i].name, "input.%d", i); | ^~ drivers/net/virtio_net.c:4551:17: note: ‘sprintf’ output between 8 and 18 bytes into a destination of size 16 4551 | sprintf(vi->rq[i].name, "input.%d", i); | ^~ drivers/net/virtio_net.c: In function ‘init_vqs’: drivers/net/virtio_net.c:4552:49: warning: ‘%d’ directive writing between 1 and 11 bytes into a region of size 9 [-Wformat-overflow=] 4552 | sprintf(vi->sq[i].name, "output.%d", i); | ^~ In function ‘virtnet_find_vqs’, inlined from ‘init_vqs’ at drivers/net/virtio_net.c:4645:8: drivers/net/virtio_net.c:4552:41: note: directive argument in the range [-2147483643, 65534] 4552 | sprintf(vi->sq[i].name, "output.%d", i); | ^~~ drivers/net/virtio_net.c:4552:17: note: ‘sprintf’ output between 9 and 19 bytes into a destination of size 16 4552 | sprintf(vi->sq[i].name, "output.%d", i); " Reviewed-by: Xuan Zhuo Signed-off-by: Zhu Yanjun Link: https://lore.kernel.org/r/20240104020902.2753599-1-yanjun@intel.com Signed-off-by: Jakub Kicinski Signed-off-by: Sasha Levin --- drivers/net/virtio_net.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index deb2229ab4d8..7cb0548d17a3 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -4096,10 +4096,11 @@ static int virtnet_find_vqs(struct virtnet_info *vi) { vq_callback_t **callbacks; struct virtqueue **vqs; - int ret = -ENOMEM; - int i, total_vqs; const char **names; + int ret = -ENOMEM; + int total_vqs; bool *ctx; + u16 i; /* We expect 1 RX virtqueue followed by 1 TX virtqueue, followed by * possible N-1 RX/TX queue pairs used in multiqueue mode, followed by @@ -4136,8 +4137,8 @@ static int virtnet_find_vqs(struct virtnet_info *vi) for (i = 0; i < vi->max_queue_pairs; i++) { callbacks[rxq2vq(i)] = skb_recv_done; callbacks[txq2vq(i)] = skb_xmit_done; - sprintf(vi->rq[i].name, "input.%d", i); - sprintf(vi->sq[i].name, "output.%d", i); + sprintf(vi->rq[i].name, "input.%u", i); + sprintf(vi->sq[i].name, "output.%u", i); names[rxq2vq(i)] = vi->rq[i].name; names[txq2vq(i)] = vi->sq[i].name; if (ctx) -- 2.43.0
[PATCH AUTOSEL 6.7 29/39] virtio_net: Fix "‘%d’ directive writing between 1 and 11 bytes into a region of size 10" warnings
From: Zhu Yanjun [ Upstream commit e3fe8d28c67bf6c291e920c6d04fa22afa14e6e4 ] Fix the warnings when building virtio_net driver. " drivers/net/virtio_net.c: In function ‘init_vqs’: drivers/net/virtio_net.c:4551:48: warning: ‘%d’ directive writing between 1 and 11 bytes into a region of size 10 [-Wformat-overflow=] 4551 | sprintf(vi->rq[i].name, "input.%d", i); |^~ In function ‘virtnet_find_vqs’, inlined from ‘init_vqs’ at drivers/net/virtio_net.c:4645:8: drivers/net/virtio_net.c:4551:41: note: directive argument in the range [-2147483643, 65534] 4551 | sprintf(vi->rq[i].name, "input.%d", i); | ^~ drivers/net/virtio_net.c:4551:17: note: ‘sprintf’ output between 8 and 18 bytes into a destination of size 16 4551 | sprintf(vi->rq[i].name, "input.%d", i); | ^~ drivers/net/virtio_net.c: In function ‘init_vqs’: drivers/net/virtio_net.c:4552:49: warning: ‘%d’ directive writing between 1 and 11 bytes into a region of size 9 [-Wformat-overflow=] 4552 | sprintf(vi->sq[i].name, "output.%d", i); | ^~ In function ‘virtnet_find_vqs’, inlined from ‘init_vqs’ at drivers/net/virtio_net.c:4645:8: drivers/net/virtio_net.c:4552:41: note: directive argument in the range [-2147483643, 65534] 4552 | sprintf(vi->sq[i].name, "output.%d", i); | ^~~ drivers/net/virtio_net.c:4552:17: note: ‘sprintf’ output between 9 and 19 bytes into a destination of size 16 4552 | sprintf(vi->sq[i].name, "output.%d", i); " Reviewed-by: Xuan Zhuo Signed-off-by: Zhu Yanjun Link: https://lore.kernel.org/r/20240104020902.2753599-1-yanjun@intel.com Signed-off-by: Jakub Kicinski Signed-off-by: Sasha Levin --- drivers/net/virtio_net.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 51b1868d2f22..1caf21fd5032 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -4096,10 +4096,11 @@ static int virtnet_find_vqs(struct virtnet_info *vi) { vq_callback_t **callbacks; struct virtqueue **vqs; - int ret = -ENOMEM; - int i, total_vqs; const char **names; + int ret = -ENOMEM; + int total_vqs; bool *ctx; + u16 i; /* We expect 1 RX virtqueue followed by 1 TX virtqueue, followed by * possible N-1 RX/TX queue pairs used in multiqueue mode, followed by @@ -4136,8 +4137,8 @@ static int virtnet_find_vqs(struct virtnet_info *vi) for (i = 0; i < vi->max_queue_pairs; i++) { callbacks[rxq2vq(i)] = skb_recv_done; callbacks[txq2vq(i)] = skb_xmit_done; - sprintf(vi->rq[i].name, "input.%d", i); - sprintf(vi->sq[i].name, "output.%d", i); + sprintf(vi->rq[i].name, "input.%u", i); + sprintf(vi->sq[i].name, "output.%u", i); names[rxq2vq(i)] = vi->rq[i].name; names[txq2vq(i)] = vi->sq[i].name; if (ctx) -- 2.43.0
[PATCH AUTOSEL 6.7 19/39] tracefs/eventfs: Use root and instance inodes as default ownership
From: "Steven Rostedt (Google)" [ Upstream commit 8186fff7ab649085e2c60d032d9a20a85af1d87c ] Instead of walking the dentries on mount/remount to update the gid values of all the dentries if a gid option is specified on mount, just update the root inode. Add .getattr, .setattr, and .permissions on the tracefs inode operations to update the permissions of the files and directories. For all files and directories in the top level instance: /sys/kernel/tracing/* It will use the root inode as the default permissions. The inode that represents: /sys/kernel/tracing (or wherever it is mounted). When an instance is created: mkdir /sys/kernel/tracing/instance/foo The directory "foo" and all its files and directories underneath will use the default of what foo is when it was created. A remount of tracefs will not affect it. If a user were to modify the permissions of any file or directory in tracefs, it will also no longer be modified by a change in ownership of a remount. The events directory, if it is in the top level instance, will use the tracefs root inode as the default ownership for itself and all the files and directories below it. For the events directory in an instance ("foo"), it will keep the ownership of what it was when it was created, and that will be used as the default ownership for the files and directories beneath it. Link: https://lore.kernel.org/linux-trace-kernel/CAHk-=wjvdgkjdxbbvln2wbznqp4ush46e3gqj9m7ug6dpx2...@mail.gmail.com/ Link: https://lore.kernel.org/linux-trace-kernel/20240103215016.1e0c9...@gandalf.local.home Cc: Masami Hiramatsu Cc: Mathieu Desnoyers Cc: Linus Torvalds Cc: Al Viro Cc: Christian Brauner Cc: Greg Kroah-Hartman Signed-off-by: Steven Rostedt (Google) Signed-off-by: Sasha Levin --- fs/tracefs/event_inode.c | 79 +++- fs/tracefs/inode.c | 198 ++- fs/tracefs/internal.h| 3 + 3 files changed, 190 insertions(+), 90 deletions(-) diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c index f0677ea0ec24..517f1ae1a058 100644 --- a/fs/tracefs/event_inode.c +++ b/fs/tracefs/event_inode.c @@ -45,6 +45,7 @@ enum { EVENTFS_SAVE_MODE = BIT(16), EVENTFS_SAVE_UID= BIT(17), EVENTFS_SAVE_GID= BIT(18), + EVENTFS_TOPLEVEL= BIT(19), }; #define EVENTFS_MODE_MASK (EVENTFS_SAVE_MODE - 1) @@ -117,10 +118,17 @@ static int eventfs_set_attr(struct mnt_idmap *idmap, struct dentry *dentry, * The events directory dentry is never freed, unless its * part of an instance that is deleted. It's attr is the * default for its child files and directories. -* Do not update it. It's not used for its own mode or ownership +* Do not update it. It's not used for its own mode or ownership. */ - if (!ei->is_events) + if (ei->is_events) { + /* But it still needs to know if it was modified */ + if (iattr->ia_valid & ATTR_UID) + ei->attr.mode |= EVENTFS_SAVE_UID; + if (iattr->ia_valid & ATTR_GID) + ei->attr.mode |= EVENTFS_SAVE_GID; + } else { update_attr(&ei->attr, iattr); + } } else { name = dentry->d_name.name; @@ -138,9 +146,66 @@ static int eventfs_set_attr(struct mnt_idmap *idmap, struct dentry *dentry, return ret; } +static void update_top_events_attr(struct eventfs_inode *ei, struct dentry *dentry) +{ + struct inode *inode; + + /* Only update if the "events" was on the top level */ + if (!ei || !(ei->attr.mode & EVENTFS_TOPLEVEL)) + return; + + /* Get the tracefs root inode. */ + inode = d_inode(dentry->d_sb->s_root); + ei->attr.uid = inode->i_uid; + ei->attr.gid = inode->i_gid; +} + +static void set_top_events_ownership(struct inode *inode) +{ + struct tracefs_inode *ti = get_tracefs(inode); + struct eventfs_inode *ei = ti->private; + struct dentry *dentry; + + /* The top events directory doesn't get automatically updated */ + if (!ei || !ei->is_events || !(ei->attr.mode & EVENTFS_TOPLEVEL)) + return; + + dentry = ei->dentry; + + update_top_events_attr(ei, dentry); + + if (!(ei->attr.mode & EVENTFS_SAVE_UID)) + inode->i_uid = ei->attr.uid; + + if (!(ei->attr.mode & EVENTFS_SAVE_GID)) + inode->i_gid = ei->attr.gid; +} + +static int eventfs_get_attr(struct mnt_idmap *idmap, + const struct path *path, struct kstat *stat, + u32 request_mask, unsigned int flags) +{ + struct dentry *dentry = path->dentry; + struct inode *inode = d_backing_inode(dentry); + + set_top_events_ownership(inode); + +
[PATCH AUTOSEL 6.7 13/39] ring-buffer: Do no swap cpu buffers if order is different
From: "Steven Rostedt (Google)" [ Upstream commit b81e03a24966dca0b119eff0549a4e44befff419 ] As all the subbuffer order (subbuffer sizes) must be the same throughout the ring buffer, check the order of the buffers that are doing a CPU buffer swap in ring_buffer_swap_cpu() to make sure they are the same. If the are not the same, then fail to do the swap, otherwise the ring buffer will think the CPU buffer has a specific subbuffer size when it does not. Link: https://lore.kernel.org/linux-trace-kernel/20231219185629.467894...@goodmis.org Cc: Masami Hiramatsu Cc: Mark Rutland Cc: Mathieu Desnoyers Cc: Andrew Morton Cc: Tzvetomir Stoyanov Cc: Vincent Donnefort Cc: Kent Overstreet Signed-off-by: Steven Rostedt (Google) Signed-off-by: Sasha Levin --- kernel/trace/ring_buffer.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c index 9286f88fcd32..f9d9309884d1 100644 --- a/kernel/trace/ring_buffer.c +++ b/kernel/trace/ring_buffer.c @@ -5461,6 +5461,9 @@ int ring_buffer_swap_cpu(struct trace_buffer *buffer_a, if (cpu_buffer_a->nr_pages != cpu_buffer_b->nr_pages) goto out; + if (buffer_a->subbuf_order != buffer_b->subbuf_order) + goto out; + ret = -EAGAIN; if (atomic_read(&buffer_a->record_disabled)) -- 2.43.0
Re: [PATCH net-next 2/2] tun: AF_XDP Rx zero-copy support
Hi Yunjian, kernel test robot noticed the following build warnings: [auto build test WARNING on net-next/main] url: https://github.com/intel-lab-lkp/linux/commits/Yunjian-Wang/xsk-Remove-non-zero-dma_page-check-in-xp_assign_dev/20240124-174011 base: net-next/main patch link: https://lore.kernel.org/r/1706089075-16084-1-git-send-email-wangyunjian%40huawei.com patch subject: [PATCH net-next 2/2] tun: AF_XDP Rx zero-copy support config: i386-randconfig-062-20240127 (https://download.01.org/0day-ci/archive/20240128/202401281735.bx1dign9-...@intel.com/config) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240128/202401281735.bx1dign9-...@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot | Closes: https://lore.kernel.org/oe-kbuild-all/202401281735.bx1dign9-...@intel.com/ sparse warnings: (new ones prefixed by >>) >> drivers/net/tun.c:1298:5: sparse: sparse: symbol 'tun_xsk_pool_setup' was >> not declared. Should it be static? drivers/net/tun.c: note: in included file (through include/linux/mmzone.h, include/linux/gfp.h, include/linux/umh.h, include/linux/kmod.h, ...): include/linux/page-flags.h:242:46: sparse: sparse: self-comparison always evaluates to false vim +/tun_xsk_pool_setup +1298 drivers/net/tun.c 1297 > 1298 int tun_xsk_pool_setup(struct net_device *dev, struct xsk_buff_pool > *pool, 1299 u16 qid) 1300 { 1301 return pool ? tun_xsk_pool_enable(dev, pool, qid) : 1302 tun_xsk_pool_disable(dev, qid); 1303 } 1304 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
Re: [RFC PATCH 2/5] mfd: add 88pm88x driver
Lee, thank you for your feedback. On Thu Jan 25, 2024 at 1:26 PM CET, Lee Jones wrote: [...] > > +#define PM88X_REG_INT_STATUS1 0x05 > > + > > +#define PM88X_REG_INT_ENA_10x0a > > +#define PM88X_INT_ENA1_ONKEY BIT(0) > > + > > +enum pm88x_irq_number { > > + PM88X_IRQ_ONKEY, > > + > > + PM88X_MAX_IRQ > > +}; > > An enum for a single IRQ? There will be a lot more IRQs but I have only added this one so far as it is the only one used by this series -- is that OK? > > +static struct reg_sequence pm886_presets[] = { > > + /* disable watchdog */ > > + REG_SEQ0(PM88X_REG_WDOG, 0x01), > > Easier to read if you place spaces between them. > > > + /* GPIO1: DVC, GPIO0: input */ > > + REG_SEQ0(PM88X_REG_GPIO_CTRL1, 0x40), > > Shouldn't you set these up using Pintrl? You mean to add a new MFD cell for the pins and write the respective driver? The downstream implementation has no such thing so I'm not sure if I would be able to do that from scratch. > > + /* GPIO2: input */ > > + REG_SEQ0(PM88X_REG_GPIO_CTRL2, 0x00), > > + /* DVC2, DVC1 */ > > Please unify all of the comments. > > They all use a different structure. > > > + REG_SEQ0(PM88X_REG_GPIO_CTRL3, 0x44), > > + /* GPIO5V_1:input, GPIO5V_2: input */ > > + REG_SEQ0(PM88X_REG_GPIO_CTRL4, 0x00), > > + /* output 32 kHz from XO */ > > + REG_SEQ0(PM88X_REG_AON_CTRL2, 0x2a), > > + /* OSC_FREERUN = 1, to lock FLL */ > > + REG_SEQ0(PM88X_REG_BK_OSC_CTRL1, 0x0f), > > + /* XO_LJ = 1, enable low jitter for 32 kHz */ > > + REG_SEQ0(PM88X_REG_LOWPOWER2, 0x20), > > + /* OV_VSYS and UV_VSYS1 comparators on VSYS disabled, VSYS_OVER_TH : > > 5.6V */ > > + REG_SEQ0(PM88X_REG_LOWPOWER4, 0xc8), > > + /* set the duty cycle of charger DC/DC to max */ > > + REG_SEQ0(PM88X_REG_BK_OSC_CTRL3, 0xc0), > > These all looks like they should be handled in their respective drivers? > > "patch"ing these in seems like a hack. To be honest, I don't really know why these are required and what effect they have -- the comments above taken from the downstream version are the only thing I have to go by. I might try removing them to see if there is any noticable change and whether they could be added only later with the respective drivers. > > > +}; > > Why this instead of What are you refering to here please? > > +static struct resource onkey_resources[] = { > > + DEFINE_RES_IRQ_NAMED(PM88X_IRQ_ONKEY, "88pm88x-onkey"), > > +}; > > + > > +static struct mfd_cell pm88x_devs[] = { > > + { > > + .name = "88pm88x-onkey", > > + .num_resources = ARRAY_SIZE(onkey_resources), > > + .resources = onkey_resources, > > + .id = -1, > > + }, > > +}; > > It's not an MFD if it only supports a single device. As I have noted above with respect to the IRQ enum and also in the commit message, I have so far only added the parts which there is already use for. I intend to add the other parts along with the respective subdevice drivers, please see my regulator series [1] for an example. I thought this approach would make for shorter and simpler patches and also would allow me to make more informed decisions as I familiarize myself with the downstream subdevice drivers more closely one by one. > > + i2c_set_clientdata(client, chip); > > + > > + device_init_wakeup(&client->dev, 1); > > + > > + chip->regmaps[PM88X_REGMAP_BASE] = devm_regmap_init_i2c(client, > > &pm88x_i2c_regmap); > > + if (IS_ERR(chip->regmaps[PM88X_REGMAP_BASE])) { > > Just define different regmaps if you really need them. You mean not to use an array of regmaps but add new struct members instead? One for each regmap? > > diff --git a/include/linux/mfd/88pm88x.h b/include/linux/mfd/88pm88x.h > > new file mode 100644 > > index ..a34c57447827 > > --- /dev/null > > +++ b/include/linux/mfd/88pm88x.h [...] > > +#define PM88X_REG_ID 0x00 > > + > > +#define PM88X_REG_STATUS1 0x01 > > +#define PM88X_ONKEY_STS1 BIT(0) > > + > > +#define PM88X_REG_MISC_CONFIG1 0x14 > > +#define PM88X_SW_PDOWN BIT(5) > > + > > +#define PM88X_REG_MISC_CONFIG2 0x15 > > +#define PM88X_INT_INV BIT(0) > > +#define PM88X_INT_CLEARBIT(1) > > +#define PM88X_INT_RC 0x00 > > +#define PM88X_INT_WC BIT(1) > > +#define PM88X_INT_MASK_MODEBIT(2) > > + > > +#define PM88X_REG_WDOG 0x1d > > + > > +#define PM88X_REG_LOWPOWER20x21 > > +#define PM88X_REG_LOWPOWER40x23 > > + > > +#define PM88X_REG_GPIO_CTRL1 0x30 > > These don't really need to be spaced out, do they? I have spaced them out already as I expect to add some related definitions to each of these in the future and thought it would then perhaps be more easily readable like this. [1] https://lore.kernel.org/al
Re: [PATCH net-next 2/2] tun: AF_XDP Rx zero-copy support
Hi Yunjian, kernel test robot noticed the following build warnings: [auto build test WARNING on net-next/main] url: https://github.com/intel-lab-lkp/linux/commits/Yunjian-Wang/xsk-Remove-non-zero-dma_page-check-in-xp_assign_dev/20240124-174011 base: net-next/main patch link: https://lore.kernel.org/r/1706089075-16084-1-git-send-email-wangyunjian%40huawei.com patch subject: [PATCH net-next 2/2] tun: AF_XDP Rx zero-copy support config: s390-defconfig (https://download.01.org/0day-ci/archive/20240128/202401281639.ybajz4sh-...@intel.com/config) compiler: s390-linux-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240128/202401281639.ybajz4sh-...@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot | Closes: https://lore.kernel.org/oe-kbuild-all/202401281639.ybajz4sh-...@intel.com/ All warnings (new ones prefixed by >>): >> drivers/net/tun.c:1298:5: warning: no previous prototype for >> 'tun_xsk_pool_setup' [-Wmissing-prototypes] 1298 | int tun_xsk_pool_setup(struct net_device *dev, struct xsk_buff_pool *pool, | ^~ vim +/tun_xsk_pool_setup +1298 drivers/net/tun.c 1297 > 1298 int tun_xsk_pool_setup(struct net_device *dev, struct xsk_buff_pool > *pool, 1299 u16 qid) 1300 { 1301 return pool ? tun_xsk_pool_enable(dev, pool, qid) : 1302 tun_xsk_pool_disable(dev, qid); 1303 } 1304 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki