[PATCH] eventfs: Remember what dentries were created on dir open
From: "Steven Rostedt (Google)"
Using the following code with libtracefs:
int dfd;
// create the directory events/kprobes/kp1
tracefs_kprobe_raw(NULL, "kp1", "schedule_timeout", "time=$arg1");
// Open the kprobes directory
dfd = tracefs_instance_file_open(NULL, "events/kprobes", O_RDONLY);
// Do a lookup of the kprobes/kp1 directory (by looking at enable)
tracefs_file_exists(NULL, "events/kprobes/kp1/enable");
// Now create a new entry in the kprobes directory
tracefs_kprobe_raw(NULL, "kp2", "schedule_hrtimeout", "expires=$arg1");
// Do another lookup to create the dentries
tracefs_file_exists(NULL, "events/kprobes/kp2/enable"))
// Close the directory
close(dfd);
What happened above, the first open (dfd) will call
dcache_dir_open_wrapper() that will create the dentries and up their ref
counts.
Now the creation of "kp2" will add another dentry within the kprobes
directory.
Upon the close of dfd, eventfs_release() will now do a dput for all the
entries in kprobes. But this is where the problem lies. The open only
upped the dentry of kp1 and not kp2. Now the close is decrementing both
kp1 and kp2, which causes kp2 to get a negative count.
Doing a "trace-cmd reset" which deletes all the kprobes cause the kernel
to crash! (due to the messed up accounting of the ref counts).
To solve this, save all the dentries that are opened in the
dcache_dir_open_wrapper() into an array, and use this array to know what
dentries to do a dput on in eventfs_release().
Since the dcache_dir_open_wrapper() calls dcache_dir_open() which uses the
file->private_data, we need to also add a wrapper around dcache_readdir()
that uses the cursor assigned to the file->private_data. This is because
the dentries need to also be saved in the file->private_data. To do this
create the structure:
struct dentry_list {
void*cursor;
struct dentry **dentries;
};
Which will hold both the cursor and the dentries. Some shuffling around is
needed to make sure that dcache_dir_open() and dcache_readdir() only see
the cursor.
Link:
https://lore.kernel.org/linux-trace-kernel/20230919211804.230ed...@gandalf.local.home/
Fixes: 63940449555e7 ("eventfs: Implement eventfs lookup, read, open functions")
Reported-by: "Masami Hiramatsu (Google)"
Signed-off-by: Steven Rostedt (Google)
---
fs/tracefs/event_inode.c | 87
1 file changed, 70 insertions(+), 17 deletions(-)
diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c
index b23bb0957bb4..1c5c6a1ff4cc 100644
--- a/fs/tracefs/event_inode.c
+++ b/fs/tracefs/event_inode.c
@@ -30,6 +30,7 @@ static struct dentry *eventfs_root_lookup(struct inode *dir,
struct dentry *dentry,
unsigned int flags);
static int dcache_dir_open_wrapper(struct inode *inode, struct file *file);
+static int dcache_readdir_wrapper(struct file *file, struct dir_context *ctx);
static int eventfs_release(struct inode *inode, struct file *file);
static const struct inode_operations eventfs_root_dir_inode_operations = {
@@ -39,7 +40,7 @@ static const struct inode_operations
eventfs_root_dir_inode_operations = {
static const struct file_operations eventfs_file_operations = {
.open = dcache_dir_open_wrapper,
.read = generic_read_dir,
- .iterate_shared = dcache_readdir,
+ .iterate_shared = dcache_readdir_wrapper,
.llseek = generic_file_llseek,
.release= eventfs_release,
};
@@ -356,6 +357,11 @@ static struct dentry *eventfs_root_lookup(struct inode
*dir,
return ret;
}
+struct dentry_list {
+ void*cursor;
+ struct dentry **dentries;
+};
+
/**
* eventfs_release - called to release eventfs file/dir
* @inode: inode to be released
@@ -364,26 +370,25 @@ static struct dentry *eventfs_root_lookup(struct inode
*dir,
static int eventfs_release(struct inode *inode, struct file *file)
{
struct tracefs_inode *ti;
- struct eventfs_inode *ei;
- struct eventfs_file *ef;
- struct dentry *dentry;
- int idx;
+ struct dentry_list *dlist = file->private_data;
+ void *cursor;
+ int i;
ti = get_tracefs(inode);
if (!(ti->flags & TRACEFS_EVENT_INODE))
return -EINVAL;
- ei = ti->private;
- idx = srcu_read_lock(_srcu);
- list_for_each_entry_srcu(ef, >e_top_files, list,
-srcu_read_lock_held(_srcu)) {
- mutex_lock(_mutex);
- dentry = ef->dentry;
- mutex_unlock(_mutex);
- if (dentry)
- dput(dentry);
+ if (WARN_ON_ONCE(!dlist))
+ return -EINVAL;
+
+ for (i = 0; dlist->dentries[i]; i++) {
+
Re: [PATCH] module: Fix comment typo
On Sun, Jun 18, 2023 at 10:58:43PM +0800, zhumao...@208suo.com wrote: > > Delete duplicated word in comment. > > Signed-off-by: Zhu Mao > --- Applied and pushed, next time please consider sending a slew of typo fixes, just one word is a bit too much trouble for just one patch. Luis
Re: [PATCH 0/2] module: Do some small changes
On Fri, Jun 16, 2023 at 09:51:31AM +0800, Tiezhu Yang wrote: > The first patch is suggested by Xi Zhang offline, > the second patch is inspired by the first patch, no functional changes. Applied and pushed, thanks! Luis
Re: [PATCH 1/2 v3] eventfs: Remove eventfs_file and just use eventfs_inode
On Tue, 19 Sep 2023 21:18:04 -0400
Steven Rostedt wrote:
> Hmm, actually looking at this, it's worse than what you stated. This is
> called when a directory is closed. So if you had:
>
> open(dir);
>
> // look at all the content of this dir to create dentries
>
> // another task creates a new entry and looks at it too.
>
> close(dir);
>
> Now we iterate over all the dentries of the dir and dput it.
>
> I think this will cause the ref counts to get out of sync. I'll have to try
> to create this scenario and see what happens.
And yes it does break :-p
Even without this patch it breaks. That is, this bug exists currently upstream.
I run the attached file (requires libtracefs)
and then run:
# cd /sys/kernel/tracing
# echo > buffer_size_kb&
Wait a bit.
This will cause the ref counts to go negative.
Then do a: trace-cmd reset
Which will remove the kprobes created by the attached program, and will
crash the kernel :-p
I have an idea on how to fix it. Let my try it out.
-- Steve
#include
#include
#include
#include
#include
#include
#include
#include
static char *argv0;
static char *get_this_name(void)
{
static char *this_name;
char *arg;
char *p;
if (this_name)
return this_name;
arg = argv0;
p = arg+strlen(arg);
while (p >= arg && *p != '/')
p--;
p++;
this_name = p;
return p;
}
static void usage(void)
{
char *p = get_this_name();
printf("usage: %s [-c comm] trace.dat\n"
"\n"
" Run this after running: trace-cmd record -e sched\n"
"\n"
" Do some work and then hit Ctrl^C to stop the recording.\n"
" Run this on the resulting trace.dat file\n"
"\n"
"-c comm - to look at only a specific process called 'comm'\n"
"\n",p);
exit(-1);
}
static void __vdie(const char *fmt, va_list ap, int err)
{
int ret = errno;
char *p = get_this_name();
if (err && errno)
perror(p);
else
ret = -1;
fprintf(stderr, " ");
vfprintf(stderr, fmt, ap);
fprintf(stderr, "\n");
exit(ret);
}
void die(const char *fmt, ...)
{
va_list ap;
va_start(ap, fmt);
__vdie(fmt, ap, 0);
va_end(ap);
}
void pdie(const char *fmt, ...)
{
va_list ap;
va_start(ap, fmt);
__vdie(fmt, ap, 1);
va_end(ap);
}
int main (int argc, char **argv)
{
int dfd;
int ret;
ret = tracefs_kprobe_raw(NULL, "kp1", "schedule_timeout", "time=$arg1");
if (ret < 0)
pdie("Can't create schedule_timeout kprobe");
dfd = tracefs_instance_file_open(NULL, "events/kprobes", O_RDONLY);
if (dfd < 0)
pdie("Can't open events/kprobes");
if (!tracefs_file_exists(NULL, "events/kprobes/kp1/enable"))
pdie("kp1/enable does not exist");
ret = tracefs_kprobe_raw(NULL, "kp2", "schedule_hrtimeout", "expires=$arg1");
if (ret < 0)
pdie("Can't create schedule_hrtimeout kprobe");
if (!tracefs_file_exists(NULL, "events/kprobes/kp2/enable"))
pdie("kp2/enable does not exist");
close(dfd);
// tracefs_dynevent_destroy_all(TRACEFS_DYNEVENT_KPROBE, true);
return 0;
}
Re: [PATCH] module/decompress: use vmalloc() for gzip decompression workspace
On Wed, Aug 30, 2023 at 05:58:20PM +0200, Andrea Righi wrote:
> Use a similar approach as commit a419beac4a07 ("module/decompress: use
> vmalloc() for zstd decompression workspace") and replace kmalloc() with
> vmalloc() also for the gzip module decompression workspace.
>
> In this case the workspace is represented by struct inflate_workspace
> that can be fairly large for kmalloc() and it can potentially lead to
> allocation errors on certain systems:
>
> $ pahole inflate_workspace
> struct inflate_workspace {
> struct inflate_state inflate_state;/* 0 9544 */
> /* --- cacheline 149 boundary (9536 bytes) was 8 bytes ago --- */
> unsigned char working_window[32768]; /* 9544 32768 */
>
> /* size: 42312, cachelines: 662, members: 2 */
> /* last cacheline: 8 bytes */
> };
>
> Considering that there is no need to use continuous physical memory,
> simply switch to vmalloc() to provide a more reliable in-kernel module
> decompression.
>
> Fixes: b1ae6dc41eaa ("module: add in-kernel support for decompressing")
> Signed-off-by: Andrea Righi
Applied, and pushed, thanks!
Luis
[PATCH] MAINTAINERS: add include/linux/module*.h to modules
Use glob include/linux/module*.h to capture all module changes. Suggested-by: Kees Cook Signed-off-by: Luis Chamberlain --- MAINTAINERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index 3be1bdfe8ecc..b78cf8dc7f16 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -14289,7 +14289,7 @@ L: linux-kernel@vger.kernel.org S: Maintained T: git git://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git modules-next F: include/linux/kmod.h -F: include/linux/module.h +F: include/linux/module*.h F: kernel/module/ F: lib/test_kmod.c F: scripts/module* -- 2.39.2
Re: [PATCH v2 0/8] sysctl: Remove sentinel elements from arch
On Wed, Sep 13, 2023 at 11:10:54AM +0200, Joel Granados via B4 Relay wrote:
> V2:
> * Added clarification both in the commit messages and the coverletter as
> to why this patch is safe to apply.
> * Added {Acked,Reviewed,Tested}-by from list
> * Link to v1:
> https://lore.kernel.org/r/20230906-jag-sysctl_remove_empty_elem_arch-v1-0-3935d4854...@samsung.com
Thanks! I've merged this onto sysctl-next.
Luis
Re: This list is being migrated to new infrastructure
On Wed, Sep 20, 2023 at 03:52:05PM -0400, Konstantin Ryabitsev wrote: > Hello, all: > > This list is being migrated to the new vger infrastructure. This should be a > fully transparent process and you don't need to change anything about how you > participate with the list or how you receive mail. > > There will be a brief 20-minute delay with archives on lore.kernel.org. I will > follow up once the archive migration has been completed. This work is now completed. I will monitor the performance to make sure that the new infrastructure is still successfully coping with the email traffic. If you notice any problems, please report them to helpdesk at kernel.org. -K
Re: (subset) [PATCH v2 0/7] Initial support for the Fairphone 5 smartphone
On Tue, 19 Sep 2023 14:45:54 +0200, Luca Weiss wrote: > Add support to boot up mainline kernel on the QCM6490-based Fairphone 5 > smartphone. > > These patches only cover a part of the functionality brought up on > mainline so far, with the rest needing larger dts and driver changes or > depend on patches that are not yet merged. I will work on sending those > once these base patches here have settled. > > [...] Applied, thanks! [1/7] arm64: dts: qcom: sc7280: Mark some nodes as 'reserved' commit: 6da24ba932082bae110feb917a64bb54637fa7c0 [3/7] arm64: dts: qcom: pm7250b: make SID configurable commit: 8e2d56f64572e0432c355093a7601bde29677490 [4/7] arm64: dts: qcom: pm8350c: Add flash led node commit: bfd4412a023b2a3a2f858f2ffc13705aaeef5737 [6/7] dt-bindings: arm: qcom: Add QCM6490 Fairphone 5 commit: 4b1a16d776b474345b12f834de1fd42bca226d90 [7/7] arm64: dts: qcom: qcm6490: Add device-tree for Fairphone 5 commit: eee9602ad6498eee9ddab1b7eb6aede288f0b934 Best regards, -- Bjorn Andersson
Re: (subset) [PATCH v2 00/14] Clean up RPM bus clocks remnants
On Tue, 12 Sep 2023 15:31:38 +0200, Konrad Dybcio wrote: > After the recent cleanups ([1], [2]) some in-tree abusers that directly > accessed the RPM bus clocks, effectively circumventing and working > against the efforts of the interconnect framework, were found. > > Patches 1-5 drop deprecated references and the rest attempt to stop > direct bus clock abuses. > > [...] Applied, thanks! [08/14] dt-bindings: remoteproc: qcom,adsp: Remove AGGRE2 clock commit: c4c5b47958529bc1de10260df0c583710853b516 [09/14] dt-bindings: remoteproc: qcom,msm8996-mss-pil: Remove PNoC clock commit: e7781901449cbcff129d80a5d9021e9e96084ec4 [10/14] remoteproc: qcom: q6v5-mss: Remove PNoC clock from 8996 MSS commit: e1592981c51bac38ea2041b642777b3ba30606a8 Best regards, -- Bjorn Andersson
Re: (subset) [PATCH v3 0/7] MSM8976 PLL,RPMPD and DTS changes
On Sat, 12 Aug 2023 13:24:43 +0200, Adam Skladowski wrote: > This patch series fixes introduce support for msm8976 pll, > also brings some adjustments and fixes domains setup and few dts nitpicks. > > Changes since v1 > > 1. Fixed few styling issues > 2. Changed compatibles for plls > 3. Added fixes: tag to first patch > > [...] Applied, thanks! [6/7] arm64: dts: qcom: msm8976: Split lpass region commit: 31c133b4a07e3db456a7e661c96653cd65a25bc6 [7/7] arm64: dts: qcom: msm8976: Fix ipc bit shifts commit: 684277525c70f329300cc687e27248e405a4ff9e Best regards, -- Bjorn Andersson
Re: [PATCH v4] scripts/link-vmlinux.sh: Add alias to duplicate symbols for kallsyms
Hello Francis, Thanks a lot for the review. Il giorno mer 20 set 2023 alle ore 12:53 Francis Laniel ha scritto: > > Hi. > > Le mardi 19 septembre 2023, 22:39:48 EEST Alessandro Carminati (Red Hat) a > écrit : > > It is not uncommon for drivers or modules related to similar peripherals > > to have symbols with the exact same name. > > While this is not a problem for the kernel's binary itself, it becomes an > > issue when attempting to trace or probe specific functions using > > infrastructure like ftrace or kprobe. > > > > The tracing subsystem relies on the `nm -n vmlinux` output, which provides > > symbol information from the kernel's ELF binary. However, when multiple > > symbols share the same name, the standard nm output does not differentiate > > between them. This can lead to confusion and difficulty when trying to > > probe the intended symbol. > > > > ~ # cat /proc/kallsyms | grep " name_show" > > 8c4f76d0 t name_show > > 8c9cccb0 t name_show > > 8cb0ac20 t name_show > > 8cc728c0 t name_show > > 8ce0efd0 t name_show > > 8ce126c0 t name_show > > 8ce1dd20 t name_show > > 8ce24e70 t name_show > > 8d1104c0 t name_show > > 8d1fe480 t name_show > > > > kas_alias addresses this challenge by enhancing symbol names with > > meaningful suffixes generated from the source file and line number > > during the kernel build process. > > These newly generated aliases provide tracers with the ability to > > comprehend the symbols they are interacting with when utilizing the > > ftracefs interface. > > This approach may also allow for the probing by name of previously > > inaccessible symbols. > > > > ~ # cat /proc/kallsyms | grep gic_mask_irq > > d15671e505ac t gic_mask_irq > > d15671e505ac t gic_mask_irq@drivers_irqchip_irq_gic_c_167 > > d15671e532a4 t gic_mask_irq > > d15671e532a4 t gic_mask_irq@drivers_irqchip_irq_gic_v3_c_407 > > ~ # > > > > Changes from v1: > > - Integrated changes requested by Masami to exclude symbols with prefixes > > "_cfi" and "_pfx". > > - Introduced a small framework to handle patterns that need to be excluded > > from the alias production. > > - Excluded other symbols using the framework. > > - Introduced the ability to discriminate between text and data symbols. > > - Added two new config symbols in this version: CONFIG_KALLSYMS_ALIAS_DATA, > > which allows data for data, and CONFIG_KALLSYMS_ALIAS_DATA_ALL, which > > excludes all filters and provides an alias for each duplicated symbol. > > > > https://lore.kernel.org/all/20230711151925.1092080-1-alessandro.carminati@gm > > ail.com/ > > > > Changes from v2: > > - Alias tags are created by querying DWARF information from the vmlinux. > > - The filename + line number is normalized and appended to the original > > name. > > - The tag begins with '@' to indicate the symbol source. > > - Not a change, but worth mentioning, since the alias is added to the > > existing list, the old duplicated name is preserved, and the livepatch > > way of dealing with duplicates is maintained. > > - Acknowledging the existence of scenarios where inlined functions > > declared in header files may result in multiple copies due to compiler > > behavior, though it is not actionable as it does not pose an operational > > issue. > > - Highlighting a single exception where the same name refers to different > > functions: the case of "compat_binfmt_elf.c," which directly includes > > "binfmt_elf.c" producing identical function copies in two separate > > modules. > > > > https://lore.kernel.org/all/20230714150326.1152359-1-alessandro.carminati@gm > > ail.com/ > > > > Changes from v3: > > - kas_alias was rewritten in Python to create a more concise and > > maintainable codebase. > > - The previous automation process used by kas_alias to locate the vmlinux > > and the addr2line has been replaced with an explicit command-line switch > > for specifying these requirements. > > - addr2line has been added into the main Makefile. > > - A new command-line switch has been introduced, enabling users to extend > > the alias to global data names. > > > > https://lore.kernel.org/all/20230828080423.3539686-1-alessandro.carminati@gm > > ail.com/ > > > > NOTE: > > About the symbols name duplication that happens as consequence of the > > inclusion compat_binfmt_elf.c does, it is evident that this corner is > > inherently challenging the addr2line approach. > > Attempting to conceal this limitation would be counterproductive. > > > > compat_binfmt_elf.c includes directly binfmt_elf.c, addr2line can't help > > but report all functions and data declared by that file, coming from > > binfmt_elf.c. > > > > My position is that, rather than producing a more complicated pipeline > > to handle this corner case, it is better to fix the compat_binfmt_elf.c > > anomaly. > > > > This patch does not deal with the two potentially problematic symbols
Re: (subset) [PATCH v3 0/4] sc7180: Add ADSP
On Thu, 07 Sep 2023 15:02:33 +0500, Nikita Travkin wrote: > sc7180 has an ADSP remoteproc that can be used to control the sound > hardware. This remoteproc has to be used on those devices that use > Qualcomm firmware and thus are locked out of driving the lpass directly. > > Introducing the ADSP would allow multiple WoA laptops such as Aspire 1 > to provide sound. It's also useful for the sm7125 devices that are to be > included to the kernel [1] > > [...] Applied, thanks! [3/4] arm64: dts: qcom: sc7180: Add tertiary mi2s pinctrl commit: 828298a9efb237b76fa667bb74a6450d1df3eeed [4/4] arm64: dts: qcom: sc7180: Add ADSP commit: a3d5fb3b084c0c67f9918a866b92cbe09b9e1d77 Best regards, -- Bjorn Andersson
Re: [PATCH] clk: qcom: mmcc-msm8974: remove ocmemcx_ahb_clk
On Sat, 02 Sep 2023 19:34:23 +0200, Luca Weiss wrote: > According to a commit in the 3.4 vendor kernel sources[0] the > ocmemcx_ahb_clk clock "is controlled by RPM and should not be touched by > APPS.". > > [0] > https://git.codelinaro.org/clo/la/kernel/msm/-/commit/37df5f2d91b4d5768b37fcaacaeea958dd683ebc > > And indeed, when using MDSS+GPU+OCMEM on MSM8226 and not using > clk_ignore_unused, when Linux tries to disable the clock the device > crashes and reboots. > > [...] Applied, thanks! [1/1] clk: qcom: mmcc-msm8974: remove ocmemcx_ahb_clk commit: 471e2875f8904539985e62220afd6c88e70779fa Best regards, -- Bjorn Andersson
Re: [PATCH v5 0/3] SM6375 remoteprocs
On Thu, 27 Jul 2023 19:33:20 +0200, Konrad Dybcio wrote: > Resending as the previous revision was mostly ignored on the rproc side. > > Changes since v3: > - Pick up krzk's rb on bindings > - Drop patch 4 (applied) > Link to v3: > https://lore.kernel.org/linux-arm-msm/20230109135647.339224-1-konrad.dyb...@linaro.org/ > > [...] Applied, thanks! [1/3] dt-bindings: remoteproc: qcom,sm6375-pas: Document remoteprocs commit: 6d3211e015b0f236986b16c042f71cce8d36d727 [2/3] remoteproc: qcom: pas: Add SM6375 ADSP & CDSP commit: a6df21cf0c93cab57059e2592c7c99b424965374 [3/3] remoteproc: qcom: pas: Add SM6375 MPSS commit: 93f875645c9da9c788224964499e68fa9664e80f Best regards, -- Bjorn Andersson
Re: (subset) [PATCH v2 0/2] Fix tcsr_mutex register for IPQ6018
On Tue, 05 Sep 2023 15:25:33 +0530, Vignesh Viswanathan wrote: > IPQ6018 has 32 tcsr_mutex hwlock registers of 0x1000 size each. > The compatible string qcom,ipq6018-tcsr-mutex is mapped to > of_msm8226_tcsr_mutex which has 32 locks configured with stride of 0x80 > and doesn't match the HW present in IPQ6018. > > This series fixes the following: > 1. Fix the tcsr_mutex register size to 0x2 in IPQ6018 DTSI. > 2. Remove IPQ6018 specific compatible in hwspinlock driver so that it > falls back to pick of_tcsr_mutex data. > > [...] Applied, thanks! [2/2] hwspinlock: qcom: Remove IPQ6018 SOC specific compatible commit: 823313068617bf2414c6067504b4e2ce5768e601 Best regards, -- Bjorn Andersson
Re: [PATCH v2] dt-bindings: remoteproc: pru: Add Interrupt property
On Mon, 14 Aug 2023 15:21:41 +0530, MD Danish Anwar wrote: > Add interrupts and interrupt-names protperties for PRU and RTU cores. > > Applied, thanks! [1/1] dt-bindings: remoteproc: pru: Add Interrupt property commit: d93f191b95bec3c913978eb18c6297e797915993 Best regards, -- Bjorn Andersson
[PATCH V2] tracing/timerlat: Hotplug support for the user-space interface
The osnoise/per_cpu/CPU$/timerlat_fd is create for each possible
CPU, but it might create confusion if the CPU is not online.
Create the file only for online CPUs, also follow hotplug by
creating and deleting as CPUs come and go.
Fixes: e88ed227f639 ("tracing/timerlat: Add user-space interface")
Signed-off-by: Daniel Bristot de Oliveira
---
Changes from V1:
- Fix compilation issue when !HOTPLUG
- Fix init interface | hotplug race
Link:
https://lore.kernel.org/lkml/b619d9fd08a3bb47018cf40afa95783844a3c1fd.1694789910.git.bris...@kernel.org/
kernel/trace/trace_osnoise.c | 112 +++
1 file changed, 86 insertions(+), 26 deletions(-)
diff --git a/kernel/trace/trace_osnoise.c b/kernel/trace/trace_osnoise.c
index bd0d01d00fb9..8422562d9864 100644
--- a/kernel/trace/trace_osnoise.c
+++ b/kernel/trace/trace_osnoise.c
@@ -229,6 +229,19 @@ static inline struct osnoise_variables
*this_cpu_osn_var(void)
}
#ifdef CONFIG_TIMERLAT_TRACER
+
+/*
+ * osnoise/per_cpu dir
+ */
+static struct dentry *osnoise_per_cpu_fd;
+
+struct osnoise_per_cpu_dir {
+ struct dentry *root;
+ struct dentry *timerlat_fd;
+};
+
+static DEFINE_PER_CPU(struct osnoise_per_cpu_dir, osnoise_per_cpu_dir);
+
/*
* Runtime information for the timer mode.
*/
@@ -2000,6 +2013,9 @@ static int start_kthread(unsigned int cpu)
char comm[24];
if (timerlat_enabled()) {
+ if (!test_bit(OSN_WORKLOAD, _options))
+ return 0;
+
snprintf(comm, 24, "timerlat/%d", cpu);
main = timerlat_main;
} else {
@@ -2065,19 +2081,64 @@ static int start_per_cpu_kthreads(void)
return retval;
}
+#ifdef CONFIG_TIMERLAT_TRACER
+static const struct file_operations timerlat_fd_fops;
+static int timerlat_add_per_cpu_interface(long cpu)
+{
+ struct dentry *timerlat_fd, *cpu_dir_fd;
+ char cpu_str[30]; /* see trace.c: tracing_init_tracefs_percpu() */
+
+ if (!osnoise_per_cpu_fd)
+ return 0;
+
+ snprintf(cpu_str, 30, "cpu%ld", cpu);
+ cpu_dir_fd = tracefs_create_dir(cpu_str, osnoise_per_cpu_fd);
+
+ if (cpu_dir_fd) {
+ timerlat_fd = trace_create_file("timerlat_fd", TRACE_MODE_READ,
+ cpu_dir_fd, NULL,
_fd_fops);
+ WARN_ON_ONCE(!timerlat_fd);
+ per_cpu_ptr(_per_cpu_dir, cpu)->root = cpu_dir_fd;
+ per_cpu_ptr(_per_cpu_dir, cpu)->timerlat_fd =
timerlat_fd;
+
+ /* Record the CPU */
+ d_inode(timerlat_fd)->i_cdev = (void *)(cpu);
+
+ return 0;
+ }
+
+ return -ENOMEM;
+}
+
+static void timerlat_rm_per_cpu_interface(long cpu)
+{
+ struct dentry *cpu_dir = per_cpu_ptr(_per_cpu_dir, cpu)->root;
+
+ if (cpu_dir) {
+ tracefs_remove(cpu_dir);
+ per_cpu_ptr(_per_cpu_dir, cpu)->root = NULL;
+ per_cpu_ptr(_per_cpu_dir, cpu)->timerlat_fd = NULL;
+ }
+}
+#elif defined(CONFIG_HOTPLUG_CPU)
+static int timerlat_add_per_cpu_interface(long cpu) { return 0; };
+static void timerlat_rm_per_cpu_interface(long cpu) {};
+#endif
+
#ifdef CONFIG_HOTPLUG_CPU
static void osnoise_hotplug_workfn(struct work_struct *dummy)
{
unsigned int cpu = smp_processor_id();
mutex_lock(_types_lock);
-
- if (!osnoise_has_registered_instances())
- goto out_unlock_trace;
-
mutex_lock(_lock);
cpus_read_lock();
+ timerlat_add_per_cpu_interface(cpu);
+
+ if (!osnoise_has_registered_instances())
+ goto out_unlock;
+
if (!cpumask_test_cpu(cpu, _cpumask))
goto out_unlock;
@@ -2086,7 +2147,6 @@ static void osnoise_hotplug_workfn(struct work_struct
*dummy)
out_unlock:
cpus_read_unlock();
mutex_unlock(_lock);
-out_unlock_trace:
mutex_unlock(_types_lock);
}
@@ -2106,6 +2166,7 @@ static int osnoise_cpu_init(unsigned int cpu)
*/
static int osnoise_cpu_die(unsigned int cpu)
{
+ timerlat_rm_per_cpu_interface(cpu);
stop_kthread(cpu);
return 0;
}
@@ -2708,10 +2769,7 @@ static int init_timerlat_stack_tracefs(struct dentry
*top_dir)
static int osnoise_create_cpu_timerlat_fd(struct dentry *top_dir)
{
- struct dentry *timerlat_fd;
- struct dentry *per_cpu;
- struct dentry *cpu_dir;
- char cpu_str[30]; /* see trace.c: tracing_init_tracefs_percpu() */
+ int retval;
long cpu;
/*
@@ -2720,29 +2778,24 @@ static int osnoise_create_cpu_timerlat_fd(struct dentry
*top_dir)
* Because osnoise/timerlat have a single workload, having
* multiple files like these are wast of memory.
*/
- per_cpu = tracefs_create_dir("per_cpu", top_dir);
- if (!per_cpu)
+ osnoise_per_cpu_fd = tracefs_create_dir("per_cpu", top_dir);
+ if (!osnoise_per_cpu_fd)
Re: [PATCH v2 7/7] arm64: dts: qcom: qcm6490: Add device-tree for Fairphone 5
On 9/19/23 14:46, Luca Weiss wrote: Add device tree for the Fairphone 5 smartphone which is based on the QCM6490 SoC. Supported features are, as of now: * Bluetooth * Debug UART * Display via simplefb * Flash/torch LED * Flip cover sensor * Power & volume buttons * RTC * SD card * USB * Various plumbing like regulators, i2c, spi, etc Signed-off-by: Luca Weiss --- Reviewed-by: Konrad Dybcio Tested-by: Konrad Dybcio Konrad
Re: [PATCH] remoteproc: mediatek: Refactor single core check and fix retrocompatibility
On 9/19/23 11:23, AngeloGioacchino Del Regno wrote:
> In older devicetrees we had the ChromeOS EC in a node called "cros-ec"
> instead of the newer "cros-ec-rpmsg", but this driver is now checking
> only for the latter, breaking compatibility with those.
>
> Besides, we can check if the SCP is single or dual core by simply
> walking through the children of the main SCP node and checking if
> if there's more than one "mediatek,scp-core" compatible node.
>
> Fixes: 1fdbf0cdde98 ("remoteproc: mediatek: Probe SCP cluster on multi-core
> SCP")
> Signed-off-by: AngeloGioacchino Del Regno
>
> ---
> drivers/remoteproc/mtk_scp.c | 18 +++---
> 1 file changed, 7 insertions(+), 11 deletions(-)
>
Tested on asurada (spherion) and jacuzzi (juniper). The issue was detected by
KernelCI, so:
Reported-by: "kernelci.org bot"
Tested-by: Laura Nao
Thanks!
Laura
> diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
> index ea227b566c54..a35409eda0cf 100644
> --- a/drivers/remoteproc/mtk_scp.c
> +++ b/drivers/remoteproc/mtk_scp.c
> @@ -1144,29 +1144,25 @@ static int scp_add_multi_core(struct platform_device
> *pdev,
> return ret;
> }
>
> -static int scp_is_single_core(struct platform_device *pdev)
> +static bool scp_is_single_core(struct platform_device *pdev)
> {
> struct device *dev = >dev;
> struct device_node *np = dev_of_node(dev);
> struct device_node *child;
> + int num_cores = 0;
>
> - child = of_get_next_available_child(np, NULL);
> - if (!child)
> - return dev_err_probe(dev, -ENODEV, "No child node\n");
> + for_each_child_of_node(np, child)
> + if (of_device_is_compatible(child, "mediatek,scp-core"))
> + num_cores++;
>
> - of_node_put(child);
> - return of_node_name_eq(child, "cros-ec-rpmsg");
> + return num_cores < 2;
> }
>
> static int scp_cluster_init(struct platform_device *pdev, struct
> mtk_scp_of_cluster *scp_cluster)
> {
> int ret;
>
> - ret = scp_is_single_core(pdev);
> - if (ret < 0)
> - return ret;
> -
> - if (ret)
> + if (scp_is_single_core(pdev))
> ret = scp_add_single_core(pdev, scp_cluster);
> else
> ret = scp_add_multi_core(pdev, scp_cluster);
Re: [PATCH 2/2] arm64: dts: qcom: msm8939-huawei-kiwi: Add initial device tree
On 9/16/23 15:41, Lukas Walter wrote:
This dts adds support for Huawei Honor 5X / GR5 (2016) smartphone
released in 2015.
Add device tree with initial support for:
- GPIO keys
- Hall sensor
- SDHCI (internal and external storage)
- WCNSS (BT/WIFI)
- Sensors (accelerometer, proximity and gyroscope)
- Vibrator
- Touchscreen
Signed-off-by: Lukas Walter
Signed-off-by: Raymond Hackley
---
Beyond the signoff question from Krzysztof, this looks really good.
Some comments below.
[...]
+
+ reserved-memory {
+ reserved@84a0 {
+ reg = <0x0 0x84a0 0x0 0x160>;
+ no-map;
+ };
Do we know what this is for?
+ };
+
+ gpio-hall-sensor {
+ compatible = "gpio-keys";
+
+ pinctrl-0 = <_hall_sensor_default>;
+ pinctrl-names = "default";
+
+ label = "GPIO Hall Effect Sensor";
I think we can have both hall sensor and V+ under gpio-keys
And then I am not sure how useful the label is for the container
node, maybe you or somebody else can tell me whether it's used
anywhere
+
+ event-hall-sensor {
+ label = "Hall Effect Sensor";
+ gpios = < 69 GPIO_ACTIVE_LOW>;
+ linux,input-type = ;
+ linux,code = ;
+ linux,can-disable;
Should this not be a wakeup-source btw?
+ };
+ };
+
[...]
+ /*
+* NOTE: vdd is not directly supplied by pm8916_l16, it seems
to be a
+* fixed regulator that is automatically enabled by pm8916_l16.
That sounds reasonable, many boards have such circuits
Konrad
Re: (2) [PATCH] vmscan: add trace events for lru_gen
On Wed, 20 Sep 2023 16:49:48 +0900 김재원 wrote: > Great. Thank you for your comment. > > For the putting the struct scan_control *sc inside the trace, > I couldn't do that because struct scan_control is defined in mm/vmscan.c. > I think I should not move it to a seperate header file. Well if you ever decide to do so, one thing to do is to move the trace/events/vmscan.h into mm/ as trace_vmscan.h so that it would have access to local header files. Then all you need to do is to move the struct scan_control into a local mm/X.h header file. > > As you may expect, I just made this by copying the existing > trace_mm_vmscan_lru_isolate and trace_mm_vmscan_lru_shrink_inactive > > I've tried to change like this. > Would this be good for you? The below looks fine to me. Thanks. -- Steve > > > --- a/include/trace/events/vmscan.h > +++ b/include/trace/events/vmscan.h > @@ -327,7 +327,7 @@ TRACE_EVENT(mm_vmscan_lru_isolate, > __print_symbolic(__entry->lru, LRU_NAMES)) > ); > > -TRACE_EVENT(mm_vmscan_lru_gen_scan, > +TRACE_EVENT_CONDITION(mm_vmscan_lru_gen_scan, > TP_PROTO(int highest_zoneidx, > int order, > unsigned long nr_requested, > @@ -339,6 +339,8 @@ TRACE_EVENT(mm_vmscan_lru_gen_scan, > > TP_ARGS(highest_zoneidx, order, nr_requested, nr_scanned, nr_skipped, > nr_taken, isolate_mode, lru), > > + TP_CONDITION(nr_scanned), > + > TP_STRUCT__entry( > __field(int, highest_zoneidx) > __field(int, order) > @@ -494,7 +496,6 @@ TRACE_EVENT(mm_vmscan_lru_gen_evict, > TP_ARGS(nid, nr_reclaimed, stat, priority, file), > > TP_STRUCT__entry( > - __field(int, nid) > __field(unsigned long, nr_reclaimed) > __field(unsigned long, nr_dirty) > __field(unsigned long, nr_writeback) > @@ -504,6 +505,7 @@ TRACE_EVENT(mm_vmscan_lru_gen_evict, > __field(unsigned int, nr_activate1) > __field(unsigned long, nr_ref_keep) > __field(unsigned long, nr_unmap_fail) > + __field(int, nid) > __field(int, priority) > __field(int, reclaim_flags) > ), > > --- a/mm/vmscan.c > +++ b/mm/vmscan.c > @@ -5131,10 +5131,9 @@ static int scan_folios(struct lruvec *lruvec, struct > scan_control *sc, > __count_memcg_events(memcg, PGREFILL, sorted); > __count_vm_events(PGSCAN_ANON + type, isolated); > > - if (scanned) > - trace_mm_vmscan_lru_gen_scan(sc->reclaim_idx, sc->order, > - MAX_LRU_BATCH, scanned, skipped, isolated, > - sc->may_unmap ? 0 : ISOLATE_UNMAPPED, type); > + trace_mm_vmscan_lru_gen_scan(sc->reclaim_idx, sc->order, > MAX_LRU_BATCH, > + scanned, skipped, isolated, > + sc->may_unmap ? 0 : ISOLATE_UNMAPPED, type); > > > > >
Re: [PATCH] remoteproc: mediatek: Refactor single core check and fix retrocompatibility
On Tue, Sep 19, 2023 at 11:23:36AM +0200, AngeloGioacchino Del Regno wrote:
> In older devicetrees we had the ChromeOS EC in a node called "cros-ec"
> instead of the newer "cros-ec-rpmsg", but this driver is now checking
> only for the latter, breaking compatibility with those.
>
> Besides, we can check if the SCP is single or dual core by simply
> walking through the children of the main SCP node and checking if
> if there's more than one "mediatek,scp-core" compatible node.
>
> Fixes: 1fdbf0cdde98 ("remoteproc: mediatek: Probe SCP cluster on multi-core
> SCP")
> Signed-off-by: AngeloGioacchino Del Regno
>
> ---
> drivers/remoteproc/mtk_scp.c | 18 +++---
> 1 file changed, 7 insertions(+), 11 deletions(-)
>
I find this patch to be the most appropriate to fix this problem. Laura,
Chen-Yu and Tinghan, please test the patch and reply public with your Testeb-by
tags.
Thanks,
Mathieu
> diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
> index ea227b566c54..a35409eda0cf 100644
> --- a/drivers/remoteproc/mtk_scp.c
> +++ b/drivers/remoteproc/mtk_scp.c
> @@ -1144,29 +1144,25 @@ static int scp_add_multi_core(struct platform_device
> *pdev,
> return ret;
> }
>
> -static int scp_is_single_core(struct platform_device *pdev)
> +static bool scp_is_single_core(struct platform_device *pdev)
> {
> struct device *dev = >dev;
> struct device_node *np = dev_of_node(dev);
> struct device_node *child;
> + int num_cores = 0;
>
> - child = of_get_next_available_child(np, NULL);
> - if (!child)
> - return dev_err_probe(dev, -ENODEV, "No child node\n");
> + for_each_child_of_node(np, child)
> + if (of_device_is_compatible(child, "mediatek,scp-core"))
> + num_cores++;
>
> - of_node_put(child);
> - return of_node_name_eq(child, "cros-ec-rpmsg");
> + return num_cores < 2;
> }
>
> static int scp_cluster_init(struct platform_device *pdev, struct
> mtk_scp_of_cluster *scp_cluster)
> {
> int ret;
>
> - ret = scp_is_single_core(pdev);
> - if (ret < 0)
> - return ret;
> -
> - if (ret)
> + if (scp_is_single_core(pdev))
> ret = scp_add_single_core(pdev, scp_cluster);
> else
> ret = scp_add_multi_core(pdev, scp_cluster);
> --
> 2.42.0
>
Re: [PATCH v4] scripts/link-vmlinux.sh: Add alias to duplicate symbols for kallsyms
Hi. Le mardi 19 septembre 2023, 22:39:48 EEST Alessandro Carminati (Red Hat) a écrit : > It is not uncommon for drivers or modules related to similar peripherals > to have symbols with the exact same name. > While this is not a problem for the kernel's binary itself, it becomes an > issue when attempting to trace or probe specific functions using > infrastructure like ftrace or kprobe. > > The tracing subsystem relies on the `nm -n vmlinux` output, which provides > symbol information from the kernel's ELF binary. However, when multiple > symbols share the same name, the standard nm output does not differentiate > between them. This can lead to confusion and difficulty when trying to > probe the intended symbol. > > ~ # cat /proc/kallsyms | grep " name_show" > 8c4f76d0 t name_show > 8c9cccb0 t name_show > 8cb0ac20 t name_show > 8cc728c0 t name_show > 8ce0efd0 t name_show > 8ce126c0 t name_show > 8ce1dd20 t name_show > 8ce24e70 t name_show > 8d1104c0 t name_show > 8d1fe480 t name_show > > kas_alias addresses this challenge by enhancing symbol names with > meaningful suffixes generated from the source file and line number > during the kernel build process. > These newly generated aliases provide tracers with the ability to > comprehend the symbols they are interacting with when utilizing the > ftracefs interface. > This approach may also allow for the probing by name of previously > inaccessible symbols. > > ~ # cat /proc/kallsyms | grep gic_mask_irq > d15671e505ac t gic_mask_irq > d15671e505ac t gic_mask_irq@drivers_irqchip_irq_gic_c_167 > d15671e532a4 t gic_mask_irq > d15671e532a4 t gic_mask_irq@drivers_irqchip_irq_gic_v3_c_407 > ~ # > > Changes from v1: > - Integrated changes requested by Masami to exclude symbols with prefixes > "_cfi" and "_pfx". > - Introduced a small framework to handle patterns that need to be excluded > from the alias production. > - Excluded other symbols using the framework. > - Introduced the ability to discriminate between text and data symbols. > - Added two new config symbols in this version: CONFIG_KALLSYMS_ALIAS_DATA, > which allows data for data, and CONFIG_KALLSYMS_ALIAS_DATA_ALL, which > excludes all filters and provides an alias for each duplicated symbol. > > https://lore.kernel.org/all/20230711151925.1092080-1-alessandro.carminati@gm > ail.com/ > > Changes from v2: > - Alias tags are created by querying DWARF information from the vmlinux. > - The filename + line number is normalized and appended to the original > name. > - The tag begins with '@' to indicate the symbol source. > - Not a change, but worth mentioning, since the alias is added to the > existing list, the old duplicated name is preserved, and the livepatch > way of dealing with duplicates is maintained. > - Acknowledging the existence of scenarios where inlined functions > declared in header files may result in multiple copies due to compiler > behavior, though it is not actionable as it does not pose an operational > issue. > - Highlighting a single exception where the same name refers to different > functions: the case of "compat_binfmt_elf.c," which directly includes > "binfmt_elf.c" producing identical function copies in two separate > modules. > > https://lore.kernel.org/all/20230714150326.1152359-1-alessandro.carminati@gm > ail.com/ > > Changes from v3: > - kas_alias was rewritten in Python to create a more concise and > maintainable codebase. > - The previous automation process used by kas_alias to locate the vmlinux > and the addr2line has been replaced with an explicit command-line switch > for specifying these requirements. > - addr2line has been added into the main Makefile. > - A new command-line switch has been introduced, enabling users to extend > the alias to global data names. > > https://lore.kernel.org/all/20230828080423.3539686-1-alessandro.carminati@gm > ail.com/ > > NOTE: > About the symbols name duplication that happens as consequence of the > inclusion compat_binfmt_elf.c does, it is evident that this corner is > inherently challenging the addr2line approach. > Attempting to conceal this limitation would be counterproductive. > > compat_binfmt_elf.c includes directly binfmt_elf.c, addr2line can't help > but report all functions and data declared by that file, coming from > binfmt_elf.c. > > My position is that, rather than producing a more complicated pipeline > to handle this corner case, it is better to fix the compat_binfmt_elf.c > anomaly. > > This patch does not deal with the two potentially problematic symbols > defined by compat_binfmt_elf.c First, thank you for the v4, you will find in the remaining of the messages some comments but for now, I did not test it (this is planned). On a general way, using python really helps here as the code is more straightforward, thank you for this change. Regarding the problem
Re: [PATCH] livepatch: Fix missing newline character in klp_resolve_symbols()
On Thu 2023-09-14 15:26:44, Zheng Yejian wrote:
> Without the newline character, the log may not be printed immediately
> after the error occurs.
>
> Fixes: ca376a937486 ("livepatch: Prevent module-specific KLP rela sections
> from referencing vmlinux symbols")
> Signed-off-by: Zheng Yejian
JFYI, the patch has been committed into livepatching.git, branch for-6.7.
Best Regards,
Petr
Re: [PATCH] livepatch: Fix missing newline character in klp_resolve_symbols()
On Thu 2023-09-14 15:26:44, Zheng Yejian wrote:
> Without the newline character, the log may not be printed immediately
> after the error occurs.
>
> Fixes: ca376a937486 ("livepatch: Prevent module-specific KLP rela sections
> from referencing vmlinux symbols")
> Signed-off-by: Zheng Yejian
Reviewed-by: Petr Mladek
Best Regards,
Petr
Re: [PATCH v17 00/14] Add support for MT8195 SCP 2nd core
On Tue, 2023-09-19 at 13:07 +0800, Chen-Yu Tsai wrote: > On Tue, Sep 19, 2023 at 9:17 AM Mathieu Poirier > wrote: > > > > On Mon, Sep 18, 2023 at 06:44:25PM +0800, Chen-Yu Tsai wrote: > > > On Mon, Sep 18, 2023 at 6:32 PM Laura Nao wrote: > > > > > > > > > Other than patch 2 and 14, I have applied this set. The remaining > > > > > patches will > > > > > have to be resent to Matthias. > > > > > Thanks, > > > > > Mathieu > > > > > > > > Hello, > > > > > > > > With patch 2 missing, the SCP is not probed correctly anymore on > > > > asurada (MT8192) and kukui (MT8183). The mtk-scp driver relies on the > > > > existence of the `cros-ec-rpmsg` node in the dt to determine if the SCP > > > > is single or multicore. Without patch 2 the driver wrongly assumes the > > > > SCP on MT8192 and MT8183 are multicore, leading to the following errors > > > > during initialization: > > > > > > > > 10696 04:33:59.126671 <3>[ 15.465714] platform 1050.scp:cros-ec: > > > > invalid resource (null) > > > > 10697 04:33:59.142855 <3>[ 15.478560] platform 1050.scp:cros-ec: > > > > Failed to parse and map sram memory > > > > 10698 04:33:59.149650 <3>[ 15.486121] mtk-scp 1050.scp: Failed > > > > to initialize core 0 rproc > > > > > > > > The issue was caught by KernelCI, complete logs can be found here: > > > > - asurada: > > > > https://urldefense.com/v3/__https://storage.kernelci.org/next/master/next-20230914/arm64/defconfig*arm64-chromebook*videodec/gcc-10/lab-collabora/baseline-nfs-mt8192-asurada-spherion-r0.html__;Kys!!CTRNKA9wMg0ARbw!iEMruK4bVWfCmhRmyauJtkTioHdQbYP3DwhnGUZNxbKYhMzusmoIjYOnpVNALoMobUdKhooUYw6OxUmrqNE$ > > > > > > > > - kukui: > > > > https://urldefense.com/v3/__https://storage.kernelci.org/next/master/next-20230914/arm64/defconfig*arm64-chromebook*videodec/gcc-10/lab-collabora/baseline-nfs-mt8183-kukui-jacuzzi-juniper-sku16.html__;Kys!!CTRNKA9wMg0ARbw!iEMruK4bVWfCmhRmyauJtkTioHdQbYP3DwhnGUZNxbKYhMzusmoIjYOnpVNALoMobUdKhooUYw6OhRxz6NQ$ > > > > > > > > > > > > Reporting the issue so that patch 2 and 14 can be resent and merged > > > > soon. > > > > > > This being a backward incompatible DT binding change, maybe we should > > > revert > > > the node name change. Or, the driver could simply count the number of > > > child > > > nodes that have the "mediatek,rpmsg-name" property, which is required. > > > > > > > You have a point. Can someone send a patch that makes this patchset > > backward > > compatible? Please do so as quickly as possible to that it can go in the > > next > > merge window with the rest of this feature. Otherwize I'll have to back > > out the > > whole thing. > > I sent out a patch [1] implementing my proposed change. > > ChenYu > > [1] > https://lore.kernel.org/linux-remoteproc/20230919050305.3817347-1-we...@chromium.org/ My version[1] is inspired by Angelo's work[2]. [1] https://lore.kernel.org/all/20230920084611.30890-1-tinghan.s...@mediatek.com/ [2] https://lore.kernel.org/all/20230919092336.51007-1-angelogioacchino.delre...@collabora.com/ Best regards, TingHan
[PATCH] remoteproc: mediatek: Fix compatibility of probing SCP cores
The SCP dt-binding introduces a new SCP node format to describe
the multi-core SCP HW. However, the old format for single-core
SCP is co-existed with the new format. It suggests that
there are two different approaches, using either the old style or
the new format, to describe the single-core SCP HW in devicetree.
the driver checks the immediate child node name in the SCP node to
determin the format that is being utilized. The node name is expected
"scp" for new format and "cros-ec-rpmsg" for the old format. However,
the expected node name for old format didn't defined in earlier
SCP dt-binding and the old node name is "cros_ec" in old devicetree.
So, this checking breaks the compatibility with old SCP devicetree.
In order to distinguish between the various forms of SCP devicetree and
maintain compatibility with the previous SCP devicetree, this fix
verifies the existence of the compatible name "mediatek,scp-core"
introduced in the new format.
Reported-by: Laura Nao
Fixes: 1fdbf0cdde98 ("remoteproc: mediatek: Probe SCP cluster on multi-core
SCP")
Signed-off-by: Tinghan Shen
---
drivers/remoteproc/mtk_scp.c | 22 +++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
index ea227b566c54..76472e50c8f5 100644
--- a/drivers/remoteproc/mtk_scp.c
+++ b/drivers/remoteproc/mtk_scp.c
@@ -1144,29 +1144,29 @@ static int scp_add_multi_core(struct platform_device
*pdev,
return ret;
}
-static int scp_is_single_core(struct platform_device *pdev)
+static bool scp_find_core_node(struct platform_device *pdev)
{
struct device *dev = >dev;
struct device_node *np = dev_of_node(dev);
struct device_node *child;
+ int found = 0;
- child = of_get_next_available_child(np, NULL);
- if (!child)
- return dev_err_probe(dev, -ENODEV, "No child node\n");
+ for_each_child_of_node(np, child) {
+ if (of_device_is_compatible(child, "mediatek,scp-core")) {
+ found = 1;
+ of_node_put(child);
+ break;
+ }
+ }
- of_node_put(child);
- return of_node_name_eq(child, "cros-ec-rpmsg");
+ return found;
}
static int scp_cluster_init(struct platform_device *pdev, struct
mtk_scp_of_cluster *scp_cluster)
{
int ret;
- ret = scp_is_single_core(pdev);
- if (ret < 0)
- return ret;
-
- if (ret)
+ if (!scp_find_core_node(pdev))
ret = scp_add_single_core(pdev, scp_cluster);
else
ret = scp_add_multi_core(pdev, scp_cluster);
--
2.18.0
RE:(2) [PATCH] vmscan: add trace events for lru_gen
>On Tue, 19 Sep 2023 11:52:16 +0900
>Jaewon Kim wrote:
>
>> /*
>> * Now redefine the EM() and EMe() macros to map the enums to the strings
>> diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h
>> index d2123dd960d5..e8f9d0452e89 100644
>> --- a/include/trace/events/vmscan.h
>> +++ b/include/trace/events/vmscan.h
>> @@ -327,6 +327,55 @@ TRACE_EVENT(mm_vmscan_lru_isolate,
>> __print_symbolic(__entry->lru, LRU_NAMES))
>> );
>>
>> +TRACE_EVENT(mm_vmscan_lru_gen_scan,
>> +TP_PROTO(int highest_zoneidx,
>> +int order,
>> +unsigned long nr_requested,
>> +unsigned long nr_scanned,
>> +unsigned long nr_skipped,
>> +unsigned long nr_taken,
>> +isolate_mode_t isolate_mode,
>> +int lru),
>
>This is a lot of parameter passing, can you consolidate it?
>
>(see below to where you call this)
>
>> +
>> +TP_ARGS(highest_zoneidx, order, nr_requested, nr_scanned, nr_skipped,
>> nr_taken, isolate_mode, lru),
>> +
>> +TP_STRUCT__entry(
>> +__field(int, highest_zoneidx)
>> +__field(int, order)
>> +__field(unsigned long, nr_requested)
>> +__field(unsigned long, nr_scanned)
>> +__field(unsigned long, nr_skipped)
>> +__field(unsigned long, nr_taken)
>> +__field(unsigned int, isolate_mode)
>> +__field(int, lru)
>> +),
>> +
>> +TP_fast_assign(
>> +__entry->highest_zoneidx = highest_zoneidx;
>> +__entry->order = order;
>> +__entry->nr_requested = nr_requested;
>> +__entry->nr_scanned = nr_scanned;
>> +__entry->nr_skipped = nr_skipped;
>> +__entry->nr_taken = nr_taken;
>> +__entry->isolate_mode = (__force unsigned int)isolate_mode;
>> +__entry->lru = lru;
>> +),
>> +
>> +/*
>> + * classzone is previous name of the highest_zoneidx.
>> + * Reason not to change it is the ABI requirement of the tracepoint.
>> + */
>> +TP_printk("isolate_mode=%d classzone=%d order=%d nr_requested=%lu
>> nr_scanned=%lu nr_skipped=%lu nr_taken=%lu lru=%s",
>> +__entry->isolate_mode,
>> +__entry->highest_zoneidx,
>> +__entry->order,
>> +__entry->nr_requested,
>> +__entry->nr_scanned,
>> +__entry->nr_skipped,
>> +__entry->nr_taken,
>> +__print_symbolic(__entry->lru, LRU_GEN_NAMES))
>> +);
>> +
>> TRACE_EVENT(mm_vmscan_write_folio,
>>
>> TP_PROTO(struct folio *folio),
>> @@ -437,6 +486,53 @@ TRACE_EVENT(mm_vmscan_lru_shrink_active,
>> show_reclaim_flags(__entry->reclaim_flags))
>> );
>>
>> +TRACE_EVENT(mm_vmscan_lru_gen_evict,
>> +
>> +TP_PROTO(int nid, unsigned long nr_reclaimed,
>> +struct reclaim_stat *stat, int priority, int file),
>> +
>> +TP_ARGS(nid, nr_reclaimed, stat, priority, file),
>> +
>> +TP_STRUCT__entry(
>> +__field(int, nid)
>
>On 64 bit architectures, this causes a 4 byte hole in the ring buffer
>layout. Please keep 32 bit size fields paired with other 32 bit size if
>possible. That is, move the above "int nid" down where it doesn't cause a
>long field to be 4 bytes away.
>
>> +__field(unsigned long, nr_reclaimed)
>> +__field(unsigned long, nr_dirty)
>> +__field(unsigned long, nr_writeback)
>> +__field(unsigned long, nr_congested)
>> +__field(unsigned long, nr_immediate)
>> +__field(unsigned int, nr_activate0)
>> +__field(unsigned int, nr_activate1)
>> +__field(unsigned long, nr_ref_keep)
>> +__field(unsigned long, nr_unmap_fail)
>
> __field(int, nid)
>
>here!
>
>> +__field(int, priority)
>> +__field(int, reclaim_flags)
>> +),
>> +
>> +TP_fast_assign(
>> +__entry->nid = nid;
>> +__entry->nr_reclaimed = nr_reclaimed;
>> +__entry->nr_dirty = stat->nr_dirty;
>> +__entry->nr_writeback = stat->nr_writeback;
>> +__entry->nr_congested = stat->nr_congested;
>> +__entry->nr_immediate = stat->nr_immediate;
>> +__entry->nr_activate0 = stat->nr_activate[0];
>> +__entry->nr_activate1 = stat->nr_activate[1];
>> +__entry->nr_ref_keep = stat->nr_ref_keep;
>> +__entry->nr_unmap_fail = stat->nr_unmap_fail;
>> +__entry->priority = priority;
>> +__entry->reclaim_flags = trace_reclaim_flags(file);
>> +),
>> +
>> +TP_printk("nid=%d nr_reclaimed=%ld nr_dirty=%ld nr_writeback=%ld
>> nr_congested=%ld nr_immediate=%ld nr_activate_anon=%d nr_activate_file=%d
>> nr_ref_keep=%ld nr_unmap_fail=%ld priority=%d flags=%s",
>> +__entry->nid, __entry->nr_reclaimed,
>> +__entry->nr_dirty, __entry->nr_writeback,
>> +
Re: [PATCH v4] scripts/link-vmlinux.sh: Add alias to duplicate symbols for kallsyms
Hello Masami, Thank you for the feedback. Il giorno mer 20 set 2023 alle ore 01:52 Masami Hiramatsu ha scritto: > > On Tue, 19 Sep 2023 19:39:48 + > "Alessandro Carminati (Red Hat)" wrote: > > > It is not uncommon for drivers or modules related to similar peripherals > > to have symbols with the exact same name. > > While this is not a problem for the kernel's binary itself, it becomes an > > issue when attempting to trace or probe specific functions using > > infrastructure like ftrace or kprobe. > > > > The tracing subsystem relies on the `nm -n vmlinux` output, which provides > > symbol information from the kernel's ELF binary. However, when multiple > > symbols share the same name, the standard nm output does not differentiate > > between them. This can lead to confusion and difficulty when trying to > > probe the intended symbol. > > > > ~ # cat /proc/kallsyms | grep " name_show" > > 8c4f76d0 t name_show > > 8c9cccb0 t name_show > > 8cb0ac20 t name_show > > 8cc728c0 t name_show > > 8ce0efd0 t name_show > > 8ce126c0 t name_show > > 8ce1dd20 t name_show > > 8ce24e70 t name_show > > 8d1104c0 t name_show > > 8d1fe480 t name_show > > > > kas_alias addresses this challenge by enhancing symbol names with > > meaningful suffixes generated from the source file and line number > > during the kernel build process. > > These newly generated aliases provide tracers with the ability to > > comprehend the symbols they are interacting with when utilizing the > > ftracefs interface. > > This approach may also allow for the probing by name of previously > > inaccessible symbols. > > > > ~ # cat /proc/kallsyms | grep gic_mask_irq > > d15671e505ac t gic_mask_irq > > d15671e505ac t gic_mask_irq@drivers_irqchip_irq_gic_c_167 > > d15671e532a4 t gic_mask_irq > > d15671e532a4 t gic_mask_irq@drivers_irqchip_irq_gic_v3_c_407 > > ~ # > > > > Changes from v1: > > - Integrated changes requested by Masami to exclude symbols with prefixes > > "_cfi" and "_pfx". > > - Introduced a small framework to handle patterns that need to be excluded > > from the alias production. > > - Excluded other symbols using the framework. > > - Introduced the ability to discriminate between text and data symbols. > > - Added two new config symbols in this version: CONFIG_KALLSYMS_ALIAS_DATA, > > which allows data for data, and CONFIG_KALLSYMS_ALIAS_DATA_ALL, which > > excludes all filters and provides an alias for each duplicated symbol. > > > > https://lore.kernel.org/all/20230711151925.1092080-1-alessandro.carmin...@gmail.com/ > > > > Changes from v2: > > - Alias tags are created by querying DWARF information from the vmlinux. > > - The filename + line number is normalized and appended to the original > > name. > > - The tag begins with '@' to indicate the symbol source. > > - Not a change, but worth mentioning, since the alias is added to the > > existing list, the old duplicated name is preserved, and the livepatch > > way of dealing with duplicates is maintained. > > - Acknowledging the existence of scenarios where inlined functions > > declared in header files may result in multiple copies due to compiler > > behavior, though it is not actionable as it does not pose an operational > > issue. > > - Highlighting a single exception where the same name refers to different > > functions: the case of "compat_binfmt_elf.c," which directly includes > > "binfmt_elf.c" producing identical function copies in two separate > > modules. > > > > https://lore.kernel.org/all/20230714150326.1152359-1-alessandro.carmin...@gmail.com/ > > > > Changes from v3: > > - kas_alias was rewritten in Python to create a more concise and > > maintainable codebase. > > - The previous automation process used by kas_alias to locate the vmlinux > > and the addr2line has been replaced with an explicit command-line switch > > for specifying these requirements. > > - addr2line has been added into the main Makefile. > > - A new command-line switch has been introduced, enabling users to extend > > the alias to global data names. > > > > https://lore.kernel.org/all/20230828080423.3539686-1-alessandro.carmin...@gmail.com/ > > > > NOTE: > > About the symbols name duplication that happens as consequence of the > > inclusion compat_binfmt_elf.c does, it is evident that this corner is > > inherently challenging the addr2line approach. > > Attempting to conceal this limitation would be counterproductive. > > > > compat_binfmt_elf.c includes directly binfmt_elf.c, addr2line can't help > > but report all functions and data declared by that file, coming from > > binfmt_elf.c. > > > > My position is that, rather than producing a more complicated pipeline > > to handle this corner case, it is better to fix the compat_binfmt_elf.c > > anomaly. > > > > This patch does not deal with the two potentially problematic symbols > > defined by
