Re: [PATCH v2] selftests/bpf: Validate UDP length in cls_redirect test
> I don't quite see the point. > This is a test prog. It's not supposed to be used as production code. It was marked as a TODO and so I sent a patch for it. I'm sorry I didn't think of the practical implications of it.
[PATCH 0/2] module: Fix memory deallocation on error path in move_module()
The first patch is an actual fix. The second patch is a minor related cleanup. Petr Pavlu (2): module: Fix memory deallocation on error path in move_module() module: Avoid unnecessary return value initialization in move_module() kernel/module/main.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) base-commit: bdc7f8c5adad50dad2ec762e317f8b212f5782ac -- 2.49.0
[PATCH 2/2] module: Avoid unnecessary return value initialization in move_module()
All error conditions in move_module() set the return value by updating the ret variable. Therefore, it is not necessary to the initialize the variable when declaring it. Remove the unnecessary initialization. Signed-off-by: Petr Pavlu --- kernel/module/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/module/main.c b/kernel/module/main.c index 322b38c0a782..06511607075c 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -2615,7 +2615,7 @@ static int move_module(struct module *mod, struct load_info *info) { int i; enum mod_mem_type t; - int ret = -ENOMEM; + int ret; bool codetag_section_found = false; for_each_mod_mem_type(type) { -- 2.49.0
[PATCH 1/2] module: Fix memory deallocation on error path in move_module()
The function move_module() uses the variable t to track how many memory types it has allocated and consequently how many should be freed if an error occurs. The variable is initially set to 0 and is updated when a call to module_memory_alloc() fails. However, move_module() can fail for other reasons as well, in which case t remains set to 0 and no memory is freed. Fix the problem by setting t to MOD_MEM_NUM_TYPES after all memory types have been allocated. Additionally, make the deallocation loop more robust by not relying on the mod_mem_type_t enum having a signed integer as its underlying type. Fixes: c7ee8aebf6c0 ("module: add stop-grap sanity check on module memcpy()") Signed-off-by: Petr Pavlu --- kernel/module/main.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/kernel/module/main.c b/kernel/module/main.c index 08b59c37735e..322b38c0a782 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -2614,7 +2614,7 @@ static int find_module_sections(struct module *mod, struct load_info *info) static int move_module(struct module *mod, struct load_info *info) { int i; - enum mod_mem_type t = 0; + enum mod_mem_type t; int ret = -ENOMEM; bool codetag_section_found = false; @@ -2630,6 +2630,7 @@ static int move_module(struct module *mod, struct load_info *info) goto out_err; } } + t = MOD_MEM_NUM_TYPES; /* Transfer each section which specifies SHF_ALLOC */ pr_debug("Final section addresses for %s:\n", mod->name); @@ -2693,8 +2694,8 @@ static int move_module(struct module *mod, struct load_info *info) return 0; out_err: module_memory_restore_rox(mod); - for (t--; t >= 0; t--) - module_memory_free(mod, t); + for (; t > 0; t--) + module_memory_free(mod, t - 1); if (codetag_section_found) codetag_free_module_sections(mod); -- 2.49.0
Re: [PATCH v4 6/7] mm/maps: read proc/pid/maps under per-vma lock
Hi Suren, Forgive me but I am going to ask a lot of questions here :p just want to make sure I'm getting everything right here. On Wed, Jun 04, 2025 at 04:11:50PM -0700, Suren Baghdasaryan wrote: > With maple_tree supporting vma tree traversal under RCU and per-vma > locks, /proc/pid/maps can be read while holding individual vma locks > instead of locking the entire address space. Nice :) > Completely lockless approach would be quite complex with the main issue > being get_vma_name() using callbacks which might not work correctly with > a stable vma copy, requiring original (unstable) vma. Hmmm can you expand on what a 'completely lockless' design might comprise of? It's super un-greppable and I've not got clangd set up with an allmod kernel to triple-check but I'm seeing at least 2 (are there more?): gate_vma_name() which is: return "[vsyscall]"; special_mapping_name() which is: return ((struct vm_special_mapping *)vma->vm_private_data)->name; Which I'm guessing is the issue because it's a double pointer deref... Seems such a silly issue to get stuck on, I wonder if we can't just change this to function correctly? > When per-vma lock acquisition fails, we take the mmap_lock for reading, > lock the vma, release the mmap_lock and continue. This guarantees the > reader to make forward progress even during lock contention. This will Ah that fabled constant forward progress ;) > interfere with the writer but for a very short time while we are > acquiring the per-vma lock and only when there was contention on the > vma reader is interested in. > One case requiring special handling is when vma changes between the > time it was found and the time it got locked. A problematic case would > be if vma got shrunk so that it's start moved higher in the address > space and a new vma was installed at the beginning: > > reader found: |VMA A| > VMA is modified:|-VMA B-|VMA A| > reader locks modified VMA A > reader reports VMA A: | gap |VMA A| > > This would result in reporting a gap in the address space that does not > exist. To prevent this we retry the lookup after locking the vma, however > we do that only when we identify a gap and detect that the address space > was changed after we found the vma. OK so in this case we have 1. Find VMA A - nothing is locked yet, but presumably we are under RCU so are... safe? From unmaps? Or are we? I guess actually the detach mechanism sorts this out for us perhaps? 2. We got unlucky and did this immediately prior to VMA A having its vma->vm_start, vm_end updated to reflect the split. 3. We lock VMA A, now position with an apparent gap after the prior VMA which, in practice does not exist. So I am guessing that by observing sequence numbers you are able to detect that a change has occurred and thus retry the operation in this situation? I know we previously discussed the possibility of this retry mechanism going on forever, I guess I will see the resolution to this in the code :) > This change is designed to reduce mmap_lock contention and prevent a > process reading /proc/pid/maps files (often a low priority task, such > as monitoring/data collection services) from blocking address space > updates. Note that this change has a userspace visible disadvantage: > it allows for sub-page data tearing as opposed to the previous mechanism > where data tearing could happen only between pages of generated output > data. Since current userspace considers data tearing between pages to be > acceptable, we assume is will be able to handle sub-page data tearing > as well. By tearing do you mean for instance seeing a VMA more than once due to e.g. a VMA expanding in a racey way? Pedantic I know, but it might be worth goiing through all the merge case, split and remap scenarios and explaining what might happen in each one (or perhaps do that as some form of documentation?) I can try to put together a list of all of the possibilities if that would be helpful. > > Signed-off-by: Suren Baghdasaryan > --- > fs/proc/internal.h | 6 ++ > fs/proc/task_mmu.c | 177 +++-- > 2 files changed, 175 insertions(+), 8 deletions(-) I really hate having all this logic in the proc/task_mmu.c file. This is really delicate stuff and I'd really like it to live in mm if possible. I reallise this might be a total pain, but I'm quite worried about us putting super-delicate, carefully written VMA handling code in different places. Also having stuff in mm/vma.c opens the door to userland testing which, when I finally have time to really expand that, would allow for some really nice stress testing here. > > diff --git a/fs/proc/internal.h b/fs/proc/internal.h > index 96122e91c645..3728c9012687 100644 > --- a/fs/proc/internal.h > +++ b/fs/proc/internal.h > @@ -379,6 +379,12 @@ struct proc_maps_private { > struct task_struct *task; > struct mm_struct *m
[PATCH] vhost-scsi: Fix check for inline_sg_cnt exceeding preallocated limit
The condition comparing ret to VHOST_SCSI_PREALLOC_SGLS was incorrect, as ret holds the result of kstrtouint() (typically 0 on success), not the parsed value. Update the check to use cnt, which contains the actual user-provided value. prevents silently accepting values exceeding the maximum inline_sg_cnt. Fixes: bca939d5bcd0 ("vhost-scsi: Dynamically allocate scatterlists") Signed-off-by: Alok Tiwari --- drivers/vhost/scsi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c index c12a0d4e6386..8d655b2d15d9 100644 --- a/drivers/vhost/scsi.c +++ b/drivers/vhost/scsi.c @@ -71,7 +71,7 @@ static int vhost_scsi_set_inline_sg_cnt(const char *buf, if (ret) return ret; - if (ret > VHOST_SCSI_PREALLOC_SGLS) { + if (cnt > VHOST_SCSI_PREALLOC_SGLS) { pr_err("Max inline_sg_cnt is %u\n", VHOST_SCSI_PREALLOC_SGLS); return -EINVAL; } -- 2.47.1
Re: [PATCH] vhost-scsi: Fix check for inline_sg_cnt exceeding preallocated limit
On 6/7/25 2:40 PM, Alok Tiwari wrote: > The condition comparing ret to VHOST_SCSI_PREALLOC_SGLS was incorrect, > as ret holds the result of kstrtouint() (typically 0 on success), > not the parsed value. Update the check to use cnt, which contains the > actual user-provided value. > > prevents silently accepting values exceeding the maximum inline_sg_cnt. > > Fixes: bca939d5bcd0 ("vhost-scsi: Dynamically allocate scatterlists") > Signed-off-by: Alok Tiwari > --- > drivers/vhost/scsi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c > index c12a0d4e6386..8d655b2d15d9 100644 > --- a/drivers/vhost/scsi.c > +++ b/drivers/vhost/scsi.c > @@ -71,7 +71,7 @@ static int vhost_scsi_set_inline_sg_cnt(const char *buf, > if (ret) > return ret; > > - if (ret > VHOST_SCSI_PREALLOC_SGLS) { > + if (cnt > VHOST_SCSI_PREALLOC_SGLS) { > pr_err("Max inline_sg_cnt is %u\n", VHOST_SCSI_PREALLOC_SGLS); > return -EINVAL; > } Thanks. Reviewed-by: Mike Christie
Re: [PATCH v4 2/4] media: qcom: camss: Add support for MSM8939
Le vendredi 06 juin 2025 à 13:59 +0300, Vladimir Zapolskiy a écrit : > Hello Vincent. > > On 6/2/25 20:27, Vincent Knecht via B4 Relay wrote: > > From: Vincent Knecht > > > > The camera subsystem for the MSM8939 is the same as MSM8916 except with > > 3 CSID instead of 2, and some higher clock rates. > > > > As a quirk, this SoC needs writing values to 2 VFE VBIF registers > > (see downstream msm8939-camera.dtsi vbif-{regs,settings} properties). > > This fixes black stripes across sensor and garbage in CSID TPG outputs. > > > > Add support for the MSM8939 camera subsystem. > > > > Reviewed-by: Bryan O'Donoghue > > Signed-off-by: Vincent Knecht > > There was a preceding and partially reviewed changeset published on > linux-media [1] before v1 of the MSM8939 platform support in CAMSS, > due to a merge conflict this platform changeset should be rebased IMHO. > > [1] > https://lore.kernel.org/all/20250513142353.2572563-4-vladimir.zapols...@linaro.org/ > > -- > Best wishes, > Vladimir Thank you, I'll look into it
Re: [PATCH v4 6/7] mm/maps: read proc/pid/maps under per-vma lock
On Sat, Jun 7, 2025 at 10:43 AM Lorenzo Stoakes wrote: > > Hi Suren, > > Forgive me but I am going to ask a lot of questions here :p just want to > make sure I'm getting everything right here. No worries and thank you for reviewing! > > On Wed, Jun 04, 2025 at 04:11:50PM -0700, Suren Baghdasaryan wrote: > > With maple_tree supporting vma tree traversal under RCU and per-vma > > locks, /proc/pid/maps can be read while holding individual vma locks > > instead of locking the entire address space. > > Nice :) > > > Completely lockless approach would be quite complex with the main issue > > being get_vma_name() using callbacks which might not work correctly with > > a stable vma copy, requiring original (unstable) vma. > > Hmmm can you expand on what a 'completely lockless' design might comprise of? In my previous implementation (https://lore.kernel.org/all/20250418174959.1431962-1-sur...@google.com/) I was doing this under RCU while checking mmap_lock seq counter to detect address space changes. That's what I meant by a completely lockless approach here. > > It's super un-greppable and I've not got clangd set up with an allmod kernel > to > triple-check but I'm seeing at least 2 (are there more?): > > gate_vma_name() which is: > > return "[vsyscall]"; > > special_mapping_name() which is: > > return ((struct vm_special_mapping *)vma->vm_private_data)->name; > > Which I'm guessing is the issue because it's a double pointer deref... Correct but in more general terms, depending on implementation of the vm_ops.name callback, vma->vm_ops->name(vma) might not work correctly with a vma copy. special_mapping_name() is an example of that. > > Seems such a silly issue to get stuck on, I wonder if we can't just change > this to function correctly? I was thinking about different ways to overcome that but once I realized per-vma locks result in even less contention and the implementation is simpler and more robust, I decided that per-vma locks direction is better. > > > When per-vma lock acquisition fails, we take the mmap_lock for reading, > > lock the vma, release the mmap_lock and continue. This guarantees the > > reader to make forward progress even during lock contention. This will > > Ah that fabled constant forward progress ;) > > > interfere with the writer but for a very short time while we are > > acquiring the per-vma lock and only when there was contention on the > > vma reader is interested in. > > One case requiring special handling is when vma changes between the > > time it was found and the time it got locked. A problematic case would > > be if vma got shrunk so that it's start moved higher in the address > > space and a new vma was installed at the beginning: > > > > reader found: |VMA A| > > VMA is modified:|-VMA B-|VMA A| > > reader locks modified VMA A > > reader reports VMA A: | gap |VMA A| > > > > This would result in reporting a gap in the address space that does not > > exist. To prevent this we retry the lookup after locking the vma, however > > we do that only when we identify a gap and detect that the address space > > was changed after we found the vma. > > OK so in this case we have > > 1. Find VMA A - nothing is locked yet, but presumably we are under RCU so >are... safe? From unmaps? Or are we? I guess actually the detach >mechanism sorts this out for us perhaps? Yes, VMAs are RCU-safe and we do detect if it got detached after we found it but before we locked it. > > 2. We got unlucky and did this immediately prior to VMA A having its >vma->vm_start, vm_end updated to reflect the split. Yes, the split happened after we found it and before we locked it. > > 3. We lock VMA A, now position with an apparent gap after the prior VMA > which, in practice does not exist. Correct. > > So I am guessing that by observing sequence numbers you are able to detect > that a change has occurred and thus retry the operation in this situation? Yes, we detect the gap and we detect that address space has changed, so to endure we did not miss a split we fall back to mmap_read_lock, lock the VMA while holding mmap_read_lock, drop mmap_read_lock and retry. > > I know we previously discussed the possibility of this retry mechanism > going on forever, I guess I will see the resolution to this in the code :) Retry in this case won't go forever because we take mmap_read_lock during the retry. In the worst case we will be constantly falling back to mmap_read_lock but that's a very unlikely case (the writer should be constantly splitting the vma right before the reader locks it). > > > This change is designed to reduce mmap_lock contention and prevent a > > process reading /proc/pid/maps files (often a low priority task, such > > as monitoring/data collection services) from blocking address space > > updates. Note that this change has a userspace visible disadvantage: > > it allows for sub-page data tearing a
[PATCH 0/2] remoteproc: cv1800b: Add initial support for C906L processor
This patch series introduces initial support for the C906L remote processor in the Sophgo CV1800B SoC. The CV1800B SoC integrates multiple cores, including a main application core running Linux, a C906L core typically running RTOS, and an 8051 MCU. The C906L is an asymmetric processor designed to typically run RTOS. This patch adds the basic infrastructure to support remoteproc management of the C906L processor, including firmware loading and basic control (start/stop) from the main Linux core. Mailbox-related functionality will be added in a separate patch. The C906L remoteproc relies on the reset controller [1] to function correctly. A branch for testing is available at [2]. Link: https://lore.kernel.org/all/20250209122936.2338821-1-inochi...@gmail.com/ [1] Link: https://github.com/pigmoral/linux/tree/cv1800-rproc-test [2] --- Junhui Liu (2): dt-bindings: remoteproc: Add C906L rproc for Sophgo CV1800B SoC drivers: remoteproc: Add C906L controller for Sophgo CV1800B SoC .../bindings/remoteproc/sophgo,cv1800b-c906l.yaml | 68 ++ drivers/remoteproc/Kconfig | 9 + drivers/remoteproc/Makefile| 1 + drivers/remoteproc/sophgo_cv1800b_c906l.c | 233 + 4 files changed, 311 insertions(+) --- base-commit: 8630c59e99363c4b655788fd01134aef9bcd9264 change-id: 20250527-cv1800-rproc-08b3a9d2e461 Best regards, -- Junhui Liu
[PATCH 1/2] dt-bindings: remoteproc: Add C906L rproc for Sophgo CV1800B SoC
Add C906L remote processor for CV1800B SoC, which is an asymmetric processor typically running RTOS. Signed-off-by: Junhui Liu --- .../bindings/remoteproc/sophgo,cv1800b-c906l.yaml | 68 ++ 1 file changed, 68 insertions(+) diff --git a/Documentation/devicetree/bindings/remoteproc/sophgo,cv1800b-c906l.yaml b/Documentation/devicetree/bindings/remoteproc/sophgo,cv1800b-c906l.yaml new file mode 100644 index ..455e957dec01c16424c49ebe5ef451883b0c3d4a --- /dev/null +++ b/Documentation/devicetree/bindings/remoteproc/sophgo,cv1800b-c906l.yaml @@ -0,0 +1,68 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/remoteproc/sophgo,cv1800b-c906l.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Sophgo C906L remote processor controller for CV1800B SoC + +maintainers: + - Junhui Liu + +description: + Document the bindings for the C906L remoteproc component that loads and boots + firmwares on the CV1800B SoC. + +properties: + compatible: +const: sophgo,cv1800b-c906l + + firmware-name: +$ref: /schemas/types.yaml#/definitions/string +description: + The name of the firmware file to load for this remote processor, relative + to the firmware search path (typically /lib/firmware/). + + memory-region: +description: + Phandle to a reserved memory region that is used to load the firmware for + this remote processor. The remote processor will use this memory region + as its execution memory. + + resets: +maxItems: 1 + + sophgo,syscon: +$ref: /schemas/types.yaml#/definitions/phandle +description: + A phandle to the SEC_SYS region, used for configuration of the remote processor. + +required: + - compatible + - firmware-name + - memory-region + - resets + - sophgo,syscon + +additionalProperties: false + +examples: + - | +reserved-memory { +#address-cells = <1>; +#size-cells = <1>; +ranges; + +c906l_mem: region@83f4 { +reg = <0x83f4 0xc>; +no-map; +}; +}; + +c906l-rproc { +compatible = "sophgo,cv1800b-c906l"; +firmware-name = "c906l-firmware.elf"; +memory-region = <&c906l_mem>; +resets = <&rst 294>; +sophgo,syscon = <&sec_sys>; +}; -- 2.49.0
[PATCH 2/2] drivers: remoteproc: Add C906L controller for Sophgo CV1800B SoC
Add initial support for the C906L remote processor found in the Sophgo CV1800B SoC. The C906L is an asymmetric core typically used to run an RTOS. This driver enables firmware loading and start/stop control of the C906L processor via the remoteproc framework. The C906L and the main application processor can communicate through mailboxes [1]. Support for mailbox-based functionality will be added in a separate patch. Link: https://lore.kernel.org/linux-riscv/20250520-cv18xx-mbox-v4-0-fd4f1c676...@pigmoral.tech/ [1] Signed-off-by: Junhui Liu --- drivers/remoteproc/Kconfig| 9 ++ drivers/remoteproc/Makefile | 1 + drivers/remoteproc/sophgo_cv1800b_c906l.c | 233 ++ 3 files changed, 243 insertions(+) diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig index 83962a114dc9fdb3260e6e922602f2da53106265..7b09a8f00332605ee528ff7c21c31091c10c2bf5 100644 --- a/drivers/remoteproc/Kconfig +++ b/drivers/remoteproc/Kconfig @@ -299,6 +299,15 @@ config RCAR_REMOTEPROC This can be either built-in or a loadable module. If compiled as module (M), the module name is rcar_rproc. +config SOPHGO_CV1800B_C906L + tristate "Sophgo CV1800B C906L remoteproc support" + depends on ARCH_SOPHGO || COMPILE_TEST + help + Say y here to support CV1800B C906L remote processor via the remote + processor framework. + + It's safe to say N here. + config ST_REMOTEPROC tristate "ST remoteproc support" depends on ARCH_STI diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile index 1c7598b8475d6057a3e044b41e3515103b7aa9f1..3c1e9387491cedc9dda8219f1e9130a84538156f 100644 --- a/drivers/remoteproc/Makefile +++ b/drivers/remoteproc/Makefile @@ -33,6 +33,7 @@ obj-$(CONFIG_QCOM_WCNSS_PIL) += qcom_wcnss_pil.o qcom_wcnss_pil-y += qcom_wcnss.o qcom_wcnss_pil-y += qcom_wcnss_iris.o obj-$(CONFIG_RCAR_REMOTEPROC) += rcar_rproc.o +obj-$(CONFIG_SOPHGO_CV1800B_C906L) += sophgo_cv1800b_c906l.o obj-$(CONFIG_ST_REMOTEPROC)+= st_remoteproc.o obj-$(CONFIG_ST_SLIM_REMOTEPROC) += st_slim_rproc.o obj-$(CONFIG_STM32_RPROC) += stm32_rproc.o diff --git a/drivers/remoteproc/sophgo_cv1800b_c906l.c b/drivers/remoteproc/sophgo_cv1800b_c906l.c new file mode 100644 index ..f3c8d8fd4f796d0cf64f8ab0dd797e017b8e8be7 --- /dev/null +++ b/drivers/remoteproc/sophgo_cv1800b_c906l.c @@ -0,0 +1,233 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (C) 2025 Junhui Liu + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +#include "remoteproc_internal.h" + +#define CV1800B_SYS_C906L_CTRL_REG 0x04 +#define CV1800B_SYS_C906L_CTRL_ENBIT(13) + +#define CV1800B_SYS_C906L_BOOTADDR_REG 0x20 + +/** + * struct cv1800b_c906l - C906L remoteproc structure + * @dev: private pointer to the device + * @reset: reset control handle + * @rproc: the remote processor handle + * @syscon: regmap for accessing security system registers + */ +struct cv1800b_c906l { + struct device *dev; + struct reset_control *reset; + struct rproc *rproc; + struct regmap *syscon; +}; + +static int cv1800b_c906l_mem_alloc(struct rproc *rproc, + struct rproc_mem_entry *mem) +{ + void *va; + + va = ioremap_wc(mem->dma, mem->len); + if (IS_ERR_OR_NULL(va)) + return -ENOMEM; + + /* Update memory entry va */ + mem->va = va; + + return 0; +} + +static int cv1800b_c906l_mem_release(struct rproc *rproc, +struct rproc_mem_entry *mem) +{ + iounmap(mem->va); + + return 0; +} + +static int cv1800b_c906l_add_carveout(struct rproc *rproc) +{ + struct device *dev = rproc->dev.parent; + struct device_node *np = dev->of_node; + struct of_phandle_iterator it; + struct rproc_mem_entry *mem; + struct reserved_mem *rmem; + + /* Register associated reserved memory regions */ + of_phandle_iterator_init(&it, np, "memory-region", NULL, 0); + while (of_phandle_iterator_next(&it) == 0) { + rmem = of_reserved_mem_lookup(it.node); + if (!rmem) { + of_node_put(it.node); + return -EINVAL; + } + + mem = rproc_mem_entry_init(dev, NULL, (dma_addr_t)rmem->base, + rmem->size, rmem->base, + cv1800b_c906l_mem_alloc, + cv1800b_c906l_mem_release, + it.node->name); + + if (!mem) { + of_node_put(it.node); + return -ENOMEM; + } + + rproc_a