Re: [PATCH 0/4] CAN TRANSCEIVER: Add support for CAN transceivers
On 09.04.2021 19:10:50, Aswath Govindraju wrote: > The following series of patches add support for CAN transceivers. > > TCAN1042 has a standby signal that needs to be pulled high for > sending/receiving messages[1]. TCAN1043 has a enable signal along with > standby signal that needs to be pulled up for sending/receiving > messages[2], and other combinations of the two lines can be used to put the > transceiver in different states to reduce power consumption. On boards > like the AM654-idk and J721e-evm these signals are controlled using gpios. > > Patch 1 models the transceiver as a phy device tree node with properties > for max bit rate supported, gpio properties for indicating gpio pin numbers > to which standby and enable signals are connected. Please add a patch that adds the driver and the bindings to the MAINTAINERS file. Feel free to add it CAN NETWORK DRIVERS. regards, Marc -- Pengutronix e.K. | Marc Kleine-Budde | Embedded Linux | https://www.pengutronix.de | Vertretung West/Dortmund | Phone: +49-231-2826-924 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | signature.asc Description: PGP signature
Re: [Outreachy kernel] [PATCH] :staging: rtl8723bs: Remove useless led_blink_hdl()
On Wed, Apr 14, 2021 at 08:33:48AM +0200, Fabio M. De Francesco wrote: > On Wednesday, April 14, 2021 7:21:50 AM CEST Dan Carpenter wrote: > > On Tue, Apr 13, 2021 at 10:08:32PM +0200, Fabio M. De Francesco wrote: > > > On Tuesday, April 13, 2021 9:48:44 PM CEST Matthew Wilcox wrote: > > > > On Tue, Apr 13, 2021 at 09:45:03PM +0200, Fabio M. De Francesco > wrote: > > > > > 1) The driver doesn't call that function from anywhere else than > > > > > the > > > > > macro. 2) You have explained that the macro add its symbol to a > > > > > slot > > > > > in an array that would shift all the subsequent elements down if > > > > > that > > > > > macro is not used exactly in the line where it is. > > > > > 3) Dan Carpenter said that that array is full of null functions (or > > > > > empty slots?). > > > > > > > > > > Unless that function is called anonymously dereferencing its > > > > > address > > > > > from the position it occupies in the array, I'm not able to see > > > > > what > > > > > else means can any caller use. > > > > > > > > > > I know I have much less experience than you with C: what can go > > > > > wrong? > > > > > > > > Here's where the driver calls that function: > > > > > > > > $ git grep wlancmds drivers/staging/rtl8723bs/ > > > > drivers/staging/rtl8723bs/core/rtw_cmd.c:static struct cmd_hdl > > > > wlancmds[] = { drivers/staging/rtl8723bs/core/rtw_cmd.c: > > > > if > > > > (pcmd->cmdcode < ARRAY_SIZE(wlancmds)) { > > > > drivers/staging/rtl8723bs/core/rtw_cmd.c: > > > > cmd_hdl > > > > = wlancmds[pcmd->cmdcode].h2cfuns; > > > > > > OK, I had imagined an anonymous call from its location in the array (as > > > I wrote in the last phrase of my message). However, I thought that it > > > could have been an improbable possibility, not a real one. > > > > > > Linux uses a lot of interesting ideas that newcomers like me should > > > learn. Things here are trickier than they appear at first sight. > > > > One trick would be to build the Smatch cross function database. > > > > https://www.spinics.net/lists/smatch/msg00568.html > > > > Then you could do: > > > > $ ~/path/to/smatch_data/db/smdb.py led_blink_hdl > > file | caller | function | type | parameter | key | value | > > drivers/staging/rtl8723bs/core/rtw_cmd.c | rtw_cmd_thread | > > rtw_cmd_thread ptr cmd_hdl | INTERNAL | -1 | | > > uchar(*)(struct adapter*, uchar*) > > drivers/staging/rtl8188eu/core/rtw_cmd.c | rtw_cmd_thread | > > rtw_cmd_thread ptr cmd_hdl | INTERNAL | -1 | | > > uchar(*)(struct adapter*, uchar*) > > drivers/staging/rtl8188eu/core/rtw_cmd.c | rtw_cmd_thread | > > rtw_cmd_thread ptr cmd_hdl | BUF_SIZE | 1 |pbuf | > > 1,4,6,8,12,14,16,19-20,23-24,48,740,884,892,900,960 > > > > > > Which says that led_blink_hdl() is called as a function pointer called > > "cmd_hdl" from rtw_cmd_thread(). > > > > Hm... It says it can be called from either rtw_cmd_thread() function > > (the rtl8723bs or rtl8188eu version) which is not ideal. But also > > basically harmless so whatever... > > > > regards, > > dan carpenter > > > Nice tool, thanks. I'll surely use it when it is needed to find out which > callers use a function pointer. > > However I cannot see how it can help in this context. That function *does* > something, even if I cannot understand why someone needs a function to test > the initialization of a pointer. Furthermore it is actually called by > rtw_cmd_thread() (as you found out by using smatch) that expect one of the > two possible values that led_blink_hdl() returns. > > That said, what trick could I use for the purpose of getting rid of that > function? At this point I'm not sure it could be made. If you look at how this is called: drivers/staging/rtl8723bs/core/rtw_cmd.c 449 memcpy(pcmdbuf, pcmd->parmbuf, pcmd->cmdsz); 450 451 if (pcmd->cmdcode < ARRAY_SIZE(wlancmds)) { 452 cmd_hdl = wlancmds[pcmd->cmdcode].h2cfuns; 453 454 if (cmd_hdl) { 455 ret = cmd_hdl(pcmd->padapter, pcmdbuf); ^^^ 456 pcmd->res = ret; 457 } 458 459 pcmdpriv->cmd_seq++; 460 } else { 461 pcmd->res = H2C_PARAMETERS_ERROR; 462 } 463 464 cmd_hdl = NULL; The led_blink_hdl() function returns success if "pcmdbuf" is non-NULL and fail if it's NULL. "pcmdbuf" is never supposed to be NULL. (The "supposed" caveat is because there may be a race in rtw_sdio_if1_init() which briefly allows a NULL "pcmdbuf", but that is an unrelated bug). Anyway, there is no point to the led_blink_hdl() function. Likely they intended
Re: [PATCH 4/7] mm: Introduce verify_page_range()
On Tue, Apr 13, 2021 at 08:01:08PM -0700, Kees Cook wrote: > So the addr can just be encoded in "int", and no structure is needed at: > > typedef bool (*vpr_fn_t)(pte_t pte); > > static int vpr_fn(pte_t *pte, unsigned long addr, void *data) > { > vpr_fn_t callback = data; > > if (!callback(*pte)) > return addr >> PAGE_SIZE; > return 0; > } > > unsigned long verify_page_range(struct mm_struct *mm, > unsigned long addr, unsigned long size, > vpr_fn_t callback) > { > return apply_to_page_range(mm, addr, size, vpr_fn, callback) << > PAGE_SIZE; > } > > But maybe I'm missing something? That covers only (32+12) bits of address space and will mostly work, but we definitely support architectures (very much including x86_64) with larger address spaces than that.
Re: [RFC/PATCH] powerpc/smp: Add SD_SHARE_PKG_RESOURCES flag to MC sched-domain
Hello Mel, On Mon, Apr 12, 2021 at 04:24:44PM +0100, Mel Gorman wrote: > On Mon, Apr 12, 2021 at 02:21:47PM +0200, Vincent Guittot wrote: > > > > Peter, Valentin, Vincent, Mel, etal > > > > > > > > On architectures where we have multiple levels of cache access latencies > > > > within a DIE, (For example: one within the current LLC or SMT core and > > > > the > > > > other at MC or Hemisphere, and finally across hemispheres), do you have > > > > any > > > > suggestions on how we could handle the same in the core scheduler? > > > > I would say that SD_SHARE_PKG_RESOURCES is there for that and doesn't > > only rely on cache > > > > >From topology.c > > SD_SHARE_PKG_RESOURCES - describes shared caches > Yes, I was aware of this shared caches, but this current patch was the simplest way to achieve the effect, though the cores in the MC domain on POWER10 do not share a cache. However, it is relatively faster to transfer data across the cores within the MC domain compared to the cores outside the MC domain in the Die. > I'm guessing here because I am not familiar with power10 but the central > problem appears to be when to prefer selecting a CPU sharing L2 or L3 > cache and the core assumes the last-level-cache is the only relevant one. > On POWER we have traditionally preferred to keep the LLC at the sched-domain comprising of groups of CPUs that share the L2 (since L3 is a victim cache on POWER). On POWER9, the L2 was shared by the threads of a pair of SMT4 cores, while on POWER10, L2 is shared by threads of a single SMT4 core. Thus, the current task wake-up logic would have a lower probability of finding an idle core inside an LLC since it has only one core to search in the LLC. This is why moving the LLC to the parent domain (MC) consisting of a group of SMT4 cores among which snooping the cache-data is faster is helpful for workloads that require the single threaded performance. > For this patch, I wondered if setting SD_SHARE_PKG_RESOURCES would have > unintended consequences for load balancing because load within a die may > not be spread between SMT4 domains if SD_SHARE_PKG_RESOURCES was set at > the MC level. Since we are adding the SD_SHARE_PKG_RESOURCES to the parent of the the only sched-domain (which is a SMT4 domain) which currently has this flag set, would it cause issues in spreading the load between the SMT4 domains ? Are there any tests/benchmarks that can help bring this out? It could be good to understand this. > > > > > > > Minimally I think it would be worth detecting when there are multiple > > > LLCs per node and detecting that in generic code as a static branch. In > > > select_idle_cpu, consider taking two passes -- first on the LLC domain > > > and if no idle CPU is found then taking a second pass if the search depth > > > > We have done a lot of changes to reduce and optimize the fast path and > > I don't think re adding another layer in the fast path makes sense as > > you will end up unrolling the for_each_domain behind some > > static_banches. > > > > Searching the node would only happen if a) there was enough search depth > left and b) there were no idle CPUs at the LLC level. As no new domain > is added, it's not clear to me why for_each_domain would change. > > But still, your comment reminded me that different architectures have > different requirements > > Power 10 appears to prefer CPU selection sharing L2 cache but desires > spillover to L3 when selecting and idle CPU. > Indeed, so on POWER10, the preference would be 1) idle core in the L2 domain. 2) idle core in the MC domain. 3) idle CPU in the L2 domain 4) idle CPU in the MC domain. This patch is able to achieve this *implicitly* because of the way the select_idle_cpu() and the select_idle_core() is currently coded, where in the presence of idle cores in the MC level, the select_idle_core() searches for the idle core starting with the core of the target-CPU. If I understand your proposal correctly it would be to make this explicit into a two level search where we first search in the LLC domain, failing which, we carry on the search in the rest of the die (assuming that the LLC is not in the die). > X86 varies, it might want the Power10 approach for some families and prefer > L3 spilling over to a CPU on the same node in others. > > S390 cares about something called books and drawers although I've no > what it means as such and whether it has any preferences on > search order. > > ARM has similar requirements again according to "scheduler: expose the > topology of clusters and add cluster scheduler" and that one *does* > add another domain. > > I had forgotten about the ARM patches but remembered that they were > interesting because they potentially help the Zen situation but I didn't > get the chance to review them before they fell off my radar again. About > all I recall is that I thought the "cluster" terminology was vague. > > The only commo
Re: [RFC/PATCH] powerpc/smp: Add SD_SHARE_PKG_RESOURCES flag to MC sched-domain
On Mon, Apr 12, 2021 at 06:33:55PM +0200, Michal Suchánek wrote: > On Mon, Apr 12, 2021 at 04:24:44PM +0100, Mel Gorman wrote: > > On Mon, Apr 12, 2021 at 02:21:47PM +0200, Vincent Guittot wrote: > > > > > Peter, Valentin, Vincent, Mel, etal > > > > > > > > > > On architectures where we have multiple levels of cache access > > > > > latencies > > > > > within a DIE, (For example: one within the current LLC or SMT core > > > > > and the > > > > > other at MC or Hemisphere, and finally across hemispheres), do you > > > > > have any > > > > > suggestions on how we could handle the same in the core scheduler? > > > > > > I would say that SD_SHARE_PKG_RESOURCES is there for that and doesn't > > > only rely on cache > > > > > > > From topology.c > > > > SD_SHARE_PKG_RESOURCES - describes shared caches > > > > I'm guessing here because I am not familiar with power10 but the central > > problem appears to be when to prefer selecting a CPU sharing L2 or L3 > > cache and the core assumes the last-level-cache is the only relevant one. > > It does not seem to be the case according to original description: > > When the scheduler tries to wakeup a task, it chooses between the > waker-CPU and the wakee's previous-CPU. Suppose this choice is called > the "target", then in the target's LLC domain, the scheduler > > a) tries to find an idle core in the LLC. This helps exploit the > This is the same as (b) Should this be SMT^^^ ? On POWER10, without this patch, the LLC is at SMT sched-domain domain. The difference between a) and b) is a) searches for an idle core, while b) searches for an idle CPU. > SMT folding that the wakee task can benefit from. If an idle > core is found, the wakee is woken up on it. > > b) Failing to find an idle core, the scheduler tries to find an idle > CPU in the LLC. This helps minimise the wakeup latency for the > wakee since it gets to run on the CPU immediately. > > c) Failing this, it will wake it up on target CPU. > > Thus, with P9-sched topology, since the CACHE domain comprises of two > SMT4 cores, there is a decent chance that we get an idle core, failing > which there is a relatively higher probability of finding an idle CPU > among the 8 threads in the domain. > > However, in P10-sched topology, since the SMT domain is the LLC and it > contains only a single SMT4 core, the probability that we find that > core to be idle is less. Furthermore, since there are only 4 CPUs to > search for an idle CPU, there is lower probability that we can get an > idle CPU to wake up the task on. > > > > > For this patch, I wondered if setting SD_SHARE_PKG_RESOURCES would have > > unintended consequences for load balancing because load within a die may > > not be spread between SMT4 domains if SD_SHARE_PKG_RESOURCES was set at > > the MC level. > > Not spreading load between SMT4 domains within MC is exactly what setting LLC > at MC level would address, wouldn't it? > > As in on P10 we have two relevant levels but the topology as is describes only > one, and moving the LLC level lower gives two levels the scheduler looks at > again. Or am I missing something? This is my current understanding as well, since with this patch we would then be able to move tasks quickly between the SMT4 cores, perhaps at the expense of losing out on cache-affinity. Which is why it would be good to verify this using a test/benchmark. > > Thanks > > Michal > -- Thanks and Regards gautham.
Re: [PATCH] implement flush_cache_vmap for RISC-V
Hi, Le 4/12/21 à 3:08 AM, Jisheng Zhang a écrit : Hi Jiuyang, On Mon, 12 Apr 2021 00:05:30 + Jiuyang Liu wrote: This patch implements flush_cache_vmap for RISC-V, since it modifies PTE. Without this patch, SFENCE.VMA won't be added to related codes, which might introduce a bug in the out-of-order micro-architecture implementations. Signed-off-by: Jiuyang Liu Reviewed-by: Alexandre Ghiti Reviewed-by: Palmer Dabbelt IIRC, Palmer hasn't given this Reviewed-by tag. --- Could you plz add version and changes? IIRC, this is the v3. arch/riscv/include/asm/cacheflush.h | 6 ++ 1 file changed, 6 insertions(+) diff --git a/arch/riscv/include/asm/cacheflush.h b/arch/riscv/include/asm/cacheflush.h index 23ff70350992..3fd528badc35 100644 --- a/arch/riscv/include/asm/cacheflush.h +++ b/arch/riscv/include/asm/cacheflush.h @@ -30,6 +30,12 @@ static inline void flush_dcache_page(struct page *page) #define flush_icache_user_page(vma, pg, addr, len) \ flush_icache_mm(vma->vm_mm, 0) +/* + * flush_cache_vmap is invoked after map_kernel_range() has installed the page + * table entries, which modifies PTE, SFENCE.VMA should be inserted. Just my humble opinion, flush_cache_vmap() may not be necessary. vmalloc_fault can take care of this, and finally sfence.vma is inserted in related path. I believe Palmer and Jisheng are right, my initial proposal to implement flush_cache_vmap is wrong. But then, Jiuyang should not have noticed any problem here, so what's wrong? @Jiuyang: Does implementing flush_cache_vmap fix your issue? And regarding flush_cache_vunmap, from Jisheng call stack, it seems also not necessary. @Jiuyang: Can you tell us more about what you noticed? Regards + */ +#define flush_cache_vmap(start, end) flush_tlb_all() + #ifndef CONFIG_SMP #define flush_icache_all() local_flush_icache_all() -- 2.31.1 ___ linux-riscv mailing list linux-ri...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv
Re: Question on KASAN calltrace record in RT
On Wed, Apr 14, 2021 at 8:58 AM Zhang, Qiang wrote: > > 发件人: Dmitry Vyukov > 发送时间: 2021年4月13日 23:29 > 收件人: Zhang, Qiang > 抄送: Andrew Halaney; andreyk...@gmail.com; ryabinin@gmail.com; > a...@linux-foundation.org; linux-kernel@vger.kernel.org; > kasan-...@googlegroups.com > 主题: Re: Question on KASAN calltrace record in RT > > [Please note: This e-mail is from an EXTERNAL e-mail address] > > On Tue, Apr 6, 2021 at 10:26 AM Zhang, Qiang > wrote: > > > > Hello everyone > > > > In RT system, after Andrew test, found the following calltrace , > > in KASAN, we record callstack through stack_depot_save(), in this function, > > may be call alloc_pages, but in RT, the spin_lock replace with > > rt_mutex in alloc_pages(), if before call this function, the irq is > > disabled, > > will trigger following calltrace. > > > > maybe add array[KASAN_STACK_DEPTH] in struct kasan_track to record > > callstack in RT system. > > > > Is there a better solution ? > > >Hi Qiang, > > > >Adding 2 full stacks per heap object can increase memory usage too >much. > >The stackdepot has a preallocation mechanism, I would start with > >adding interrupts check here: > >https://elixir.bootlin.com/linux/v5.12-rc7/source/lib/stackdepot.c#L294 > >and just not do preallocation in interrupt context. This will solve > >the problem, right? > > It seems to be useful, however, there are the following situations > If there is a lot of stack information that needs to be saved in interrupts, > the memory which has been allocated to hold the stack information is > depletion, when need to save stack again in interrupts, there will be no > memory available . Yes, this is true. This also true now because we allocate with GFP_ATOMIC. This is deliberate design decision. Note that a unique allocation stack is saved only once, so it's enough to be lucky only once per stack. Also interrupts don't tend to allocate thousands of objects. So I think all in all it should work fine in practice. If it turns out to be a problem, we could simply preallocate more memory in RT config. > Thanks > Qiang > > > > Thanks > > Qiang > > > > BUG: sleeping function called from invalid context at > > kernel/locking/rtmutex.c:951 > > [ 14.522262] in_atomic(): 1, irqs_disabled(): 1, non_block: 0, pid: 640, > > name: mount > > [ 14.522304] Call Trace: > > [ 14.522306] dump_stack+0x92/0xc1 > > [ 14.522313] ___might_sleep.cold.99+0x1b0/0x1ef > > [ 14.522319] rt_spin_lock+0x3e/0xc0 > > [ 14.522329] local_lock_acquire+0x52/0x3c0 > > [ 14.522332] get_page_from_freelist+0x176c/0x3fd0 > > [ 14.522543] __alloc_pages_nodemask+0x28f/0x7f0 > > [ 14.522559] stack_depot_save+0x3a1/0x470 > > [ 14.522564] kasan_save_stack+0x2f/0x40 > > [ 14.523575] kasan_record_aux_stack+0xa3/0xb0 > > [ 14.523580] insert_work+0x48/0x340 > > [ 14.523589] __queue_work+0x430/0x1280 > > [ 14.523595] mod_delayed_work_on+0x98/0xf0 > > [ 14.523607] kblockd_mod_delayed_work_on+0x17/0x20 > > [ 14.523611] blk_mq_run_hw_queue+0x151/0x2b0 > > [ 14.523620] blk_mq_sched_insert_request+0x2ad/0x470 > > [ 14.523633] blk_mq_submit_bio+0xd2a/0x2330 > > [ 14.523675] submit_bio_noacct+0x8aa/0xfe0 > > [ 14.523693] submit_bio+0xf0/0x550 > > [ 14.523714] submit_bio_wait+0xfe/0x200 > > [ 14.523724] xfs_rw_bdev+0x370/0x480 [xfs] > > [ 14.523831] xlog_do_io+0x155/0x320 [xfs] > > [ 14.524032] xlog_bread+0x23/0xb0 [xfs] > > [ 14.524133] xlog_find_head+0x131/0x8b0 [xfs] > > [ 14.524375] xlog_find_tail+0xc8/0x7b0 [xfs] > > [ 14.524828] xfs_log_mount+0x379/0x660 [xfs] > > [ 14.524927] xfs_mountfs+0xc93/0x1af0 [xfs] > > [ 14.525424] xfs_fs_fill_super+0x923/0x17f0 [xfs] > > [ 14.525522] get_tree_bdev+0x404/0x680 > > [ 14.525622] vfs_get_tree+0x89/0x2d0 > > [ 14.525628] path_mount+0xeb2/0x19d0 > > [ 14.525648] do_mount+0xcb/0xf0 > > [ 14.525665] __x64_sys_mount+0x162/0x1b0 > > [ 14.525670] do_syscall_64+0x33/0x40 > > [ 14.525674] entry_SYSCALL_64_after_hwframe+0x44/0xae > > [ 14.525677] RIP: 0033:0x7fd6c15eaade
Re: [PATCH] riscv: locks: introduce ticket-based spinlock implementation
On Wed, Apr 14, 2021 at 10:26:57AM +0800, Guo Ren wrote: > Thx Peter, > > On Tue, Apr 13, 2021 at 4:17 PM Peter Zijlstra wrote: > > > > On Tue, Apr 13, 2021 at 10:03:01AM +0200, Peter Zijlstra wrote: > > > > > For ticket locks you really only needs atomic_fetch_add() and > > > smp_store_release() and an architectural guarantees that the > > > atomic_fetch_add() has fwd progress under contention and that a sub-word > > > store (through smp_store_release()) will fail the SC. > > > > > > Then you can do something like: > > > > > > void lock(atomic_t *lock) > > > { > > > u32 val = atomic_fetch_add(1<<16, lock); /* SC, gives us RCsc */ > > > u16 ticket = val >> 16; > > > > > > for (;;) { > > > if (ticket == (u16)val) > > > break; > > > cpu_relax(); > > > val = atomic_read_acquire(lock); > > > } > Should it be? >for (;;) { >if (ticket == (u16)val) { >__atomic_acquire_fence(); >break; >} No, atomic_fetch_add() is full smp_mb(), it even has a comment on that says so. Also, __atomic_acquire_fence() is an implementation detail of atomic, and architectures need not provide it. On top of that, IIRC the atomic _acquire/_release have RCpc ordering, where we want our locks to have RCsc ordering (and very much not weaker than RCtso). Even more so, adding barriers to atomics should really not be conditional.
Re: [PATCH v15 00/12] SMMUv3 Nested Stage Setup (IOMMU part)
Hi Shameer, On 2021/4/14 14:56, Shameerali Kolothum Thodi wrote: -Original Message- From: wangxingang Sent: 14 April 2021 03:36 To: Eric Auger ; eric.auger@gmail.com; jean-phili...@linaro.org; io...@lists.linux-foundation.org; linux-kernel@vger.kernel.org; k...@vger.kernel.org; kvm...@lists.cs.columbia.edu; w...@kernel.org; m...@kernel.org; robin.mur...@arm.com; j...@8bytes.org; alex.william...@redhat.com; t...@semihalf.com; zhukeqian Cc: jacob.jun@linux.intel.com; yi.l@intel.com; zhangfei@linaro.org; zhangfei@gmail.com; vivek.gau...@arm.com; Shameerali Kolothum Thodi ; yuzenghui ; nicoleots...@gmail.com; lushenming ; vse...@nvidia.com; chenxiang (M) ; vdu...@nvidia.com; jiangkunkun Subject: Re: [PATCH v15 00/12] SMMUv3 Nested Stage Setup (IOMMU part) Hi Eric, Jean-Philippe On 2021/4/11 19:12, Eric Auger wrote: SMMUv3 Nested Stage Setup (IOMMU part) This series brings the IOMMU part of HW nested paging support in the SMMUv3. The VFIO part is submitted separately. This is based on Jean-Philippe's [PATCH v14 00/10] iommu: I/O page faults for SMMUv3 https://www.spinics.net/lists/arm-kernel/msg886518.html (including the patches that were not pulled for 5.13) The IOMMU API is extended to support 2 new API functionalities: 1) pass the guest stage 1 configuration 2) pass stage 1 MSI bindings Then those capabilities gets implemented in the SMMUv3 driver. The virtualizer passes information through the VFIO user API which cascades them to the iommu subsystem. This allows the guest to own stage 1 tables and context descriptors (so-called PASID table) while the host owns stage 2 tables and main configuration structures (STE). Best Regards Eric This series can be found at: v5.12-rc6-jean-iopf-14-2stage-v15 (including the VFIO part in its last version: v13) I am testing the performance of an accelerator with/without SVA/vSVA, and found there might be some potential performance loss risk for SVA/vSVA. I use a Network and computing encryption device (SEC), and send 1MB request for 1 times. I trigger mm fault before I send the request, so there should be no iopf. Here's what I got: physical scenario: performance:SVA:9MB/s NOSVA:9MB/s tlb_miss: SVA:302,651 NOSVA:1,223 trans_table_walk_access:SVA:302,276 NOSVA:1,237 VM scenario: performance:vSVA:9MB/s NOvSVA:6MB/s about 30~40% loss tlb_miss: vSVA:4,423,897 NOvSVA:1,907 trans_table_walk_access:vSVA:61,928,430 NOvSVA:21,948 In physical scenario, there's almost no performance loss, but the tlb_miss and trans_table_walk_access of stage 1 for SVA is quite high, comparing to NOSVA. In VM scenario, there's about 30~40% performance loss, this is because the two stage tlb_miss and trans_table_walk_access is even higher, and impact the performance. I compare the procedure of building page table of SVA and NOSVA, and found that NOSVA uses 2MB mapping as far as possible, while SVA uses only 4KB. I retest with huge page, and huge page could solve this problem, the performance of SVA/vSVA is almost the same as NOSVA. I am wondering do you have any other solution for the performance loss of vSVA, or any other method to reduce the tlb_miss/trans_table_walk. Hi Xingang, Just curious, do you have DVM enabled on this board or does it use explicit SMMU TLB invalidations? Thanks, Shameer For now, DVM is enabled and TLBI is not explicit used. And by the way the performance data above is performance:vSVA:9GB/s(not 9MB/s) NOvSVA:6GB/s(not 6GB/s) Thanks Xingang .
Re: [PATCH v3 01/12] iommu: Introduce dirty log tracking framework
Hi Keqian, On 4/13/21 4:54 PM, Keqian Zhu wrote: Some types of IOMMU are capable of tracking DMA dirty log, such as ARM SMMU with HTTU or Intel IOMMU with SLADE. This introduces the dirty log tracking framework in the IOMMU base layer. Three new essential interfaces are added, and we maintaince the status of dirty log tracking in iommu_domain. 1. iommu_switch_dirty_log: Perform actions to start|stop dirty log tracking 2. iommu_sync_dirty_log: Sync dirty log from IOMMU into a dirty bitmap 3. iommu_clear_dirty_log: Clear dirty log of IOMMU by a mask bitmap A new dev feature are added to indicate whether a specific type of iommu hardware supports and its driver realizes them. Signed-off-by: Keqian Zhu Signed-off-by: Kunkun Jiang --- drivers/iommu/iommu.c | 150 ++ include/linux/iommu.h | 53 +++ 2 files changed, 203 insertions(+) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index d0b0a15dba84..667b2d6d2fc0 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -1922,6 +1922,7 @@ static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus, domain->type = type; /* Assume all sizes by default; the driver may override this later */ domain->pgsize_bitmap = bus->iommu_ops->pgsize_bitmap; + mutex_init(&domain->switch_log_lock); return domain; } @@ -2720,6 +2721,155 @@ int iommu_domain_set_attr(struct iommu_domain *domain, } EXPORT_SYMBOL_GPL(iommu_domain_set_attr); +int iommu_switch_dirty_log(struct iommu_domain *domain, bool enable, + unsigned long iova, size_t size, int prot) +{ + const struct iommu_ops *ops = domain->ops; + int ret; + + if (unlikely(!ops || !ops->switch_dirty_log)) + return -ENODEV; + + mutex_lock(&domain->switch_log_lock); + if (enable && domain->dirty_log_tracking) { + ret = -EBUSY; + goto out; + } else if (!enable && !domain->dirty_log_tracking) { + ret = -EINVAL; + goto out; + } + + ret = ops->switch_dirty_log(domain, enable, iova, size, prot); + if (ret) + goto out; + + domain->dirty_log_tracking = enable; +out: + mutex_unlock(&domain->switch_log_lock); + return ret; +} +EXPORT_SYMBOL_GPL(iommu_switch_dirty_log); Since you also added IOMMU_DEV_FEAT_HWDBM, I am wondering what's the difference between iommu_switch_dirty_log(on) vs. iommu_dev_enable_feature(IOMMU_DEV_FEAT_HWDBM) iommu_switch_dirty_log(off) vs. iommu_dev_disable_feature(IOMMU_DEV_FEAT_HWDBM) + +int iommu_sync_dirty_log(struct iommu_domain *domain, unsigned long iova, +size_t size, unsigned long *bitmap, +unsigned long base_iova, unsigned long bitmap_pgshift) +{ + const struct iommu_ops *ops = domain->ops; + unsigned int min_pagesz; + size_t pgsize; + int ret = 0; + + if (unlikely(!ops || !ops->sync_dirty_log)) + return -ENODEV; + + min_pagesz = 1 << __ffs(domain->pgsize_bitmap); + if (!IS_ALIGNED(iova | size, min_pagesz)) { + pr_err("unaligned: iova 0x%lx size 0x%zx min_pagesz 0x%x\n", + iova, size, min_pagesz); + return -EINVAL; + } + + mutex_lock(&domain->switch_log_lock); + if (!domain->dirty_log_tracking) { + ret = -EINVAL; + goto out; + } + + while (size) { + pgsize = iommu_pgsize(domain, iova, size); + + ret = ops->sync_dirty_log(domain, iova, pgsize, + bitmap, base_iova, bitmap_pgshift); Any reason why do you want to do this in a per-4K page manner? This can lead to a lot of indirect calls and bad performance. How about a sync_dirty_pages()? The same comment applies to other places in this patch series. + if (ret) + break; + + pr_debug("dirty_log_sync handle: iova 0x%lx pagesz 0x%zx\n", +iova, pgsize); + + iova += pgsize; + size -= pgsize; + } +out: + mutex_unlock(&domain->switch_log_lock); + return ret; +} +EXPORT_SYMBOL_GPL(iommu_sync_dirty_log); + +static int __iommu_clear_dirty_log(struct iommu_domain *domain, + unsigned long iova, size_t size, + unsigned long *bitmap, + unsigned long base_iova, + unsigned long bitmap_pgshift) +{ + const struct iommu_ops *ops = domain->ops; + size_t pgsize; + int ret = 0; + + if (unlikely(!ops || !ops->clear_dirty_log)) + return -ENODEV; + + while (size) { + pgsize = iommu_pgsize(domain, iova, size); + + ret = ops->clear_dirty_log(domain, iova, pgsize,
Re: [PATCH v2] drivers: pinctrl: qcom: fix Kconfig dependency on GPIOLIB
On Wed, Apr 14, 2021 at 4:51 AM Julian Braha wrote: > When PINCTRL_MSM is enabled, and GPIOLIB is disabled, > Kbuild gives the following warning: > > WARNING: unmet direct dependencies detected for GPIOLIB_IRQCHIP > Depends on [n]: GPIOLIB [=n] > Selected by [y]: > - PINCTRL_MSM [=y] && PINCTRL [=y] && (ARCH_QCOM || COMPILE_TEST [=y]) > > This is because PINCTRL_MSM selects GPIOLIB_IRQCHIP, > without selecting or depending on GPIOLIB, despite > GPIOLIB_IRQCHIP depending on GPIOLIB. Having PINCTRL_MSM > select GPIOLIB will cause a recursive dependency error. > > Signed-off-by: Julian Braha Patch applied. Yours, Linus Walleij
Re: [PATCH V2 1/1] sched/fair:Reduce unnecessary check preempt in the sched tick
On Wed, Apr 14, 2021 at 10:22:29AM +0800, qianjun.ker...@gmail.com wrote: > From: jun qian > > As you are already set the TIF_NEED_RESCHED, there is no need > to check resched again. Still no justification; does this actually help anything? > Signed-off-by: jun qian > --- > kernel/sched/fair.c | 11 +-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c > index 794c2cb945f8..1a69b5fffe4a 100644 > --- a/kernel/sched/fair.c > +++ b/kernel/sched/fair.c > @@ -4360,19 +4360,26 @@ check_preempt_tick(struct cfs_rq *cfs_rq, struct > sched_entity *curr) > { > unsigned long ideal_runtime, delta_exec; > struct sched_entity *se; > + struct rq *rq = rq_of(cfs_rq); > s64 delta; > > ideal_runtime = sched_slice(cfs_rq, curr); > delta_exec = curr->sum_exec_runtime - curr->prev_sum_exec_runtime; > if (delta_exec > ideal_runtime) { > - resched_curr(rq_of(cfs_rq)); > + if (!test_tsk_need_resched(rq->curr)) > + resched_curr(rq_of(cfs_rq)); > /* >* The current task ran long enough, ensure it doesn't get >* re-elected due to buddy favours. >*/ > clear_buddies(cfs_rq, curr); > return; > - } > + /* > + * If here with TIF_NEED_RESCHED already set from the early entity_tick, > + * there is no need to check again. > + */ > + } else if (test_tsk_need_resched(rq->curr)) > + return; This is horrific style. And, afaict, completely unnecessary.
Re: [PATCH v2 1/3] dt-bindings: gpio: add YAML description for rockchip,gpio-bank
On Tue, Apr 13, 2021 at 12:36 AM Johan Jonker wrote: > Current dts files with "rockchip,gpio-bank" subnodes > are manually verified. In order to automate this process > the text that describes the compatible in rockchip,pinctrl.txt > is removed and converted to YAML in rockchip,gpio-bank.yaml. > > Signed-off-by: Johan Jonker > --- > Changed V2: > changed example gpio nodename Reviewed-by: Linus Walleij Yours, Linus Walleij
Re: [tip: core/rcu] softirq: Don't try waking ksoftirqd before it has been spawned
On 2021-04-12 11:36:45 [-0700], Paul E. McKenney wrote: > > Color me confused. I did not follow the discussion around this > > completely, but wasn't it agreed on that this rcu torture muck can wait > > until the threads are brought up? > > Yes, we can cause rcutorture to wait. But in this case, rcutorture > is just the messenger, and making it wait would simply be ignoring > the message. The message is that someone could invoke any number of > things that wait on a softirq handler's invocation during the interval > before ksoftirqd has been spawned. My memory on this is that the only user, that required this early behaviour, was kprobe which was recently changed to not need it anymore. Which makes the test as the only user that remains. Therefore I thought that this test will be moved to later position (when ksoftirqd is up and running) and that there is no more requirement for RCU to be completely up that early in the boot process. Did I miss anything? > > Thanx, Paul Sebastian
[PATCH] atm: idt77252: remove unused function
Fix the following clang warning: drivers/atm/idt77252.c:1787:1: warning: unused function 'idt77252_fbq_level' [-Wunused-function]. Reported-by: Abaci Robot Signed-off-by: Jiapeng Chong --- drivers/atm/idt77252.c | 6 -- 1 file changed, 6 deletions(-) diff --git a/drivers/atm/idt77252.c b/drivers/atm/idt77252.c index 0c13cac..9e4bd75 100644 --- a/drivers/atm/idt77252.c +++ b/drivers/atm/idt77252.c @@ -1784,12 +1784,6 @@ static int idt77252_proc_read(struct atm_dev *dev, loff_t * pos, /*/ static __inline__ int -idt77252_fbq_level(struct idt77252_dev *card, int queue) -{ - return (readl(SAR_REG_STAT) >> (16 + (queue << 2))) & 0x0f; -} - -static __inline__ int idt77252_fbq_full(struct idt77252_dev *card, int queue) { return (readl(SAR_REG_STAT) >> (16 + (queue << 2))) == 0x0f; -- 1.8.3.1
Re: [PATCH] x86/kprobes: Simplify alloc_insn_page() with __vmalloc_node_range
On Tue, 13 Apr 2021 22:00:30 +0900 Masami Hiramatsu wrote: > > > Hi, Hi > > On Tue, 13 Apr 2021 18:03:24 +0800 > Jisheng Zhang wrote: > > > Use the __vmalloc_node_range() to simplify x86's alloc_insn_page() > > implementation. > > Have you checked this is equivarent to the original code on > all architecture? IIRC, some arch has a special module_alloc(), Indeed, this isn't equivarent to the original code. FWICT, the differences on x86 are: 1) module_alloc() allocates a special vmalloc range 2) module_alloc() randomizes the return address via. module_load_offset() 3) module_alloc() also supports kasan instrumentation by kasan_module_alloc() But I'm not sure whether the above differences are useful for kprobes ss insn slot page or not. Take 1) for example, special range in module_alloc is due to relative jump limitation, modules need to call kernel .text. does kprobes ss ins slot needs this limitation too? Thanks > thus I NACKed similar patch previously. > > Thank you, > > > > > Signed-off-by: Jisheng Zhang > > --- > > arch/x86/kernel/kprobes/core.c | 24 > > 1 file changed, 4 insertions(+), 20 deletions(-) > > > > diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c > > index df776cdca327..75081f3dbe44 100644 > > --- a/arch/x86/kernel/kprobes/core.c > > +++ b/arch/x86/kernel/kprobes/core.c > > @@ -383,26 +383,10 @@ static int prepare_boost(kprobe_opcode_t *buf, struct > > kprobe *p, > > /* Make page to RO mode when allocate it */ > > void *alloc_insn_page(void) > > { > > - void *page; > > - > > - page = module_alloc(PAGE_SIZE); > > - if (!page) > > - return NULL; > > - > > - set_vm_flush_reset_perms(page); > > - /* > > - * First make the page read-only, and only then make it executable to > > - * prevent it from being W+X in between. > > - */ > > - set_memory_ro((unsigned long)page, 1); > > - > > - /* > > - * TODO: Once additional kernel code protection mechanisms are set, > > ensure > > - * that the page was not maliciously altered and it is still zeroed. > > - */ > > - set_memory_x((unsigned long)page, 1); > > - > > - return page; > > + return __vmalloc_node_range(PAGE_SIZE, PAGE_SIZE, VMALLOC_START, > > + VMALLOC_END, GFP_KERNEL, PAGE_KERNEL_ROX, > > + VM_FLUSH_RESET_PERMS, NUMA_NO_NODE, > > + __builtin_return_address(0)); > > } > > > > /* Recover page to RW mode before releasing it */ > > -- > > 2.31.0 > > > > > -- > Masami Hiramatsu
Re: [PATCH v2 resend] mm/memory_hotplug: Make unpopulated zones PCP structures unreachable during hot remove
On Mon, Apr 12, 2021 at 01:08:42PM +0100, Mel Gorman wrote: > zone_pcp_reset allegedly protects against a race with drain_pages > using local_irq_save but this is bogus. local_irq_save only operates > on the local CPU. If memory hotplug is running on CPU A and drain_pages > is running on CPU B, disabling IRQs on CPU A does not affect CPU B and > offers no protection. > > This patch deletes IRQ disable/enable on the grounds that IRQs protect > nothing and assumes the existing hotplug paths guarantees the PCP cannot be > used after zone_pcp_enable(). That should be the case already because all > the pages have been freed and there is no page to put on the PCP lists. > > Signed-off-by: Mel Gorman Reviewed-by: Oscar Salvador -- Oscar Salvador SUSE L3
[PATCH v5 0/2] Add Mediatek Efuse Device Node for MT8192 SoC
From: Ryan Wu This patch adds efuse to read via NVMEM. Ryan Wu (2): dt-bindings: nvmem: mediatek: add support for MediaTek mt8192 SoC arm64: dts: mt8192: add eFuse support for MT8192 SoC Documentation/devicetree/bindings/nvmem/mtk-efuse.txt | 1 + arch/arm64/boot/dts/mediatek/mt8192.dtsi | 5 + 2 files changed, 6 insertions(+) -- 2.6.4
[PATCH v5 1/2] dt-bindings: nvmem: mediatek: add support for MediaTek mt8192 SoC
From: Ryan Wu This updates dt-binding documentation for MediaTek mt8192 Signed-off-by: Ryan Wu --- This patch is based on v5.12-rc2. --- Documentation/devicetree/bindings/nvmem/mtk-efuse.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/nvmem/mtk-efuse.txt b/Documentation/devicetree/bindings/nvmem/mtk-efuse.txt index ef93c3b..dbd4da8 100644 --- a/Documentation/devicetree/bindings/nvmem/mtk-efuse.txt +++ b/Documentation/devicetree/bindings/nvmem/mtk-efuse.txt @@ -7,6 +7,7 @@ Required properties: "mediatek,mt7622-efuse", "mediatek,efuse": for MT7622 "mediatek,mt7623-efuse", "mediatek,efuse": for MT7623 "mediatek,mt8173-efuse" or "mediatek,efuse": for MT8173 + "mediatek,mt8192-efuse", "mediatek,efuse": for MT8192 "mediatek,mt8516-efuse", "mediatek,efuse": for MT8516 - reg: Should contain registers location and length -- 2.6.4
[PATCH v5 2/2] arm64: dts: mt8192: add eFuse support for MT8192 SoC
From: Ryan Wu Add eFuse node to read Mediatek eFuse Signed-off-by: Ryan Wu --- This patch dependents on "arm64: dts: Add Mediatek SoC MT8192 and evaluation board dts and Makefile"[1] mt8192.dtsi file is needed for this patch. Please also accept this patch together with [1]. [1]http://lists.infradead.org/pipermail/linux-mediatek/2020-November/019378.html --- arch/arm64/boot/dts/mediatek/mt8192.dtsi | 5 + 1 file changed, 5 insertions(+) diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi index 9757138..4d4e4de 100644 --- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi +++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi @@ -436,6 +436,11 @@ status = "disable"; }; + efuse: efuse@11c1 { + compatible = "mediatek,mt8192-efuse", +"mediatek,efuse"; + }; + i2c3: i2c3@11cb { compatible = "mediatek,mt8192-i2c"; reg = <0 0x11cb 0 0x1000>, -- 2.6.4
Re: [PATCH] mac80211_hwsim: indicate support for 60GHz channels
On Mon, 2021-04-12 at 22:06 -0300, Ramon Fontes wrote: > Advertise 60GHz channels to mac80211. This is wrong. mac80211 doesn't support 60 GHz operation. johannes
[PATCH v6 0/2] staging: rtl8192e: Clean up patchset for style issues in rtl819x_HTProc.c
Changes from v5:- Rebased these patches and dropped 2/3 and made this a patchset of 2. [PATCH v5 1/2]:- No changes. [PATCH V5 2/2]:- No changes. Changes from v4:- [PATCH v4 1/2]:- No changes. [PATCH V4 2/2]:- Removed casts and parentheses. Changes from v3:- Changed subject line to match prefix on the patches. [PATCH v3 1/2]:- No changes. [PATCH V3 2/2]:- No changes. Changes from v2:- [PATCH v2 1/2]:- Modified subject description. Changes has been made in v3. [PATCH v2 2/2]:- Rectified spelling mistake in subject description. Changes has been made in v3. Changes from v1:- [PATCH 1/2]:- Removed unnecessary parentheses around boolean expression. Changes has been made in v2. [PATCH 2/2]:- No changes. Mitali Borkar (2): staging: rtl8192e: remove parentheses around boolean expression staging: rtl8192e: remove casts and parentheses drivers/staging/rtl8192e/rtl819x_HTProc.c | 14 +-- 1 file changed, 5 insertions(+), 9 deletions(-) -- 2.30.2
Re: [PATCH] objtool: prevent memory leak in error paths
On Wed, Apr 14, 2021 at 01:45:11AM +0500, Muhammad Usama Anjum wrote: > Memory allocated by sym and sym->name isn't being freed if some error > occurs in elf_create_undef_symbol(). Free the sym and sym->name if error > is detected before returning NULL. > > Addresses-Coverity: ("Prevent memory leak") -EDONTCARE, objtool is single-shot by design, on error we quit, which frees all memory. Please exclude all of objtool from this class of problems in your Coverity thing.
[PATCH v6 1/2] staging: rtl8192e: remove parentheses around boolean expression
Removed unnecessary parentheses around '!xyz' boolean expression as '!' has higher precedance than '||' Signed-off-by: Mitali Borkar --- Changes from v5:- No changes. Changes from v4:- No changes. Changes from v3:- No changes. Changes from v2:- Modified subject description. Changes has been made in v3. Changes from v1:- Removed unnecessary parentheses around boolean expression. Changes has been made in v2. drivers/staging/rtl8192e/rtl819x_HTProc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/rtl8192e/rtl819x_HTProc.c b/drivers/staging/rtl8192e/rtl819x_HTProc.c index b1fa8e9a4f28..431202927036 100644 --- a/drivers/staging/rtl8192e/rtl819x_HTProc.c +++ b/drivers/staging/rtl8192e/rtl819x_HTProc.c @@ -276,7 +276,7 @@ void HTConstructCapabilityElement(struct rtllib_device *ieee, u8 *posHTCap, struct rt_hi_throughput *pHT = ieee->pHTInfo; struct ht_capab_ele *pCapELE = NULL; - if ((!posHTCap) || (!pHT)) { + if (!posHTCap || !pHT) { netdev_warn(ieee->dev, "%s(): posHTCap and pHTInfo are null\n", __func__); return; -- 2.30.2
[PATCH v6 2/2] staging: rtl8192e: remove casts and parentheses
Removed unnecessary (void *) cast and parentheses to meet linux kernel coding style. Signed-off-by: Mitali Borkar --- Changes from v5:- No changes. Changes from v4:- Removed unnecessary casts and parentheses. Changes from v3:- No changes. Changes from v2:- Rectified spelling mistake in subject description. Changes has been made in v3. Changes from v1:- No changes. drivers/staging/rtl8192e/rtl819x_HTProc.c | 12 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/drivers/staging/rtl8192e/rtl819x_HTProc.c b/drivers/staging/rtl8192e/rtl819x_HTProc.c index 431202927036..ec6b46166e84 100644 --- a/drivers/staging/rtl8192e/rtl819x_HTProc.c +++ b/drivers/staging/rtl8192e/rtl819x_HTProc.c @@ -646,14 +646,10 @@ void HTInitializeHTInfo(struct rtllib_device *ieee) pHTInfo->CurrentMPDUDensity = pHTInfo->MPDU_Density; pHTInfo->CurrentAMPDUFactor = pHTInfo->AMPDU_Factor; - memset((void *)(&pHTInfo->SelfHTCap), 0, - sizeof(pHTInfo->SelfHTCap)); - memset((void *)(&pHTInfo->SelfHTInfo), 0, - sizeof(pHTInfo->SelfHTInfo)); - memset((void *)(&pHTInfo->PeerHTCapBuf), 0, - sizeof(pHTInfo->PeerHTCapBuf)); - memset((void *)(&pHTInfo->PeerHTInfoBuf), 0, - sizeof(pHTInfo->PeerHTInfoBuf)); + memset(&pHTInfo->SelfHTCap, 0, sizeof(pHTInfo->SelfHTCap)); + memset(&pHTInfo->SelfHTInfo, 0, sizeof(pHTInfo->SelfHTInfo)); + memset(&pHTInfo->PeerHTCapBuf, 0, sizeof(pHTInfo->PeerHTCapBuf)); + memset(&pHTInfo->PeerHTInfoBuf, 0, sizeof(pHTInfo->PeerHTInfoBuf)); pHTInfo->bSwBwInProgress = false; -- 2.30.2
Re: [PATCH 0/4 POC] Allow executing code and syscalls in another address space
On 14/04/2021 06:52, Andrei Vagin wrote: We already have process_vm_readv and process_vm_writev to read and write to a process memory faster than we can do this with ptrace. And now it is time for process_vm_exec that allows executing code in an address space of another process. We can do this with ptrace but it is much slower. = Use-cases = Here are two known use-cases. The first one is “application kernel” sandboxes like User-mode Linux and gVisor. In this case, we have a process that runs the sandbox kernel and a set of stub processes that are used to manage guest address spaces. Guest code is executed in the context of stub processes but all system calls are intercepted and handled in the sandbox kernel. Right now, these sort of sandboxes use PTRACE_SYSEMU to trap system calls, but the process_vm_exec can significantly speed them up. Certainly interesting, but will require um to rework most of its memory management and we will most likely need extra mm support to make use of it in UML. We are not likely to get away just with one syscall there. Another use-case is CRIU (Checkpoint/Restore in User-space). Several process properties can be received only from the process itself. Right now, we use a parasite code that is injected into the process. We do this with ptrace but it is slow, unsafe, and tricky. process_vm_exec can simplify the process of injecting a parasite code and it will allow pre-dump memory without stopping processes. The pre-dump here is when we enable a memory tracker and dump the memory while a process is continue running. On each interaction we dump memory that has been changed from the previous iteration. In the final step, we will stop processes and dump their full state. Right now the most effective way to dump process memory is to create a set of pipes and splice memory into these pipes from the parasite code. With process_vm_exec, we will be able to call vmsplice directly. It means that we will not need to stop a process to inject the parasite code. = How it works = process_vm_exec has two modes: * Execute code in an address space of a target process and stop on any signal or system call. * Execute a system call in an address space of a target process. int process_vm_exec(pid_t pid, struct sigcontext uctx, unsigned long flags, siginfo_t siginfo, sigset_t *sigmask, size_t sizemask) PID - target process identification. We can consider to use pidfd instead of PID here. sigcontext contains a process state with what the process will be resumed after switching the address space and then when a process will be stopped, its sate will be saved back to sigcontext. siginfo is information about a signal that has interrupted the process. If a process is interrupted by a system call, signfo will contain a synthetic siginfo of the SIGSYS signal. sigmask is a set of signals that process_vm_exec returns via signfo. # How fast is it In the fourth patch, you can find two benchmarks that execute a function that calls system calls in a loop. ptrace_vm_exe uses ptrace to trap system calls, proces_vm_exec uses the process_vm_exec syscall to do the same thing. ptrace_vm_exec: 1446 ns/syscall ptrocess_vm_exec: 289 ns/syscall PS: This version is just a prototype. Its goal is to collect the initial feedback, to discuss the interfaces, and maybe to get some advice on implementation.. Cc: Andrew Morton Cc: Andy Lutomirski Cc: Anton Ivanov Cc: Christian Brauner Cc: Dmitry Safonov <0x7f454...@gmail.com> Cc: Ingo Molnar Cc: Jeff Dike Cc: Mike Rapoport Cc: Michael Kerrisk (man-pages) Cc: Oleg Nesterov Cc: Peter Zijlstra Cc: Richard Weinberger Cc: Thomas Gleixner Andrei Vagin (4): signal: add a helper to restore a process state from sigcontex arch/x86: implement the process_vm_exec syscall arch/x86: allow to execute syscalls via process_vm_exec selftests: add tests for process_vm_exec arch/Kconfig | 15 ++ arch/x86/Kconfig | 1 + arch/x86/entry/common.c | 19 +++ arch/x86/entry/syscalls/syscall_64.tbl| 1 + arch/x86/include/asm/sigcontext.h | 2 + arch/x86/kernel/Makefile | 1 + arch/x86/kernel/process_vm_exec.c | 160 ++ arch/x86/kernel/signal.c | 125 ++ include/linux/entry-common.h | 2 + include/linux/process_vm_exec.h | 17 ++ include/linux/sched.h | 7 + include/linux/syscalls.h | 6 + include/uapi/asm-generic/unistd.h | 4 +- include/uapi/linux/process_vm_exec.h | 8 + kernel/entry/common.c | 2 +- kernel/fork.c | 9 + kernel/sys_ni.c | 2 + .../selftests/process_vm_exec/Makefile| 7 + tools/t
Re: [PATCH v4 2/2] remoteproc: stm32: add capability to detach
Hello Bjorn On 4/13/21 11:34 PM, Bjorn Andersson wrote: > On Wed 31 Mar 02:33 CDT 2021, Arnaud Pouliquen wrote: > >> A mechanism similar to the shutdown mailbox signal is implemented to >> detach a remote processor. >> >> Upon detachment, a signal is sent to the remote firmware, allowing it >> to perform specific actions such as stopping rpmsg communication. >> >> The Cortex-M hold boot is also disabled to allow the remote processor >> to restart in case of crash. >> >> Signed-off-by: Arnaud Pouliquen >> Reviewed-by: Mathieu Poirier >> Tested-by: Mathieu Poirier >> --- >> drivers/remoteproc/stm32_rproc.c | 39 ++-- >> 1 file changed, 37 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/remoteproc/stm32_rproc.c >> b/drivers/remoteproc/stm32_rproc.c >> index 3d45f51de4d0..7353f9e7e7af 100644 >> --- a/drivers/remoteproc/stm32_rproc.c >> +++ b/drivers/remoteproc/stm32_rproc.c >> @@ -28,7 +28,7 @@ >> #define RELEASE_BOOT1 >> >> #define MBOX_NB_VQ 2 >> -#define MBOX_NB_MBX 3 >> +#define MBOX_NB_MBX 4 >> >> #define STM32_SMC_RCC 0x82001000 >> #define STM32_SMC_REG_WRITE 0x1 >> @@ -38,6 +38,7 @@ >> #define STM32_MBX_VQ1 "vq1" >> #define STM32_MBX_VQ1_ID1 >> #define STM32_MBX_SHUTDOWN "shutdown" >> +#define STM32_MBX_DETACH"detach" >> >> #define RSC_TBL_SIZE1024 >> >> @@ -336,6 +337,15 @@ static const struct stm32_mbox >> stm32_rproc_mbox[MBOX_NB_MBX] = { >> .tx_done = NULL, >> .tx_tout = 500, /* 500 ms time out */ >> }, >> +}, >> +{ >> +.name = STM32_MBX_DETACH, >> +.vq_id = -1, >> +.client = { >> +.tx_block = true, >> +.tx_done = NULL, >> +.tx_tout = 200, /* 200 ms time out to detach should be >> fair enough */ >> +}, >> } >> }; >> >> @@ -461,6 +471,25 @@ static int stm32_rproc_attach(struct rproc *rproc) >> return stm32_rproc_set_hold_boot(rproc, true); >> } >> >> +static int stm32_rproc_detach(struct rproc *rproc) >> +{ >> +struct stm32_rproc *ddata = rproc->priv; >> +int err, dummy_data, idx; >> + >> +/* Inform the remote processor of the detach */ >> +idx = stm32_rproc_mbox_idx(rproc, STM32_MBX_DETACH); >> +if (idx >= 0 && ddata->mb[idx].chan) { >> +/* A dummy data is sent to allow to block on transmit */ >> +err = mbox_send_message(ddata->mb[idx].chan, >> +&dummy_data); > > Seems I posted my comment on v1, rather than this latest version. Please > let me know if we should do anything about this dummy_data. Thanks for pointing this out, you are right, the mailbox driver is stm32_ipcc and it only sends a signal to the remote processor. As message can be queued by the mailbox framework using a local variable seems not a good option. As this code is a copy/past of the kick and stop? I propose to get this one and I will send a new patch to fix the usage in the whole driver. Thanks, Arnaud > > Regards, > Bjorn > >> +if (err < 0) >> +dev_warn(&rproc->dev, "warning: remote FW detach >> without ack\n"); >> +} >> + >> +/* Allow remote processor to auto-reboot */ >> +return stm32_rproc_set_hold_boot(rproc, false); >> +} >> + >> static int stm32_rproc_stop(struct rproc *rproc) >> { >> struct stm32_rproc *ddata = rproc->priv; >> @@ -597,7 +626,12 @@ stm32_rproc_get_loaded_rsc_table(struct rproc *rproc, >> size_t *table_sz) >> } >> >> done: >> -/* Assuming the resource table fits in 1kB is fair */ >> +/* >> + * Assuming the resource table fits in 1kB is fair. >> + * Notice for the detach, that this 1 kB memory area has to be reserved >> in the coprocessor >> + * firmware for the resource table. On detach, the remoteproc core >> re-initializes this >> + * entire area by overwriting it with the initial values stored in >> rproc->clean_table. >> + */ >> *table_sz = RSC_TBL_SIZE; >> return (struct resource_table *)ddata->rsc_va; >> } >> @@ -607,6 +641,7 @@ static const struct rproc_ops st_rproc_ops = { >> .start = stm32_rproc_start, >> .stop = stm32_rproc_stop, >> .attach = stm32_rproc_attach, >> +.detach = stm32_rproc_detach, >> .kick = stm32_rproc_kick, >> .load = rproc_elf_load_segments, >> .parse_fw = stm32_rproc_parse_fw, >> -- >> 2.17.1 >>
Re: [PATCH v3 02/12] iommu: Add iommu_split_block interface
On 4/13/21 4:54 PM, Keqian Zhu wrote: Block(largepage) mapping is not a proper granule for dirty log tracking. Take an extreme example, if DMA writes one byte, under 1G mapping, the dirty amount reported is 1G, but under 4K mapping, the dirty amount is just 4K. This adds a new interface named iommu_split_block in IOMMU base layer. A specific IOMMU driver can invoke it during start dirty log. If so, the driver also need to realize the split_block iommu ops. We flush all iotlbs after the whole procedure is completed to ease the pressure of IOMMU, as we will hanle a huge range of mapping in general. Signed-off-by: Keqian Zhu Signed-off-by: Kunkun Jiang --- drivers/iommu/iommu.c | 41 + include/linux/iommu.h | 11 +++ 2 files changed, 52 insertions(+) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 667b2d6d2fc0..bb413a927870 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -2721,6 +2721,47 @@ int iommu_domain_set_attr(struct iommu_domain *domain, } EXPORT_SYMBOL_GPL(iommu_domain_set_attr); +int iommu_split_block(struct iommu_domain *domain, unsigned long iova, + size_t size) +{ + const struct iommu_ops *ops = domain->ops; + unsigned int min_pagesz; + size_t pgsize; + bool flush = false; + int ret = 0; + + if (unlikely(!ops || !ops->split_block)) + return -ENODEV; + + min_pagesz = 1 << __ffs(domain->pgsize_bitmap); + if (!IS_ALIGNED(iova | size, min_pagesz)) { + pr_err("unaligned: iova 0x%lx size 0x%zx min_pagesz 0x%x\n", + iova, size, min_pagesz); + return -EINVAL; + } + + while (size) { + flush = true; + + pgsize = iommu_pgsize(domain, iova, size); + + ret = ops->split_block(domain, iova, pgsize); + if (ret) + break; + + pr_debug("split handled: iova 0x%lx size 0x%zx\n", iova, pgsize); + + iova += pgsize; + size -= pgsize; + } + + if (flush) + iommu_flush_iotlb_all(domain); + + return ret; +} +EXPORT_SYMBOL_GPL(iommu_split_block); Do you really have any consumers of this interface other than the dirty bit tracking? If not, I don't suggest to make this as a generic IOMMU interface. There is an implicit requirement for such interfaces. The iommu_map/unmap(iova, size) shouldn't be called at the same time. Currently there's no such sanity check in the iommu core. A poorly written driver could mess up the kernel by misusing this interface. This also applies to iommu_merge_page(). Best regards, baolu
Re: [PATCH] x86/kprobes: Simplify alloc_insn_page() with __vmalloc_node_range
Jisheng Zhang wrote: > > > Hi, Hi > > On Tue, 13 Apr 2021 18:03:24 +0800 > Jisheng Zhang wrote: > > > Use the __vmalloc_node_range() to simplify x86's alloc_insn_page() > > implementation. > > Have you checked this is equivarent to the original code on all > architecture? IIRC, some arch has a special module_alloc(), > Indeed, this isn't equivarent to the original code. FWICT, the differences on > x86 are: > 1) module_alloc() allocates a special vmalloc range > 2) module_alloc() randomizes the return address via. module_load_offset() > 3) module_alloc() also supports kasan instrumentation by kasan_module_alloc() > But I'm not sure whether the above differences are useful for kprobes ss > insn slot page or not. Take 1) for example, special range in module_alloc > is due to relative jump limitation, modules need to call kernel .text. does > kprobes ss ins slot needs this limitation too? Oops, I found this wonderful thread: https://www.lkml.org/lkml/2020/7/28/1413 So kprobes ss ins slot page "must be in the range of relative branching only for x86 and arm" And Jarkko's "arch/x86: kprobes: Remove MODULES dependency" series look much better. The last version is v5, I'm not sure whether Jarkko will send new version to mainline the series. thanks
Re: [RFC Part2 PATCH 01/30] x86: Add the host SEV-SNP initialization support
On Wed, Mar 24, 2021 at 12:04:07PM -0500, Brijesh Singh wrote: > @@ -538,6 +540,10 @@ > #define MSR_K8_SYSCFG0xc0010010 > #define MSR_K8_SYSCFG_MEM_ENCRYPT_BIT23 > #define MSR_K8_SYSCFG_MEM_ENCRYPTBIT_ULL(MSR_K8_SYSCFG_MEM_ENCRYPT_BIT) > +#define MSR_K8_SYSCFG_SNP_EN_BIT 24 > +#define MSR_K8_SYSCFG_SNP_EN BIT_ULL(MSR_K8_SYSCFG_SNP_EN_BIT) > +#define MSR_K8_SYSCFG_SNP_VMPL_EN_BIT25 > +#define MSR_K8_SYSCFG_SNP_VMPL_ENBIT_ULL(MSR_K8_SYSCFG_SNP_VMPL_EN_BIT) > #define MSR_K8_INT_PENDING_MSG 0xc0010055 > /* C1E active bits in int pending message */ > #define K8_INTP_C1E_ACTIVE_MASK 0x1800 Ok, I believe it is finally time to make this MSR architectural and drop this silliness with "K8" in the name. If you wanna send me a prepatch which converts all like this: MSR_K8_SYSCFG -> MSR_AMD64_SYSCFG I'll gladly take it. If you prefer me to do it, I'll gladly do it. > @@ -44,12 +45,16 @@ u64 sev_check_data __section(".data") = 0; > EXPORT_SYMBOL(sme_me_mask); > DEFINE_STATIC_KEY_FALSE(sev_enable_key); > EXPORT_SYMBOL_GPL(sev_enable_key); > +DEFINE_STATIC_KEY_FALSE(snp_enable_key); > +EXPORT_SYMBOL_GPL(snp_enable_key); > > bool sev_enabled __section(".data"); > > /* Buffer used for early in-place encryption by BSP, no locking needed */ > static char sme_early_buffer[PAGE_SIZE] __initdata __aligned(PAGE_SIZE); > > +static unsigned long rmptable_start, rmptable_end; __ro_after_init I guess. > + > /* > * When SNP is active, this routine changes the page state from private to > shared before > * copying the data from the source to destination and restore after the > copy. This is required > @@ -528,3 +533,82 @@ void __init mem_encrypt_init(void) > print_mem_encrypt_feature_info(); > } > > +static __init void snp_enable(void *arg) > +{ > + u64 val; > + > + rdmsrl_safe(MSR_K8_SYSCFG, &val); Why is this one _safe but the wrmsr isn't? Also, _safe returns a value - check it pls and return early. > + > + val |= MSR_K8_SYSCFG_SNP_EN; > + val |= MSR_K8_SYSCFG_SNP_VMPL_EN; > + > + wrmsrl(MSR_K8_SYSCFG, val); > +} > + > +static __init int rmptable_init(void) > +{ > + u64 rmp_base, rmp_end; > + unsigned long sz; > + void *start; > + u64 val; > + > + rdmsrl_safe(MSR_AMD64_RMP_BASE, &rmp_base); > + rdmsrl_safe(MSR_AMD64_RMP_END, &rmp_end); Ditto, why _safe if you're checking CPUID? > + > + if (!rmp_base || !rmp_end) { > + pr_info("SEV-SNP: Memory for the RMP table has not been > reserved by BIOS\n"); > + return 1; > + } > + > + sz = rmp_end - rmp_base + 1; > + > + start = memremap(rmp_base, sz, MEMREMAP_WB); > + if (!start) { > + pr_err("SEV-SNP: Failed to map RMP table 0x%llx-0x%llx\n", > rmp_base, rmp_end); ^^^ That prefix is done by doing #undef pr_fmt #define pr_fmt(fmt) "SEV-SNP: " fmt before the SNP-specific functions. > + return 1; > + } > + > + /* > + * Check if SEV-SNP is already enabled, this can happen if we are > coming from kexec boot. > + * Do not initialize the RMP table when SEV-SNP is already. > + */ comment can be 80 cols wide. > + rdmsrl_safe(MSR_K8_SYSCFG, &val); As above. > + if (val & MSR_K8_SYSCFG_SNP_EN) > + goto skip_enable; > + > + /* Initialize the RMP table to zero */ > + memset(start, 0, sz); > + > + /* Flush the caches to ensure that data is written before we enable the > SNP */ > + wbinvd_on_all_cpus(); > + > + /* Enable the SNP feature */ > + on_each_cpu(snp_enable, NULL, 1); What happens if you boot only a subset of the CPUs and then others get hotplugged later? IOW, you need a CPU hotplug notifier which enables the feature bit on newly arrived CPUs. Which makes me wonder whether it makes sense to have this in an initcall and not put it instead in init_amd(): the BSP will do the init work and the APs coming in will see that it has been enabled and only call snp_enable(). Which solves the hotplug thing automagically. > + > +skip_enable: > + rmptable_start = (unsigned long)start; > + rmptable_end = rmptable_start + sz; > + > + pr_info("SEV-SNP: RMP table physical address 0x%016llx - 0x%016llx\n", > rmp_base, rmp_end); "RMP table at ..." also, why is this issued in skip_enable? You want to issue it only once, on enable. also, rmp_base and rmp_end look redundant - you can simply use rmptable_start and rmptable_end. Which reminds me - that function needs to check as the very first thing on entry whether SNP is enabled and exit if so - there's no need to read MSR_AMD64_RMP_BASE and MSR_AMD64_RMP_END unnecessarily. > + > + return 0; > +} > + > +static int __init mem_encrypt_snp_init(void) > +{ > + if (!boot_cpu_has(X86_FEATURE_SEV_SNP)) > + return 1; > + > + if (rmptable_init()) { > +
Re: [Outreachy kernel] Re: [PATCH v3 4/4] staging: media: intel-ipu3: remove space before tabs
On Wed, Apr 14, 2021 at 12:05:04AM +0200, Julia Lawall wrote: > > > On Wed, 14 Apr 2021, Mitali Borkar wrote: > > > On Tue, Apr 13, 2021 at 09:17:12PM +0300, Dan Carpenter wrote: > > > On Tue, Apr 13, 2021 at 08:59:34PM +0530, Mitali Borkar wrote: > > > > Removed unnecessary space before tabs to adhere to linux kernel coding > > > > style. > > > > Reported by checkpatch. > > > > > > > > Signed-off-by: Mitali Borkar > > > > --- > > > > > > > > Changes from v2:- No changes. > > > > Changes from v1:- No changes. > > > > > > > > drivers/staging/media/ipu3/include/intel-ipu3.h | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/staging/media/ipu3/include/intel-ipu3.h > > > > b/drivers/staging/media/ipu3/include/intel-ipu3.h > > > > index 47e98979683c..42edac5ee4e4 100644 > > > > --- a/drivers/staging/media/ipu3/include/intel-ipu3.h > > > > +++ b/drivers/staging/media/ipu3/include/intel-ipu3.h > > > > @@ -633,7 +633,7 @@ struct > > > > ipu3_uapi_bnr_static_config_wb_gains_thr_config { > > > > * @cg:Gain coefficient for threshold calculation, [0, 31], > > > > default 8. > > > > * @ci:Intensity coefficient for threshold calculation. range > > > > [0, 0x1f] > > > > * default 6. > > > > - * format: u3.2 (3 most significant bits represent whole number, > > > > + *format: u3.2 (3 most significant bits represent whole number, > > > > * 2 least significant bits represent the fractional part > > > > > > Just remove the spaces, don't remove the tab. It's looks silly now. > > > > > Okay Sir, do I have to send a v4 patch on this now? > > Yes. If you get feedback on a patch, you should send a new version. v2 of this patch can be used as well, it's fine. (I missed this change in v3.) -- Sakari Ailus
[PATCH V2] watchdog: mtk: support pre-timeout when the bark irq is available
Use the bark interrupt as the pretimeout notifier if available. By default, the pretimeout notification shall occur one second earlier than the timeout. V2: - panic() by default if WATCHDOG_PRETIMEOUT_GOV is not enabled Signed-off-by: Wang Qing --- drivers/watchdog/mtk_wdt.c | 57 ++ 1 file changed, 52 insertions(+), 5 deletions(-) diff --git a/drivers/watchdog/mtk_wdt.c b/drivers/watchdog/mtk_wdt.c index 97ca993..b0ec933 --- a/drivers/watchdog/mtk_wdt.c +++ b/drivers/watchdog/mtk_wdt.c @@ -25,6 +25,7 @@ #include #include #include +#include #define WDT_MAX_TIMEOUT31 #define WDT_MIN_TIMEOUT1 @@ -234,18 +235,41 @@ static int mtk_wdt_start(struct watchdog_device *wdt_dev) void __iomem *wdt_base = mtk_wdt->wdt_base; int ret; - ret = mtk_wdt_set_timeout(wdt_dev, wdt_dev->timeout); + ret = mtk_wdt_set_timeout(wdt_dev, wdt_dev->timeout - wdt_dev->pretimeout); if (ret < 0) return ret; reg = ioread32(wdt_base + WDT_MODE); - reg &= ~(WDT_MODE_IRQ_EN | WDT_MODE_DUAL_EN); + reg &= ~WDT_MODE_IRQ_EN; + if (wdt_dev->pretimeout) + reg |= WDT_MODE_IRQ_EN; + else + reg &= ~WDT_MODE_IRQ_EN; reg |= (WDT_MODE_EN | WDT_MODE_KEY); iowrite32(reg, wdt_base + WDT_MODE); return 0; } +static int mtk_wdt_set_pretimeout(struct watchdog_device *wdd, + unsigned int timeout) +{ + wdd->pretimeout = timeout; + return mtk_wdt_start(wdd); +} + +static irqreturn_t mtk_wdt_isr(int irq, void *arg) +{ + struct watchdog_device *wdd = arg; +if (IS_ENABLED(CONFIG_WATCHDOG_PRETIMEOUT_GOV)) + watchdog_notify_pretimeout(wdd); +else + //panic by decault instead of WDT_SWRST; + panic("MTk Watchdog bark!\n"); + + return IRQ_HANDLED; +} + static const struct watchdog_info mtk_wdt_info = { .identity = DRV_NAME, .options= WDIOF_SETTIMEOUT | @@ -253,12 +277,21 @@ static const struct watchdog_info mtk_wdt_info = { WDIOF_MAGICCLOSE, }; +static const struct watchdog_info mtk_wdt_pt_info = { + .identity = DRV_NAME, + .options= WDIOF_SETTIMEOUT | + WDIOF_PRETIMEOUT | + WDIOF_KEEPALIVEPING | + WDIOF_MAGICCLOSE, +}; + static const struct watchdog_ops mtk_wdt_ops = { .owner = THIS_MODULE, .start = mtk_wdt_start, .stop = mtk_wdt_stop, .ping = mtk_wdt_ping, .set_timeout= mtk_wdt_set_timeout, + .set_pretimeout = mtk_wdt_set_pretimeout, .restart= mtk_wdt_restart, }; @@ -267,7 +300,7 @@ static int mtk_wdt_probe(struct platform_device *pdev) struct device *dev = &pdev->dev; struct mtk_wdt_dev *mtk_wdt; const struct mtk_wdt_data *wdt_data; - int err; + int err, irq; mtk_wdt = devm_kzalloc(dev, sizeof(*mtk_wdt), GFP_KERNEL); if (!mtk_wdt) @@ -279,7 +312,22 @@ static int mtk_wdt_probe(struct platform_device *pdev) if (IS_ERR(mtk_wdt->wdt_base)) return PTR_ERR(mtk_wdt->wdt_base); - mtk_wdt->wdt_dev.info = &mtk_wdt_info; + irq = platform_get_irq(pdev, 0); + if (irq > 0) { + err = devm_request_irq(&pdev->dev, irq, mtk_wdt_isr, 0, "wdt_bark", + &mtk_wdt->wdt_dev); + if (err) + return err; + + mtk_wdt->wdt_dev.info = &mtk_wdt_pt_info; + mtk_wdt->wdt_dev.pretimeout = 1; + } else { + if (irq == -EPROBE_DEFER) + return -EPROBE_DEFER; + + mtk_wdt->wdt_dev.info = &mtk_wdt_info; + } + mtk_wdt->wdt_dev.ops = &mtk_wdt_ops; mtk_wdt->wdt_dev.timeout = WDT_MAX_TIMEOUT; mtk_wdt->wdt_dev.max_hw_heartbeat_ms = WDT_MAX_TIMEOUT * 1000; @@ -360,7 +408,6 @@ static struct platform_driver mtk_wdt_driver = { }; module_platform_driver(mtk_wdt_driver); - module_param(timeout, uint, 0); MODULE_PARM_DESC(timeout, "Watchdog heartbeat in seconds"); -- 2.7.4
Re: [PATCH v2 0/7] gpio-rockchip driver
On Sun, Apr 11, 2021 at 3:30 PM Peter Geis wrote: > Separate gpio driver from pinctrl driver, and support v2 controller. > > Tested on rk3566-quartz64 prototype board. > > Patch History: > V2 - Rebase to latest linux-next. > > Tested-by: Peter Geis This does not apply to the pin control tree, the problem with basing stuff on -next is that it sometimes does not apply to any development tree and now that happened (because of conflicts in the GPIO tree). You can either resend this based on the pinctrl "devel" branch: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/log/?h=devel ... or you can wait until kernel v5.13-rc1 is out and then we can merge it, but it might even require rebasing after that. Yours, Linus Walleij
回复: Question on KASAN calltrace record in RT
发件人: Mike Galbraith 发送时间: 2021年4月14日 12:00 收件人: Dmitry Vyukov; Zhang, Qiang 抄送: Andrew Halaney; andreyk...@gmail.com; ryabinin@gmail.com; a...@linux-foundation.org; linux-kernel@vger.kernel.org; kasan-...@googlegroups.com 主题: Re: Question on KASAN calltrace record in RT [Please note: This e-mail is from an EXTERNAL e-mail address] On Tue, 2021-04-13 at 17:29 +0200, Dmitry Vyukov wrote: > On Tue, Apr 6, 2021 at 10:26 AM Zhang, Qiang > wrote: > > > > Hello everyone > > > > In RT system, after Andrew test, found the following calltrace , > > in KASAN, we record callstack through stack_depot_save(), in this function, > > may be call alloc_pages, but in RT, the spin_lock replace with > > rt_mutex in alloc_pages(), if before call this function, the irq is > > disabled, > > will trigger following calltrace. > > > > maybe add array[KASAN_STACK_DEPTH] in struct kasan_track to record > > callstack in RT system. > > > > Is there a better solution ? > > Hi Qiang, > > Adding 2 full stacks per heap object can increase memory usage too much. > The stackdepot has a preallocation mechanism, I would start with > adding interrupts check here: > https://elixir.bootlin.com/linux/v5.12-rc7/source/lib/stackdepot.c#L294 > and just not do preallocation in interrupt context. This will solve > the problem, right? Hm, this thing might actually be (sorta?) working, modulo one startup gripe. The CRASH_DUMP inspired gripe I get with !RT appeared (and shut up when told I don't care given kdump has worked just fine for ages:), but no more might_sleep() gripeage. CONFIG_KASAN_SHADOW_OFFSET=0xdc00 CONFIG_HAVE_ARCH_KASAN=y CONFIG_HAVE_ARCH_KASAN_VMALLOC=y CONFIG_CC_HAS_KASAN_GENERIC=y CONFIG_KASAN=y CONFIG_KASAN_GENERIC=y CONFIG_KASAN_OUTLINE=y # CONFIG_KASAN_INLINE is not set CONFIG_KASAN_STACK=1 CONFIG_KASAN_VMALLOC=y # CONFIG_KASAN_MODULE_TEST is not set --- lib/stackdepot.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) --- a/lib/stackdepot.c +++ b/lib/stackdepot.c @@ -71,7 +71,7 @@ static void *stack_slabs[STACK_ALLOC_MAX static int depot_index; static int next_slab_inited; static size_t depot_offset; -static DEFINE_SPINLOCK(depot_lock); +static DEFINE_RAW_SPINLOCK(depot_lock); static bool init_stack_slab(void **prealloc) { @@ -265,7 +265,7 @@ depot_stack_handle_t stack_depot_save(un struct page *page = NULL; void *prealloc = NULL; unsigned long flags; - u32 hash; + u32 hash, preemptible = !IS_ENABLED(CONFIG_PREEMPT_RT) || preemptible(); if CONFIG_PREEMPT_RT is enabled and but not in preemptible, the prealloc should be allowed should be change like this: may_prealloc = !(IS_ENABLED(CONFIG_PREEMPT_RT) && preemptible()); Thanks Qiang if (unlikely(nr_entries == 0) || stack_depot_disable) goto fast_exit; @@ -291,7 +291,7 @@ depot_stack_handle_t stack_depot_save(un * The smp_load_acquire() here pairs with smp_store_release() to * |next_slab_inited| in depot_alloc_stack() and init_stack_slab(). */ - if (unlikely(!smp_load_acquire(&next_slab_inited))) { + if (unlikely(!smp_load_acquire(&next_slab_inited) && may_prealloc)) { /* * Zero out zone modifiers, as we don't have specific zone * requirements. Keep the flags related to allocation in atomic @@ -305,7 +305,7 @@ depot_stack_handle_t stack_depot_save(un prealloc = page_address(page); } - spin_lock_irqsave(&depot_lock, flags); + raw_spin_lock_irqsave(&depot_lock, flags); found = find_stack(*bucket, entries, nr_entries, hash); if (!found) { @@ -329,7 +329,7 @@ depot_stack_handle_t stack_depot_save(un WARN_ON(!init_stack_slab(&prealloc)); } - spin_unlock_irqrestore(&depot_lock, flags); + raw_spin_unlock_irqrestore(&depot_lock, flags); exit: if (prealloc) { /* Nobody used this memory, ok to free it. */ [0.692437] BUG: sleeping function called from invalid context at kernel/locking/rtmutex.c:943 [0.692439] in_atomic(): 1, irqs_disabled(): 1, non_block: 0, pid: 1, name: swapper/0 [0.692442] Preemption disabled at: [0.692443] [] on_each_cpu_cond_mask+0x30/0xb0 [0.692451] CPU: 5 PID: 1 Comm: swapper/0 Not tainted 5.12.0.g2afefec-tip-rt #5 [0.692454] Hardware name: MEDION MS-7848/MS-7848, BIOS M7848W08.20C 09/23/2013 [0.692456] Call Trace: [0.692458] ? on_each_cpu_cond_mask+0x30/0xb0 [0.692462] dump_stack+0x8a/0xb5 [0.692467] ___might_sleep.cold+0xfe/0x112 [0.692471] rt_spin_lock+0x1c/0x60 [0.692475] free_unref_page+0x117/0x3c0 [0.692481] qlist_free_all+0x60/0xd0 [0.692485] per_cpu_remove_cache+0x5b/0x70 [0.692488] smp_call_function_many_cond+0x185/0x3d0 [0.692492] ? qlist_move_cache+0xe0/0xe0 [0.692495] ? qlist_move_cache+0xe0/0
[PATCH v2] arm64: dts: mt8183: Add power-domains properity to mfgcfg
mfgcfg clock is under MFG_ASYNC power domain Signed-off-by: Weiyi Lu Signed-off-by: Ikjoon Jang --- Changes in v2: Fix a wrong power domain reference (scpsys to spm). Link(v1): https://patchwork.kernel.org/project/linux-mediatek/patch/20210224091742.1060508-1-i...@chromium.org/#23997681 --- arch/arm64/boot/dts/mediatek/mt8183.dtsi | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi index 0ff7b67a6806..64813634c3df 100644 --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi @@ -1116,6 +1116,7 @@ mfgcfg: syscon@1300 { compatible = "mediatek,mt8183-mfgcfg", "syscon"; reg = <0 0x1300 0 0x1000>; #clock-cells = <1>; + power-domains = <&spm MT8183_POWER_DOMAIN_MFG_ASYNC>; }; mmsys: syscon@1400 { -- 2.31.1.295.g9ea45b61b8-goog
Re: [PATCH 0/4 POC] Allow executing code and syscalls in another address space
On Wed, 2021-04-14 at 08:22 +0100, Anton Ivanov wrote: > On 14/04/2021 06:52, Andrei Vagin wrote: > > We already have process_vm_readv and process_vm_writev to read and write > > to a process memory faster than we can do this with ptrace. And now it > > is time for process_vm_exec that allows executing code in an address > > space of another process. We can do this with ptrace but it is much > > slower. > > > > = Use-cases = > > > > Here are two known use-cases. The first one is “application kernel” > > sandboxes like User-mode Linux and gVisor. In this case, we have a > > process that runs the sandbox kernel and a set of stub processes that > > are used to manage guest address spaces. Guest code is executed in the > > context of stub processes but all system calls are intercepted and > > handled in the sandbox kernel. Right now, these sort of sandboxes use > > PTRACE_SYSEMU to trap system calls, but the process_vm_exec can > > significantly speed them up. > > Certainly interesting, but will require um to rework most of its memory > management and we will most likely need extra mm support to make use of > it in UML. We are not likely to get away just with one syscall there. Might help the seccomp mode though: https://patchwork.ozlabs.org/project/linux-um/list/?series=231980 johannes
Re: [PATCH v2 3/9] userfaultfd/shmem: support minor fault registration for shmem
On Mon, 12 Apr 2021, Axel Rasmussen wrote: > This patch allows shmem-backed VMAs to be registered for minor faults. > Minor faults are appropriately relayed to userspace in the fault path, > for VMAs with the relevant flag. > > This commit doesn't hook up the UFFDIO_CONTINUE ioctl for shmem-backed > minor faults, though, so userspace doesn't yet have a way to resolve > such faults. This is a very odd way to divide up the series: an "Intermission" half way through the implementation of MINOR/CONTINUE: this 3/9 makes little sense without the 4/9 to mm/userfaultfd.c which follows. But, having said that, I won't object and Peter did not object, and I don't know of anyone else looking here: it will only give each of us more trouble to insist on repartitioning the series, and it's the end state that's far more important to me and to all of us. And I'll even seize on it, to give myself an intermission after this one, until tomorrow (when I'll look at 4/9 and 9/9 - but shall not look at the selftests ones at all). Most of this is okay, except the mm/shmem.c part; and I've just now realized that somewhere (whether in this patch or separately) there needs to be an update to Documentation/admin-guide/mm/userfaultfd.rst (admin-guide? how weird, but not this series' business to correct). > > Signed-off-by: Axel Rasmussen > --- > fs/userfaultfd.c | 6 +++--- > include/uapi/linux/userfaultfd.h | 7 ++- > mm/memory.c | 8 +--- > mm/shmem.c | 10 +- > 4 files changed, 23 insertions(+), 8 deletions(-) > > diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c > index 14f92285d04f..9f3b8684cf3c 100644 > --- a/fs/userfaultfd.c > +++ b/fs/userfaultfd.c > @@ -1267,8 +1267,7 @@ static inline bool vma_can_userfault(struct > vm_area_struct *vma, > } > > if (vm_flags & VM_UFFD_MINOR) { > - /* FIXME: Add minor fault interception for shmem. */ > - if (!is_vm_hugetlb_page(vma)) > + if (!(is_vm_hugetlb_page(vma) || vma_is_shmem(vma))) > return false; > } > > @@ -1941,7 +1940,8 @@ static int userfaultfd_api(struct userfaultfd_ctx *ctx, > /* report all available features and ioctls to userland */ > uffdio_api.features = UFFD_API_FEATURES; > #ifndef CONFIG_HAVE_ARCH_USERFAULTFD_MINOR > - uffdio_api.features &= ~UFFD_FEATURE_MINOR_HUGETLBFS; > + uffdio_api.features &= > + ~(UFFD_FEATURE_MINOR_HUGETLBFS | UFFD_FEATURE_MINOR_SHMEM); > #endif > uffdio_api.ioctls = UFFD_API_IOCTLS; > ret = -EFAULT; > diff --git a/include/uapi/linux/userfaultfd.h > b/include/uapi/linux/userfaultfd.h > index bafbeb1a2624..159a74e9564f 100644 > --- a/include/uapi/linux/userfaultfd.h > +++ b/include/uapi/linux/userfaultfd.h > @@ -31,7 +31,8 @@ > UFFD_FEATURE_MISSING_SHMEM | \ > UFFD_FEATURE_SIGBUS |\ > UFFD_FEATURE_THREAD_ID | \ > -UFFD_FEATURE_MINOR_HUGETLBFS) > +UFFD_FEATURE_MINOR_HUGETLBFS | \ > +UFFD_FEATURE_MINOR_SHMEM) > #define UFFD_API_IOCTLS \ > ((__u64)1 << _UFFDIO_REGISTER | \ >(__u64)1 << _UFFDIO_UNREGISTER | \ > @@ -185,6 +186,9 @@ struct uffdio_api { >* UFFD_FEATURE_MINOR_HUGETLBFS indicates that minor faults >* can be intercepted (via REGISTER_MODE_MINOR) for >* hugetlbfs-backed pages. > + * > + * UFFD_FEATURE_MINOR_SHMEM indicates the same support as > + * UFFD_FEATURE_MINOR_HUGETLBFS, but for shmem-backed pages instead. >*/ > #define UFFD_FEATURE_PAGEFAULT_FLAG_WP (1<<0) > #define UFFD_FEATURE_EVENT_FORK (1<<1) > @@ -196,6 +200,7 @@ struct uffdio_api { > #define UFFD_FEATURE_SIGBUS (1<<7) > #define UFFD_FEATURE_THREAD_ID (1<<8) > #define UFFD_FEATURE_MINOR_HUGETLBFS (1<<9) > +#define UFFD_FEATURE_MINOR_SHMEM (1<<10) > __u64 features; > > __u64 ioctls; > diff --git a/mm/memory.c b/mm/memory.c > index 4e358601c5d6..cc71a445c76c 100644 > --- a/mm/memory.c > +++ b/mm/memory.c > @@ -3972,9 +3972,11 @@ static vm_fault_t do_read_fault(struct vm_fault *vmf) >* something). >*/ > if (vma->vm_ops->map_pages && fault_around_bytes >> PAGE_SHIFT > 1) { > - ret = do_fault_around(vmf); > - if (ret) > - return ret; > + if (likely(!userfaultfd_minor(vmf->vma))) { > + ret = do_fault_around(vmf); > + if (ret) > + return ret; > + } > } > > ret = __do_fault(vmf); > diff --git a/mm/shmem.c b/mm/shmem.c > index b72c55aa07fc..3f48cb5e8404 100644 > --- a/mm/shmem.c > +++ b/mm/shmem.c > @@ -1785,7 +
[tip: x86/cleanups] x86/pat: Do not compile stubbed functions when X86_PAT is off
The following commit has been merged into the x86/cleanups branch of tip: Commit-ID: 16854b567dff767e5ec5e6dc23021271136733a5 Gitweb: https://git.kernel.org/tip/16854b567dff767e5ec5e6dc23021271136733a5 Author:Jan Kiszka AuthorDate:Mon, 26 Oct 2020 18:39:06 +01:00 Committer: Borislav Petkov CommitterDate: Wed, 14 Apr 2021 08:21:41 +02:00 x86/pat: Do not compile stubbed functions when X86_PAT is off Those are already provided by linux/io.h as stubs. The conflict remains invisible until someone would pull linux/io.h into memtype.c. This fixes a build error when this file is used outside of the kernel tree. [ bp: Massage commit message. ] Signed-off-by: Jan Kiszka Signed-off-by: Borislav Petkov Link: https://lkml.kernel.org/r/a9351615-7a0d-9d47-af65-d9e2fffe8...@siemens.com --- arch/x86/mm/pat/memtype.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/x86/mm/pat/memtype.c b/arch/x86/mm/pat/memtype.c index 6084d14..3112ca7 100644 --- a/arch/x86/mm/pat/memtype.c +++ b/arch/x86/mm/pat/memtype.c @@ -800,6 +800,7 @@ void memtype_free_io(resource_size_t start, resource_size_t end) memtype_free(start, end); } +#ifdef CONFIG_X86_PAT int arch_io_reserve_memtype_wc(resource_size_t start, resource_size_t size) { enum page_cache_mode type = _PAGE_CACHE_MODE_WC; @@ -813,6 +814,7 @@ void arch_io_free_memtype_wc(resource_size_t start, resource_size_t size) memtype_free_io(start, start + size); } EXPORT_SYMBOL(arch_io_free_memtype_wc); +#endif pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn, unsigned long size, pgprot_t vma_prot)
Re: [Outreachy kernel] [PATCH] :staging: rtl8723bs: Remove useless led_blink_hdl()
On Wed, Apr 14, 2021 at 08:21:50AM +0300, Dan Carpenter wrote: > On Tue, Apr 13, 2021 at 10:08:32PM +0200, Fabio M. De Francesco wrote: > > On Tuesday, April 13, 2021 9:48:44 PM CEST Matthew Wilcox wrote: > > > On Tue, Apr 13, 2021 at 09:45:03PM +0200, Fabio M. De Francesco wrote: > > > > 1) The driver doesn't call that function from anywhere else than the > > > > macro. 2) You have explained that the macro add its symbol to a slot > > > > in an array that would shift all the subsequent elements down if that > > > > macro is not used exactly in the line where it is. > > > > 3) Dan Carpenter said that that array is full of null functions (or > > > > empty slots?). > > > > > > > > Unless that function is called anonymously dereferencing its address > > > > from the position it occupies in the array, I'm not able to see what > > > > else means can any caller use. > > > > > > > > I know I have much less experience than you with C: what can go wrong? > > > > > > Here's where the driver calls that function: > > > > > > $ git grep wlancmds drivers/staging/rtl8723bs/ > > > drivers/staging/rtl8723bs/core/rtw_cmd.c:static struct cmd_hdl wlancmds[] > > > = { drivers/staging/rtl8723bs/core/rtw_cmd.c: if > > > (pcmd->cmdcode < ARRAY_SIZE(wlancmds)) { > > > drivers/staging/rtl8723bs/core/rtw_cmd.c: cmd_hdl > > > = wlancmds[pcmd->cmdcode].h2cfuns; > > > > > OK, I had imagined an anonymous call from its location in the array (as I > > wrote in the last phrase of my message). However, I thought that it could > > have been an improbable possibility, not a real one. > > > > Linux uses a lot of interesting ideas that newcomers like me should learn. > > Things here are trickier than they appear at first sight. > > One trick would be to build the Smatch cross function database. > > https://www.spinics.net/lists/smatch/msg00568.html > > Then you could do: > > $ ~/path/to/smatch_data/db/smdb.py led_blink_hdl > file | caller | function | type | parameter | key | value | > drivers/staging/rtl8723bs/core/rtw_cmd.c | rtw_cmd_thread | > rtw_cmd_thread ptr cmd_hdl | INTERNAL | -1 | | > uchar(*)(struct adapter*, uchar*) > drivers/staging/rtl8188eu/core/rtw_cmd.c | rtw_cmd_thread | > rtw_cmd_thread ptr cmd_hdl | INTERNAL | -1 | | > uchar(*)(struct adapter*, uchar*) > drivers/staging/rtl8188eu/core/rtw_cmd.c | rtw_cmd_thread | > rtw_cmd_thread ptr cmd_hdl | BUF_SIZE | 1 |pbuf | > 1,4,6,8,12,14,16,19-20,23-24,48,740,884,892,900,960 > > > Which says that led_blink_hdl() is called as a function pointer called > "cmd_hdl" from rtw_cmd_thread(). > > Hm... It says it can be called from either rtw_cmd_thread() function > (the rtl8723bs or rtl8188eu version) which is not ideal. But also > basically harmless so whatever... > > regards, > dan carpenter > very powerful tool. I tried this: fabio@agape:~/src/git/kernels/staging$ ~/src/git/smatch/smatch_data/db/smdb.py led_blink_hdl Traceback (most recent call last): File "/home/fabio/src/git/smatch/smatch_data/db/smdb.py", line 725, in print_caller_info("", func) File "/home/fabio/src/git/smatch/smatch_data/db/smdb.py", line 366, in print_caller_info ptrs = get_function_pointers(func) File "/home/fabio/src/git/smatch/smatch_data/db/smdb.py", line 53, in get_function_pointers get_function_pointers_helper(func) File "/home/fabio/src/git/smatch/smatch_data/db/smdb.py", line 38, in get_function_pointers_helper cur.execute("select distinct ptr from function_ptr where function = '%s';" %(func)) sqlite3.OperationalError: no such table: function_ptr I run smatch version 1.71 on Debian Buster machine what's happened? thanks in advance, fabio
Re: Regression v5.12-rc3: net: stmmac: re-init rx buffers when mac resume back
On Wed, Apr 14, 2021 at 02:18:58AM +, Joakim Zhang wrote: > > > -Original Message- > > From: Thierry Reding > > Sent: 2021年4月14日 0:07 > > To: David S. Miller ; Jakub Kicinski > > Cc: Joakim Zhang ; Jon Hunter > > ; Giuseppe Cavallaro ; > > Alexandre Torgue ; Jose Abreu > > ; net...@vger.kernel.org; Linux Kernel Mailing List > > ; linux-tegra > > Subject: Re: Regression v5.12-rc3: net: stmmac: re-init rx buffers when mac > > resume back > > > > On Tue, Apr 13, 2021 at 12:13:01PM +, Joakim Zhang wrote: > > > > > > Hi Jon, > > > > > > > -Original Message- > > > > From: Jon Hunter > > > > Sent: 2021年4月13日 16:41 > > > > To: Joakim Zhang ; Giuseppe Cavallaro > > > > ; Alexandre Torgue > > > > ; Jose Abreu > > > > Cc: net...@vger.kernel.org; Linux Kernel Mailing List > > > > ; linux-tegra > > > > ; Jakub Kicinski > > > > Subject: Re: Regression v5.12-rc3: net: stmmac: re-init rx buffers > > > > when mac resume back > > > > > > > > > > > > On 01/04/2021 17:28, Jon Hunter wrote: > > > > > > > > > > On 31/03/2021 12:41, Joakim Zhang wrote: > > > > > > > > > > ... > > > > > > > > > >>> In answer to your question, resuming from suspend does work on > > > > >>> this board without your change. We have been testing > > > > >>> suspend/resume now on this board since Linux v5.8 and so we have > > > > >>> the ability to bisect such regressions. So it is clear to me > > > > >>> that this is the change that caused > > > > this, but I am not sure why. > > > > >> > > > > >> Yes, I know this issue is regression caused by my patch. I just > > > > >> want to > > > > analyze the potential reasons. Due to the code change only related > > > > to the page recycle and reallocate. > > > > >> So I guess if this page operate need IOMMU works when IOMMU is > > enabled. > > > > Could you help check if IOMMU driver resume before STMMAC? Our > > > > common desire is to find the root cause, right? > > > > > > > > > > > > > > > Yes of course that is the desire here indeed. I had assumed that > > > > > the suspend/resume order was good because we have never seen any > > > > > problems, but nonetheless it is always good to check. Using ftrace > > > > > I enabled tracing of the appropriate suspend/resume functions and > > > > > this is what I see ... > > > > > > > > > > # tracer: function > > > > > # > > > > > # entries-in-buffer/entries-written: 4/4 #P:6 > > > > > # > > > > > #_-=> irqs-off > > > > > # / _=> need-resched > > > > > # | / _---=> hardirq/softirq > > > > > # || / _--=> preempt-depth > > > > > # ||| / delay > > > > > # TASK-PID CPU# TIMESTAMP FUNCTION > > > > > # | | | | | > > > > > rtcwake-748 [000] ...1 536.700777: > > > > stmmac_pltfr_suspend <-platform_pm_suspend > > > > > rtcwake-748 [000] ...1 536.735532: > > > > arm_smmu_pm_suspend <-platform_pm_suspend > > > > > rtcwake-748 [000] ...1 536.757290: > > > > arm_smmu_pm_resume <-platform_pm_resume > > > > > rtcwake-748 [003] ...1 536.856771: > > > > stmmac_pltfr_resume <-platform_pm_resume > > > > > > > > > > > > > > > So I don't see any ordering issues that could be causing this. > > > > > > > > > > > > Another thing I have found is that for our platform, if the driver > > > > for the ethernet PHY (in this case broadcom PHY) is enabled, then it > > > > fails to resume but if I disable the PHY in the kernel > > > > configuration, then resume works. I have found that if I move the > > > > reinit of the RX buffers to before the startup of the phy, then it can > > > > resume > > OK with the PHY enabled. > > > > > > > > Does the following work for you? Does your platform use a specific > > > > ethernet PHY driver? > > > > > > I am also looking into this issue these days, we use the Realtek > > > RTL8211FDI > > PHY, driver is drivers/net/phy/realtek.c. > > > > > > For our EQOS MAC integrated in our SoC, Rx side logic depends on RXC clock > > from PHY, so we need phylink_start before MAC. > > > > > > I will test below code change tomorrow to see if it can work at my side, > > > since > > it is only re-init memory, need not RXC clock. > > > > > > > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > > > b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > > > index 208cae344ffa..071d15d86dbe 100644 > > > > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > > > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > > > @@ -5416,19 +5416,20 @@ int stmmac_resume(struct device *dev) > > > > return ret; > > > > } > > > > + rtnl_lock(); > > > > + mutex_lock(&priv->lock); > > > > + stmmac_reinit_rx_buffers(priv); > > > > + mutex_unlock(&priv->lock); > > > > + > > > > if (!device
Re: [PATCH v7 3/7] mm,hugetlb: Clear HPageFreed outside of the lock
On Wed, Apr 14, 2021 at 08:04:21AM +0200, Michal Hocko wrote: > On Tue 13-04-21 14:19:03, Mike Kravetz wrote: > > On 4/13/21 6:23 AM, Michal Hocko wrote: > > The only place where page->private may not be initialized is when we do > > allocations at boot time from memblock. In this case, we will add the > > pages to the free list via put_page/free_huge_page so the appropriate > > flags will be cleared before anyone notices. > > Pages allocated by the bootmem should be pre initialized from the boot, > no? I guess Mike means: hugetlb_hstate_alloc_pages alloc_bootmem_huge_page __alloc_bootmem_huge_page memblock_alloc_try_nid_raw and AFAICS, memblock_alloc_try_nid_raw() does not zero the memory. Then these pages are initialized in: gather_bootmem_prealloc prep_compound_huge_page prep_new_huge_page But as can be noticed, no one touches page->private when coming from that path. -- Oscar Salvador SUSE L3
Re: [PATCH] ACPI: x86: Call acpi_boot_table_init() after acpi_table_upgrade()
On 13.04.21 19:53, Rafael J. Wysocki wrote: On Tue, Apr 13, 2021 at 7:43 PM David Hildenbrand wrote: On 13.04.21 16:01, Rafael J. Wysocki wrote: From: Rafael J. Wysocki Commit 1a1c130ab757 ("ACPI: tables: x86: Reserve memory occupied by ACPI tables") attempted to address an issue with reserving the memory occupied by ACPI tables, but it broke the initrd-based table override mechanism relied on by multiple users. To restore the initrd-based ACPI table override functionality, move the acpi_boot_table_init() invocation in setup_arch() on x86 after the acpi_table_upgrade() one. Fixes: 1a1c130ab757 ("ACPI: tables: x86: Reserve memory occupied by ACPI tables") Reported-by: Hans de Goede Tested-by: Hans de Goede Signed-off-by: Rafael J. Wysocki --- George, can you please check if this reintroduces the issue addressed by the above commit for you? --- arch/x86/kernel/setup.c |5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) Index: linux-pm/arch/x86/kernel/setup.c === --- linux-pm.orig/arch/x86/kernel/setup.c +++ linux-pm/arch/x86/kernel/setup.c @@ -1045,9 +1045,6 @@ void __init setup_arch(char **cmdline_p) cleanup_highmap(); - /* Look for ACPI tables and reserve memory occupied by them. */ - acpi_boot_table_init(); - memblock_set_current_limit(ISA_END_ADDRESS); e820__memblock_setup(); @@ -1132,6 +1129,8 @@ void __init setup_arch(char **cmdline_p) reserve_initrd(); acpi_table_upgrade(); + /* Look for ACPI tables and reserve memory occupied by them. */ + acpi_boot_table_init(); vsmp_init(); This is fairly late; especially, it's after actual allocations -- see e820__memblock_alloc_reserved_mpc_new(). Can't the table upgrade mechanism fix up when adjusting something? Not at this point of the cycle I'm afraid. Some details on what actually breaks would be helpful. Generally speaking, the table overrides that come from the initrd are not taken into account if acpi_boot_table_init() runs before acpi_table_upgrade() and the latter cannot run before reserve_initrd(). I see. (looking at Documentation/acpi/initrd_table_override.txt I understand what acpi table overrides are for :) ) Honestly, I'm not sure how much effort it would take to untangle this ATM. Also true; ideally, we wouldn't have any allocations (find+reserve) before ordinary reservations are done. However, I have no idea if we can move e820__memblock_alloc_reserved_mpc_new() and reserve_real_mode() around easily. Also, reserve_initrd()->relocate_initrd() does allocations. This is a mess. -- Thanks, David / dhildenb
Re: [PATCH] ASoC: Intel: Handle device properties with software node API
On Tue, Apr 13, 2021 at 10:47:49AM -0500, Pierre-Louis Bossart wrote: > > > On 4/13/21 9:05 AM, Heikki Krogerus wrote: > > On Tue, Apr 13, 2021 at 03:20:45PM +0300, Heikki Krogerus wrote: > > > On Mon, Apr 12, 2021 at 03:36:20PM -0500, Pierre-Louis Bossart wrote: > > > > I took the code and split it in two for BYT/CHT (modified to remove > > > > devm_) > > > > and SoundWire parts (added as is). > > > > > > > > https://github.com/thesofproject/linux/pull/2810 > > > > > > > > Both cases result in a refcount error on device_remove_sof when > > > > removing the > > > > platform device. I don't understand the code well enough to figure out > > > > what > > > > happens, but it's likely a case of the software node being removed > > > > twice? > > > > > > Right. Because you are injecting the node to an already existing > > > device, the node does not get linked with the device in sysfs. That > > > would increment the reference count in a normal case. It all happens > > > in the function software_node_notify(). Driver core calls it when a > > > device is added and also when it's removed. In this case it is only > > > called when it's removed. > > > > > > I think the best way to handle this now is to simply not decrementing > > > the ref count when you add the properties, so don't call > > > fwnode_handle_put() there (but add a comment explaining why you are > > > not calling it). > > > > No, sorry... That's just too hacky. Let's not do that after all. > > > > We can also fix this in the software node code. I'm attaching a patch > > that should make it possible to inject the software nodes also > > afterwards safely. This is definitely also not without its problems, > > but we can go with that if it works. Let me know. > > I tested manually on bytcr w/ RT5640 and used the SOF CI farm to test the > SoundWire cases, I don't see any issues with your patch. The refcount issue > is gone and the module load/unload cycles don't report any problems. > > Would you queue it for 5.13-rc1, or is this too late already? I'll send it out now. Let's see what happens. thanks, > > > For a better solution you would call device_reprobe() after you have > > > injected the software node, but before that you need to modify > > > device_reprobe() so it calls device_platform_notify() (which it really > > > should call in any case). But this should probable be done later, > > > separately. > > > > > > thanks, > > > > > > P.S. > > > > > > Have you guys considered the possibility of describing the connections > > > between all these components by using one of the methods that we now > > > have for that in kernel, for example device graph? It can now be > > > used also with software nodes (OF graph and ACPI device graph are of > > > course already fully supported). > > I must admit I've never heard of a 'device graph'. Any pointers or APIs you > can think of? > It's a good comment since we are planning to rework the SOF clients and > machine driver handling. -- heikki
[PATCH] usb: typec: silence a static checker warning
Smatch complains about a potential missing error code: drivers/usb/typec/port-mapper.c:168 typec_link_port() warn: missing error code 'ret' This is a false positive and returning zero is intentional. Let's re-arrange the code to silence the warning and make the intent more clear. Signed-off-by: Dan Carpenter --- drivers/usb/typec/port-mapper.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/usb/typec/port-mapper.c b/drivers/usb/typec/port-mapper.c index fae736eb0601..9b0991bdf391 100644 --- a/drivers/usb/typec/port-mapper.c +++ b/drivers/usb/typec/port-mapper.c @@ -157,15 +157,17 @@ int typec_link_port(struct device *port) { struct device *connector; struct port_node *node; - int ret = 0; + int ret; node = create_port_node(port); if (IS_ERR(node)) return PTR_ERR(node); connector = find_connector(node); - if (!connector) + if (!connector) { + ret = 0; goto remove_node; + } ret = link_port(to_typec_port(connector), node); if (ret) -- 2.30.2
Re: [PATCH v14 6/6] locking/qspinlock: Introduce the shuffle reduction optimization into CNA
On Thu, Apr 01, 2021 at 11:31:56AM -0400, Alex Kogan wrote: > This performance optimization chooses probabilistically to avoid moving > threads from the main queue into the secondary one when the secondary queue > is empty. > > It is helpful when the lock is only lightly contended. In particular, it > makes CNA less eager to create a secondary queue, but does not introduce > any extra delays for threads waiting in that queue once it is created. > > Signed-off-by: Alex Kogan > Reviewed-by: Steve Sistare > Reviewed-by: Waiman Long > --- > kernel/locking/qspinlock_cna.h | 39 ++ > 1 file changed, 39 insertions(+) > > diff --git a/kernel/locking/qspinlock_cna.h b/kernel/locking/qspinlock_cna.h > index 29c3abbd3d94..983c6a47a221 100644 > --- a/kernel/locking/qspinlock_cna.h > +++ b/kernel/locking/qspinlock_cna.h > @@ -5,6 +5,7 @@ > > #include > #include > +#include > > /* > * Implement a NUMA-aware version of MCS (aka CNA, or compact NUMA-aware > lock). > @@ -86,6 +87,34 @@ static inline bool intra_node_threshold_reached(struct > cna_node *cn) > return current_time - threshold > 0; > } > > +/* > + * Controls the probability for enabling the ordering of the main queue > + * when the secondary queue is empty. The chosen value reduces the amount > + * of unnecessary shuffling of threads between the two waiting queues > + * when the contention is low, while responding fast enough and enabling > + * the shuffling when the contention is high. > + */ > +#define SHUFFLE_REDUCTION_PROB_ARG (7) Out of curiosity: Have you used other values and done measurements what's an efficient value for SHUFFLE_REDUCTION_PROB_ARG? Maybe I miscalculated it, but if I understand it correctly this value implies that the propability is 0.9921875 that below function returns true. My question is probably answered by following statement from referenced paper: "In our experiments with the shuffle reduction optimization enabled, we set THRESHOLD2 to 0xff." (page with figure 5) > + > +/* Per-CPU pseudo-random number seed */ > +static DEFINE_PER_CPU(u32, seed); > + > +/* > + * Return false with probability 1 / 2^@num_bits. > + * Intuitively, the larger @num_bits the less likely false is to be returned. > + * @num_bits must be a number between 0 and 31. > + */ > +static bool probably(unsigned int num_bits) > +{ > + u32 s; > + > + s = this_cpu_read(seed); > + s = next_pseudo_random32(s); > + this_cpu_write(seed, s); > + > + return s & ((1 << num_bits) - 1); > +} > + > static void __init cna_init_nodes_per_cpu(unsigned int cpu) > { > struct mcs_spinlock *base = per_cpu_ptr(&qnodes[0].mcs, cpu); > @@ -293,6 +322,16 @@ static __always_inline u32 cna_wait_head_or_lock(struct > qspinlock *lock, > { > struct cna_node *cn = (struct cna_node *)node; > > + if (node->locked <= 1 && probably(SHUFFLE_REDUCTION_PROB_ARG)) { Again if I understand it correctly with SHUFFLE_REDUCTION_PROB_ARG==7 it's roughly 1 out of 100 cases where probably() returns false. Why/when is this beneficial? I assume it has to do with following statement in the referenced paper: "The superior performance over MCS at 4 threads is the result of the shuffling that does take place once in a while, organizing threads’ arrivals to the lock in a way that reduces the inter-socket lock migration without the need to continuously modify the main queue. ..." (page with figure 9; the paper has no page numbers) But OTHO why this pseudo randomness? How about deterministically treating every 100th execution differently (it also matches "once in a while") and thus entirely removing the pseudo randomness? Have you tried this? If so why was it worse than pseudo randomness? (Or maybe I missed something and pseudo randomness is required for other reasons there.) > + /* > + * When the secondary queue is empty, skip the call to > + * cna_order_queue() below with high probability. This > optimization > + * reduces the overhead of unnecessary shuffling of threads > + * between waiting queues when the lock is only lightly > contended. > + */ > + return 0; > + } > + > if (!cn->start_time || !intra_node_threshold_reached(cn)) { > /* >* We are at the head of the wait queue, no need to use > -- > 2.24.3 (Apple Git-128) > -- Regards, Andreas
Re: [Outreachy kernel] [PATCH] :staging: rtl8723bs: Remove useless led_blink_hdl()
On Wed, Apr 14, 2021 at 09:40:36AM +0200, Fabio Aiuto wrote: > On Wed, Apr 14, 2021 at 08:21:50AM +0300, Dan Carpenter wrote: > > On Tue, Apr 13, 2021 at 10:08:32PM +0200, Fabio M. De Francesco wrote: > > > On Tuesday, April 13, 2021 9:48:44 PM CEST Matthew Wilcox wrote: > > > > On Tue, Apr 13, 2021 at 09:45:03PM +0200, Fabio M. De Francesco wrote: > > > > > 1) The driver doesn't call that function from anywhere else than the > > > > > macro. 2) You have explained that the macro add its symbol to a slot > > > > > in an array that would shift all the subsequent elements down if that > > > > > macro is not used exactly in the line where it is. > > > > > 3) Dan Carpenter said that that array is full of null functions (or > > > > > empty slots?). > > > > > > > > > > Unless that function is called anonymously dereferencing its address > > > > > from the position it occupies in the array, I'm not able to see what > > > > > else means can any caller use. > > > > > > > > > > I know I have much less experience than you with C: what can go wrong? > > > > > > > > Here's where the driver calls that function: > > > > > > > > $ git grep wlancmds drivers/staging/rtl8723bs/ > > > > drivers/staging/rtl8723bs/core/rtw_cmd.c:static struct cmd_hdl > > > > wlancmds[] > > > > = { drivers/staging/rtl8723bs/core/rtw_cmd.c: if > > > > (pcmd->cmdcode < ARRAY_SIZE(wlancmds)) { > > > > drivers/staging/rtl8723bs/core/rtw_cmd.c: cmd_hdl > > > > = wlancmds[pcmd->cmdcode].h2cfuns; > > > > > > > OK, I had imagined an anonymous call from its location in the array (as I > > > wrote in the last phrase of my message). However, I thought that it could > > > have been an improbable possibility, not a real one. > > > > > > Linux uses a lot of interesting ideas that newcomers like me should > > > learn. > > > Things here are trickier than they appear at first sight. > > > > One trick would be to build the Smatch cross function database. > > > > https://www.spinics.net/lists/smatch/msg00568.html > > > > Then you could do: > > > > $ ~/path/to/smatch_data/db/smdb.py led_blink_hdl > > file | caller | function | type | parameter | key | value | > > drivers/staging/rtl8723bs/core/rtw_cmd.c | rtw_cmd_thread | > > rtw_cmd_thread ptr cmd_hdl | INTERNAL | -1 | | > > uchar(*)(struct adapter*, uchar*) > > drivers/staging/rtl8188eu/core/rtw_cmd.c | rtw_cmd_thread | > > rtw_cmd_thread ptr cmd_hdl | INTERNAL | -1 | | > > uchar(*)(struct adapter*, uchar*) > > drivers/staging/rtl8188eu/core/rtw_cmd.c | rtw_cmd_thread | > > rtw_cmd_thread ptr cmd_hdl | BUF_SIZE | 1 |pbuf | > > 1,4,6,8,12,14,16,19-20,23-24,48,740,884,892,900,960 > > > > > > Which says that led_blink_hdl() is called as a function pointer called > > "cmd_hdl" from rtw_cmd_thread(). > > > > Hm... It says it can be called from either rtw_cmd_thread() function > > (the rtl8723bs or rtl8188eu version) which is not ideal. But also > > basically harmless so whatever... > > > > regards, > > dan carpenter > > > > very powerful tool. > > I tried this: > > fabio@agape:~/src/git/kernels/staging$ > ~/src/git/smatch/smatch_data/db/smdb.py led_blink_hdl > Traceback (most recent call last): > File "/home/fabio/src/git/smatch/smatch_data/db/smdb.py", line 725, in > > print_caller_info("", func) > File "/home/fabio/src/git/smatch/smatch_data/db/smdb.py", line 366, in > print_caller_info > ptrs = get_function_pointers(func) > File "/home/fabio/src/git/smatch/smatch_data/db/smdb.py", line 53, in > get_function_pointers > get_function_pointers_helper(func) > File "/home/fabio/src/git/smatch/smatch_data/db/smdb.py", line 38, in > get_function_pointers_helper > cur.execute("select distinct ptr from function_ptr where function = > '%s';" %(func)) > sqlite3.OperationalError: no such table: function_ptr > > I run smatch version 1.71 on Debian Buster machine > It takes a few hours to build the DB. The instructions are in the email. ~/path/to/smatch/smatch_scripts/build_kernel_data.sh regards, dan carpenter
Re: [PATCH v3 0/3] rseq: minor optimizations
On Tue, Apr 13, 2021 at 04:40:33PM -0400, Mathieu Desnoyers wrote: > - On Apr 13, 2021, at 4:33 PM, Eric Dumazet eric.duma...@gmail.com wrote: > > > From: Eric Dumazet > > > > rseq is a heavy user of copy to/from user data in fast paths. > > This series tries to reduce the cost. > > > > v3: Third patch going back to v1 (only deal with 64bit arches) > > v2: Addressed Peter and Mathieu feedbacks, thanks ! > > For the whole series: > > Reviewed-by: Mathieu Desnoyers Thanks!
Re: [PATCH 0/8] i2c: fix reference leak when pm_runtime_get_sync fails
On Tue, Dec 01, 2020 at 05:29:24PM +0800, Qinglang Miao wrote: > pm_runtime_get_sync will increment the PM reference count > even failed. Forgetting to putting operation will result > in a reference leak here. > > Replace it with pm_runtime_resume_and_get to keep usage > counter balanced. > > BTW, pm_runtime_resume_and_get is introduced in v5.10-rc5 as > dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get > to dealwith usage counter") > > Qinglang Miao (8): > i2c: cadence: fix reference leak when pm_runtime_get_sync fails > i2c: img-scb: fix reference leak when pm_runtime_get_sync fails > i2c: imx-lpi2c: fix reference leak when pm_runtime_get_sync fails > i2c: imx: fix reference leak when pm_runtime_get_sync fails > i2c: omap: fix reference leak when pm_runtime_get_sync fails > i2c: sprd: fix reference leak when pm_runtime_get_sync fails > i2c: stm32f7: fix reference leak when pm_runtime_get_sync fails > i2c: xiic: fix reference leak when pm_runtime_get_sync fails I applied this series now to for-next, thanks! signature.asc Description: PGP signature
Re: [PATCH] i2c: omap: Fix rumtime PM imbalance on error
On Wed, Apr 07, 2021 at 11:30:30AM +0800, Dinghao Liu wrote: > pm_runtime_get_sync() will increase the rumtime PM counter > even it returns an error. Thus a pairing decrement is needed > to prevent refcount leak. Fix this by replacing this API with > pm_runtime_resume_and_get(), which will not change the runtime > PM counter on error. > > Signed-off-by: Dinghao Liu Thanks, yet I applied this series now: http://patchwork.ozlabs.org/project/linux-i2c/list/?series=217733&state=* signature.asc Description: PGP signature
[PATCH] software node: Allow node addition to already existing device
If the node is added to an already exiting device, the node needs to be also linked to the device separately. This will make sure the reference count is kept in balance also when the node is injected to a device afterwards. Reported-by: Pierre-Louis Bossart Fixes: e68d0119e328 ("software node: Introduce device_add_software_node()") Signed-off-by: Heikki Krogerus --- drivers/base/swnode.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c index 740333629b420..3cc11b813f28c 100644 --- a/drivers/base/swnode.c +++ b/drivers/base/swnode.c @@ -1045,6 +1045,7 @@ int device_add_software_node(struct device *dev, const struct software_node *nod } set_secondary_fwnode(dev, &swnode->fwnode); + software_node_notify(dev, KOBJ_ADD); return 0; } @@ -1118,8 +1119,8 @@ int software_node_notify(struct device *dev, unsigned long action) switch (action) { case KOBJ_ADD: - ret = sysfs_create_link(&dev->kobj, &swnode->kobj, - "software_node"); + ret = sysfs_create_link_nowarn(&dev->kobj, &swnode->kobj, + "software_node"); if (ret) break; -- 2.30.2
RE: [PATCH v2 3/3] rseq: optimise rseq_get_rseq_cs() and clear_rseq_cs()
From: Arjun Roy > Sent: 13 April 2021 23:04 > > On Tue, Apr 13, 2021 at 2:19 PM David Laight wrote: > > > > > If we're special-casing 64-bit architectures anyways - unrolling the > > > 32B copy_from_user() for struct rseq_cs appears to be roughly 5-10% > > > savings on x86-64 when I measured it (well, in a microbenchmark, not > > > in rseq_get_rseq_cs() directly). Perhaps that could be an additional > > > avenue for improvement here. > > > > The killer is usually 'user copy hardening'. > > It significantly slows down sendmsg() and recvmsg(). > > I've got measurable performance improvements by > > using __copy_from_user() when the buffer since has > > already been checked - but isn't a compile-time constant. > > > > There is also scope for using _get_user() when reading > > iovec[] (instead of copy_from_user()) and doing all the > > bound checks (etc) in the loop. > > That gives a measurable improvement for writev("/dev/null"). > > I must sort those patches out again. > > > > David > > > > In this case I mean replacing copy_from_user(rseq_cs, urseq_cs, > sizeof(*rseq_cs)) with 4 (x8B=32 total) unsafe_get_user() (wrapped in > user_read_access_begin/end()) which I think would just bypass user > copy hardening (as far as I can tell). Yes that is one advantage over any of the get_user() calls. You also lose all the 'how shall we optimise this' checks in copy_from_user(). Repeated unsafe_get_user() calls are crying out for an optimisation. You get something like: failed = 0; copy(); if (failed) goto error; copy(); if (failed) goto error; Where 'failed' is set by the fault handler. This could be optimised to: failed = 0; copy(); copy(); if (failed) goto error; Even if it faults on every invalid address it probably doesn't matter - no one cares about that path. I've not really looked at how it could be achieved though. It might be that the 'asm goto with outputs' variant manages to avoid the compare and jump. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Re: [PATCH v2 4/5] staging: rtl8192e: rectified spelling mistake and replace memcmp with ether_oui_equal
On Wed, Apr 14, 2021 at 12:26:01PM +0530, Mitali Borkar wrote: > Added a generic function of static inline bool in > include/linux/etherdevice.h to replace memcmp with > ether_oui_equal throughout the execution. > Corrected the misspelled words in this file. > > Signed-off-by: Mitali Borkar > --- > > Changes from v1:- Rectified spelling mistake and replaced memcmp with > ether_oui_equal. > > drivers/staging/rtl8192e/rtl819x_HTProc.c | 48 +++ > include/linux/etherdevice.h | 5 +++ ^^^ This is networking code and not staging code, but the netdev mailing list isn't CC'd. > 2 files changed, 29 insertions(+), 24 deletions(-) > > diff --git a/drivers/staging/rtl8192e/rtl819x_HTProc.c > b/drivers/staging/rtl8192e/rtl819x_HTProc.c > index ec6b46166e84..ce58feb2af9a 100644 > --- a/drivers/staging/rtl8192e/rtl819x_HTProc.c > +++ b/drivers/staging/rtl8192e/rtl819x_HTProc.c > @@ -43,7 +43,7 @@ u16 MCS_DATA_RATE[2][2][77] = { >810, 720, 810, 900, 900, 990} } > }; > > -static u8 UNKNOWN_BORADCOM[3] = {0x00, 0x14, 0xbf}; > +static u8 UNKNOWN_BROADCOM[3] = {0x00, 0x14, 0xbf}; Please pull this spelling fix into its own patch. [ snip ] > diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h > index 2e5debc0373c..6a1a63168319 100644 > --- a/include/linux/etherdevice.h > +++ b/include/linux/etherdevice.h > @@ -87,6 +87,11 @@ static inline bool is_link_local_ether_addr(const u8 *addr) > #endif > } > > +static inline bool ether_oui_equal(const u8 *addr, const u8 *oui) > +{ > +return addr[0] == oui[0] && addr[1] == oui[1] && addr[2] == oui[2]; > +} The indenting is messed up on this. regards, dan carpenter
Re: [PATCH v3 1/3] usb: typec: tcpm: Honour pSnkStdby requirement during negotiation
On Tue, Apr 13, 2021 at 07:39:58PM -0700, Badhri Jagan Sridharan wrote: > >From PD Spec: > The Sink Shall transition to Sink Standby before a positive or > negative voltage transition of VBUS. During Sink Standby > the Sink Shall reduce its power draw to pSnkStdby. This allows > the Source to manage the voltage transition as well as > supply sufficient operating current to the Sink to maintain PD > operation during the transition. The Sink Shall > complete this transition to Sink Standby within tSnkStdby > after evaluating the Accept Message from the Source. The > transition when returning to Sink operation from Sink Standby > Shall be completed within tSnkNewPower. The > pSnkStdby requirement Shall only apply if the Sink power draw > is higher than this level. > > The above requirement needs to be met to prevent hard resets > from port partner. > > Without the patch: (5V/3A during SNK_DISCOVERY all the way through > explicit contract) > [ 95.711984] CC1: 0 -> 0, CC2: 0 -> 5 [state TOGGLING, polarity 0, > connected] > [ 95.712007] state change TOGGLING -> SNK_ATTACH_WAIT [rev3 NONE_AMS] > [ 95.712017] pending state change SNK_ATTACH_WAIT -> SNK_DEBOUNCED @ 170 ms > [rev3 NONE_AMS] > [ 95.837190] VBUS on > [ 95.882075] state change SNK_ATTACH_WAIT -> SNK_DEBOUNCED [delayed 170 ms] > [ 95.882082] state change SNK_DEBOUNCED -> SNK_ATTACHED [rev3 NONE_AMS] > [ 95.882086] polarity 1 > [ 95.883151] set_auto_vbus_discharge_threshold mode:0 pps_active:n > vbus:5000 ret:0 > [ 95.883441] enable vbus discharge ret:0 > [ 95.883445] Requesting mux state 1, usb-role 2, orientation 2 > [ 95.883776] state change SNK_ATTACHED -> SNK_STARTUP [rev3 NONE_AMS] > [ 95.883879] pending state change SNK_STARTUP -> SNK_DISCOVERY @ 500 ms > [rev3 NONE_AMS] > [ 96.038960] VBUS on > [ 96.383939] state change SNK_STARTUP -> SNK_DISCOVERY [delayed 500 ms] > [ 96.383946] Setting voltage/current limit 5000 mV 3000 mA > [ 96.383961] vbus=0 charge:=1 > [ 96.386044] state change SNK_DISCOVERY -> SNK_WAIT_CAPABILITIES [rev3 > NONE_AMS] > [ 96.386309] pending state change SNK_WAIT_CAPABILITIES -> HARD_RESET_SEND > @ 450 ms [rev3 NONE_AMS] > [ 96.394404] PD RX, header: 0x2161 [1] > [ 96.394408] PDO 0: type 0, 5000 mV, 3000 mA [E] > [ 96.394410] PDO 1: type 0, 9000 mV, 2000 mA [] > [ 96.394412] state change SNK_WAIT_CAPABILITIES -> > SNK_NEGOTIATE_CAPABILITIES [rev2 POWER_NEGOTIATION] > [ 96.394416] Setting usb_comm capable false > [ 96.395083] cc=0 cc1=0 cc2=5 vbus=0 vconn=sink polarity=1 > [ 96.395089] Requesting PDO 1: 9000 mV, 2000 mA > [ 96.395093] PD TX, header: 0x1042 > [ 96.397404] PD TX complete, status: 0 > [ 96.397424] pending state change SNK_NEGOTIATE_CAPABILITIES -> > HARD_RESET_SEND @ 60 ms [rev2 POWER_NEGOTIATION] > [ 96.400826] PD RX, header: 0x363 [1] > [ 96.400829] state change SNK_NEGOTIATE_CAPABILITIES -> SNK_TRANSITION_SINK > [rev2 POWER_NEGOTIATION] > [ 96.400832] pending state change SNK_TRANSITION_SINK -> HARD_RESET_SEND @ > 500 ms [rev2 POWER_NEGOTIATION] > [ 96.577315] PD RX, header: 0x566 [1] > [ 96.577321] Setting voltage/current limit 9000 mV 2000 mA > [ 96.578363] set_auto_vbus_discharge_threshold mode:3 pps_active:n > vbus:9000 ret:0 > [ 96.578370] state change SNK_TRANSITION_SINK -> SNK_READY [rev2 > POWER_NEGOTIATION] > > With the patch: > [ 168.398573] CC1: 0 -> 0, CC2: 0 -> 5 [state TOGGLING, polarity 0, > connected] > [ 168.398605] state change TOGGLING -> SNK_ATTACH_WAIT [rev3 NONE_AMS] > [ 168.398619] pending state change SNK_ATTACH_WAIT -> SNK_DEBOUNCED @ 170 ms > [rev3 NONE_AMS] > [ 168.522348] VBUS on > [ 168.568676] state change SNK_ATTACH_WAIT -> SNK_DEBOUNCED [delayed 170 ms] > [ 168.568684] state change SNK_DEBOUNCED -> SNK_ATTACHED [rev3 NONE_AMS] > [ 168.568688] polarity 1 > [ 168.569867] set_auto_vbus_discharge_threshold mode:0 pps_active:n > vbus:5000 ret:0 > [ 168.570158] enable vbus discharge ret:0 > [ 168.570161] Requesting mux state 1, usb-role 2, orientation 2 > [ 168.570504] state change SNK_ATTACHED -> SNK_STARTUP [rev3 NONE_AMS] > [ 168.570634] pending state change SNK_STARTUP -> SNK_DISCOVERY @ 500 ms > [rev3 NONE_AMS] > [ 169.070689] state change SNK_STARTUP -> SNK_DISCOVERY [delayed 500 ms] > [ 169.070695] Setting voltage/current limit 5000 mV 3000 mA > [ 169.070702] vbus=0 charge:=1 > [ 169.072719] state change SNK_DISCOVERY -> SNK_WAIT_CAPABILITIES [rev3 > NONE_AMS] > [ 169.073145] pending state change SNK_WAIT_CAPABILITIES -> HARD_RESET_SEND > @ 450 ms [rev3 NONE_AMS] > [ 169.077162] PD RX, header: 0x2161 [1] > [ 169.077172] PDO 0: type 0, 5000 mV, 3000 mA [E] > [ 169.077178] PDO 1: type 0, 9000 mV, 2000 mA [] > [ 169.077183] state change SNK_WAIT_CAPABILITIES -> > SNK_NEGOTIATE_CAPABILITIES [rev2 POWER_NEGOTIATION] > [ 169.077191] Setting usb_comm capable false > [ 169.077753] cc=0 cc1=0 cc2=5 vbus=0 vconn=sink polarity=1 > [ 169.077759] Requesting PDO 1: 9000 mV,
Re: [PATCH v3 2/3] usb: typec: tcpm: Allow slow charging loops to comply to pSnkStby
On Tue, Apr 13, 2021 at 07:39:59PM -0700, Badhri Jagan Sridharan wrote: > When a PD charger advertising Rp-3.0 is connected to a sink port, the > sink port current limit would 3A, during SNK_DISCOVERY, till power > negotiation starts. Once the negotiation starts the power limit needs > to drop down to pSnkStby(500mA @ 5V) and to negotiated current limit > once the explicit contract is in place. Not all charging loops can ramp > up to 3A and drop down to 500mA within tSnkStdby which is 15ms. The port > partner might hard reset if tSnkStdby is not met. > > To solve this problem, this patch introduces slow-charger-loop which > when set makes the port request PD_P_SNK_STDBY_MW upon entering > SNK_DISCOVERY(instead of 3A or the 1.5A during SNK_DISCOVERY) and the > actual currrent limit after RX of PD_CTRL_PSRDY for PD link or during > SNK_READY for non-pd link. > > Signed-off-by: Badhri Jagan Sridharan Reviewed-by: Heikki Krogerus > --- > Changes since V2: > * Refactored code based on Heikki's suggestion > --- > drivers/usb/typec/tcpm/tcpm.c | 17 ++--- > 1 file changed, 14 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c > index aedc8bb9532a..2ad5e14a6867 100644 > --- a/drivers/usb/typec/tcpm/tcpm.c > +++ b/drivers/usb/typec/tcpm/tcpm.c > @@ -459,6 +459,12 @@ struct tcpm_port { > /* Auto vbus discharge status */ > bool auto_vbus_discharge_enabled; > > + /* > + * When set, port requests PD_P_SNK_STDBY_MW upon entering > SNK_DISCOVERY and > + * the actual currrent limit after RX of PD_CTRL_PSRDY for PD link, > + * SNK_READY for non-pd link. > + */ > + bool slow_charger_loop; > #ifdef CONFIG_DEBUG_FS > struct dentry *dentry; > struct mutex logbuffer_lock;/* log buffer access lock */ > @@ -4047,9 +4053,11 @@ static void run_state_machine(struct tcpm_port *port) > break; > case SNK_DISCOVERY: > if (port->vbus_present) { > - tcpm_set_current_limit(port, > -tcpm_get_current_limit(port), > -5000); > + u32 current_lim = tcpm_get_current_limit(port); > + > + if (port->slow_charger_loop || (current_lim > > PD_P_SNK_STDBY_MW / 5)) > + current_lim = PD_P_SNK_STDBY_MW / 5; > + tcpm_set_current_limit(port, current_lim, 5000); > tcpm_set_charge(port, true); > tcpm_set_state(port, SNK_WAIT_CAPABILITIES, 0); > break; > @@ -4161,6 +4169,8 @@ static void run_state_machine(struct tcpm_port *port) > port->pwr_opmode = TYPEC_PWR_MODE_PD; > } > > + if (!port->pd_capable && port->slow_charger_loop) > + tcpm_set_current_limit(port, > tcpm_get_current_limit(port), 5000); > tcpm_swap_complete(port, 0); > tcpm_typec_connect(port); > mod_enable_frs_delayed_work(port, 0); > @@ -5763,6 +5773,7 @@ static int tcpm_fw_get_caps(struct tcpm_port *port, > port->typec_caps.type = ret; > port->port_type = port->typec_caps.type; > > + port->slow_charger_loop = fwnode_property_read_bool(fwnode, > "slow-charger-loop"); > if (port->port_type == TYPEC_PORT_SNK) > goto sink; > > -- > 2.31.1.295.g9ea45b61b8-goog thanks, -- heikki
Re: [PATCH -next] i2c: imx-lpi2c: fix PM reference leak in lpi2c_imx_master_enable()
On Thu, Apr 08, 2021 at 07:19:30PM +0800, Ye Weihua wrote: > The PM reference count is not expected to be incremented on return in > ipi2c_imx_master_enable(). > > However, pm_runtime_get_sync() will increment the PM reference count > even on failure. forgetting to put the reference again will result in > a leak. > > Replace it with pm_runtime_resume_and_get() to keep the usage counter > balanced. > > Reported-by: Hulk Robot > Signed-off-by: Ye Weihua Thanks, yet I applied this series now: http://patchwork.ozlabs.org/project/linux-i2c/list/?series=217733&state=* signature.asc Description: PGP signature
Re: [PATCH -next] i2c: cadence: Fix PM reference leak in cdns_i2c_master_xfer()
On Thu, Apr 08, 2021 at 07:23:52PM +0800, Pu Lehui wrote: > pm_runtime_get_sync() will increment pm usage counter even it failed. > Forgetting to putting operation will result in reference leak here. > Fix it by replacing it with pm_runtime_resume_and_get() to keep usage > counter balanced. > > Signed-off-by: Pu Lehui Thanks, yet I applied this series now: http://patchwork.ozlabs.org/project/linux-i2c/list/?series=217733&state=* signature.asc Description: PGP signature
Re: 回复: Question on KASAN calltrace record in RT
On Wed, 2021-04-14 at 07:29 +, Zhang, Qiang wrote: > > if CONFIG_PREEMPT_RT is enabled and but not in preemptible, the prealloc > should be allowed No, you can't take an rtmutex when not preemptible. -Mike
Re: [PATCH -next] i2c: img-scb: fix PM reference leak in img_i2c_xfer()
On Thu, Apr 08, 2021 at 07:29:10PM +0800, Pu Lehui wrote: > pm_runtime_get_sync() will increment pm usage counter even it failed. > Forgetting to putting operation will result in reference leak here. > Fix it by replacing it with pm_runtime_resume_and_get() to keep usage > counter balanced. > > Signed-off-by: Pu Lehui Thanks, yet I applied this series now: http://patchwork.ozlabs.org/project/linux-i2c/list/?series=217733&state=* signature.asc Description: PGP signature
Re: [PATCH] MIPS: Fix strnlen_user access check
On Tue, Apr 13, 2021 at 04:01:13PM +, David Laight wrote: > From: Thomas Bogendoerfer > > Sent: 13 April 2021 16:19 > > > > On Tue, Apr 13, 2021 at 12:37:25PM +, David Laight wrote: > > > From: Thomas Bogendoerfer > > > > Sent: 13 April 2021 12:15 > > > ... > > > > > The __access_ok() is noted with `Ensure that the range [addr, > > > > > addr+size) > > > > > is within the process's address space`. Does the range checked by > > > > > __access_ok() on MIPS is [addr, addr+size]. So if we want to use > > > > > access_ok(s, 1), should we modify __access_ok()? Or my > > > > > misunderstanding? > > > > > > > > you are right, I'm going to apply > > > > > > > > https://patchwork.kernel.org/project/linux-mips/patch/20190209194718.1294-1-paul.bur...@mips.com/ > > > > > > > > to fix that. > > > > > > Isn't that still wrong? > > > If an application does: > > > write(fd, (void *)0x, 0); > > > it should return 0, not -1 and EFAULT/SIGSEGV. > > > > WRITE(2) Linux Programmer's Manual > > WRITE(2) > > [...] > >If count is zero and fd refers to a regular file, then write() > > may > >return a failure status if one of the errors below is detected. If > > no > >errors are detected, or error detection is not performed, 0 will > > be > >returned without causing any other effect. If count is zero and > > fd > >refers to a file other than a regular file, the results are not > > speci- > >fied. > > [...] > >EFAULT buf is outside your accessible address space. > > > > at least it's covered by the man page on my Linux system. > > Something related definitely caused grief in the setsockopt() changes. > > > > There is also the question about why this makes any difference > > > to the original problem of logging in via the graphical interface. > > > > kernel/module.c:mod->args = strndup_user(uargs, ~0UL >> 1); > > > > and strndup_user does a strnlen_user. > > That call is just gross. > Why did it work before the removal of set_fs() etc. strnlen_user just did the equivalent of access_ok(s, 0) and I copy&pasted the wrong access_ok() statement :-( > Or was there another change that affected strndup_user() ? no, just the change in strnlen_user. Thomas. -- Crap can work. Given enough thrust pigs will fly, but it's not necessarily a good idea.[ RFC1925, 2.3 ]
Re: [PATCH v2 00/16] Multigenerational LRU Framework
On Wed, Apr 14, 2021 at 12:15 AM Huang, Ying wrote: > > Yu Zhao writes: > > > On Tue, Apr 13, 2021 at 8:30 PM Rik van Riel wrote: > >> > >> On Wed, 2021-04-14 at 09:14 +1000, Dave Chinner wrote: > >> > On Tue, Apr 13, 2021 at 10:13:24AM -0600, Jens Axboe wrote: > >> > > >> > > The initial posting of this patchset did no better, in fact it did > >> > > a bit > >> > > worse. Performance dropped to the same levels and kswapd was using > >> > > as > >> > > much CPU as before, but on top of that we also got excessive > >> > > swapping. > >> > > Not at a high rate, but 5-10MB/sec continually. > >> > > > >> > > I had some back and forths with Yu Zhao and tested a few new > >> > > revisions, > >> > > and the current series does much better in this regard. Performance > >> > > still dips a bit when page cache fills, but not nearly as much, and > >> > > kswapd is using less CPU than before. > >> > > >> > Profiles would be interesting, because it sounds to me like reclaim > >> > *might* be batching page cache removal better (e.g. fewer, larger > >> > batches) and so spending less time contending on the mapping tree > >> > lock... > >> > > >> > IOWs, I suspect this result might actually be a result of less lock > >> > contention due to a change in batch processing characteristics of > >> > the new algorithm rather than it being a "better" algorithm... > >> > >> That seems quite likely to me, given the issues we have > >> had with virtual scan reclaim algorithms in the past. > > > > Hi Rik, > > > > Let paste the code so we can move beyond the "batching" hypothesis: > > > > static int __remove_mapping(struct address_space *mapping, struct page > > *page, > > bool reclaimed, struct mem_cgroup *target_memcg) > > { > > unsigned long flags; > > int refcount; > > void *shadow = NULL; > > > > BUG_ON(!PageLocked(page)); > > BUG_ON(mapping != page_mapping(page)); > > > > xa_lock_irqsave(&mapping->i_pages, flags); > > > >> SeongJae, what is this algorithm supposed to do when faced > >> with situations like this: > > > > I'll assume the questions were directed at me, not SeongJae. > > > >> 1) Running on a system with 8 NUMA nodes, and > >> memory > >>pressure in one of those nodes. > >> 2) Running PostgresQL or Oracle, with hundreds of > >>processes mapping the same (very large) shared > >>memory segment. > >> > >> How do you keep your algorithm from falling into the worst > >> case virtual scanning scenarios that were crippling the > >> 2.4 kernel 15+ years ago on systems with just a few GB of > >> memory? > > > > There is a fundamental shift: that time we were scanning for cold pages, > > and nowadays we are scanning for hot pages. > > > > I'd be surprised if scanning for cold pages didn't fall apart, because it'd > > find most of the entries accessed, if they are present at all. > > > > Scanning for hot pages, on the other hand, is way better. Let me just > > reiterate: > > 1) It will not scan page tables from processes that have been sleeping > >since the last scan. > > 2) It will not scan PTE tables under non-leaf PMD entries that do not > >have the accessed bit set, when > >CONFIG_HAVE_ARCH_PARENT_PMD_YOUNG=y. > > 3) It will not zigzag between the PGD table and the same PMD or PTE > >table spanning multiple VMAs. In other words, it finishes all the > >VMAs with the range of the same PMD or PTE table before it returns > >to the PGD table. This optimizes workloads that have large numbers > >of tiny VMAs, especially when CONFIG_PGTABLE_LEVELS=5. > > > > So the cost is roughly proportional to the number of referenced pages it > > discovers. If there is no memory pressure, no scanning at all. For a system > > under heavy memory pressure, most of the pages are referenced (otherwise > > why would it be under memory pressure?), and if we use the rmap, we need to > > scan a lot of pages anyway. Why not just scan them all? > > This may be not the case. For rmap scanning, it's possible to scan only > a small portion of memory. But with the page table scanning, you need > to scan almost all (I understand you have some optimization as above). Hi Ying, Let's take a step back. For the sake of discussion, when does the scanning have to happen? Can we agree that the simplest answer is when we have evicted all inactive pages? If so, my next question is who's filled in the memory space previously occupied by those inactive pages? Newly faulted in pages, right? They have the accessed bit set, and we can't evict them without scanning them first, would you agree? And there are also existing active pages, and they were protected from eviction. But now we need to deactivate some of them. Do you think whether they'd have been used or not since the last scan? (Remember they were active.) You mentioned "a small portion" and "almost all". How do you interpret them in terms of these steps? Intuitively, "a small portion" and "a
Re: [Outreachy kernel] [PATCH] :staging: rtl8723bs: Remove useless led_blink_hdl()
On Wednesday, April 14, 2021 9:00:25 AM CEST Dan Carpenter wrote: > On Wed, Apr 14, 2021 at 08:33:48AM +0200, Fabio M. De Francesco wrote: > > On Wednesday, April 14, 2021 7:21:50 AM CEST Dan Carpenter wrote: > > > On Tue, Apr 13, 2021 at 10:08:32PM +0200, Fabio M. De Francesco wrote: > > > > On Tuesday, April 13, 2021 9:48:44 PM CEST Matthew Wilcox wrote: > > > > > On Tue, Apr 13, 2021 at 09:45:03PM +0200, Fabio M. De Francesco > > > > wrote: > > > > > > 1) The driver doesn't call that function from anywhere else > > > > > > than > > > > > > the > > > > > > macro. 2) You have explained that the macro add its symbol to a > > > > > > slot > > > > > > in an array that would shift all the subsequent elements down > > > > > > if > > > > > > that > > > > > > macro is not used exactly in the line where it is. > > > > > > 3) Dan Carpenter said that that array is full of null functions > > > > > > (or > > > > > > empty slots?). > > > > > > > > > > > > Unless that function is called anonymously dereferencing its > > > > > > address > > > > > > from the position it occupies in the array, I'm not able to see > > > > > > what > > > > > > else means can any caller use. > > > > > > > > > > > > I know I have much less experience than you with C: what can go > > > > > > wrong? > > > > > > > > > > Here's where the driver calls that function: > > > > > > > > > > $ git grep wlancmds drivers/staging/rtl8723bs/ > > > > > drivers/staging/rtl8723bs/core/rtw_cmd.c:static struct cmd_hdl > > > > > > > > > > wlancmds[] = { drivers/staging/rtl8723bs/core/rtw_cmd.c: > > > > > if > > > > > > > > > > (pcmd->cmdcode < ARRAY_SIZE(wlancmds)) { > > > > > drivers/staging/rtl8723bs/core/rtw_cmd.c: > > > > > cmd_hdl > > > > > = wlancmds[pcmd->cmdcode].h2cfuns; > > > > > > > > OK, I had imagined an anonymous call from its location in the array > > > > (as > > > > I wrote in the last phrase of my message). However, I thought that > > > > it > > > > could have been an improbable possibility, not a real one. > > > > > > > > Linux uses a lot of interesting ideas that newcomers like me should > > > > learn. Things here are trickier than they appear at first sight. > > > > > > One trick would be to build the Smatch cross function database. > > > > > > https://www.spinics.net/lists/smatch/msg00568.html > > > > > > Then you could do: > > > > > > $ ~/path/to/smatch_data/db/smdb.py led_blink_hdl > > > file | caller | function | type | parameter | key | value | > > > drivers/staging/rtl8723bs/core/rtw_cmd.c | rtw_cmd_thread | > > > rtw_cmd_thread ptr cmd_hdl | INTERNAL | -1 | > > > | > > > uchar(*)(struct adapter*, uchar*) > > > drivers/staging/rtl8188eu/core/rtw_cmd.c | rtw_cmd_thread | > > > rtw_cmd_thread ptr cmd_hdl | INTERNAL | -1 | > > > | > > > uchar(*)(struct adapter*, uchar*) > > > drivers/staging/rtl8188eu/core/rtw_cmd.c | rtw_cmd_thread | > > > rtw_cmd_thread ptr cmd_hdl | BUF_SIZE | 1 | > > > pbuf | > > > 1,4,6,8,12,14,16,19-20,23-24,48,740,884,892,900,960 > > > > > > > > > Which says that led_blink_hdl() is called as a function pointer > > > called > > > "cmd_hdl" from rtw_cmd_thread(). > > > > > > Hm... It says it can be called from either rtw_cmd_thread() function > > > (the rtl8723bs or rtl8188eu version) which is not ideal. But also > > > basically harmless so whatever... > > > > > > regards, > > > dan carpenter > > > > Nice tool, thanks. I'll surely use it when it is needed to find out > > which callers use a function pointer. > > > > However I cannot see how it can help in this context. That function > > *does* something, even if I cannot understand why someone needs a > > function to test the initialization of a pointer. Furthermore it is > > actually called by rtw_cmd_thread() (as you found out by using smatch) > > that expect one of the two possible values that led_blink_hdl() > > returns. > > > > That said, what trick could I use for the purpose of getting rid of > > that > > function? At this point I'm not sure it could be made. > > If you look at how this is called: > > drivers/staging/rtl8723bs/core/rtw_cmd.c >449 memcpy(pcmdbuf, pcmd->parmbuf, pcmd->cmdsz); >450 >451 if (pcmd->cmdcode < ARRAY_SIZE(wlancmds)) { >452 cmd_hdl = > wlancmds[pcmd->cmdcode].h2cfuns; 453 >454 if (cmd_hdl) { >455 ret = cmd_hdl(pcmd->padapter, > pcmdbuf); ^^^ > >456 pcmd->res = ret; >457 } >458 >459 pcmdpriv->cmd_seq++; >460 } else { >461 pcmd->res = H2C_PARAMETERS_ERROR; >462 } >463 >464 cmd_hdl = NULL; > > The led_blink_hdl() function returns success if "pcmdbuf" is non-NULL > and fail if it's NULL.
[PATCH v6] hwmon: Add driver for fsp-3y PSUs and PDUs
This patch adds support for these devices: - YH-5151E - the PDU - YM-2151E - the PSU The device datasheet says that the devices support PMBus 1.2, but in my testing, a lot of the commands aren't supported and if they are, they sometimes behave strangely or inconsistently. For example, writes to the PAGE command requires using PEC, otherwise the write won't work and the page won't switch, even though, the standard says that PEC is optional. On the other hand, writes to SMBALERT don't require PEC. Because of this, the driver is mostly reverse engineered with the help of a tool called pmbus_peek written by David Brownell (and later adopted by my colleague Jan Kundrát). The device also has some sort of a timing issue when switching pages, which is explained further in the code. Because of this, the driver support is limited. It exposes only the values, that have been tested to work correctly. Signed-off-by: Václav Kubernát --- Documentation/hwmon/fsp-3y.rst | 28 Documentation/hwmon/index.rst | 1 + drivers/hwmon/pmbus/Kconfig| 10 ++ drivers/hwmon/pmbus/Makefile | 1 + drivers/hwmon/pmbus/fsp-3y.c | 253 + 5 files changed, 293 insertions(+) create mode 100644 Documentation/hwmon/fsp-3y.rst create mode 100644 drivers/hwmon/pmbus/fsp-3y.c diff --git a/Documentation/hwmon/fsp-3y.rst b/Documentation/hwmon/fsp-3y.rst new file mode 100644 index ..5693d83a2035 --- /dev/null +++ b/Documentation/hwmon/fsp-3y.rst @@ -0,0 +1,28 @@ +.. SPDX-License-Identifier: GPL-2.0 + +Kernel driver fsp3y +== +Supported devices: + * 3Y POWER YH-5151E + * 3Y POWER YM-2151E + +Author: Václav Kubernát + +Description +--- +This driver implements limited support for two 3Y POWER devices. + +Sysfs entries +- + * in1_inputinput voltage + * in2_input12V output voltage + * in3_input5V output voltage + * curr1_input input current + * curr2_input 12V output current + * curr3_input 5V output current + * fan1_input fan rpm + * temp1_input temperature 1 + * temp2_input temperature 2 + * temp3_input temperature 3 + * power1_input input power + * power2_input output power diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst index fcb870ce6286..55c9f014c248 100644 --- a/Documentation/hwmon/index.rst +++ b/Documentation/hwmon/index.rst @@ -63,6 +63,7 @@ Hardware Monitoring Kernel Drivers f71805f f71882fg fam15h_power + fsp-3y ftsteutates g760a g762 diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig index 03606d4298a4..9d12d446396c 100644 --- a/drivers/hwmon/pmbus/Kconfig +++ b/drivers/hwmon/pmbus/Kconfig @@ -56,6 +56,16 @@ config SENSORS_BEL_PFE This driver can also be built as a module. If so, the module will be called bel-pfe. +config SENSORS_FSP_3Y + tristate "FSP/3Y-Power power supplies" + help + If you say yes here you get hardware monitoring support for + FSP/3Y-Power hot-swap power supplies. + Supported models: YH-5151E, YM-2151E + + This driver can also be built as a module. If so, the module will + be called fsp-3y. + config SENSORS_IBM_CFFPS tristate "IBM Common Form Factor Power Supply" depends on LEDS_CLASS diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile index 6a4ba0fdc1db..bfe218ad898f 100644 --- a/drivers/hwmon/pmbus/Makefile +++ b/drivers/hwmon/pmbus/Makefile @@ -8,6 +8,7 @@ obj-$(CONFIG_SENSORS_PMBUS) += pmbus.o obj-$(CONFIG_SENSORS_ADM1266) += adm1266.o obj-$(CONFIG_SENSORS_ADM1275) += adm1275.o obj-$(CONFIG_SENSORS_BEL_PFE) += bel-pfe.o +obj-$(CONFIG_SENSORS_FSP_3Y) += fsp-3y.o obj-$(CONFIG_SENSORS_IBM_CFFPS)+= ibm-cffps.o obj-$(CONFIG_SENSORS_INSPUR_IPSPS) += inspur-ipsps.o obj-$(CONFIG_SENSORS_IR35221) += ir35221.o diff --git a/drivers/hwmon/pmbus/fsp-3y.c b/drivers/hwmon/pmbus/fsp-3y.c new file mode 100644 index ..564649e87e34 --- /dev/null +++ b/drivers/hwmon/pmbus/fsp-3y.c @@ -0,0 +1,253 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Hardware monitoring driver for FSP 3Y-Power PSUs + * + * Copyright (c) 2021 Václav Kubernát, CESNET + * + * This driver is mostly reverse engineered with the help of a tool called pmbus_peek written by + * David Brownell (and later adopted by Jan Kundrát). The device has some sort of a timing issue + * when switching pages, details are explained in the code. The driver support is limited. It + * exposes only the values, that have been tested to work correctly. Unsupported values either + * aren't supported by the devices or their encondings are unknown. + */ + +#include +#include +#include +#include +#include "pmbus.h" + +#define YM2151_PAGE_12V_LOG0x00 +#define YM2151_PAGE_12V_REAL 0x00 +#define YM2151_PAGE_5VSB_LOG 0x0
Re: [PATCH -next] i2c: sprd: Fix PM reference leak in sprd_i2c_master_xfer()
On Thu, Apr 08, 2021 at 08:59:15PM +0800, Li Huafei wrote: > pm_runtime_get_sync will increment pm usage counter even it failed. > Forgetting to putting operation will result in reference leak here. Fix > it by replacing it with pm_runtime_resume_and_get to keep usage counter > balanced. > > Reported-by: Hulk Robot > Signed-off-by: Li Huafei Thanks, yet I applied this series now: http://patchwork.ozlabs.org/project/linux-i2c/list/?series=217733&state=* signature.asc Description: PGP signature
Re: [PATCH -next] i2c: omap: fix PM reference leak in omap_i2c_probe()
On Thu, Apr 08, 2021 at 08:56:48PM +0800, Li Huafei wrote: > pm_runtime_get_sync will increment pm usage counter even it failed. > Forgetting to putting operation will result in reference leak here. Fix > it by replacing it with pm_runtime_resume_and_get to keep usage counter > balanced. > > Reported-by: Hulk Robot > Signed-off-by: Li Huafei Thanks, yet I applied this series now: http://patchwork.ozlabs.org/project/linux-i2c/list/?series=217733&state=* signature.asc Description: PGP signature
Re: [PATCH] usb: typec: silence a static checker warning
On Wed, Apr 14, 2021 at 10:44:40AM +0300, Dan Carpenter wrote: > Smatch complains about a potential missing error code: > > drivers/usb/typec/port-mapper.c:168 typec_link_port() > warn: missing error code 'ret' > > This is a false positive and returning zero is intentional. Let's > re-arrange the code to silence the warning and make the intent more > clear. > > Signed-off-by: Dan Carpenter Reviewed-by: Heikki Krogerus > --- > drivers/usb/typec/port-mapper.c | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/typec/port-mapper.c b/drivers/usb/typec/port-mapper.c > index fae736eb0601..9b0991bdf391 100644 > --- a/drivers/usb/typec/port-mapper.c > +++ b/drivers/usb/typec/port-mapper.c > @@ -157,15 +157,17 @@ int typec_link_port(struct device *port) > { > struct device *connector; > struct port_node *node; > - int ret = 0; > + int ret; > > node = create_port_node(port); > if (IS_ERR(node)) > return PTR_ERR(node); > > connector = find_connector(node); > - if (!connector) > + if (!connector) { > + ret = 0; > goto remove_node; > + } > > ret = link_port(to_typec_port(connector), node); > if (ret) > -- > 2.30.2 thanks, -- heikki
[PATCH 1/2] audit: Add syscall return code handling for compat task
When 32-bit userspace application is running on 64-bit kernel, the 32-bit syscall return code would be changed from u32 to u64 in regs_return_value and then changed to s64. Hence the negative return code recorded by audit would end up being a big positive number like below. type=SYSCALL msg=audit(160715.887:582): arch=4028 syscall=322 success=yes exit=4294967283 This patch forces the u32->s32->s64 for compat tasks. Signed-off-by: He Zhe --- include/linux/audit.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/include/linux/audit.h b/include/linux/audit.h index 82b7c1116a85..32cb853f3029 100644 --- a/include/linux/audit.h +++ b/include/linux/audit.h @@ -334,7 +334,9 @@ static inline void audit_syscall_exit(void *pt_regs) { if (unlikely(audit_context())) { int success = is_syscall_success(pt_regs); - long return_code = regs_return_value(pt_regs); + long return_code = is_compat_task() ? + (s64)(s32)regs_return_value(pt_regs) : + regs_return_value(pt_regs); __audit_syscall_exit(success, return_code); } -- 2.17.1
Re: [PATCH] staging: media: omap4iss: Remove unused macro functions
On 13/04/2021 20:21, ascordeiro wrote: > Em ter, 2021-04-13 às 17:06 +0200, Hans Verkuil escreveu: >> On 12/04/2021 15:42, Aline Santana Cordeiro wrote: >>> Remove unused macro functions "to_iss_device()", "to_device()", >>> and "v4l2_dev_to_iss_device(dev)". >> >> 'git grep to_iss_device drivers/staging/omap4iss' gives me lots of >> hits! >> Same for to_device. Only v4l2_dev_to_iss_device appears to be unused. >> >> Regards, >> >> Hans >> > This command is really helpful, I didin't know. > Thank you for the tip. > > May I send a v2 removing just v4l2_dev_to_iss_device? Sure, that's fine. Regards, Hans > > Thank you in advance, > Aline > >>> >>> Signed-off-by: Aline Santana Cordeiro < >>> alinesantanacorde...@gmail.com> >>> --- >>> drivers/staging/media/omap4iss/iss.h | 8 >>> 1 file changed, 8 deletions(-) >>> >>> diff --git a/drivers/staging/media/omap4iss/iss.h >>> b/drivers/staging/media/omap4iss/iss.h >>> index b88f952..a354d5f 100644 >>> --- a/drivers/staging/media/omap4iss/iss.h >>> +++ b/drivers/staging/media/omap4iss/iss.h >>> @@ -29,11 +29,6 @@ >>> >>> struct regmap; >>> >>> -#define to_iss_device(ptr_module) \ >>> - container_of(ptr_module, struct iss_device, ptr_module) >>> -#define >>> to_device(ptr_module) \ >>> - (to_iss_device(ptr_module)->dev) >>> - >>> enum iss_mem_resources { >>> OMAP4_ISS_MEM_TOP, >>> OMAP4_ISS_MEM_CSI2_A_REGS1, >>> @@ -119,9 +114,6 @@ struct iss_device { >>> unsigned int isp_subclk_resources; >>> }; >>> >>> -#define v4l2_dev_to_iss_device(dev) \ >>> - container_of(dev, struct iss_device, v4l2_dev) >>> - >>> int omap4iss_get_external_info(struct iss_pipeline *pipe, >>> struct media_link *link); >>> >>> >> > >
[PATCH 2/2] ptrace: is_syscall_success: Add syscall return code handling for compat task
When 32-bit userspace application is running on 64-bit kernel, the 32-bit syscall return code would be changed from u32 to u64 in regs_return_value and then changed to s64. Hence the negative return code would be treated as a positive number and results in a non-error in, for example, audit like below. type=SYSCALL msg=audit(160715.887:582): arch=4028 syscall=322 success=yes exit=4294967283 This patch forces the u32->s32->s64 for compat tasks. Signed-off-by: He Zhe --- include/linux/ptrace.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h index b5ebf6c01292..bc3056fff8a6 100644 --- a/include/linux/ptrace.h +++ b/include/linux/ptrace.h @@ -260,7 +260,9 @@ static inline void ptrace_release_task(struct task_struct *task) * is an error value. On some systems like ia64 and powerpc they have different * indicators of success/failure and must define their own. */ -#define is_syscall_success(regs) (!IS_ERR_VALUE((unsigned long)(regs_return_value(regs +#define is_syscall_success(regs) (!IS_ERR_VALUE(is_compat_task() ? \ + (unsigned long)(s64)(s32)(regs_return_value(regs)) : \ + (unsigned long)(regs_return_value(regs #endif /* -- 2.17.1
Re: [PATCH v5] hwmon: Add driver for fsp-3y PSUs and PDUs
st 14. 4. 2021 v 5:29 odesílatel Guenter Roeck napsal: > > On Wed, Apr 14, 2021 at 02:13:06AM +0200, Václav Kubernát wrote: > > This patch adds support for these devices: > > - YH-5151E - the PDU > > - YM-2151E - the PSU > > > > The device datasheet says that the devices support PMBus 1.2, but in my > > testing, a lot of the commands aren't supported and if they are, they > > sometimes behave strangely or inconsistently. For example, writes to the > > PAGE command requires using PEC, otherwise the write won't work and the > > page won't switch, even though, the standard says that PEC is optional. > > On the other hand, writes to SMBALERT don't require PEC. Because of > > this, the driver is mostly reverse engineered with the help of a tool > > called pmbus_peek written by David Brownell (and later adopted by my > > colleague Jan Kundrát). > > > > The device also has some sort of a timing issue when switching pages, > > which is explained further in the code. > > > > Because of this, the driver support is limited. It exposes only the > > values, that have been tested to work correctly. > > > > Signed-off-by: Václav Kubernát > > checkpatch says: > > WARNING: Missing or malformed SPDX-License-Identifier tag in line 1 > #108: FILE: Documentation/hwmon/fsp-3y.rst:1: > +Kernel driver fsp3y > > WARNING: line length of 137 exceeds 100 columns > #409: FILE: drivers/hwmon/pmbus/fsp-3y.c:225: > + dev_warn(&client->dev, "Device mismatch: Configured %s (%d), > detected %d\n", id->name, (int)id->driver_data, data->chip); > > Please fix and resubmit. > Done. Václav > Thanks, > Guenter
Re: [PATCH -next] i2c: imx: Fix PM reference leak in i2c_imx_reg_slave()
On Thu, Apr 08, 2021 at 07:06:38PM +0800, Ye Weihua wrote: > The PM reference count is not expected to be incremented on return in > these functions. > > However, pm_runtime_get_sync() will increment the PM reference count > even on failure. forgetting to put the reference again will result in > a leak. > > Replace it with pm_runtime_resume_and_get() to keep the usage counter > balanced. > > Reported-by: Hulk Robot > Signed-off-by: Ye Weihua After rebasing, only one hunk was left: > @@ -801,7 +801,7 @@ static int i2c_imx_reg_slave(struct i2c_client *client) > i2c_imx->last_slave_event = I2C_SLAVE_STOP; > > /* Resume */ > - ret = pm_runtime_get_sync(i2c_imx->adapter.dev.parent); > + ret = pm_runtime_resume_and_get(i2c_imx->adapter.dev.parent); > if (ret < 0) { > dev_err(&i2c_imx->adapter.dev, "failed to resume i2c > controller"); > return ret; I applied this to for-next, thanks! signature.asc Description: PGP signature
Re: [PATCH v3 2/3] drm: bridge: add it66121 driver
On Wed, 14 Apr 2021 at 08:13, Neil Armstrong wrote: > > Hi Rob, > > Le 13/04/2021 à 22:21, Robert Foss a écrit : > > Hey Neil & Phong, > > > > Thanks for submitting this series! > > > >> + > >> +static const struct drm_bridge_funcs it66121_bridge_funcs = { > >> + .attach = it66121_bridge_attach, > >> + .enable = it66121_bridge_enable, > >> + .disable = it66121_bridge_disable, > >> + .mode_set = it66121_bridge_mode_set, > >> + .mode_valid = it66121_bridge_mode_valid, > >> + .detect = it66121_bridge_detect, > >> + .get_edid = it66121_bridge_get_edid, > >> + .atomic_get_output_bus_fmts = > >> it66121_bridge_atomic_get_output_bus_fmts, > >> + .atomic_get_input_bus_fmts = > >> it66121_bridge_atomic_get_input_bus_fmts, > >> +}; > > > > I would like to see an implementation of HPD, since it is supported by > > the hardware[1] (and required by the documentation). IRQ status bit 0 > > seems to be the responsible for notifying us about hot plug detection > > events. > > It's implemented in the IRQ handler with the IT66121_INT_STATUS1_HPD_STATUS > event. > I didn't even get that far :) Either way, the HPD support should be exposed in drm_bridge_funcs (.hpd_enable, .hpd_disable (and possibly .hpd_notify)) and drm_bridge.ops (DRM_BRIDGE_OP_HPD).
RE: Regression v5.12-rc3: net: stmmac: re-init rx buffers when mac resume back
> -Original Message- > From: Thierry Reding > Sent: 2021年4月14日 15:41 > To: Joakim Zhang > Cc: David S. Miller ; Jakub Kicinski ; > Jon Hunter ; Giuseppe Cavallaro > ; Alexandre Torgue ; > Jose Abreu ; net...@vger.kernel.org; Linux Kernel > Mailing List ; linux-tegra > > Subject: Re: Regression v5.12-rc3: net: stmmac: re-init rx buffers when mac > resume back > > On Wed, Apr 14, 2021 at 02:18:58AM +, Joakim Zhang wrote: > > > > > -Original Message- > > > From: Thierry Reding > > > Sent: 2021年4月14日 0:07 > > > To: David S. Miller ; Jakub Kicinski > > > > > > Cc: Joakim Zhang ; Jon Hunter > > > ; Giuseppe Cavallaro ; > > > Alexandre Torgue ; Jose Abreu > > > ; net...@vger.kernel.org; Linux Kernel Mailing > > > List ; linux-tegra > > > > > > Subject: Re: Regression v5.12-rc3: net: stmmac: re-init rx buffers > > > when mac resume back > > > > > > On Tue, Apr 13, 2021 at 12:13:01PM +, Joakim Zhang wrote: > > > > > > > > Hi Jon, > > > > > > > > > -Original Message- > > > > > From: Jon Hunter > > > > > Sent: 2021年4月13日 16:41 > > > > > To: Joakim Zhang ; Giuseppe Cavallaro > > > > > ; Alexandre Torgue > > > > > ; Jose Abreu > > > > > Cc: net...@vger.kernel.org; Linux Kernel Mailing List > > > > > ; linux-tegra > > > > > ; Jakub Kicinski > > > > > Subject: Re: Regression v5.12-rc3: net: stmmac: re-init rx > > > > > buffers when mac resume back > > > > > > > > > > > > > > > On 01/04/2021 17:28, Jon Hunter wrote: > > > > > > > > > > > > On 31/03/2021 12:41, Joakim Zhang wrote: > > > > > > > > > > > > ... > > > > > > > > > > > >>> In answer to your question, resuming from suspend does work > > > > > >>> on this board without your change. We have been testing > > > > > >>> suspend/resume now on this board since Linux v5.8 and so we > > > > > >>> have the ability to bisect such regressions. So it is clear > > > > > >>> to me that this is the change that caused > > > > > this, but I am not sure why. > > > > > >> > > > > > >> Yes, I know this issue is regression caused by my patch. I > > > > > >> just want to > > > > > analyze the potential reasons. Due to the code change only > > > > > related to the page recycle and reallocate. > > > > > >> So I guess if this page operate need IOMMU works when IOMMU > > > > > >> is > > > enabled. > > > > > Could you help check if IOMMU driver resume before STMMAC? Our > > > > > common desire is to find the root cause, right? > > > > > > > > > > > > > > > > > > Yes of course that is the desire here indeed. I had assumed > > > > > > that the suspend/resume order was good because we have never > > > > > > seen any problems, but nonetheless it is always good to check. > > > > > > Using ftrace I enabled tracing of the appropriate > > > > > > suspend/resume functions and this is what I see ... > > > > > > > > > > > > # tracer: function > > > > > > # > > > > > > # entries-in-buffer/entries-written: 4/4 #P:6 > > > > > > # > > > > > > #_-=> irqs-off > > > > > > # / _=> need-resched > > > > > > # | / _---=> hardirq/softirq > > > > > > # || / _--=> preempt-depth > > > > > > # ||| / delay > > > > > > # TASK-PID CPU# TIMESTAMP > FUNCTION > > > > > > # | | | | | > > > > > > rtcwake-748 [000] ...1 536.700777: > > > > > stmmac_pltfr_suspend <-platform_pm_suspend > > > > > > rtcwake-748 [000] ...1 536.735532: > > > > > arm_smmu_pm_suspend <-platform_pm_suspend > > > > > > rtcwake-748 [000] ...1 536.757290: > > > > > arm_smmu_pm_resume <-platform_pm_resume > > > > > > rtcwake-748 [003] ...1 536.856771: > > > > > stmmac_pltfr_resume <-platform_pm_resume > > > > > > > > > > > > > > > > > > So I don't see any ordering issues that could be causing this. > > > > > > > > > > > > > > > Another thing I have found is that for our platform, if the > > > > > driver for the ethernet PHY (in this case broadcom PHY) is > > > > > enabled, then it fails to resume but if I disable the PHY in the > > > > > kernel configuration, then resume works. I have found that if I > > > > > move the reinit of the RX buffers to before the startup of the > > > > > phy, then it can resume > > > OK with the PHY enabled. > > > > > > > > > > Does the following work for you? Does your platform use a > > > > > specific ethernet PHY driver? > > > > > > > > I am also looking into this issue these days, we use the Realtek > > > > RTL8211FDI > > > PHY, driver is drivers/net/phy/realtek.c. > > > > > > > > For our EQOS MAC integrated in our SoC, Rx side logic depends on > > > > RXC clock > > > from PHY, so we need phylink_start before MAC. > > > > > > > > I will test below code change tomorrow to see if it can work at my > > > > side, since > > > it is only re-init memory, need not RXC clock. > > > > > > > > > > > > > diff --g
Re: [Outreachy kernel] [PATCH] :staging: rtl8723bs: Remove useless led_blink_hdl()
On Wed, Apr 14, 2021 at 09:59:36AM +0200, Fabio M. De Francesco wrote: > Fine. I'm about to submit a patch. > > Is there any formal means to acknowledge (in the patch) your contribution > to the resolution of this problem? It doesn't matter. Suggested-by. regards, dan carpenter
Re: [PATCH v3 2/3] drm: bridge: add it66121 driver
On 14/04/2021 10:06, Robert Foss wrote: > On Wed, 14 Apr 2021 at 08:13, Neil Armstrong wrote: >> >> Hi Rob, >> >> Le 13/04/2021 à 22:21, Robert Foss a écrit : >>> Hey Neil & Phong, >>> >>> Thanks for submitting this series! >>> + +static const struct drm_bridge_funcs it66121_bridge_funcs = { + .attach = it66121_bridge_attach, + .enable = it66121_bridge_enable, + .disable = it66121_bridge_disable, + .mode_set = it66121_bridge_mode_set, + .mode_valid = it66121_bridge_mode_valid, + .detect = it66121_bridge_detect, + .get_edid = it66121_bridge_get_edid, + .atomic_get_output_bus_fmts = it66121_bridge_atomic_get_output_bus_fmts, + .atomic_get_input_bus_fmts = it66121_bridge_atomic_get_input_bus_fmts, +}; >>> >>> I would like to see an implementation of HPD, since it is supported by >>> the hardware[1] (and required by the documentation). IRQ status bit 0 >>> seems to be the responsible for notifying us about hot plug detection >>> events. >> >> It's implemented in the IRQ handler with the IT66121_INT_STATUS1_HPD_STATUS >> event. >> > > I didn't even get that far :) > > Either way, the HPD support should be exposed in drm_bridge_funcs > (.hpd_enable, .hpd_disable (and possibly .hpd_notify)) and > drm_bridge.ops (DRM_BRIDGE_OP_HPD). > Indeed I forgot these calls in the NO_CONNECTOR implementation... Neil
Re: [PATCH 02/10] mm/numa: automatically generate node migration order
On Fri, Apr 09, 2021 at 08:07:08PM -0700, Wei Xu wrote: > On Thu, Apr 1, 2021 at 11:35 AM Dave Hansen > wrote: > > + * When Node 0 fills up, its memory should be migrated to > > + * Node 1. When Node 1 fills up, it should be migrated to > > + * Node 2. The migration path start on the nodes with the > > + * processors (since allocations default to this node) and > > + * fast memory, progress through medium and end with the > > + * slow memory: > > + * > > + * 0 -> 1 -> 2 -> stop > > + * 3 -> 4 -> 5 -> stop > > + * > > + * This is represented in the node_demotion[] like this: > > + * > > + * { 1, // Node 0 migrates to 1 > > + *2, // Node 1 migrates to 2 > > + * -1, // Node 2 does not migrate > > + *4, // Node 3 migrates to 4 > > + *5, // Node 4 migrates to 5 > > + * -1} // Node 5 does not migrate > > + */ > > In this example, if we want to support multiple nodes as the demotion > target of a source node, we can group these nodes into three tiers > (classes): > > fast class: > 0 -> {1, 4} // 1 is the preferred > 3 -> {4, 1} // 4 is the preferred > > medium class: > 1 -> {2, 5} // 2 is the preferred > 4 -> {5, 2} // 5 is the preferred > > slow class: > 2 -> stop > 5 -> stop Hi Wei Xu, I have some questions about it Fast class/memory are pictured as those nodes with CPUs, while Slow class/memory are PMEM, right? Then, what stands for medium class/memory? In Dave's example, list is created in a way that stays local to the socket, and we go from the fast one to the slow one. In yours, lists are created taking the fastest nodes from all sockets and we work our way down, which means have cross-socket nodes in the list. How much of a penalty is that? And while I get your point, I am not sure if that is what we pretend here. This patchset aims to place cold pages that are about to be reclaim in slower nodes to give them a second chance, while your design seems more to have kind of different memory clases and be able to place applications in one of those tiers depending on its demands or sysadmin-demand. Could you expand some more? -- Oscar Salvador SUSE L3
回复: 回复: Question on KASAN calltrace record in RT
发件人: Mike Galbraith 发送时间: 2021年4月14日 15:56 收件人: Zhang, Qiang; Dmitry Vyukov 抄送: Andrew Halaney; andreyk...@gmail.com; ryabinin@gmail.com; a...@linux-foundation.org; linux-kernel@vger.kernel.org; kasan-...@googlegroups.com 主题: Re: 回复: Question on KASAN calltrace record in RT [Please note: This e-mail is from an EXTERNAL e-mail address] On Wed, 2021-04-14 at 07:29 +, Zhang, Qiang wrote: > > if CONFIG_PREEMPT_RT is enabled and but not in preemptible, the prealloc > should be allowed > >No, you can't take an rtmutex when not preemptible. > Oh, I'm in a mess, Thank you for your explanation. >-Mike
Re: [PATCH 1/1] mm: Fix struct page layout on 32-bit systems
On Mon, 12 Apr 2021 02:15:32 +0100 Matthew Wilcox wrote: > On Sun, Apr 11, 2021 at 11:33:18AM +0100, Matthew Wilcox wrote: > > Basically, we have three aligned dwords here. We can either alias with > > @flags and the first word of @lru, or the second word of @lru and @mapping, > > or @index and @private. @flags is a non-starter. If we use @mapping, > > then you have to set it to NULL before you free it, and I'm not sure > > how easy that will be for you. If that's trivial, then we could use > > the layout: > > > > unsigned long _pp_flags; > > unsigned long pp_magic; > > union { > > dma_addr_t dma_addr;/* might be one or two words */ > > unsigned long _pp_align[2]; > > }; > > unsigned long pp_pfmemalloc; > > unsigned long xmi; > > I forgot about the munmap path. That calls zap_page_range() which calls > set_page_dirty() which calls page_mapping(). If we use page->mapping, > that's going to get interpreted as an address_space pointer. > > *sigh*. Foiled at every turn. Yes, indeed! - And very frustrating. It's keeping me up at night. I'm dreaming about 32 vs 64 bit data structures. My fitbit stats tell me that I don't sleep well with these kind of dreams ;-) > I'm kind of inclined towards using two (or more) bits for PageSlab as > we discussed here: > > https://lore.kernel.org/linux-mm/01000163efe179fe-d6270c58-eaba-482f-a6bd-334667250ef7-000...@email.amazonses.com/ > > so we have PageKAlloc that's true for PageSlab, PagePool, PageDMAPool, > PageVMalloc, PageFrag and maybe a few other kernel-internal allocations. I actually like this idea a lot. I also think it will solve or remove Matteo/Ilias'es[2] need to introduce the pp_magic signature. Ilias do say[1] that page_pool pages could be used for TCP RX zerocopy, but I don't think we should "allow" that (meaning page_pool should drop the DMA-mapping and give up recycling). I should argue why in that thread. That said, I think we need to have a quicker fix for the immediate issue with 64-bit bit dma_addr on 32-bit arch and the misalignment hole it leaves[3] in struct page. In[3] you mention ppc32, does it only happens on certain 32-bit archs? I'm seriously considering removing page_pool's support for doing/keeping DMA-mappings on 32-bit arch's. AFAIK only a single driver use this. > (see also here:) > https://lore.kernel.org/linux-mm/20180518194519.3820-18-wi...@infradead.org/ [1] https://lore.kernel.org/netdev/YHHuE7g73mZNrMV4@enceladus/ [2] https://patchwork.kernel.org/project/netdevbpf/patch/20210409223801.104657-3-mcr...@linux.microsoft.com/ [3] https://lore.kernel.org/linux-mm/20210410024313.gx2531...@casper.infradead.org/ -- Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat LinkedIn: http://www.linkedin.com/in/brouer
Re: [PATCH 02/10] mm/numa: automatically generate node migration order
On Wed, Apr 14, 2021 at 10:08:54AM +0200, Oscar Salvador wrote: > In Dave's example, list is created in a way that stays local to the socket, > and we go from the fast one to the slow one. Or maybe it is just because find_next_best_node() does not know any better and creates the list that way? -- Oscar Salvador SUSE L3
Re: [PATCH v2] usb: dwc3: core: Add shutdown callback for dwc3
Sandeep Maheswaram writes: > This patch adds a shutdown callback to USB DWC core driver to ensure that > it is properly shutdown in reboot/shutdown path. This is required > where SMMU address translation is enabled like on SC7180 > SoC and few others. If the hardware is still accessing memory after > SMMU translation is disabled as part of SMMU shutdown callback in > system reboot or shutdown path, then IOVAs(I/O virtual address) > which it was using will go on the bus as the physical addresses which > might result in unknown crashes (NoC/interconnect errors). > > Signed-off-by: Sandeep Maheswaram Acked-by: Felipe Balbi -- balbi signature.asc Description: PGP signature
Re: [PATCH 02/10] mm/numa: automatically generate node migration order
On 14.04.21 10:08, Oscar Salvador wrote: On Fri, Apr 09, 2021 at 08:07:08PM -0700, Wei Xu wrote: On Thu, Apr 1, 2021 at 11:35 AM Dave Hansen wrote: + * When Node 0 fills up, its memory should be migrated to + * Node 1. When Node 1 fills up, it should be migrated to + * Node 2. The migration path start on the nodes with the + * processors (since allocations default to this node) and + * fast memory, progress through medium and end with the + * slow memory: + * + * 0 -> 1 -> 2 -> stop + * 3 -> 4 -> 5 -> stop + * + * This is represented in the node_demotion[] like this: + * + * { 1, // Node 0 migrates to 1 + *2, // Node 1 migrates to 2 + * -1, // Node 2 does not migrate + *4, // Node 3 migrates to 4 + *5, // Node 4 migrates to 5 + * -1} // Node 5 does not migrate + */ In this example, if we want to support multiple nodes as the demotion target of a source node, we can group these nodes into three tiers (classes): fast class: 0 -> {1, 4} // 1 is the preferred 3 -> {4, 1} // 4 is the preferred medium class: 1 -> {2, 5} // 2 is the preferred 4 -> {5, 2} // 5 is the preferred slow class: 2 -> stop 5 -> stop Hi Wei Xu, I have some questions about it Fast class/memory are pictured as those nodes with CPUs, while Slow class/memory are PMEM, right? Then, what stands for medium class/memory? My guest best is that fast class is something like HBM (High Bandwidth Memory), medium class is ordinary RAM, slow class is PMEM. -- Thanks, David / dhildenb
Re: [PATCH] ACPI: x86: Call acpi_boot_table_init() after acpi_table_upgrade()
On Wed, Apr 14, 2021 at 09:42:01AM +0200, David Hildenbrand wrote: > On 13.04.21 19:53, Rafael J. Wysocki wrote: > > On Tue, Apr 13, 2021 at 7:43 PM David Hildenbrand wrote: > > > > > > On 13.04.21 16:01, Rafael J. Wysocki wrote: > > > > From: Rafael J. Wysocki > > > > > > > > Commit 1a1c130ab757 ("ACPI: tables: x86: Reserve memory occupied by > > > > ACPI tables") attempted to address an issue with reserving the memory > > > > occupied by ACPI tables, but it broke the initrd-based table override > > > > mechanism relied on by multiple users. > > > > > > > > To restore the initrd-based ACPI table override functionality, move > > > > the acpi_boot_table_init() invocation in setup_arch() on x86 after > > > > the acpi_table_upgrade() one. > > > > > > > > Fixes: 1a1c130ab757 ("ACPI: tables: x86: Reserve memory occupied by > > > > ACPI tables") > > > > Reported-by: Hans de Goede > > > > Tested-by: Hans de Goede > > > > Signed-off-by: Rafael J. Wysocki > > > > --- > > > > > > > > George, can you please check if this reintroduces the issue addressed by > > > > the above commit for you? > > > > > > > > --- > > > >arch/x86/kernel/setup.c |5 ++--- > > > >1 file changed, 2 insertions(+), 3 deletions(-) > > > > > > > > Index: linux-pm/arch/x86/kernel/setup.c > > > > === > > > > --- linux-pm.orig/arch/x86/kernel/setup.c > > > > +++ linux-pm/arch/x86/kernel/setup.c > > > > @@ -1045,9 +1045,6 @@ void __init setup_arch(char **cmdline_p) > > > > > > > >cleanup_highmap(); > > > > > > > > - /* Look for ACPI tables and reserve memory occupied by them. */ > > > > - acpi_boot_table_init(); > > > > - > > > >memblock_set_current_limit(ISA_END_ADDRESS); > > > >e820__memblock_setup(); > > > > > > > > @@ -1132,6 +1129,8 @@ void __init setup_arch(char **cmdline_p) > > > >reserve_initrd(); > > > > > > > >acpi_table_upgrade(); > > > > + /* Look for ACPI tables and reserve memory occupied by them. */ > > > > + acpi_boot_table_init(); > > > > > > > >vsmp_init(); > > > > > > This is fairly late; especially, it's after actual allocations -- see > > > e820__memblock_alloc_reserved_mpc_new(). > > > > > > Can't the table upgrade mechanism fix up when adjusting something? > > > > Not at this point of the cycle I'm afraid. > > > > > Some details on what actually breaks would be helpful. > > > > Generally speaking, the table overrides that come from the initrd are > > not taken into account if acpi_boot_table_init() runs before > > acpi_table_upgrade() and the latter cannot run before > > reserve_initrd(). > > I see. (looking at Documentation/acpi/initrd_table_override.txt I understand > what acpi table overrides are for :) ) > > > > > Honestly, I'm not sure how much effort it would take to untangle this ATM. > > Also true; ideally, we wouldn't have any allocations (find+reserve) before > ordinary reservations are done. > > However, I have no idea if we can move > e820__memblock_alloc_reserved_mpc_new() and reserve_real_mode() around > easily. Also, reserve_initrd()->relocate_initrd() does allocations. Even if we can move e820__memblock_alloc_reserved_mpc_new() and reserve_real_mode(), the allocation in reserve_initrd() has to be before the tables override, we would only reduce the probability of allocating an ACPI page. I think what we can do is to override the ACPI tables separately from their initial parsing. Rafael, what do you say? > This is a mess. True :( -- Sincerely yours, Mike.
Re: [PATCH v1 1/1] mfd: lpc_sch: Partially revert "Add support for Intel Quark X1000"
On Wed, 03 Mar 2021, Andy Shevchenko wrote: > The IRQ support for SCH GPIO is not specific to the Intel Quark SoC. > Moreover the IRQ routing is quite interesting there, so while it's > needs a special support, the driver haven't it anyway yet. > > Due to above remove basically redundant code of IRQ support. > > This reverts commit ec689a8a8155ce8b966bd5d7737a3916f5e48be3. > > Signed-off-by: Andy Shevchenko > --- > drivers/mfd/lpc_sch.c | 32 ++-- > 1 file changed, 6 insertions(+), 26 deletions(-) Applied, thanks. -- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
Re: [PATCH v1 2/2] i2c: designware: Get rid of legacy platform data
On Tue, 06 Apr 2021, Wolfram Sang wrote: > On Wed, Mar 31, 2021 at 06:48:51PM +0300, Andy Shevchenko wrote: > > Platform data is a legacy interface to supply device properties > > to the driver. In this case we don't have anymore in-kernel users > > for it. Just remove it for good. > > > > Signed-off-by: Andy Shevchenko > > Acked-by: Wolfram Sang Do you require an immutable branch? -- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
Re: [PATCH v2 2/3] arm64: dts: qcom: sm8150: add i2c nodes
Caleb Connolly writes: > Tested on the OnePlus 7 Pro (including DMA). > > Signed-off-by: Caleb Connolly > Reviewed-by: Vinod Koul > Reviewed-by: Bhupesh Sharma Tested on Microsoft Surface Duo (DTS will be sent after -rc1) Tested-by: Felipe Balbi -- balbi signature.asc Description: PGP signature
Re: [PATCH 02/10] mm/numa: automatically generate node migration order
On Wed, Apr 14, 2021 at 10:12:29AM +0200, David Hildenbrand wrote: > My guest best is that fast class is something like HBM (High Bandwidth > Memory), medium class is ordinary RAM, slow class is PMEM. I see, thanks for the hint David ;-) -- Oscar Salvador SUSE L3
Re: [PATCH v3 1/3] dt-bindings: display: bridge: add it66121 bindings
On 13/04/2021 18:03, Laurent Pinchart wrote: > Hi Neil, > > Thank you for the patch. > > On Mon, Apr 12, 2021 at 05:46:46PM +0200, Neil Armstrong wrote: >> From: Phong LE >> >> Add the ITE bridge HDMI it66121 bindings. >> >> Signed-off-by: Phong LE >> Signed-off-by: Neil Armstrong >> --- >> .../bindings/display/bridge/ite,it66121.yaml | 123 ++ >> 1 file changed, 123 insertions(+) >> create mode 100644 >> Documentation/devicetree/bindings/display/bridge/ite,it66121.yaml >> >> diff --git >> a/Documentation/devicetree/bindings/display/bridge/ite,it66121.yaml >> b/Documentation/devicetree/bindings/display/bridge/ite,it66121.yaml >> new file mode 100644 >> index ..61ed6dc7740b >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/display/bridge/ite,it66121.yaml >> @@ -0,0 +1,123 @@ >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/display/bridge/ite,it66121.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: ITE it66121 HDMI bridge Device Tree Bindings >> + >> +maintainers: >> + - Phong LE >> + - Neil Armstrong >> + >> +description: | >> + The IT66121 is a high-performance and low-power single channel HDMI >> + transmitter, fully compliant with HDMI 1.3a, HDCP 1.2 and backward >> compatible >> + to DVI 1.0 specifications. >> + >> +properties: >> + compatible: >> +const: ite,it66121 >> + >> + reg: >> +maxItems: 1 >> +description: base I2C address of the device > > You can drop the description. Done > >> + >> + reset-gpios: >> +maxItems: 1 >> +description: GPIO connected to active low reset >> + >> + vrf12-supply: >> +description: Regulator for 1.2V analog core power. >> + >> + vcn33-supply: >> +description: Regulator for 3.3V digital core power. >> + >> + vcn18-supply: >> +description: Regulator for 1.8V IO core power. >> + >> + interrupts: >> +maxItems: 1 >> + >> + ports: >> +$ref: /schemas/graph.yaml#/properties/ports >> + >> +properties: >> + port@0: >> +$ref: /schemas/graph.yaml#/$defs/port-base >> +unevaluatedProperties: false >> +description: DPI input port. >> + >> +properties: >> + endpoint: >> +$ref: /schemas/graph.yaml#/$defs/endpoint-base >> +unevaluatedProperties: false >> + >> +properties: >> + bus-width: >> +description: >> + Endpoint bus width. >> +enum: >> + - 12 # 12 data lines connected and dual-edge mode >> + - 24 # 24 data lines connected and single-edge mode >> +default: 24 >> + >> + port@1: >> +$ref: /schemas/graph.yaml#/properties/port >> +description: HDMI Connector port. >> + >> +required: >> + - port@0 >> + - port@1 >> + >> +required: >> + - compatible >> + - reg >> + - reset-gpios >> + - vrf12-supply >> + - vcn33-supply >> + - vcn18-supply >> + - interrupts >> + - ports >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> +i2c { >> + #address-cells = <1>; >> + #size-cells = <0>; > > It's customary to indent DT examples with 4 spaces. Done > >> + >> + it66121hdmitx: it66121hdmitx@4c { >> +compatible = "ite,it66121"; >> +pinctrl-names = "default"; >> +pinctrl-0 = <&ite_pins_default>; >> +vcn33-supply = <&mt6358_vcn33_wifi_reg>; >> +vcn18-supply = <&mt6358_vcn18_reg>; >> +vrf12-supply = <&mt6358_vrf12_reg>; >> +reset-gpios = <&pio 160 1 /* GPIO_ACTIVE_LOW */>; > > You can #include the necessary headers at the top of the example, and > use GPIO_ACTIVE_LOW and IRQ_TYPE_LEVEL_LOW to replace the numerical > values. Done > > Reviewed-by: Laurent Pinchart Thanks, Neil > >> +interrupt-parent = <&pio>; >> +interrupts = <4 8 /* IRQ_TYPE_LEVEL_LOW */>; >> +reg = <0x4c>; >> + >> +ports { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + port@0 { >> +reg = <0>; >> +it66121_in: endpoint { >> + bus-width = <12>; >> + remote-endpoint = <&display_out>; >> +}; >> + }; >> + >> + port@1 { >> +reg = <1>; >> +hdmi_conn_out: endpoint { >> + remote-endpoint = <&hdmi_conn_in>; >> +}; >> + }; >> +}; >> + }; >> +}; >
[PATCH v2] Documentation: dev-tools: Add Testing Overview
The kernel now has a number of testing and debugging tools, and we've seen a bit of confusion about what the differences between them are. Add a basic documentation outlining the testing tools, when to use each, and how they interact. This is a pretty quick overview rather than the idealised "kernel testing guide" that'd probably be optimal, but given the number of times questions like "When do you use KUnit and when do you use Kselftest?" are being asked, it seemed worth at least having something. Hopefully this can form the basis for more detailed documentation later. Signed-off-by: David Gow --- Thanks, everyone, for the comments on the doc. I've made a few of the suggested changes. Please let me know what you think! -- David Changes since v1: https://lore.kernel.org/linux-kselftest/20210410070529.4113432-1-david...@google.com/ - Note KUnit's speed and that one should provide selftests for syscalls - Mention lockdep as a Dynamic Analysis Tool - Refer to "Dynamic Analysis Tools" instead of "Sanitizers" - A number of minor formatting tweaks and rewordings for clarity. Not changed: - I haven't included an exhaustive list of differences, advantages, etc, between KUnit and kselftest: for now, the doc continues to focus on the difference between 'in-kernel' and 'userspace' testing here. - Similarly, I'm not linking out to docs defining and describing "Unit" tests versus "End-to-end" tests. None of the existing documentation elsewhere quite matches what we do in the kernel perfectly, so it seems less confusing to focus on the 'in-kernel'/'userspace' distinction, and leave other definitions as a passing mention for those who are already familiar with the concepts. - I haven't linked to any talk videos here: a few of them are linked on (e.g.) the KUnit webpage, but I wanted to keep the Kernel documentation more self-contained for now. No objection to adding them in a follow-up patch if people feel strongly about it, though. - The link from index.rst to this doc is unchanged. I personally think that the link is prominent enough there: it's the first link, and shows up a few times. One possibility if people disagreed would be to merge this page with the index, but given not all dev-tools are going to be testing-related, it seemed a bit arrogant. :-) Documentation/dev-tools/index.rst| 3 + Documentation/dev-tools/testing-overview.rst | 117 +++ 2 files changed, 120 insertions(+) create mode 100644 Documentation/dev-tools/testing-overview.rst diff --git a/Documentation/dev-tools/index.rst b/Documentation/dev-tools/index.rst index 1b1cf4f5c9d9..f590e5860794 100644 --- a/Documentation/dev-tools/index.rst +++ b/Documentation/dev-tools/index.rst @@ -7,6 +7,8 @@ be used to work on the kernel. For now, the documents have been pulled together without any significant effort to integrate them into a coherent whole; patches welcome! +A brief overview of testing-specific tools can be found in :doc:`testing-overview`. + .. class:: toc-title Table of contents @@ -14,6 +16,7 @@ whole; patches welcome! .. toctree:: :maxdepth: 2 + testing-overview coccinelle sparse kcov diff --git a/Documentation/dev-tools/testing-overview.rst b/Documentation/dev-tools/testing-overview.rst new file mode 100644 index ..ce36a8cdf6b5 --- /dev/null +++ b/Documentation/dev-tools/testing-overview.rst @@ -0,0 +1,117 @@ +.. SPDX-License-Identifier: GPL-2.0 + + +Kernel Testing Guide + + + +There are a number of different tools for testing the Linux kernel, so knowing +when to use each of them can be a challenge. This document provides a rough +overview of their differences, and how they fit together. + + +Writing and Running Tests += + +The bulk of kernel tests are written using either the kselftest or KUnit +frameworks. These both provide infrastructure to help make running tests and +groups of tests easier, as well as providing helpers to aid in writing new +tests. + +If you're looking to verify the behaviour of the Kernel — particularly specific +parts of the kernel — then you'll want to use KUnit or kselftest. + + +The Difference Between KUnit and kselftest +-- + +KUnit (Documentation/dev-tools/kunit/index.rst) is an entirely in-kernel system +for "white box" testing: because test code is part of the kernel, it can access +internal structures and functions which aren't exposed to userspace. + +KUnit tests therefore are best written against small, self-contained parts +of the kernel, which can be tested in isolation. This aligns well with the +concept of 'unit' testing. + +For example, a KUnit test might test an individual kernel function (or even a +single codepath through a function, such as an error handling case), rather +than a feature as a whole. + +This also makes KUnit tests very fast to build and run, allowing them to be
Re: [PATCH v1 2/2] i2c: designware: Get rid of legacy platform data
> > > Platform data is a legacy interface to supply device properties > > > to the driver. In this case we don't have anymore in-kernel users > > > for it. Just remove it for good. > > > > > > Signed-off-by: Andy Shevchenko > > > > Acked-by: Wolfram Sang > > Do you require an immutable branch? Nope, but thanks for asking. signature.asc Description: PGP signature
Re: [PATCH v3 2/3] drm: bridge: add it66121 driver
Le mer. 14 avril 2021 à 8:17, Neil Armstrong a écrit : Hi, Le 13/04/2021 à 22:56, Paul Cercueil a écrit : Hi Neil, I get build failures locally: drivers/gpu/drm/bridge/ite-it66121.c: In function ‘it66121_hw_reset’: drivers/gpu/drm/bridge/ite-it66121.c:242:2: error: implicit declaration of function ‘gpiod_set_value’ [-Werror=implicit-function-declaration] 242 | gpiod_set_value(ctx->gpio_reset, 1); | ^~~ drivers/gpu/drm/bridge/ite-it66121.c: In function ‘it66121_probe’: drivers/gpu/drm/bridge/ite-it66121.c:1016:16: error: implicit declaration of function ‘FIELD_GET’; did you mean ‘FOLL_GET’? [-Werror=implicit-function-declaration] 1016 | revision_id = FIELD_GET(IT66121_REVISION_MASK, device_ids[1]); | ^ | FOLL_GET Nothing difficult to fix, but the includes should be added nonetheless. Exact, I got the CI build failures, I'll fix these for v4. Were you able to test on your setup ? The v2 always forced DDR mode, with this v3, I also switch to normal 24input mode, but totally untested. I will try to find some time today to test after work. -Paul Thanks, Neil Cheers, -Paul Le lun. 12 avril 2021 à 17:46, Neil Armstrong a écrit : From: Phong LE This commit is a simple driver for bridge HMDI it66121. The input format is RBG and there is no color conversion. Audio, HDCP and CEC are not supported yet. Signed-off-by: Phong LE Signed-off-by: Neil Armstrong --- drivers/gpu/drm/bridge/Kconfig |8 + drivers/gpu/drm/bridge/Makefile |1 + drivers/gpu/drm/bridge/ite-it66121.c | 1081 ++ 3 files changed, 1090 insertions(+) create mode 100644 drivers/gpu/drm/bridge/ite-it66121.c diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index e4110d6ca7b3..6915c38fa459 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -74,6 +74,14 @@ config DRM_LONTIUM_LT9611UXC HDMI signals Please say Y if you have such hardware. +config DRM_ITE_IT66121 +tristate "ITE IT66121 HDMI bridge" +depends on OF +select DRM_KMS_HELPER +select REGMAP_I2C +help + Support for ITE IT66121 HDMI bridge. + config DRM_LVDS_CODEC tristate "Transparent LVDS encoders and decoders support" depends on OF diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile index 86e7acc76f8d..4f725753117c 100644 --- a/drivers/gpu/drm/bridge/Makefile +++ b/drivers/gpu/drm/bridge/Makefile @@ -24,6 +24,7 @@ obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o obj-$(CONFIG_DRM_TI_TPD12S015) += ti-tpd12s015.o obj-$(CONFIG_DRM_NWL_MIPI_DSI) += nwl-dsi.o +obj-$(CONFIG_DRM_ITE_IT66121) += ite-it66121.o obj-y += analogix/ obj-y += cadence/ diff --git a/drivers/gpu/drm/bridge/ite-it66121.c b/drivers/gpu/drm/bridge/ite-it66121.c new file mode 100644 index ..73af49b29dfa --- /dev/null +++ b/drivers/gpu/drm/bridge/ite-it66121.c @@ -0,0 +1,1081 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2020 BayLibre, SAS + * Author: Phong LE + * Copyright (C) 2018-2019, Artem Mygaiev + * Copyright (C) 2017, Fresco Logic, Incorporated. + * + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include + +#define IT66121_VENDOR_ID0_REG0x00 +#define IT66121_VENDOR_ID1_REG0x01 +#define IT66121_DEVICE_ID0_REG0x02 +#define IT66121_DEVICE_ID1_REG0x03 + +#define IT66121_VENDOR_ID00x54 +#define IT66121_VENDOR_ID10x49 +#define IT66121_DEVICE_ID00x12 +#define IT66121_DEVICE_ID10x06 +#define IT66121_REVISION_MASKGENMASK(7, 4) +#define IT66121_DEVICE_ID1_MASKGENMASK(3, 0) + +#define IT66121_MASTER_SEL_REG0x10 +#define IT66121_MASTER_SEL_HOSTBIT(0) + +#define IT66121_AFE_DRV_REG0x61 +#define IT66121_AFE_DRV_RSTBIT(4) +#define IT66121_AFE_DRV_PWDBIT(5) + +#define IT66121_INPUT_MODE_REG0x70 +#define IT66121_INPUT_MODE_RGB(0 << 6) +#define IT66121_INPUT_MODE_YUV422BIT(6) +#define IT66121_INPUT_MODE_YUV444(2 << 6) +#define IT66121_INPUT_MODE_CCIR656BIT(4) +#define IT66121_INPUT_MODE_SYNCEMBBIT(3) +#define IT66121_INPUT_MODE_DDRBIT(2) + +#define IT66121_INPUT_CSC_REG0x72 +#define IT66121_INPUT_CSC_ENDITHERBIT(7) +#define IT66121_INPUT_CSC_ENUDFILTERBIT(6) +#define IT66121_INPUT_CSC_DNFREE_GOBIT(5) +#define IT66121_INPUT_CSC_RGB_TO_YUV0x02 +#define IT66121_INPUT_CSC_YUV_TO_RGB0x03 +#define IT66121_INPUT_CSC_NO_CONV0x00 + +#define IT66121_AFE_XP_REG
Re: [PATCH v3 2/3] drm: bridge: add it66121 driver
Hi Neil, On Wed, Apr 14, 2021 at 10:08:46AM +0200, Neil Armstrong wrote: > On 14/04/2021 10:06, Robert Foss wrote: > > On Wed, 14 Apr 2021 at 08:13, Neil Armstrong > > wrote: > >> Le 13/04/2021 à 22:21, Robert Foss a écrit : > >>> Hey Neil & Phong, > >>> > >>> Thanks for submitting this series! > >>> > + > +static const struct drm_bridge_funcs it66121_bridge_funcs = { > + .attach = it66121_bridge_attach, > + .enable = it66121_bridge_enable, > + .disable = it66121_bridge_disable, > + .mode_set = it66121_bridge_mode_set, > + .mode_valid = it66121_bridge_mode_valid, > + .detect = it66121_bridge_detect, > + .get_edid = it66121_bridge_get_edid, > + .atomic_get_output_bus_fmts = > it66121_bridge_atomic_get_output_bus_fmts, > + .atomic_get_input_bus_fmts = > it66121_bridge_atomic_get_input_bus_fmts, > +}; > >>> > >>> I would like to see an implementation of HPD, since it is supported by > >>> the hardware[1] (and required by the documentation). IRQ status bit 0 > >>> seems to be the responsible for notifying us about hot plug detection > >>> events. > >> > >> It's implemented in the IRQ handler with the > >> IT66121_INT_STATUS1_HPD_STATUS event. > > > > I didn't even get that far :) > > > > Either way, the HPD support should be exposed in drm_bridge_funcs > > (.hpd_enable, .hpd_disable (and possibly .hpd_notify)) and > > drm_bridge.ops (DRM_BRIDGE_OP_HPD). > > Indeed I forgot these calls in the NO_CONNECTOR implementation... For new bridges, you should no implement connector creation, only the DRM_BRIDGE_ATTACH_NO_CONNECTOR case should be supported. -- Regards, Laurent Pinchart
[PATCH] staging: rtl8723bs: fix indentation issue introduced by long line split
fix indentation of last line in if condition. Fixes: af6afdb63f17 (staging: rtl8723bs: split long lines) Reported-by: Dan Carpenter Signed-off-by: Fabio Aiuto --- drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c index f19a15a3924b..4cdc5802798d 100644 --- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c +++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c @@ -621,7 +621,7 @@ unsigned int OnProbeReq(struct adapter *padapter, union recv_frame *precv_frame) _issue_probersp: if ((check_fwstate(pmlmepriv, _FW_LINKED) && pmlmepriv->cur_network.join_res) || -check_fwstate(pmlmepriv, WIFI_ADHOC_MASTER_STATE)) + check_fwstate(pmlmepriv, WIFI_ADHOC_MASTER_STATE)) issue_probersp(padapter, get_sa(pframe), is_valid_p2p_probereq); } -- 2.20.1
Re: [PATCH v2 4/5] staging: rtl8192e: rectified spelling mistake and replace memcmp with ether_oui_equal
On Wed, Apr 14, 2021 at 12:26:01PM +0530, Mitali Borkar wrote: > Added a generic function of static inline bool in > include/linux/etherdevice.h to replace memcmp with > ether_oui_equal throughout the execution. > Corrected the misspelled words in this file. > > Signed-off-by: Mitali Borkar > --- > > Changes from v1:- Rectified spelling mistake and replaced memcmp with > ether_oui_equal. > > drivers/staging/rtl8192e/rtl819x_HTProc.c | 48 +++ > include/linux/etherdevice.h | 5 +++ > 2 files changed, 29 insertions(+), 24 deletions(-) > > diff --git a/drivers/staging/rtl8192e/rtl819x_HTProc.c > b/drivers/staging/rtl8192e/rtl819x_HTProc.c > index ec6b46166e84..ce58feb2af9a 100644 > --- a/drivers/staging/rtl8192e/rtl819x_HTProc.c > +++ b/drivers/staging/rtl8192e/rtl819x_HTProc.c > @@ -43,7 +43,7 @@ u16 MCS_DATA_RATE[2][2][77] = { >810, 720, 810, 900, 900, 990} } > }; > > -static u8 UNKNOWN_BORADCOM[3] = {0x00, 0x14, 0xbf}; > +static u8 UNKNOWN_BROADCOM[3] = {0x00, 0x14, 0xbf}; > > static u8 LINKSYSWRT330_LINKSYSWRT300_BROADCOM[3] = {0x00, 0x1a, 0x70}; > > @@ -146,16 +146,16 @@ bool IsHTHalfNmodeAPs(struct rtllib_device *ieee) > boolretValue = false; > struct rtllib_network *net = &ieee->current_network; > > - if ((memcmp(net->bssid, BELKINF5D8233V1_RALINK, 3) == 0) || > - (memcmp(net->bssid, BELKINF5D82334V3_RALINK, 3) == 0) || > - (memcmp(net->bssid, PCI_RALINK, 3) == 0) || > - (memcmp(net->bssid, EDIMAX_RALINK, 3) == 0) || > - (memcmp(net->bssid, AIRLINK_RALINK, 3) == 0) || > + if ((ether_oui_equal(net->bssid, BELKINF5D8233V1_RALINK) == 0) || > + (ether_oui_equal(net->bssid, BELKINF5D82334V3_RALINK) == 0) || > + (ether_oui_equal(net->bssid, PCI_RALINK) == 0) || > + (ether_oui_equal(net->bssid, EDIMAX_RALINK) == 0) || > + (ether_oui_equal(net->bssid, AIRLINK_RALINK) == 0) || > (net->ralink_cap_exist)) > retValue = true; > - else if (!memcmp(net->bssid, UNKNOWN_BORADCOM, 3) || > - !memcmp(net->bssid, LINKSYSWRT330_LINKSYSWRT300_BROADCOM, 3) || > - !memcmp(net->bssid, LINKSYSWRT350_LINKSYSWRT150_BROADCOM, 3) || > + else if (ether_oui_equal(net->bssid, UNKNOWN_BROADCOM) || > + ether_oui_equal(net->bssid, > LINKSYSWRT330_LINKSYSWRT300_BROADCOM) || > + ether_oui_equal(net->bssid, > LINKSYSWRT350_LINKSYSWRT150_BROADCOM) || >(net->broadcom_cap_exist)) > retValue = true; > else if (net->bssht.bd_rt2rt_aggregation) > @@ -179,26 +179,26 @@ static void HTIOTPeerDetermine(struct rtllib_device > *ieee) > pHTInfo->IOTPeer = HT_IOT_PEER_92U_SOFTAP; > } else if (net->broadcom_cap_exist) { > pHTInfo->IOTPeer = HT_IOT_PEER_BROADCOM; > - } else if (!memcmp(net->bssid, UNKNOWN_BORADCOM, 3) || > - !memcmp(net->bssid, LINKSYSWRT330_LINKSYSWRT300_BROADCOM, 3) || > - !memcmp(net->bssid, LINKSYSWRT350_LINKSYSWRT150_BROADCOM, 3)) { > + } else if (ether_oui_equal(net->bssid, UNKNOWN_BROADCOM) || > +ether_oui_equal(net->bssid, > LINKSYSWRT330_LINKSYSWRT300_BROADCOM) || > +ether_oui_equal(net->bssid, > LINKSYSWRT350_LINKSYSWRT150_BROADCOM)) { > pHTInfo->IOTPeer = HT_IOT_PEER_BROADCOM; > - } else if ((memcmp(net->bssid, BELKINF5D8233V1_RALINK, 3) == 0) || > - (memcmp(net->bssid, BELKINF5D82334V3_RALINK, 3) == 0) || > - (memcmp(net->bssid, PCI_RALINK, 3) == 0) || > - (memcmp(net->bssid, EDIMAX_RALINK, 3) == 0) || > - (memcmp(net->bssid, AIRLINK_RALINK, 3) == 0) || > - net->ralink_cap_exist) { > + } else if ((ether_oui_equal(net->bssid, BELKINF5D8233V1_RALINK) == 0) || > +(ether_oui_equal(net->bssid, BELKINF5D82334V3_RALINK) == 0) > || > +(ether_oui_equal(net->bssid, PCI_RALINK) == 0) || > +(ether_oui_equal(net->bssid, EDIMAX_RALINK) == 0) || > +(ether_oui_equal(net->bssid, AIRLINK_RALINK) == 0) || > +net->ralink_cap_exist) { > pHTInfo->IOTPeer = HT_IOT_PEER_RALINK; > } else if ((net->atheros_cap_exist) || > - (memcmp(net->bssid, DLINK_ATHEROS_1, 3) == 0) || > - (memcmp(net->bssid, DLINK_ATHEROS_2, 3) == 0)) { > +(ether_oui_equal(net->bssid, DLINK_ATHEROS_1) == 0) || > +(ether_oui_equal(net->bssid, DLINK_ATHEROS_2) == 0)) { > pHTInfo->IOTPeer = HT_IOT_PEER_ATHEROS; > - } else if ((memcmp(net->bssid, CISCO_BROADCOM, 3) == 0) || > - net->cisco_cap_exist) { > + } else if ((ether_oui_equal(net->bssid, CISCO_BROADCOM) == 0) || > +net->cisco_cap_exist) { > pHTInfo->IOTPeer = HT_IOT_PEER_CISCO; > - } else if ((memcmp(net->bssid, LINKSYS_MARVELL_440
Re: Subject: [PATCH v2 0/5] staging: rtl8192e: CLeanup patchset for style issues in rtl819x_Y.c files
On Wed, Apr 14, 2021 at 12:24:44PM +0530, Mitali Borkar wrote: > Changes from v1:- Dropped 6/6 from and made this as a patchset of 5 as > alignment of code is done in following patches. Why is "Subject:" listed in your subject line? Do not manually add it after git format-patch has created it... thanks, greg k-h
Re: [PATCH 02/10] mm/numa: automatically generate node migration order
On 14.04.21 10:14, Oscar Salvador wrote: On Wed, Apr 14, 2021 at 10:12:29AM +0200, David Hildenbrand wrote: My guest best is that fast class is something like HBM (High Bandwidth haha, whatever happened in that sentence: s/guest best/best guess/ Memory), medium class is ordinary RAM, slow class is PMEM. I see, thanks for the hint David ;-) -- Thanks, David / dhildenb
Re: [PATCH v10 1/3] PCI: portdrv: Add pcie_port_service_get_irq() function
Hi Jonathan, On 2021/04/12 17:42, Jonathan Cameron wrote: On Sat, 10 Apr 2021 01:22:16 +0900 Kunihiko Hayashi wrote: Add pcie_port_service_get_irq() that returns the virtual IRQ number for specified portdrv service. Trivial comment inline. Cc: Lorenzo Pieralisi Signed-off-by: Kunihiko Hayashi Reviewed-by: Rob Herring Acked-by: Bjorn Helgaas --- drivers/pci/pcie/portdrv.h | 1 + drivers/pci/pcie/portdrv_core.c | 16 2 files changed, 17 insertions(+) diff --git a/drivers/pci/pcie/portdrv.h b/drivers/pci/pcie/portdrv.h index 2ff5724..628a3de 100644 --- a/drivers/pci/pcie/portdrv.h +++ b/drivers/pci/pcie/portdrv.h @@ -144,4 +144,5 @@ static inline void pcie_pme_interrupt_enable(struct pci_dev *dev, bool en) {} #endif /* !CONFIG_PCIE_PME */ struct device *pcie_port_find_device(struct pci_dev *dev, u32 service); +int pcie_port_service_get_irq(struct pci_dev *dev, u32 service); #endif /* _PORTDRV_H_ */ diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c index e1fed664..b60f0f3 100644 --- a/drivers/pci/pcie/portdrv_core.c +++ b/drivers/pci/pcie/portdrv_core.c @@ -477,7 +477,22 @@ struct device *pcie_port_find_device(struct pci_dev *dev, } EXPORT_SYMBOL_GPL(pcie_port_find_device); +/* /** this is kernel-doc style, so why not mark it as such? Thank you for pointing out. I'll apply the style to this comment. Thank you, --- Best Regards Kunihiko Hayashi
Re: [PATCH] x86/kprobes: Simplify alloc_insn_page() with __vmalloc_node_range
Hi Jisheng, On Wed, 14 Apr 2021 15:27:28 +0800 Jisheng Zhang wrote: \ > > > > On Tue, 13 Apr 2021 18:03:24 +0800 > > Jisheng Zhang wrote: > > > > > Use the __vmalloc_node_range() to simplify x86's alloc_insn_page() > > > implementation. > > > > Have you checked this is equivarent to the original code on all > > architecture? IIRC, some arch has a special module_alloc(), > > > Indeed, this isn't equivarent to the original code. FWICT, the differences > > on x86 are: > > > 1) module_alloc() allocates a special vmalloc range > > 2) module_alloc() randomizes the return address via. module_load_offset() > > 3) module_alloc() also supports kasan instrumentation by > > kasan_module_alloc() > > > But I'm not sure whether the above differences are useful for kprobes ss > > insn slot page or not. Take 1) for example, special range in module_alloc > > is due to relative jump limitation, modules need to call kernel .text. does > > kprobes ss ins slot needs this limitation too? > > Oops, I found this wonderful thread: > https://www.lkml.org/lkml/2020/7/28/1413 > > So kprobes ss ins slot page "must be in the range of relative branching only > for x86 and arm" Yes, at this moment. (Not sure we can introduce similar feature on other arch too) > > And Jarkko's "arch/x86: kprobes: Remove MODULES dependency" series look > much better. The last version is v5, I'm not sure whether Jarkko will > send new version to mainline the series. I hope so. If module_alloc() itself is implemented on the generic text_alloc(), I can replace the module_alloc() with text_alloc(). Thank you, -- Masami Hiramatsu
[patch v3] translations/zh_CN: add translations to dev-tools gcov
Add translations to dev-tools gcov Signed-off-by: Bernard Zhao --- Changes since V2: * fix some inaccurate translation Changes since V1: * add index.rst in dev-tools and link to to zh_CN/index.rst * fix some inaccurate translation --- .../translations/zh_CN/dev-tools/gcov.rst | 261 ++ .../translations/zh_CN/dev-tools/index.rst| 35 +++ Documentation/translations/zh_CN/index.rst| 1 + 3 files changed, 297 insertions(+) create mode 100644 Documentation/translations/zh_CN/dev-tools/gcov.rst create mode 100644 Documentation/translations/zh_CN/dev-tools/index.rst diff --git a/Documentation/translations/zh_CN/dev-tools/gcov.rst b/Documentation/translations/zh_CN/dev-tools/gcov.rst new file mode 100644 index ..882eee4c75c5 --- /dev/null +++ b/Documentation/translations/zh_CN/dev-tools/gcov.rst @@ -0,0 +1,261 @@ +.. include:: ../disclaimer-zh_CN.rst + +:Original: Documentation/dev-tools/gcov.rst +:Translator: ? Bernard Zhao + +???Linux???gcov += + +gcov???linux???GCC? +? +linux?gcovdebug-fs? +?gcov???``-o``?? +???root?:: + +# cd /tmp/linux-out +# gcov -o /sys/kernel/debug/gcov/tmp/linux-out/kernel spinlock.c + +??? +?gcov_?lcov_??? +???linux HTML??? + +???: + +* ??? +* ?? +* ??? +??? + +_gcov: https://gcc.gnu.org/onlinedocs/gcc/Gcov.html +_lcov: http://ltp.sourceforge.net/coverage/lcov.php + + +?? +--- + +:: + +CONFIG_DEBUG_FS=y +CONFIG_GCOV_KERNEL=y + +??:: + +CONFIG_GCOV_PROFILE_ALL=y + +??? +?? +?? + +?debugfs??:: + +mount -t debugfs none /sys/kernel/debug + + +? +- + +?? +??Makefile: + +- ?main.o???:: + +GCOV_PROFILE_main.o := y + +- ???:: + +GCOV_PROFILE := y + +???CONFIG_GCOV_PROFILE_ALL?? +?:: + +GCOV_PROFILE_main.o := n + +???:: + +GCOV_PROFILE := n + +??? + + + +- + +gcov???debugfs?: + +``/sys/kernel/debug/gcov`` +gcov + +``/sys/kernel/debug/gcov/reset`` + ??:??gcov???0 + +``/sys/kernel/debug/gcov/path/to/compile/dir/file.gcda`` + gcov?? + ??gcov???0 + +``/sys/kernel/debug/gcov/path/to/compile/dir/file.gcno`` + gcov +???gcc??``-ftest-coverage``??? + + +? +--- + + +gcov +debugfs? +??debugfs? + +???gcov_persist??gcov:: + +gcov_persist = 0 + +???gcov??? +? + + +?? +- + +gcov??
Re: [PATCH 4/7] staging: rtl8723bs: replace DBG_871X_SEL_NL with netdev_dbg()
On Tue, Apr 13, 2021 at 04:56:32PM +0200, Fabio Aiuto wrote: > replace DGB_871X_SEL_NL macro with netdev_dbg(). > > DBG_871X_SEL_NL macro expands to a raw prink call or a > seq_printf if selected stream _is not_ a local > debug symbol set to null. > This second scenario never occurs so replace > all macro usages with netdev_dbg(). > > This is done with the following coccinelle script: > > @@ > expression sel; > expression list args; > identifier padapter; > identifier func; > @@ > > func(..., struct adapter *padapter, ...) { > <... > - DBG_871X_SEL_NL(sel, args); > + netdev_dbg(padapter->pnetdev, args); > ...> > } > > Signed-off-by: Fabio Aiuto > --- > drivers/staging/rtl8723bs/core/rtw_debug.c | 16 +++ > drivers/staging/rtl8723bs/core/rtw_odm.c | 49 +++--- > drivers/staging/rtl8723bs/hal/hal_com.c| 31 ++ > 3 files changed, 46 insertions(+), 50 deletions(-) > > diff --git a/drivers/staging/rtl8723bs/core/rtw_debug.c > b/drivers/staging/rtl8723bs/core/rtw_debug.c > index 324c7e5248f8..79fd968bb147 100644 > --- a/drivers/staging/rtl8723bs/core/rtw_debug.c > +++ b/drivers/staging/rtl8723bs/core/rtw_debug.c > @@ -20,7 +20,7 @@ void sd_f0_reg_dump(void *sel, struct adapter *adapter) > > for (i = 0x0; i <= 0xff; i++) { > if (i%16 == 0) > - DBG_871X_SEL_NL(sel, "0x%02x ", i); > + netdev_dbg(adapter->pnetdev, "0x%02x ", i); > > DBG_871X_SEL(sel, "%02x ", rtw_sd_f0_read8(adapter, i)); > > @@ -35,11 +35,11 @@ void mac_reg_dump(void *sel, struct adapter *adapter) > { > int i, j = 1; > > - DBG_871X_SEL_NL(sel, "=== MAC REG ===\n"); > + netdev_dbg(adapter->pnetdev, "=== MAC REG ===\n"); > > for (i = 0x0; i < 0x800; i += 4) { > if (j%4 == 1) > - DBG_871X_SEL_NL(sel, "0x%03x", i); > + netdev_dbg(adapter->pnetdev, "0x%03x", i); > DBG_871X_SEL(sel, " 0x%08x ", rtw_read32(adapter, i)); > if ((j++)%4 == 0) > DBG_871X_SEL(sel, "\n"); > @@ -50,10 +50,10 @@ void bb_reg_dump(void *sel, struct adapter *adapter) > { > int i, j = 1; > > - DBG_871X_SEL_NL(sel, "=== BB REG ===\n"); > + netdev_dbg(adapter->pnetdev, "=== BB REG ===\n"); > for (i = 0x800; i < 0x1000 ; i += 4) { > if (j%4 == 1) > - DBG_871X_SEL_NL(sel, "0x%03x", i); > + netdev_dbg(adapter->pnetdev, "0x%03x", i); > DBG_871X_SEL(sel, " 0x%08x ", rtw_read32(adapter, i)); > if ((j++)%4 == 0) > DBG_871X_SEL(sel, "\n"); > @@ -73,14 +73,14 @@ void rf_reg_dump(void *sel, struct adapter *adapter) > else > path_nums = 2; > > - DBG_871X_SEL_NL(sel, "=== RF REG ===\n"); > + netdev_dbg(adapter->pnetdev, "=== RF REG ===\n"); > > for (path = 0; path < path_nums; path++) { > - DBG_871X_SEL_NL(sel, "RF_Path(%x)\n", path); > + netdev_dbg(adapter->pnetdev, "RF_Path(%x)\n", path); > for (i = 0; i < 0x100; i++) { > value = rtw_hal_read_rfreg(adapter, path, i, > 0x); > if (j%4 == 1) > - DBG_871X_SEL_NL(sel, "0x%02x ", i); > + netdev_dbg(adapter->pnetdev, "0x%02x ", i); > DBG_871X_SEL(sel, " 0x%08x ", value); > if ((j++)%4 == 0) > DBG_871X_SEL(sel, "\n"); > diff --git a/drivers/staging/rtl8723bs/core/rtw_odm.c > b/drivers/staging/rtl8723bs/core/rtw_odm.c > index 53f7cc0444ba..084f6ae040ee 100644 > --- a/drivers/staging/rtl8723bs/core/rtw_odm.c > +++ b/drivers/staging/rtl8723bs/core/rtw_odm.c > @@ -96,12 +96,13 @@ void rtw_odm_dbg_comp_msg(void *sel, struct adapter > *adapter) > int i; > > rtw_hal_get_def_var(adapter, HW_DEF_ODM_DBG_FLAG, &dbg_comp); > - DBG_871X_SEL_NL(sel, "odm.DebugComponents = 0x%016llx\n", dbg_comp); > + netdev_dbg(adapter->pnetdev, "odm.DebugComponents = 0x%016llx\n", > +dbg_comp); > for (i = 0; i < RTW_ODM_COMP_MAX; i++) { > if (odm_comp_str[i]) > - DBG_871X_SEL_NL(sel, "%cBIT%-2d %s\n", > - (BIT0 << i) & dbg_comp ? '+' : ' ', > - i, odm_comp_str[i]); > + netdev_dbg(adapter->pnetdev, "%cBIT%-2d %s\n", > +(BIT0 << i) & dbg_comp ? '+' : ' ', i, > +odm_comp_str[i]); > } > } > > @@ -116,11 +117,11 @@ void rtw_odm_dbg_level_msg(void *sel, struct adapter > *adapter) > int i; > > rtw_hal_get_def_var(adapter, HW_DEF_ODM_DBG_LEVEL, &dbg_level); > - DBG_871X_SEL_NL(sel, "odm.DebugLevel = %u\n", dbg_level); > + netdev_d
Re: [PATCH 5/7] staging: rtl8723bs: put a new line after ';'
On Tue, Apr 13, 2021 at 04:56:33PM +0200, Fabio Aiuto wrote: > fix the following post commit hook checkpatch issue: > > ERROR: space required after that ';' (ctx:VxV) > 232: FILE: drivers/staging/rtl8723bs/core/rtw_odm.c:160: > +"AdapEn_RSSI", "IGI_LowerBound");netdev_dbg > (adapter->pnetdev, > > This was coccinelle script output coding style issue. > > Signed-off-by: Fabio Aiuto > --- > drivers/staging/rtl8723bs/core/rtw_odm.c | 17 + > 1 file changed, 9 insertions(+), 8 deletions(-) > > diff --git a/drivers/staging/rtl8723bs/core/rtw_odm.c > b/drivers/staging/rtl8723bs/core/rtw_odm.c > index 084f6ae040ee..f4a0ef428564 100644 > --- a/drivers/staging/rtl8723bs/core/rtw_odm.c > +++ b/drivers/staging/rtl8723bs/core/rtw_odm.c > @@ -157,14 +157,15 @@ void rtw_odm_adaptivity_parm_msg(void *sel, struct > adapter *adapter) > > netdev_dbg(adapter->pnetdev, "%10s %16s %8s %10s %11s %14s\n", > "TH_L2H_ini", "TH_EDCCA_HL_diff", "IGI_Base", "ForceEDCCA", > -"AdapEn_RSSI", "IGI_LowerBound");netdev_dbg(adapter->pnetdev, > -"0x%-8x %-16d > 0x%-6x %-10d %-11u %-14u\n", > - > (u8)odm->TH_L2H_ini, > - > odm->TH_EDCCA_HL_diff, > -odm->IGI_Base, > -odm->ForceEDCCA, > -odm->AdapEn_RSSI, > - > odm->IGI_LowerBound); > +"AdapEn_RSSI", "IGI_LowerBound"); > + netdev_dbg(adapter->pnetdev, > +"0x%-8x %-16d 0x%-6x %-10d %-11u %-14u\n", > +(u8)odm->TH_L2H_ini, > +odm->TH_EDCCA_HL_diff, > +odm->IGI_Base, > +odm->ForceEDCCA, > +odm->AdapEn_RSSI, > +odm->IGI_LowerBound); > } > > void rtw_odm_adaptivity_parm_set(struct adapter *adapter, s8 TH_L2H_ini, > -- > 2.20.1 This patch should not be needed, your commit that caused this error should not have done so. Please fix that instead. thanks, greg k-h
Re: [PATCH v2 00/16] Multigenerational LRU Framework
Yu Zhao writes: > On Wed, Apr 14, 2021 at 12:15 AM Huang, Ying wrote: >> >> Yu Zhao writes: >> >> > On Tue, Apr 13, 2021 at 8:30 PM Rik van Riel wrote: >> >> >> >> On Wed, 2021-04-14 at 09:14 +1000, Dave Chinner wrote: >> >> > On Tue, Apr 13, 2021 at 10:13:24AM -0600, Jens Axboe wrote: >> >> > >> >> > > The initial posting of this patchset did no better, in fact it did >> >> > > a bit >> >> > > worse. Performance dropped to the same levels and kswapd was using >> >> > > as >> >> > > much CPU as before, but on top of that we also got excessive >> >> > > swapping. >> >> > > Not at a high rate, but 5-10MB/sec continually. >> >> > > >> >> > > I had some back and forths with Yu Zhao and tested a few new >> >> > > revisions, >> >> > > and the current series does much better in this regard. Performance >> >> > > still dips a bit when page cache fills, but not nearly as much, and >> >> > > kswapd is using less CPU than before. >> >> > >> >> > Profiles would be interesting, because it sounds to me like reclaim >> >> > *might* be batching page cache removal better (e.g. fewer, larger >> >> > batches) and so spending less time contending on the mapping tree >> >> > lock... >> >> > >> >> > IOWs, I suspect this result might actually be a result of less lock >> >> > contention due to a change in batch processing characteristics of >> >> > the new algorithm rather than it being a "better" algorithm... >> >> >> >> That seems quite likely to me, given the issues we have >> >> had with virtual scan reclaim algorithms in the past. >> > >> > Hi Rik, >> > >> > Let paste the code so we can move beyond the "batching" hypothesis: >> > >> > static int __remove_mapping(struct address_space *mapping, struct page >> > *page, >> > bool reclaimed, struct mem_cgroup >> > *target_memcg) >> > { >> > unsigned long flags; >> > int refcount; >> > void *shadow = NULL; >> > >> > BUG_ON(!PageLocked(page)); >> > BUG_ON(mapping != page_mapping(page)); >> > >> > xa_lock_irqsave(&mapping->i_pages, flags); >> > >> >> SeongJae, what is this algorithm supposed to do when faced >> >> with situations like this: >> > >> > I'll assume the questions were directed at me, not SeongJae. >> > >> >> 1) Running on a system with 8 NUMA nodes, and >> >> memory >> >>pressure in one of those nodes. >> >> 2) Running PostgresQL or Oracle, with hundreds of >> >>processes mapping the same (very large) shared >> >>memory segment. >> >> >> >> How do you keep your algorithm from falling into the worst >> >> case virtual scanning scenarios that were crippling the >> >> 2.4 kernel 15+ years ago on systems with just a few GB of >> >> memory? >> > >> > There is a fundamental shift: that time we were scanning for cold pages, >> > and nowadays we are scanning for hot pages. >> > >> > I'd be surprised if scanning for cold pages didn't fall apart, because it'd >> > find most of the entries accessed, if they are present at all. >> > >> > Scanning for hot pages, on the other hand, is way better. Let me just >> > reiterate: >> > 1) It will not scan page tables from processes that have been sleeping >> >since the last scan. >> > 2) It will not scan PTE tables under non-leaf PMD entries that do not >> >have the accessed bit set, when >> >CONFIG_HAVE_ARCH_PARENT_PMD_YOUNG=y. >> > 3) It will not zigzag between the PGD table and the same PMD or PTE >> >table spanning multiple VMAs. In other words, it finishes all the >> >VMAs with the range of the same PMD or PTE table before it returns >> >to the PGD table. This optimizes workloads that have large numbers >> >of tiny VMAs, especially when CONFIG_PGTABLE_LEVELS=5. >> > >> > So the cost is roughly proportional to the number of referenced pages it >> > discovers. If there is no memory pressure, no scanning at all. For a system >> > under heavy memory pressure, most of the pages are referenced (otherwise >> > why would it be under memory pressure?), and if we use the rmap, we need to >> > scan a lot of pages anyway. Why not just scan them all? >> >> This may be not the case. For rmap scanning, it's possible to scan only >> a small portion of memory. But with the page table scanning, you need >> to scan almost all (I understand you have some optimization as above). > > Hi Ying, > > Let's take a step back. > > For the sake of discussion, when does the scanning have to happen? Can > we agree that the simplest answer is when we have evicted all inactive > pages? > > If so, my next question is who's filled in the memory space previously > occupied by those inactive pages? Newly faulted in pages, right? They > have the accessed bit set, and we can't evict them without scanning > them first, would you agree? > > And there are also existing active pages, and they were protected from > eviction. But now we need to deactivate some of them. Do you think > whether they'd have been used or not since the last scan? (Remember > they w