Re: [PATCH v4] trace/kprobe: Display the actual notrace function when rejecting a probe
On Thu, 14 Dec 2023 10:47:02 +0530
Naveen N Rao wrote:
> Trying to probe update_sd_lb_stats() using perf results in the below
> message in the kernel log:
> trace_kprobe: Could not probe notrace function _text
>
> This is because 'perf probe' specifies the kprobe location as an offset
> from '_text':
> $ sudo perf probe -D update_sd_lb_stats
> p:probe/update_sd_lb_stats _text+1830728
>
> However, the error message is misleading and doesn't help convey the
> actual notrace function that is being probed. Fix this by looking up the
> actual function name that is being probed. With this fix, we now get the
> below message in the kernel log:
> trace_kprobe: Could not probe notrace function
> update_sd_lb_stats.constprop.0
>
OK, this looks good to me. let me pick this.
Thank you!
> Signed-off-by: Naveen N Rao
> ---
> v4: Use printk format specifier %ps with probe address to lookup the
> symbol, as suggested by Masami.
>
> kernel/trace/trace_kprobe.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 3d7a180a8427..0017404d6e8d 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -487,8 +487,8 @@ static int __register_trace_kprobe(struct trace_kprobe
> *tk)
> return -EINVAL;
>
> if (within_notrace_func(tk)) {
> - pr_warn("Could not probe notrace function %s\n",
> - trace_kprobe_symbol(tk));
> + pr_warn("Could not probe notrace function %ps\n",
> + (void *)trace_kprobe_address(tk));
> return -EINVAL;
> }
>
>
> base-commit: 4758560fa268cecfa1144f015aa9f2525d164b7e
> --
> 2.43.0
>
--
Masami Hiramatsu (Google)
[PATCH V3] remoteproc: virtio: Fix wdg cannot recovery remote processor
From: Joakim Zhang
Recovery remote processor failed when wdg irq received:
[0.842574] remoteproc remoteproc0: crash detected in cix-dsp-rproc: type
watchdog
[0.842750] remoteproc remoteproc0: handling crash #1 in cix-dsp-rproc
[0.842824] remoteproc remoteproc0: recovering cix-dsp-rproc
[0.843342] remoteproc remoteproc0: stopped remote processor cix-dsp-rproc
[0.847901] rproc-virtio rproc-virtio.0.auto: Failed to associate buffer
[0.847979] remoteproc remoteproc0: failed to probe subdevices for
cix-dsp-rproc: -16
The reason is that dma coherent mem would not be released when
recovering the remote processor, due to rproc_virtio_remove()
would not be called, where the mem released. It will fail when
it try to allocate and associate buffer again.
Releasing reserved memory from rproc_virtio_dev_release(), instead of
rproc_virtio_remove().
Fixes: 1d7b61c06dc3 ("remoteproc: virtio: Create platform device for the
remoteproc_virtio")
Signed-off-by: Joakim Zhang
---
ChangeLogs:
V1->V2:
* the same for of_reserved_mem_device_release()
V2->V3:
* release reserved memory in rproc_virtio_dev_release()
---
drivers/remoteproc/remoteproc_virtio.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_virtio.c
b/drivers/remoteproc/remoteproc_virtio.c
index 83d76915a6ad..25b66b113b69 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -351,6 +351,9 @@ static void rproc_virtio_dev_release(struct device *dev)
kfree(vdev);
+ of_reserved_mem_device_release(>pdev->dev);
+ dma_release_coherent_memory(>pdev->dev);
+
put_device(>pdev->dev);
}
@@ -584,9 +587,6 @@ static void rproc_virtio_remove(struct platform_device
*pdev)
rproc_remove_subdev(rproc, >subdev);
rproc_remove_rvdev(rvdev);
- of_reserved_mem_device_release(>dev);
- dma_release_coherent_memory(>dev);
-
put_device(>dev);
}
--
2.25.1
RE: [PATCH V2] remoteproc: virtio: Fix wdg cannot recovery remote processor
Hello Arnaud,
> -Original Message-
> From: Arnaud POULIQUEN
> Sent: Saturday, December 16, 2023 12:56 AM
> To: Joakim Zhang ; anders...@kernel.org;
> mathieu.poir...@linaro.org
> Cc: linux-remotep...@vger.kernel.org; linux-kernel@vger.kernel.org; cix-
> kernel-upstream
> Subject: Re: [PATCH V2] remoteproc: virtio: Fix wdg cannot recovery remote
> processor
>
> Hello Joakim,
>
> On 12/15/23 15:50, joakim.zh...@cixtech.com wrote:
> > From: Joakim Zhang
> >
> > Recovery remote processor failed when wdg irq received:
> > [0.842574] remoteproc remoteproc0: crash detected in cix-dsp-rproc:
> type watchdog
> > [0.842750] remoteproc remoteproc0: handling crash #1 in cix-dsp-rproc
> > [0.842824] remoteproc remoteproc0: recovering cix-dsp-rproc
> > [0.843342] remoteproc remoteproc0: stopped remote processor cix-dsp-
> rproc
> > [0.847901] rproc-virtio rproc-virtio.0.auto: Failed to associate buffer
> > [0.847979] remoteproc remoteproc0: failed to probe subdevices for cix-
> dsp-rproc: -16
> >
> > The reason is that dma coherent mem would not be released when
> > recovering the remote processor, due to rproc_virtio_remove() would
> > not be called, where the mem released. It will fail when it try to
> > allocate and associate buffer again.
> >
> > We can see that dma coherent mem allocated from
> > rproc_add_virtio_dev(), so should release it from
> > rproc_remove_virtio_dev(). These functions should appear symmetrically:
> > -rproc_vdev_do_start()->rproc_add_virtio_dev()-
> >dma_declare_coherent_m
> > emory()
> > -rproc_vdev_do_stop()->rproc_remove_virtio_dev()-
> >dma_release_coherent
> > _memory()
> >
> > The same for of_reserved_mem_device_init_by_idx() and
> of_reserved_mem_device_release().
> >
> > Fixes: 1d7b61c06dc3 ("remoteproc: virtio: Create platform device for
> > the remoteproc_virtio")
> > Signed-off-by: Joakim Zhang
> > ---
> > ChangeLogs:
> > V1->V2:
> > * the same for of_reserved_mem_device_release()
> > ---
> > drivers/remoteproc/remoteproc_virtio.c | 7 ---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/remoteproc/remoteproc_virtio.c
> > b/drivers/remoteproc/remoteproc_virtio.c
> > index 83d76915a6ad..e877ee78740d 100644
> > --- a/drivers/remoteproc/remoteproc_virtio.c
> > +++ b/drivers/remoteproc/remoteproc_virtio.c
> > @@ -465,8 +465,12 @@ static int rproc_add_virtio_dev(struct rproc_vdev
> > *rvdev, int id) static int rproc_remove_virtio_dev(struct device
> > *dev, void *data) {
> > struct virtio_device *vdev = dev_to_virtio(dev);
> > + struct rproc_vdev *rvdev = vdev_to_rvdev(vdev);
> >
> > unregister_virtio_device(vdev);
> > + of_reserved_mem_device_release(>pdev->dev);
> > + dma_release_coherent_memory(>pdev->dev);
> > +
>
> At this step, the virtio device may not be released and may still be using the
> memory.
> Do you try to move this in rproc_virtio_dev_release?
Oh, yes, thanks for the hint, I tested, and it can fix the issue, I will send
v3 soon.
Joakim
> Regards,
> Arnaud
>
> > return 0;
> > }
> >
> > @@ -584,9 +588,6 @@ static void rproc_virtio_remove(struct
> platform_device *pdev)
> > rproc_remove_subdev(rproc, >subdev);
> > rproc_remove_rvdev(rvdev);
> >
> > - of_reserved_mem_device_release(>dev);
> > - dma_release_coherent_memory(>dev);
> > -
> > put_device(>dev);
> > }
> >
> > --
> > 2.25.1
Re: [PATCH] MAINTAINERS: Remove Ohad Ben-Cohen from hwspinlock subsystem
On Sat, Dec 16, 2023 at 08:59:50PM +0200, Ohad Ben Cohen wrote: > Hi Bagas, > > On Sat, Dec 16, 2023 at 1:10 PM Bagas Sanjaya wrote: > > --- a/CREDITS > > +++ b/CREDITS > > @@ -323,6 +323,7 @@ N: Ohad Ben Cohen > > E: o...@wizery.com > > D: Remote Processor (remoteproc) subsystem > > D: Remote Processor Messaging (rpmsg) subsystem > > +D: Hardware spinlock (hwspinlock) subsystem > > Please also add: > > D: OMAP hwspinlock driver > D: OMAP remoteproc driver > OK, will do in v2. Thanks. -- An old man doll... just what I always wanted! - Clara signature.asc Description: PGP signature
Re: [PATCH] MAINTAINERS: Remove Ohad Ben-Cohen from hwspinlock subsystem
Hi Bagas, On Sat, Dec 16, 2023 at 1:10 PM Bagas Sanjaya wrote: > --- a/CREDITS > +++ b/CREDITS > @@ -323,6 +323,7 @@ N: Ohad Ben Cohen > E: o...@wizery.com > D: Remote Processor (remoteproc) subsystem > D: Remote Processor Messaging (rpmsg) subsystem > +D: Hardware spinlock (hwspinlock) subsystem Please also add: D: OMAP hwspinlock driver D: OMAP remoteproc driver Thanks, Ohad.
[PATCH] x86/sgx: fix kernel-doc comment misuse
Don't use "/**" for a non-kernel-doc comment. This prevents a warning from scripts/kernel-doc: main.c:740: warning: expecting prototype for A section metric is concatenated in a way that @low bits 12(). Prototype was for sgx_calc_section_metric() instead Signed-off-by: Randy Dunlap Cc: Jarkko Sakkinen Cc: Dave Hansen Cc: linux-...@vger.kernel.org Cc: x...@kernel.org --- arch/x86/kernel/cpu/sgx/main.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff -- a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c --- a/arch/x86/kernel/cpu/sgx/main.c +++ b/arch/x86/kernel/cpu/sgx/main.c @@ -731,7 +731,7 @@ out: return 0; } -/** +/* * A section metric is concatenated in a way that @low bits 12-31 define the * bits 12-31 of the metric and @high bits 0-19 define the bits 32-51 of the * metric.
[PATCH] MAINTAINERS: Remove Ohad Ben-Cohen from hwspinlock subsystem
Commit 62c46d55688894 ("MAINTAINERS: Removing Ohad from remoteproc/rpmsg
maintenance") removes his MAINTAINERS entry in regards to remoteproc
subsystem due to his inactivity (the last commit with his Signed-off-by
is 99c429cb4e628e ("remoteproc/wkup_m3: Use MODULE_DEVICE_TABLE to
export alias") which is authored in 2015 and his last LKML message prior
to 62c46d55688894 was [1]).
Remove also his MAINTAINERS entry for hwspinlock subsystem as there is
no point of Cc'ing maintainers who never respond in a long time.
[1]:
https://lore.kernel.org/r/CAK=Wgbbcyi36ef1-PV8VS=m6nfoqnfgudwy6v7ocnkt0ddr...@mail.gmail.com/
Signed-off-by: Bagas Sanjaya
---
I was prompted to do the removal when I was reviewing kernel-doc fix
[2]. When I was digging MAINTAINERS history (`git log --no-merges --
MAINTAINERS`), I noticed that Ohad is inactive.
This patch is based on mm-nonmm-unstable as I intend to route it
through mm tree.
[2]: https://lore.kernel.org/r/zx04ymz_vdfee...@archie.me/
CREDITS | 1 +
MAINTAINERS | 4 +---
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/CREDITS b/CREDITS
index 81845c39e3cf37..cff24c62b0e8f9 100644
--- a/CREDITS
+++ b/CREDITS
@@ -323,6 +323,7 @@ N: Ohad Ben Cohen
E: o...@wizery.com
D: Remote Processor (remoteproc) subsystem
D: Remote Processor Messaging (rpmsg) subsystem
+D: Hardware spinlock (hwspinlock) subsystem
N: Krzysztof Benedyczak
E: go...@mat.uni.torun.pl
diff --git a/MAINTAINERS b/MAINTAINERS
index 5c9d3d8546714a..4acc4a3d4fcd96 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9257,7 +9257,6 @@ F:drivers/char/hw_random/
F: include/linux/hw_random.h
HARDWARE SPINLOCK CORE
-M: Ohad Ben-Cohen
M: Bjorn Andersson
R: Baolin Wang
L: linux-remotep...@vger.kernel.org
@@ -15692,9 +15691,8 @@ F:
Documentation/devicetree/bindings/gpio/ti,omap-gpio.yaml
F: drivers/gpio/gpio-omap.c
OMAP HARDWARE SPINLOCK SUPPORT
-M: Ohad Ben-Cohen
L: linux-o...@vger.kernel.org
-S: Maintained
+S: Orphan
F: drivers/hwspinlock/omap_hwspinlock.c
OMAP HS MMC SUPPORT
base-commit: cbeb59a84b8f29151b882d03a4d23d19d92f4337
--
An old man doll... just what I always wanted! - Clara
Re: [PATCH vhost v2 4/8] vdpa/mlx5: Mark vq addrs for modification in hw vq
On Fri, 2023-12-15 at 18:56 +0100, Eugenio Perez Martin wrote: > On Fri, Dec 15, 2023 at 3:13 PM Dragos Tatulea wrote: > > > > On Fri, 2023-12-15 at 12:35 +, Dragos Tatulea wrote: > > > On Thu, 2023-12-14 at 19:30 +0100, Eugenio Perez Martin wrote: > > > > On Thu, Dec 14, 2023 at 4:51 PM Dragos Tatulea > > > > wrote: > > > > > > > > > > On Thu, 2023-12-14 at 08:45 -0500, Michael S. Tsirkin wrote: > > > > > > On Thu, Dec 14, 2023 at 01:39:55PM +, Dragos Tatulea wrote: > > > > > > > On Tue, 2023-12-12 at 15:44 -0800, Si-Wei Liu wrote: > > > > > > > > > > > > > > > > On 12/12/2023 11:21 AM, Eugenio Perez Martin wrote: > > > > > > > > > On Tue, Dec 5, 2023 at 11:46 AM Dragos Tatulea > > > > > > > > > wrote: > > > > > > > > > > Addresses get set by .set_vq_address. hw vq addresses will > > > > > > > > > > be updated on > > > > > > > > > > next modify_virtqueue. > > > > > > > > > > > > > > > > > > > > Signed-off-by: Dragos Tatulea > > > > > > > > > > Reviewed-by: Gal Pressman > > > > > > > > > > Acked-by: Eugenio Pérez > > > > > > > > > I'm kind of ok with this patch and the next one about state, > > > > > > > > > but I > > > > > > > > > didn't ack them in the previous series. > > > > > > > > > > > > > > > > > > My main concern is that it is not valid to change the vq > > > > > > > > > address after > > > > > > > > > DRIVER_OK in VirtIO, which vDPA follows. Only memory maps are > > > > > > > > > ok to > > > > > > > > > change at this moment. I'm not sure about vq state in vDPA, > > > > > > > > > but vhost > > > > > > > > > forbids changing it with an active backend. > > > > > > > > > > > > > > > > > > Suspend is not defined in VirtIO at this moment though, so > > > > > > > > > maybe it is > > > > > > > > > ok to decide that all of these parameters may change during > > > > > > > > > suspend. > > > > > > > > > Maybe the best thing is to protect this with a vDPA feature > > > > > > > > > flag. > > > > > > > > I think protect with vDPA feature flag could work, while on the > > > > > > > > other > > > > > > > > hand vDPA means vendor specific optimization is possible around > > > > > > > > suspend > > > > > > > > and resume (in case it helps performance), which doesn't have > > > > > > > > to be > > > > > > > > backed by virtio spec. Same applies to vhost user backend > > > > > > > > features, > > > > > > > > variations there were not backed by spec either. Of course, we > > > > > > > > should > > > > > > > > try best to make the default behavior backward compatible with > > > > > > > > virtio-based backend, but that circles back to no suspend > > > > > > > > definition in > > > > > > > > the current virtio spec, for which I hope we don't cease > > > > > > > > development on > > > > > > > > vDPA indefinitely. After all, the virtio based vdap backend can > > > > > > > > well > > > > > > > > define its own feature flag to describe (minor difference in) > > > > > > > > the > > > > > > > > suspend behavior based on the later spec once it is formed in > > > > > > > > future. > > > > > > > > > > > > > > > So what is the way forward here? From what I understand the > > > > > > > options are: > > > > > > > > > > > > > > 1) Add a vdpa feature flag for changing device properties while > > > > > > > suspended. > > > > > > > > > > > > > > 2) Drop these 2 patches from the series for now. Not sure if this > > > > > > > makes sense as > > > > > > > this. But then Si-Wei's qemu device suspend/resume poc [0] that > > > > > > > exercises this > > > > > > > code won't work anymore. This means the series would be less well > > > > > > > tested. > > > > > > > > > > > > > > Are there other possible options? What do you think? > > > > > > > > > > > > > > [0] https://github.com/siwliu-kernel/qemu/tree/svq-resume-wip > > > > > > > > > > > > I am fine with either of these. > > > > > > > > > > > How about allowing the change only under the following conditions: > > > > > vhost_vdpa_can_suspend && vhost_vdpa_can_resume && > > > > > VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK is set > > > > > > > > > > ? > > > > > > > > I think the best option by far is 1, as there is no hint in the > > > > combination of these 3 indicating that you can change device > > > > properties in the suspended state. > > > > > > > Sure. Will respin a v3 without these two patches. > > > > > > Another series can implement option 2 and add these 2 patches on top. > > Hmm...I misunderstood your statement and sent a erroneus v3. You said that > > having a feature flag is the best option. > > > > Will add a feature flag in v4: is this similar to the > > VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK flag? > > > > Right, it should be easy to return it from .get_backend_features op if > the FW returns that capability, isn't it? > Yes, that's easy. But I wonder if we need one feature bit for each type of change: - VHOST_BACKEND_F_CHANGEABLE_VQ_ADDR_IN_SUSPEND - VHOST_BACKEND_F_CHANGEABLE_VQ_STATE_IN_SUSPEND Or would a big one
