Re: [PATCH -next] fbdev: offb: allow build when DRM_OFDRM=m
Am 23.11.22 um 04:16 schrieb Randy Dunlap: Fix build when CONFIG_FB_OF=y and CONFIG_DRM_OFDRM=m. When the latter symbol is =m, kconfig downgrades (limits) the 'select's under FB_OF to modular (=m). This causes undefined symbol references: powerpc64-linux-ld: drivers/video/fbdev/offb.o:(.data.rel.ro+0x58): undefined reference to `cfb_fillrect' powerpc64-linux-ld: drivers/video/fbdev/offb.o:(.data.rel.ro+0x60): undefined reference to `cfb_copyarea' powerpc64-linux-ld: drivers/video/fbdev/offb.o:(.data.rel.ro+0x68): undefined reference to `cfb_imageblit' Fix this by allowing FB_OF any time that DRM_OFDRM != y so that the selected FB_CFB_* symbols will become =y instead of =m. In tristate logic (for DRM_OFDRM), this changes the dependency from !DRM_OFDRM == 2 - 1 == 1 => modular only (or disabled) to (boolean) DRM_OFDRM != y == y, allowing the 'select's to cause the FB_CFB_* symbols to =y instead of =m. Fixes: c8a17756c425 ("drm/ofdrm: Add ofdrm for Open Firmware framebuffers") Signed-off-by: Randy Dunlap Suggested-by: Masahiro Yamada Cc: Thomas Zimmermann Cc: Michal Suchánek Cc: linuxppc-dev@lists.ozlabs.org Cc: Daniel Vetter Cc: Helge Deller Cc: linux-fb...@vger.kernel.org Cc: dri-de...@lists.freedesktop.org Acked-by: Thomas Zimmermann --- drivers/video/fbdev/Kconfig |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff -- a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig --- a/drivers/video/fbdev/Kconfig +++ b/drivers/video/fbdev/Kconfig @@ -455,7 +455,7 @@ config FB_ATARI config FB_OF bool "Open Firmware frame buffer device support" depends on (FB = y) && PPC && (!PPC_PSERIES || PCI) - depends on !DRM_OFDRM + depends on DRM_OFDRM != y select APERTURE_HELPERS select FB_CFB_FILLRECT select FB_CFB_COPYAREA -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH] perf test: Skip watchpoint tests if no watchpoints available
On 11/21/22 15:57, Naveen N. Rao wrote: > On IBM Power9, perf watchpoint tests fail since no hardware breakpoints > are available. Detect this by checking the error returned by > perf_event_open() and skip the tests in that case. > > Reported-by: Disha Goel > Signed-off-by: Naveen N. Rao > --- Patch looks fine to me. Also tested it in power9 box Test result without this patch changes : 23: Watchpoint : 23.1: Read Only Watchpoint: FAILED! 23.2: Write Only Watchpoint : FAILED! 23.3: Read / Write Watchpoint : FAILED! 23.4: Modify Watchpoint : FAILED! Test result with patch changes: 23: Watchpoint : 23.1: Read Only Watchpoint: Skip (missing hardware support) 23.2: Write Only Watchpoint : Skip (missing hardware support) 23.3: Read / Write Watchpoint : Skip (missing hardware support) 23.4: Modify Watchpoint : Skip (missing hardware support) Reviewed-by: Kajol Jain Tested-by: Kajol Jain Thanks, Kajol Jain > tools/perf/tests/wp.c | 12 +++- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/tools/perf/tests/wp.c b/tools/perf/tests/wp.c > index 56455da30341b4..cc8719609b19ea 100644 > --- a/tools/perf/tests/wp.c > +++ b/tools/perf/tests/wp.c > @@ -59,8 +59,10 @@ static int __event(int wp_type, void *wp_addr, unsigned > long wp_len) > get__perf_event_attr(&attr, wp_type, wp_addr, wp_len); > fd = sys_perf_event_open(&attr, 0, -1, -1, >perf_event_open_cloexec_flag()); > - if (fd < 0) > + if (fd < 0) { > + fd = -errno; > pr_debug("failed opening event %x\n", attr.bp_type); > + } > > return fd; > } > @@ -77,7 +79,7 @@ static int test__wp_ro(struct test_suite *test > __maybe_unused, > > fd = __event(HW_BREAKPOINT_R, (void *)&data1, sizeof(data1)); > if (fd < 0) > - return -1; > + return fd == -ENODEV ? TEST_SKIP : -1; > > tmp = data1; > WP_TEST_ASSERT_VAL(fd, "RO watchpoint", 1); > @@ -101,7 +103,7 @@ static int test__wp_wo(struct test_suite *test > __maybe_unused, > > fd = __event(HW_BREAKPOINT_W, (void *)&data1, sizeof(data1)); > if (fd < 0) > - return -1; > + return fd == -ENODEV ? TEST_SKIP : -1; > > tmp = data1; > WP_TEST_ASSERT_VAL(fd, "WO watchpoint", 0); > @@ -126,7 +128,7 @@ static int test__wp_rw(struct test_suite *test > __maybe_unused, > fd = __event(HW_BREAKPOINT_R | HW_BREAKPOINT_W, (void *)&data1, >sizeof(data1)); > if (fd < 0) > - return -1; > + return fd == -ENODEV ? TEST_SKIP : -1; > > tmp = data1; > WP_TEST_ASSERT_VAL(fd, "RW watchpoint", 1); > @@ -150,7 +152,7 @@ static int test__wp_modify(struct test_suite *test > __maybe_unused, int subtest _ > > fd = __event(HW_BREAKPOINT_W, (void *)&data1, sizeof(data1)); > if (fd < 0) > - return -1; > + return fd == -ENODEV ? TEST_SKIP : -1; > > data1 = tmp; > WP_TEST_ASSERT_VAL(fd, "Modify watchpoint", 1); > > base-commit: 63a3bf5e8d9e79ce456c8f73d4395a5a51d841b1
Re: [PATCH -next] fbdev: offb: allow build when DRM_OFDRM=m
On Wed, Nov 23, 2022 at 09:02:54AM +0100, Thomas Zimmermann wrote: > > Am 23.11.22 um 04:16 schrieb Randy Dunlap: > > Fix build when CONFIG_FB_OF=y and CONFIG_DRM_OFDRM=m. > > When the latter symbol is =m, kconfig downgrades (limits) the 'select's > > under FB_OF to modular (=m). This causes undefined symbol references: > > > > powerpc64-linux-ld: drivers/video/fbdev/offb.o:(.data.rel.ro+0x58): > > undefined reference to `cfb_fillrect' > > powerpc64-linux-ld: drivers/video/fbdev/offb.o:(.data.rel.ro+0x60): > > undefined reference to `cfb_copyarea' > > powerpc64-linux-ld: drivers/video/fbdev/offb.o:(.data.rel.ro+0x68): > > undefined reference to `cfb_imageblit' > > > > Fix this by allowing FB_OF any time that DRM_OFDRM != y so that the > > selected FB_CFB_* symbols will become =y instead of =m. > > > > In tristate logic (for DRM_OFDRM), this changes the dependency from > > !DRM_OFDRM == 2 - 1 == 1 => modular only (or disabled) > > to (boolean) > > DRM_OFDRM != y == y, allowing the 'select's to cause the > > FB_CFB_* symbols to =y instead of =m. > > > > Fixes: c8a17756c425 ("drm/ofdrm: Add ofdrm for Open Firmware framebuffers") > > Signed-off-by: Randy Dunlap > > Suggested-by: Masahiro Yamada > > Cc: Thomas Zimmermann > > Cc: Michal Suchánek > > Cc: linuxppc-dev@lists.ozlabs.org > > Cc: Daniel Vetter > > Cc: Helge Deller > > Cc: linux-fb...@vger.kernel.org > > Cc: dri-de...@lists.freedesktop.org > > Acked-by: Thomas Zimmermann Tested-by: Michal Suchánek > > > --- > > drivers/video/fbdev/Kconfig |2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff -- a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig > > --- a/drivers/video/fbdev/Kconfig > > +++ b/drivers/video/fbdev/Kconfig > > @@ -455,7 +455,7 @@ config FB_ATARI > > config FB_OF > > bool "Open Firmware frame buffer device support" > > depends on (FB = y) && PPC && (!PPC_PSERIES || PCI) > > - depends on !DRM_OFDRM > > + depends on DRM_OFDRM != y > > select APERTURE_HELPERS > > select FB_CFB_FILLRECT > > select FB_CFB_COPYAREA > > -- > Thomas Zimmermann > Graphics Driver Developer > SUSE Software Solutions Germany GmbH > Maxfeldstr. 5, 90409 Nürnberg, Germany > (HRB 36809, AG Nürnberg) > Geschäftsführer: Ivo Totev
Re: [PATCH -next] fbdev: offb: allow build when DRM_OFDRM=m
On Wed, Nov 23, 2022, at 04:16, Randy Dunlap wrote: > Fix build when CONFIG_FB_OF=y and CONFIG_DRM_OFDRM=m. > When the latter symbol is =m, kconfig downgrades (limits) the 'select's > under FB_OF to modular (=m). This causes undefined symbol references: > > powerpc64-linux-ld: drivers/video/fbdev/offb.o:(.data.rel.ro+0x58): > undefined reference to `cfb_fillrect' > powerpc64-linux-ld: drivers/video/fbdev/offb.o:(.data.rel.ro+0x60): > undefined reference to `cfb_copyarea' > powerpc64-linux-ld: drivers/video/fbdev/offb.o:(.data.rel.ro+0x68): > undefined reference to `cfb_imageblit' > > Fix this by allowing FB_OF any time that DRM_OFDRM != y so that the > selected FB_CFB_* symbols will become =y instead of =m. > > In tristate logic (for DRM_OFDRM), this changes the dependency from > !DRM_OFDRM== 2 - 1 == 1 => modular only (or disabled) > to (boolean) > DRM_OFDRM != y == y, allowing the 'select's to cause the > FB_CFB_* symbols to =y instead of =m. > Is it actually a useful configuration to have OFDRM=m and FB_OF=y though? I would expect in that case that the OFDRM driver never binds to a device because it's already owned by FB_OF. > diff -- a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig > --- a/drivers/video/fbdev/Kconfig > +++ b/drivers/video/fbdev/Kconfig > @@ -455,7 +455,7 @@ config FB_ATARI > config FB_OF > bool "Open Firmware frame buffer device support" > depends on (FB = y) && PPC && (!PPC_PSERIES || PCI) > - depends on !DRM_OFDRM > + depends on DRM_OFDRM != y > select APERTURE_HELPERS I would instead make this 'depends on DRM_OFDRM=n', which completely eliminates configs that have both driver enabled. A nicer change would be to make FB_OF a tristate symbol, which makes it possible to load one of the two modules if both are enabled =m, while only allowing one of them to be =y if the other is completely disabled. It looks like offb was originally written to be usable as a loadable module, but Kconfig has prevented this since at least the start of the git history. Arnd
[PATCH 5.4 027/156] perf stat: Fix printing os->prefix in CSV metrics output
From: Athira Rajeev [ Upstream commit ad353b710c7493df3d4fc2d3a51819126bed2e81 ] 'perf stat' with CSV output option prints an extra empty string as first field in metrics output line. Sample output below: # ./perf stat -x, --per-socket -a -C 1 ls S0,1,1.78,msec,cpu-clock,1785146,100.00,0.973,CPUs utilized S0,1,26,,context-switches,1781750,100.00,0.015,M/sec S0,1,1,,cpu-migrations,1780526,100.00,0.561,K/sec S0,1,1,,page-faults,1779060,100.00,0.561,K/sec S0,1,875807,,cycles,1769826,100.00,0.491,GHz S0,1,85281,,stalled-cycles-frontend,1767512,100.00,9.74,frontend cycles idle S0,1,576839,,stalled-cycles-backend,1766260,100.00,65.86,backend cycles idle S0,1,288430,,instructions,1762246,100.00,0.33,insn per cycle > ,S0,1,,,2.00,stalled cycles per insn The above command line uses field separator as "," via "-x," option and per-socket option displays socket value as first field. But here the last line for "stalled cycles per insn" has "," in the beginning. Sample output using interval mode: # ./perf stat -I 1000 -x, --per-socket -a -C 1 ls 0.001813453,S0,1,1.87,msec,cpu-clock,1872052,100.00,0.002,CPUs utilized 0.001813453,S0,1,2,,context-switches,1868028,100.00,1.070,K/sec -- 0.001813453,S0,1,85379,,instructions,1856754,100.00,0.32,insn per cycle > 0.001813453,,S0,1,,,1.34,stalled cycles per insn Above result also has an extra CSV separator after the timestamp. Patch addresses extra field separator in the beginning of the metric output line. The counter stats are displayed by function "perf_stat__print_shadow_stats" in code "util/stat-shadow.c". While printing the stats info for "stalled cycles per insn", function "new_line_csv" is used as new_line callback. The new_line_csv function has check for "os->prefix" and if prefix is not null, it will be printed along with cvs separator. Snippet from "new_line_csv": if (os->prefix) fprintf(os->fh, "%s%s", os->prefix, config->csv_sep); Here os->prefix gets printed followed by "," which is the cvs separator. The os->prefix is used in interval mode option ( -I ), to print time stamp on every new line. But prefix is already set to contain CSV separator when used in interval mode for CSV option. Reference: Function "static void print_interval" Snippet: sprintf(prefix, "%6lu.%09lu%s", ts->tv_sec, ts->tv_nsec, config->csv_sep); Also if prefix is not assigned (if not used with -I option), it gets set to empty string. Reference: function printout() in util/stat-display.c Snippet: .prefix = prefix ? prefix : "", Since prefix already set to contain cvs_sep in interval option, patch removes printing config->csv_sep in new_line_csv function to avoid printing extra field. After the patch: # ./perf stat -x, --per-socket -a -C 1 ls S0,1,2.04,msec,cpu-clock,2045202,100.00,1.013,CPUs utilized S0,1,2,,context-switches,2041444,100.00,979.289,/sec S0,1,0,,cpu-migrations,2040820,100.00,0.000,/sec S0,1,2,,page-faults,2040288,100.00,979.289,/sec S0,1,254589,,cycles,2036066,100.00,0.125,GHz S0,1,82481,,stalled-cycles-frontend,2032420,100.00,32.40,frontend cycles idle S0,1,113170,,stalled-cycles-backend,2031722,100.00,44.45,backend cycles idle S0,1,88766,,instructions,2030942,100.00,0.35,insn per cycle S0,1,,,1.27,stalled cycles per insn Fixes: 92a61f6412d3a09d ("perf stat: Implement CSV metrics output") Reported-by: Disha Goel Reviewed-By: Kajol Jain Signed-off-by: Athira Jajeev Tested-by: Disha Goel Cc: Andi Kleen Cc: Ian Rogers Cc: James Clark Cc: Jiri Olsa Cc: linuxppc-dev@lists.ozlabs.org Cc: Madhavan Srinivasan Cc: Michael Ellerman Cc: Nageswara R Sastry Cc: Namhyung Kim Link: https://lore.kernel.org/r/20221018085605.63834-1-atraj...@linux.vnet.ibm.com Signed-off-by: Arnaldo Carvalho de Melo Signed-off-by: Sasha Levin --- tools/perf/util/stat-display.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c index 93147cc40162..e18c26501a7f 100644 --- a/tools/perf/util/stat-display.c +++ b/tools/perf/util/stat-display.c @@ -193,7 +193,7 @@ static void new_line_csv(struct perf_stat_config *config, void *ctx) fputc('\n', os->fh); if (os->prefix) - fprintf(os->fh, "%s%s", os->prefix, config->csv_sep); + fprintf(os->fh, "%s", os->prefix); aggr_printout(config, os->evsel, os->id, os->nr); for (i = 0; i < os->nfields; i++) fputs(config->csv_sep, os->fh); -- 2.35.1
Re: [PATCH -next] fbdev: offb: allow build when DRM_OFDRM=m
Hi Arnd, On 11/23/22 01:08, Arnd Bergmann wrote: > On Wed, Nov 23, 2022, at 04:16, Randy Dunlap wrote: >> Fix build when CONFIG_FB_OF=y and CONFIG_DRM_OFDRM=m. >> When the latter symbol is =m, kconfig downgrades (limits) the 'select's >> under FB_OF to modular (=m). This causes undefined symbol references: >> >> powerpc64-linux-ld: drivers/video/fbdev/offb.o:(.data.rel.ro+0x58): >> undefined reference to `cfb_fillrect' >> powerpc64-linux-ld: drivers/video/fbdev/offb.o:(.data.rel.ro+0x60): >> undefined reference to `cfb_copyarea' >> powerpc64-linux-ld: drivers/video/fbdev/offb.o:(.data.rel.ro+0x68): >> undefined reference to `cfb_imageblit' >> >> Fix this by allowing FB_OF any time that DRM_OFDRM != y so that the >> selected FB_CFB_* symbols will become =y instead of =m. >> >> In tristate logic (for DRM_OFDRM), this changes the dependency from >> !DRM_OFDRM == 2 - 1 == 1 => modular only (or disabled) >> to (boolean) >> DRM_OFDRM != y == y, allowing the 'select's to cause the >> FB_CFB_* symbols to =y instead of =m. >> > > Is it actually a useful configuration to have OFDRM=m and > FB_OF=y though? I would expect in that case that the OFDRM > driver never binds to a device because it's already owned > by FB_OF. > >> diff -- a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig >> --- a/drivers/video/fbdev/Kconfig >> +++ b/drivers/video/fbdev/Kconfig >> @@ -455,7 +455,7 @@ config FB_ATARI >> config FB_OF >> bool "Open Firmware frame buffer device support" >> depends on (FB = y) && PPC && (!PPC_PSERIES || PCI) >> -depends on !DRM_OFDRM >> +depends on DRM_OFDRM != y >> select APERTURE_HELPERS > > I would instead make this 'depends on DRM_OFDRM=n', which > completely eliminates configs that have both driver enabled. Yep, that works for me. Thanks. Thomas, Michal, are you OK with that change? > A nicer change would be to make FB_OF a tristate symbol, > which makes it possible to load one of the two modules if > both are enabled =m, while only allowing one of them to > be =y if the other is completely disabled. It looks like > offb was originally written to be usable as a loadable module, > but Kconfig has prevented this since at least the start of > the git history. -- ~Randy
Re: [PATCH -next] fbdev: offb: allow build when DRM_OFDRM=m
Hi Am 23.11.22 um 11:02 schrieb Randy Dunlap: Hi Arnd, On 11/23/22 01:08, Arnd Bergmann wrote: On Wed, Nov 23, 2022, at 04:16, Randy Dunlap wrote: Fix build when CONFIG_FB_OF=y and CONFIG_DRM_OFDRM=m. When the latter symbol is =m, kconfig downgrades (limits) the 'select's under FB_OF to modular (=m). This causes undefined symbol references: powerpc64-linux-ld: drivers/video/fbdev/offb.o:(.data.rel.ro+0x58): undefined reference to `cfb_fillrect' powerpc64-linux-ld: drivers/video/fbdev/offb.o:(.data.rel.ro+0x60): undefined reference to `cfb_copyarea' powerpc64-linux-ld: drivers/video/fbdev/offb.o:(.data.rel.ro+0x68): undefined reference to `cfb_imageblit' Fix this by allowing FB_OF any time that DRM_OFDRM != y so that the selected FB_CFB_* symbols will become =y instead of =m. In tristate logic (for DRM_OFDRM), this changes the dependency from !DRM_OFDRM == 2 - 1 == 1 => modular only (or disabled) to (boolean) DRM_OFDRM != y == y, allowing the 'select's to cause the FB_CFB_* symbols to =y instead of =m. Is it actually a useful configuration to have OFDRM=m and FB_OF=y though? I would expect in that case that the OFDRM driver never binds to a device because it's already owned by FB_OF. diff -- a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig --- a/drivers/video/fbdev/Kconfig +++ b/drivers/video/fbdev/Kconfig @@ -455,7 +455,7 @@ config FB_ATARI config FB_OF bool "Open Firmware frame buffer device support" depends on (FB = y) && PPC && (!PPC_PSERIES || PCI) - depends on !DRM_OFDRM + depends on DRM_OFDRM != y select APERTURE_HELPERS I would instead make this 'depends on DRM_OFDRM=n', which completely eliminates configs that have both driver enabled. Yep, that works for me. Thanks. Thomas, Michal, are you OK with that change? Yes. A nicer change would be to make FB_OF a tristate symbol, which makes it possible to load one of the two modules if both are enabled =m, while only allowing one of them to be =y if the other is completely disabled. It looks like offb was originally written to be usable as a loadable module, but Kconfig has prevented this since at least the start of the git history. -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
[PATCH linux-next] powerpc/cell/axon_msi: replace DEFINE_SIMPLE_ATTRIBUTE with DEFINE_DEBUGFS_ATTRIBUTE
From: zhang songyi Fix the following coccicheck warning: /arch/powerpc/platforms/cell/axon_msi.c:457:0-23: WARNING: fops_msic should be defined with DEFINE_DEBUGFS_ATTRIBUTE Signed-off-by: zhang songyi --- arch/powerpc/platforms/cell/axon_msi.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/platforms/cell/axon_msi.c b/arch/powerpc/platforms/cell/axon_msi.c index 5b012abca773..ab080b5022ff 100644 --- a/arch/powerpc/platforms/cell/axon_msi.c +++ b/arch/powerpc/platforms/cell/axon_msi.c @@ -454,7 +454,7 @@ static int msic_get(void *data, u64 *val) return 0; } -DEFINE_SIMPLE_ATTRIBUTE(fops_msic, msic_get, msic_set, "%llu\n"); +DEFINE_DEBUGFS_ATTRIBUTE(fops_msic, msic_get, msic_set, "%llu\n"); void axon_msi_debug_setup(struct device_node *dn, struct axon_msic *msic) { @@ -475,6 +475,7 @@ void axon_msi_debug_setup(struct device_node *dn, struct axon_msic *msic) snprintf(name, sizeof(name), "msic_%d", of_node_to_nid(dn)); - debugfs_create_file(name, 0600, arch_debugfs_dir, msic, &fops_msic); + debugfs_create_file_unsafe(name, 0600, arch_debugfs_dir, + msic, &fops_msic); } #endif /* DEBUG */ -- 2.15.2
Re: (subset) [PATCH 000/606] i2c: Complete conversion to i2c_probe_new
On Fri, 18 Nov 2022 23:35:34 +0100, Uwe Kleine-König wrote: > since commit b8a1a4cd5a98 ("i2c: Provide a temporary .probe_new() > call-back type") from 2016 there is a "temporary" alternative probe > callback for i2c drivers. > > This series completes all drivers to this new callback (unless I missed > something). It's based on current next/master. > A part of the patches depend on commit 662233731d66 ("i2c: core: > Introduce i2c_client_get_device_id helper function"), there is a branch that > you can pull into your tree to get it: > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next Thanks! [538/606] regulator: act8865-regulator: Convert to i2c's .probe_new() (no commit info) [539/606] regulator: ad5398: Convert to i2c's .probe_new() (no commit info) [540/606] regulator: da9121-regulator: Convert to i2c's .probe_new() commit: 020cf73b47414a84b666d3e6736a6ae957e27840 [541/606] regulator: fan53555: Convert to i2c's .probe_new() (no commit info) [542/606] regulator: isl6271a-regulator: Convert to i2c's .probe_new() (no commit info) [543/606] regulator: lp3972: Convert to i2c's .probe_new() commit: 2532d5f8d5c20d5a0a8a0d57a311bc5df00dea04 [544/606] regulator: lp872x: Convert to i2c's .probe_new() (no commit info) [545/606] regulator: lp8755: Convert to i2c's .probe_new() commit: cb28f74b4809a00b40fdf0c44ccf51ab950581d3 [546/606] regulator: ltc3589: Convert to i2c's .probe_new() (no commit info) [547/606] regulator: max1586: Convert to i2c's .probe_new() commit: 3d54f7ba248b0ad1791bc356e9ad3d9020a1c472 [548/606] regulator: max8649: Convert to i2c's .probe_new() commit: 3cf4417385d0ac8f02f22888e12a6d21d97d89fc [549/606] regulator: max8660: Convert to i2c's .probe_new() (no commit info) [550/606] regulator: max8952: Convert to i2c's .probe_new() commit: c20c36735949b3b7984692fbab3d92b0e8a845ec [551/606] regulator: max8973-regulator: Convert to i2c's .probe_new() (no commit info) [552/606] regulator: pca9450-regulator: Convert to i2c's .probe_new() commit: ed56fa6e804cb13bbe29e9214792308817f6e553 [553/606] regulator: pfuze100-regulator: Convert to i2c's .probe_new() (no commit info) [554/606] regulator: pv88080-regulator: Convert to i2c's .probe_new() (no commit info) [555/606] regulator: rpi-panel-attiny-regulator: Convert to i2c's .probe_new() commit: d85d02d17a608b558d44510e9824668c5d4fe5d8 [556/606] regulator: tps51632-regulator: Convert to i2c's .probe_new() commit: d4885f306304ff29eec06b9ad5f526a1099e0418 [557/606] regulator: tps62360-regulator: Convert to i2c's .probe_new() (no commit info) [558/606] regulator: tps6286x-regulator: Convert to i2c's .probe_new() commit: e34782316281c78c5911f86d4699d4f35a607c9d [559/606] regulator: tps65023-regulator: Convert to i2c's .probe_new() (no commit info) All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
Re: [PATCH mm-unstable v1 13/20] media: videobuf-dma-sg: remove FOLL_FORCE usage
On 16/11/2022 11:26, David Hildenbrand wrote: > GUP now supports reliable R/O long-term pinning in COW mappings, such > that we break COW early. MAP_SHARED VMAs only use the shared zeropage so > far in one corner case (DAXFS file with holes), which can be ignored > because GUP does not support long-term pinning in fsdax (see > check_vma_flags()). > > Consequently, FOLL_FORCE | FOLL_WRITE | FOLL_LONGTERM is no longer required > for reliable R/O long-term pinning: FOLL_LONGTERM is sufficient. So stop > using FOLL_FORCE, which is really only for ptrace access. > > Cc: Mauro Carvalho Chehab > Signed-off-by: David Hildenbrand Acked-by: Hans Verkuil Looks good! Hans > --- > drivers/media/v4l2-core/videobuf-dma-sg.c | 14 +- > 1 file changed, 5 insertions(+), 9 deletions(-) > > diff --git a/drivers/media/v4l2-core/videobuf-dma-sg.c > b/drivers/media/v4l2-core/videobuf-dma-sg.c > index f75e5eedeee0..234e9f647c96 100644 > --- a/drivers/media/v4l2-core/videobuf-dma-sg.c > +++ b/drivers/media/v4l2-core/videobuf-dma-sg.c > @@ -151,17 +151,16 @@ static void videobuf_dma_init(struct videobuf_dmabuf > *dma) > static int videobuf_dma_init_user_locked(struct videobuf_dmabuf *dma, > int direction, unsigned long data, unsigned long size) > { > + unsigned int gup_flags = FOLL_LONGTERM; > unsigned long first, last; > - int err, rw = 0; > - unsigned int flags = FOLL_FORCE; > + int err; > > dma->direction = direction; > switch (dma->direction) { > case DMA_FROM_DEVICE: > - rw = READ; > + gup_flags |= FOLL_WRITE; > break; > case DMA_TO_DEVICE: > - rw = WRITE; > break; > default: > BUG(); > @@ -177,14 +176,11 @@ static int videobuf_dma_init_user_locked(struct > videobuf_dmabuf *dma, > if (NULL == dma->pages) > return -ENOMEM; > > - if (rw == READ) > - flags |= FOLL_WRITE; > - > dprintk(1, "init user [0x%lx+0x%lx => %lu pages]\n", > data, size, dma->nr_pages); > > - err = pin_user_pages(data & PAGE_MASK, dma->nr_pages, > - flags | FOLL_LONGTERM, dma->pages, NULL); > + err = pin_user_pages(data & PAGE_MASK, dma->nr_pages, gup_flags, > + dma->pages, NULL); > > if (err != dma->nr_pages) { > dma->nr_pages = (err >= 0) ? err : 0;
Re: [PATCH mm-unstable v1 15/20] media: pci/ivtv: remove FOLL_FORCE usage
On 16/11/2022 11:26, David Hildenbrand wrote: > FOLL_FORCE is really only for ptrace access. R/O pinning a page is > supposed to fail if the VMA misses proper access permissions (no VM_READ). > > Let's just remove FOLL_FORCE usage here; there would have to be a pretty > good reason to allow arbitrary drivers to R/O pin pages in a PROT_NONE > VMA. Most probably, FOLL_FORCE usage is just some legacy leftover. I'm pretty sure about that as well, so: Acked-by: Hans Verkuil Regards, Hans > > Cc: Andy Walls > Cc: Mauro Carvalho Chehab > Signed-off-by: David Hildenbrand > --- > drivers/media/pci/ivtv/ivtv-udma.c | 2 +- > drivers/media/pci/ivtv/ivtv-yuv.c | 5 ++--- > 2 files changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/pci/ivtv/ivtv-udma.c > b/drivers/media/pci/ivtv/ivtv-udma.c > index 210be8290f24..99b9f55ca829 100644 > --- a/drivers/media/pci/ivtv/ivtv-udma.c > +++ b/drivers/media/pci/ivtv/ivtv-udma.c > @@ -115,7 +115,7 @@ int ivtv_udma_setup(struct ivtv *itv, unsigned long > ivtv_dest_addr, > > /* Pin user pages for DMA Xfer */ > err = pin_user_pages_unlocked(user_dma.uaddr, user_dma.page_count, > - dma->map, FOLL_FORCE); > + dma->map, 0); > > if (user_dma.page_count != err) { > IVTV_DEBUG_WARN("failed to map user pages, returned %d instead > of %d\n", > diff --git a/drivers/media/pci/ivtv/ivtv-yuv.c > b/drivers/media/pci/ivtv/ivtv-yuv.c > index 4ba10c34a16a..582146f8d70d 100644 > --- a/drivers/media/pci/ivtv/ivtv-yuv.c > +++ b/drivers/media/pci/ivtv/ivtv-yuv.c > @@ -63,12 +63,11 @@ static int ivtv_yuv_prep_user_dma(struct ivtv *itv, > struct ivtv_user_dma *dma, > > /* Pin user pages for DMA Xfer */ > y_pages = pin_user_pages_unlocked(y_dma.uaddr, > - y_dma.page_count, &dma->map[0], FOLL_FORCE); > + y_dma.page_count, &dma->map[0], 0); > uv_pages = 0; /* silence gcc. value is set and consumed only if: */ > if (y_pages == y_dma.page_count) { > uv_pages = pin_user_pages_unlocked(uv_dma.uaddr, > - uv_dma.page_count, &dma->map[y_pages], > - FOLL_FORCE); > + uv_dma.page_count, &dma->map[y_pages], 0); > } > > if (y_pages != y_dma.page_count || uv_pages != uv_dma.page_count) {
Re: [PATCH mm-unstable v1 16/20] mm/frame-vector: remove FOLL_FORCE usage
Hi David, Tomasz, On 16/11/2022 11:26, David Hildenbrand wrote: > FOLL_FORCE is really only for ptrace access. According to commit > 707947247e95 ("media: videobuf2-vmalloc: get_userptr: buffers are always > writable"), get_vaddr_frames() currently pins all pages writable as a > workaround for issues with read-only buffers. I've decided to revert 707947247e95: I have not been able to reproduce the problem described in that commit, and Tomasz reported that it caused problems with a specific use-case they encountered. I'll post that patch soon and I expect it to land in 6.2. It will cause a conflict with this patch, though. If the problem described in that patch occurs again, then I will revisit it and hopefully do a better job than I did before. That commit was not my finest moment. Regards, Hans > > FOLL_FORCE, however, seems to be a legacy leftover as it predates > commit 707947247e95 ("media: videobuf2-vmalloc: get_userptr: buffers are > always writable"). Let's just remove it. > > Once the read-only buffer issue has been resolved, FOLL_WRITE could > again be set depending on the DMA direction. > > Cc: Hans Verkuil > Cc: Marek Szyprowski > Cc: Tomasz Figa > Cc: Marek Szyprowski > Cc: Mauro Carvalho Chehab > Signed-off-by: David Hildenbrand > --- > drivers/media/common/videobuf2/frame_vector.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/media/common/videobuf2/frame_vector.c > b/drivers/media/common/videobuf2/frame_vector.c > index 542dde9d2609..062e98148c53 100644 > --- a/drivers/media/common/videobuf2/frame_vector.c > +++ b/drivers/media/common/videobuf2/frame_vector.c > @@ -50,7 +50,7 @@ int get_vaddr_frames(unsigned long start, unsigned int > nr_frames, > start = untagged_addr(start); > > ret = pin_user_pages_fast(start, nr_frames, > - FOLL_FORCE | FOLL_WRITE | FOLL_LONGTERM, > + FOLL_WRITE | FOLL_LONGTERM, > (struct page **)(vec->ptrs)); > if (ret > 0) { > vec->got_ref = true;
Re: [PATCH] perf test: Skip watchpoint tests if no watchpoints available
Em Tue, Nov 22, 2022 at 12:57:05PM -0800, Ian Rogers escreveu: > On Tue, Nov 22, 2022 at 11:19 AM Christophe Leroy > wrote: > > > > > > > > Le 21/11/2022 à 11:27, Naveen N. Rao a écrit : > > > On IBM Power9, perf watchpoint tests fail since no hardware breakpoints > > > are available. Detect this by checking the error returned by > > > perf_event_open() and skip the tests in that case. > > > > > > Reported-by: Disha Goel > > > Signed-off-by: Naveen N. Rao > > > --- > > > tools/perf/tests/wp.c | 12 +++- > > > 1 file changed, 7 insertions(+), 5 deletions(-) > > > > > > diff --git a/tools/perf/tests/wp.c b/tools/perf/tests/wp.c > > > index 56455da30341b4..cc8719609b19ea 100644 > > > --- a/tools/perf/tests/wp.c > > > +++ b/tools/perf/tests/wp.c > > > @@ -59,8 +59,10 @@ static int __event(int wp_type, void *wp_addr, > > > unsigned long wp_len) > > > get__perf_event_attr(&attr, wp_type, wp_addr, wp_len); > > > fd = sys_perf_event_open(&attr, 0, -1, -1, > > >perf_event_open_cloexec_flag()); > > > - if (fd < 0) > > > + if (fd < 0) { > > > + fd = -errno; > > > pr_debug("failed opening event %x\n", attr.bp_type); > > > + } > > > > Do you really need that ? > > > > Can't you directly check errno in the caller ? > > errno is very easily clobbered and not clearly set on success - ie, > it'd be better not to do that. > > Acked-by: Ian Rogers Thanks, applied. - Arnaldo
Re: [PATCH linux-next] powerpc/cell/axon_msi: replace DEFINE_SIMPLE_ATTRIBUTE with DEFINE_DEBUGFS_ATTRIBUTE
Hi, Le 23/11/2022 à 10:06, zhang.son...@zte.com.cn a écrit : > From: zhang songyi > > Fix the following coccicheck warning: > /arch/powerpc/platforms/cell/axon_msi.c:457:0-23: WARNING: > fops_msic should be defined with DEFINE_DEBUGFS_ATTRIBUTE What's the difference between this new patch and the one that is already awaiting application here : https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20211222090655.484551-1-deng.changch...@zte.com.cn/ ? The only difference I see it that the already existing patch has a more complete description of the change, so unless I'm missing something it would be nice to avoid sending the same changes again and again. Thanks Christophe > > Signed-off-by: zhang songyi > --- > arch/powerpc/platforms/cell/axon_msi.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/arch/powerpc/platforms/cell/axon_msi.c > b/arch/powerpc/platforms/cell/axon_msi.c > index 5b012abca773..ab080b5022ff 100644 > --- a/arch/powerpc/platforms/cell/axon_msi.c > +++ b/arch/powerpc/platforms/cell/axon_msi.c > @@ -454,7 +454,7 @@ static int msic_get(void *data, u64 *val) > return 0; > } > > -DEFINE_SIMPLE_ATTRIBUTE(fops_msic, msic_get, msic_set, "%llu\n"); > +DEFINE_DEBUGFS_ATTRIBUTE(fops_msic, msic_get, msic_set, "%llu\n"); > > void axon_msi_debug_setup(struct device_node *dn, struct axon_msic *msic) > { > @@ -475,6 +475,7 @@ void axon_msi_debug_setup(struct device_node *dn, struct > axon_msic *msic) > > snprintf(name, sizeof(name), "msic_%d", of_node_to_nid(dn)); > > - debugfs_create_file(name, 0600, arch_debugfs_dir, msic, &fops_msic); > + debugfs_create_file_unsafe(name, 0600, arch_debugfs_dir, > + msic, &fops_msic); > } > #endif /* DEBUG */ > -- > 2.15.2
Re: [PATCH v7 2/2] arm64: support batched/deferred tlb shootdown during page reclamation
On 11/17/22 13:56, Yicong Yang wrote: > From: Barry Song > > on x86, batched and deferred tlb shootdown has lead to 90% > performance increase on tlb shootdown. on arm64, HW can do > tlb shootdown without software IPI. But sync tlbi is still > quite expensive. > > Even running a simplest program which requires swapout can > prove this is true, > #include > #include > #include > #include > > int main() > { > #define SIZE (1 * 1024 * 1024) > volatile unsigned char *p = mmap(NULL, SIZE, PROT_READ | PROT_WRITE, > MAP_SHARED | MAP_ANONYMOUS, -1, 0); > > memset(p, 0x88, SIZE); > > for (int k = 0; k < 1; k++) { > /* swap in */ > for (int i = 0; i < SIZE; i += 4096) { > (void)p[i]; > } > > /* swap out */ > madvise(p, SIZE, MADV_PAGEOUT); > } > } > > Perf result on snapdragon 888 with 8 cores by using zRAM > as the swap block device. > > ~ # perf record taskset -c 4 ./a.out > [ perf record: Woken up 10 times to write data ] > [ perf record: Captured and wrote 2.297 MB perf.data (60084 samples) ] > ~ # perf report > # To display the perf.data header info, please use --header/--header-only > options. > # To display the perf.data header info, please use --header/--header-only > options. > # > # > # Total Lost Samples: 0 > # > # Samples: 60K of event 'cycles' > # Event count (approx.): 35706225414 > # > # Overhead Command Shared Object Symbol > # ... . > . > # > 21.07% a.out[kernel.kallsyms] [k] _raw_spin_unlock_irq > 8.23% a.out[kernel.kallsyms] [k] _raw_spin_unlock_irqrestore > 6.67% a.out[kernel.kallsyms] [k] filemap_map_pages > 6.16% a.out[kernel.kallsyms] [k] __zram_bvec_write > 5.36% a.out[kernel.kallsyms] [k] ptep_clear_flush > 3.71% a.out[kernel.kallsyms] [k] _raw_spin_lock > 3.49% a.out[kernel.kallsyms] [k] memset64 > 1.63% a.out[kernel.kallsyms] [k] clear_page > 1.42% a.out[kernel.kallsyms] [k] _raw_spin_unlock > 1.26% a.out[kernel.kallsyms] [k] > mod_zone_state.llvm.8525150236079521930 > 1.23% a.out[kernel.kallsyms] [k] xas_load > 1.15% a.out[kernel.kallsyms] [k] zram_slot_lock > > ptep_clear_flush() takes 5.36% CPU in the micro-benchmark > swapping in/out a page mapped by only one process. If the > page is mapped by multiple processes, typically, like more > than 100 on a phone, the overhead would be much higher as > we have to run tlb flush 100 times for one single page. > Plus, tlb flush overhead will increase with the number > of CPU cores due to the bad scalability of tlb shootdown > in HW, so those ARM64 servers should expect much higher > overhead. > > Further perf annonate shows 95% cpu time of ptep_clear_flush > is actually used by the final dsb() to wait for the completion > of tlb flush. This provides us a very good chance to leverage > the existing batched tlb in kernel. The minimum modification > is that we only send async tlbi in the first stage and we send > dsb while we have to sync in the second stage. > > With the above simplest micro benchmark, collapsed time to > finish the program decreases around 5%. > > Typical collapsed time w/o patch: > ~ # time taskset -c 4 ./a.out > 0.21user 14.34system 0:14.69elapsed > w/ patch: > ~ # time taskset -c 4 ./a.out > 0.22user 13.45system 0:13.80elapsed > > Also, Yicong Yang added the following observation. > Tested with benchmark in the commit on Kunpeng920 arm64 server, > observed an improvement around 12.5% with command > `time ./swap_bench`. > w/o w/ > real0m13.460s 0m11.771s > user0m0.248s0m0.279s > sys 0m12.039s 0m11.458s > > Originally it's noticed a 16.99% overhead of ptep_clear_flush() > which has been eliminated by this patch: > > [root@localhost yang]# perf record -- ./swap_bench && perf report > [...] > 16.99% swap_bench [kernel.kallsyms] [k] ptep_clear_flush > > It is tested on 4,8,128 CPU platforms and shows to be beneficial on > large systems but may not have improvement on small systems like on > a 4 CPU platform. So make ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH depends > on CONFIG_EXPERT for this stage and make this disabled on systems > with less than 8 CPUs. User can modify this threshold according to > their own platforms by CONFIG_NR_CPUS_FOR_BATCHED_TLB. > > This patch extends arch_tlbbatch_add_mm() to take an address of the > target page to support the feature on arm64. Also rename it to > arch_tlbbatch_add_pending() to better match its function since we > don't need to handle the mm on arm64 and add_mm is not proper. > add_pen
Re: [PATCH mm-unstable v1 16/20] mm/frame-vector: remove FOLL_FORCE usage
On 23/11/2022 14:26, Hans Verkuil wrote: > Hi David, Tomasz, > > On 16/11/2022 11:26, David Hildenbrand wrote: >> FOLL_FORCE is really only for ptrace access. According to commit >> 707947247e95 ("media: videobuf2-vmalloc: get_userptr: buffers are always >> writable"), get_vaddr_frames() currently pins all pages writable as a >> workaround for issues with read-only buffers. > > I've decided to revert 707947247e95: I have not been able to reproduce the > problem > described in that commit, and Tomasz reported that it caused problems with a > specific use-case they encountered. I'll post that patch soon and I expect it > to land in 6.2. It will cause a conflict with this patch, though. > > If the problem described in that patch occurs again, then I will revisit it > and hopefully do a better job than I did before. That commit was not my > finest moment. In any case, for this patch: Acked-by: Hans Verkuil Regards, Hans > > Regards, > > Hans > >> >> FOLL_FORCE, however, seems to be a legacy leftover as it predates >> commit 707947247e95 ("media: videobuf2-vmalloc: get_userptr: buffers are >> always writable"). Let's just remove it. >> >> Once the read-only buffer issue has been resolved, FOLL_WRITE could >> again be set depending on the DMA direction. >> >> Cc: Hans Verkuil >> Cc: Marek Szyprowski >> Cc: Tomasz Figa >> Cc: Marek Szyprowski >> Cc: Mauro Carvalho Chehab >> Signed-off-by: David Hildenbrand >> --- >> drivers/media/common/videobuf2/frame_vector.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/media/common/videobuf2/frame_vector.c >> b/drivers/media/common/videobuf2/frame_vector.c >> index 542dde9d2609..062e98148c53 100644 >> --- a/drivers/media/common/videobuf2/frame_vector.c >> +++ b/drivers/media/common/videobuf2/frame_vector.c >> @@ -50,7 +50,7 @@ int get_vaddr_frames(unsigned long start, unsigned int >> nr_frames, >> start = untagged_addr(start); >> >> ret = pin_user_pages_fast(start, nr_frames, >> - FOLL_FORCE | FOLL_WRITE | FOLL_LONGTERM, >> + FOLL_WRITE | FOLL_LONGTERM, >>(struct page **)(vec->ptrs)); >> if (ret > 0) { >> vec->got_ref = true; >
Re: [PATCH 2/4] fs: define a firmware security filesystem named fwsecurityfs
On 11/22/22 18:21, Nayna wrote: From the perspective of our use case, we need to expose firmware security objects to userspace for management. Not all of the objects pre-exist and we would like to allow root to create them from userspace. From a unification perspective, I have considered a common location at /sys/firmware/security for managing any platform's security objects. And I've proposed a generic filesystem, which could be used by any platform to represent firmware security objects via /sys/firmware/security. Here are some alternatives to generic filesystem in discussion: 1. Start with a platform-specific filesystem. If more platforms would like to use the approach, it can be made generic. We would still have a common location of /sys/firmware/security and new code would live in arch. This is my preference and would be the best fit for our use case. 2. Use securityfs. This would mean modifying it to satisfy other use cases, including supporting userspace file creation. I don't know if the securityfs maintainer would find that acceptable. I would also still want some way to expose variables at /sys/firmware/security. 3. Use a sysfs-based approach. This would be a platform-specific implementation. However, sysfs has a similar issue to securityfs for file creation. When I tried it in RFC v1[1], I had to implement a workaround to achieve that. [1] https://lore.kernel.org/linuxppc-dev/20220122005637.28199-3-na...@linux.ibm.com/ Hi Greg, Based on the discussions so far, is Option 1, described above, an acceptable next step? Thanks & Regards, - Nayna
Re: [PATCH v5 3/6] crash: add phdr for possible CPUs in elfcorehdr
On 22/11/22 23:28, Eric DeVolder wrote: On 11/20/22 17:25, Sourabh Jain wrote: On architectures like PowerPC the crash notes are available for all possible CPUs. So let's populate the elfcorehdr for all possible CPUs having crash notes to avoid updating elfcorehdr during in-kernel crash update on CPU hotplug events. The similar technique was used in kexec-tool for kexec_load case. Signed-off-by: Sourabh Jain --- kernel/crash_core.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/kernel/crash_core.c b/kernel/crash_core.c index bca1b198d9e55..f6cccdcadc9f3 100644 --- a/kernel/crash_core.c +++ b/kernel/crash_core.c @@ -364,16 +364,19 @@ int crash_prepare_elf64_headers(struct kimage *image, struct crash_mem *mem, ehdr->e_ehsize = sizeof(Elf64_Ehdr); ehdr->e_phentsize = sizeof(Elf64_Phdr); - /* Prepare one phdr of type PT_NOTE for each present CPU */ - for_each_present_cpu(cpu) { + /* Prepare one phdr of type PT_NOTE for possible CPU with crash note. */ + for_each_possible_cpu(cpu) { if (IS_ENABLED(CONFIG_HOTPLUG_CPU)) { /* Skip the soon-to-be offlined cpu */ if (image->hotplug_event && (cpu == image->offlinecpu)) continue; } - phdr->p_type = PT_NOTE; notes_addr = per_cpu_ptr_to_phys(per_cpu_ptr(crash_notes, cpu)); + if (!notes_addr) + continue; + + phdr->p_type = PT_NOTE; phdr->p_offset = phdr->p_paddr = notes_addr; phdr->p_filesz = phdr->p_memsz = sizeof(note_buf_t); (ehdr->e_phnum)++; I did a quick test of this for x86_64 and it works. Reviewed-by: Eric DeVolder Thanks for testing it out. - Sourabh Jain
Re: [PATCH 2/4] fs: define a firmware security filesystem named fwsecurityfs
On Wed, Nov 23, 2022 at 10:05:49AM -0500, Nayna wrote: > > On 11/22/22 18:21, Nayna wrote: > > > > From the perspective of our use case, we need to expose firmware > > security objects to userspace for management. Not all of the objects > > pre-exist and we would like to allow root to create them from userspace. > > > > From a unification perspective, I have considered a common location at > > /sys/firmware/security for managing any platform's security objects. And > > I've proposed a generic filesystem, which could be used by any platform > > to represent firmware security objects via /sys/firmware/security. > > > > Here are some alternatives to generic filesystem in discussion: > > > > 1. Start with a platform-specific filesystem. If more platforms would > > like to use the approach, it can be made generic. We would still have a > > common location of /sys/firmware/security and new code would live in > > arch. This is my preference and would be the best fit for our use case. > > > > 2. Use securityfs. This would mean modifying it to satisfy other use > > cases, including supporting userspace file creation. I don't know if the > > securityfs maintainer would find that acceptable. I would also still > > want some way to expose variables at /sys/firmware/security. > > > > 3. Use a sysfs-based approach. This would be a platform-specific > > implementation. However, sysfs has a similar issue to securityfs for > > file creation. When I tried it in RFC v1[1], I had to implement a > > workaround to achieve that. > > > > [1] > > https://lore.kernel.org/linuxppc-dev/20220122005637.28199-3-na...@linux.ibm.com/ > > > Hi Greg, > > Based on the discussions so far, is Option 1, described above, an acceptable > next step? No, as I said almost a year ago, I do not want to see platform-only filesystems going and implementing stuff that should be shared by all platforms. thanks, greg k-h
Re: [PATCH -next] fbdev: offb: allow build when DRM_OFDRM=m
On 11/23/22 02:02, Randy Dunlap wrote: > Hi Arnd, > > On 11/23/22 01:08, Arnd Bergmann wrote: >> On Wed, Nov 23, 2022, at 04:16, Randy Dunlap wrote: >>> Fix build when CONFIG_FB_OF=y and CONFIG_DRM_OFDRM=m. >>> When the latter symbol is =m, kconfig downgrades (limits) the 'select's >>> under FB_OF to modular (=m). This causes undefined symbol references: >>> >>> powerpc64-linux-ld: drivers/video/fbdev/offb.o:(.data.rel.ro+0x58): >>> undefined reference to `cfb_fillrect' >>> powerpc64-linux-ld: drivers/video/fbdev/offb.o:(.data.rel.ro+0x60): >>> undefined reference to `cfb_copyarea' >>> powerpc64-linux-ld: drivers/video/fbdev/offb.o:(.data.rel.ro+0x68): >>> undefined reference to `cfb_imageblit' >>> >>> Fix this by allowing FB_OF any time that DRM_OFDRM != y so that the >>> selected FB_CFB_* symbols will become =y instead of =m. >>> >>> In tristate logic (for DRM_OFDRM), this changes the dependency from >>> !DRM_OFDRM == 2 - 1 == 1 => modular only (or disabled) >>> to (boolean) >>> DRM_OFDRM != y == y, allowing the 'select's to cause the >>> FB_CFB_* symbols to =y instead of =m. >>> >> >> Is it actually a useful configuration to have OFDRM=m and >> FB_OF=y though? I would expect in that case that the OFDRM >> driver never binds to a device because it's already owned >> by FB_OF. >> >>> diff -- a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig >>> --- a/drivers/video/fbdev/Kconfig >>> +++ b/drivers/video/fbdev/Kconfig >>> @@ -455,7 +455,7 @@ config FB_ATARI >>> config FB_OF >>> bool "Open Firmware frame buffer device support" >>> depends on (FB = y) && PPC && (!PPC_PSERIES || PCI) >>> - depends on !DRM_OFDRM >>> + depends on DRM_OFDRM != y >>> select APERTURE_HELPERS >> >> I would instead make this 'depends on DRM_OFDRM=n', which >> completely eliminates configs that have both driver enabled. > > Yep, that works for me. Thanks. > > Thomas, Michal, are you OK with that change? > >> A nicer change would be to make FB_OF a tristate symbol, >> which makes it possible to load one of the two modules if >> both are enabled =m, while only allowing one of them to >> be =y if the other is completely disabled. It looks like >> offb was originally written to be usable as a loadable module, >> but Kconfig has prevented this since at least the start of >> the git history. ISTM that a distro would prefer to have both DFM_OFDRM and FB_OF as tristate symbols that could both be built as loadable modules, as Arnd describes above. I'll look into that. -- ~Randy
Re: (subset) [PATCH 000/606] i2c: Complete conversion to i2c_probe_new
On Fri, 18 Nov 2022 23:35:34 +0100, Uwe Kleine-König wrote: > since commit b8a1a4cd5a98 ("i2c: Provide a temporary .probe_new() > call-back type") from 2016 there is a "temporary" alternative probe > callback for i2c drivers. > > This series completes all drivers to this new callback (unless I missed > something). It's based on current next/master. > A part of the patches depend on commit 662233731d66 ("i2c: core: > Introduce i2c_client_get_device_id helper function"), there is a branch that > you can pull into your tree to get it: > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next Thanks! [539/606] regulator: ad5398: Convert to i2c's .probe_new() commit: 7f69edba960bbdcbc829d8d0995b1117ce29e8b1 [544/606] regulator: lp872x: Convert to i2c's .probe_new() commit: 87feccb347b25f5dc6ff451123b832c9ad5dddfe [546/606] regulator: ltc3589: Convert to i2c's .probe_new() commit: 78c8f6cdb51d471928d481ed3b2c82dbc110a1ed [549/606] regulator: max8660: Convert to i2c's .probe_new() commit: dbf31dac703009174226bb87b3914bd092040327 [551/606] regulator: max8973-regulator: Convert to i2c's .probe_new() commit: 4e85e5d64f66ac5e4b0286ee4b6f8e8ce1044d42 [557/606] regulator: tps62360-regulator: Convert to i2c's .probe_new() commit: 18804160277ec2ab992373385f86c6af2322b28b [559/606] regulator: tps65023-regulator: Convert to i2c's .probe_new() commit: 3b5b07dde998f6ade7433a8db019cf816c7e35af All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
Re: [PATCH linux-next][RFC]torture: avoid offline tick_do_timer_cpu
On Wed, Nov 23, 2022 at 10:23:11AM +0800, Zhouyi Zhou wrote: > On Tue, Nov 22, 2022 at 9:37 AM Paul E. McKenney wrote: > > > > On Mon, Nov 21, 2022 at 11:51:40AM +0800, Zhouyi Zhou wrote: > > > During CPU-hotplug torture (CONFIG_NO_HZ_FULL=y), if we try to > > > offline tick_do_timer_cpu, the operation will fail because in > > > function tick_nohz_cpu_down: > > > ``` > > > if (tick_nohz_full_running && tick_do_timer_cpu == cpu) > > > return -EBUSY; > > > ``` > > > Above bug was first discovered in torture tests performed in PPC VM > > > of Open Source Lab of Oregon State University, and reproducable in RISC-V > > > and X86-64 (with additional kernel commandline cpu0_hotplug). > > > > > > In this patch, we avoid offline tick_do_timer_cpu by distribute > > > the offlining cpu among remaining cpus. > > > > > > Signed-off-by: Zhouyi Zhou > > > > Good show chasing this down! > Thank Paul for your guidance and encouragement! > > > > A couple of questions below. > The answers below. > > > > > --- > > > include/linux/tick.h| 1 + > > > kernel/time/tick-common.c | 1 + > > > kernel/time/tick-internal.h | 1 - > > > kernel/torture.c| 10 ++ > > > 4 files changed, 12 insertions(+), 1 deletion(-) > > > > > > diff --git a/include/linux/tick.h b/include/linux/tick.h > > > index bfd571f18cfd..23cc0b205853 100644 > > > --- a/include/linux/tick.h > > > +++ b/include/linux/tick.h > > > @@ -14,6 +14,7 @@ > > > #include > > > > > > #ifdef CONFIG_GENERIC_CLOCKEVENTS > > > +extern int tick_do_timer_cpu __read_mostly; > > > extern void __init tick_init(void); > > > /* Should be core only, but ARM BL switcher requires it */ > > > extern void tick_suspend_local(void); > > > diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c > > > index 46789356f856..87b9b9afa320 100644 > > > --- a/kernel/time/tick-common.c > > > +++ b/kernel/time/tick-common.c > > > @@ -48,6 +48,7 @@ ktime_t tick_next_period; > > > *procedure also covers cpu hotplug. > > > */ > > > int tick_do_timer_cpu __read_mostly = TICK_DO_TIMER_BOOT; > > > +EXPORT_SYMBOL_GPL(tick_do_timer_cpu); > > > #ifdef CONFIG_NO_HZ_FULL > > > /* > > > * tick_do_timer_boot_cpu indicates the boot CPU temporarily owns > > > diff --git a/kernel/time/tick-internal.h b/kernel/time/tick-internal.h > > > index 649f2b48e8f0..8953dca10fdd 100644 > > > --- a/kernel/time/tick-internal.h > > > +++ b/kernel/time/tick-internal.h > > > @@ -15,7 +15,6 @@ > > > > > > DECLARE_PER_CPU(struct tick_device, tick_cpu_device); > > > extern ktime_t tick_next_period; > > > -extern int tick_do_timer_cpu __read_mostly; > > > > > > extern void tick_setup_periodic(struct clock_event_device *dev, int > > > broadcast); > > > extern void tick_handle_periodic(struct clock_event_device *dev); > > > diff --git a/kernel/torture.c b/kernel/torture.c > > > index 789aeb0e1159..bccbdd33dda2 100644 > > > --- a/kernel/torture.c > > > +++ b/kernel/torture.c > > > @@ -33,6 +33,7 @@ > > > #include > > > #include > > > #include > > > +#include > > > #include > > > #include > > > #include > > > @@ -358,7 +359,16 @@ torture_onoff(void *arg) > > > schedule_timeout_interruptible(HZ / 10); > > > continue; > > > } > > > +#ifdef CONFIG_NO_HZ_FULL > > > + /* do not offline tick do timer cpu */ > > > + if (tick_nohz_full_running) { > > > + cpu = (torture_random(&rand) >> 4) % maxcpu; > > > + if (cpu >= tick_do_timer_cpu) > > > > Why is this ">=" instead of "=="? > I use probability theory here to let the remaining cpu distribute evenly. > Example: > we have cpus: 0 1 2 3 4 5 6 7 > maxcpu = 7 > tick_do_timer_cpu = 2 > remaining cpus are: 0 1 3 4 5 6 7 > if the offline cpu candidate is 2, then the result cpu is 2+1 > else if the offline cpu candidate is 3, then the result cpu is 3+1 > ... > else if the offline cpu candidate is 6, then the result cpu is 6+1 > > > > > + cpu = (cpu + 1) % (maxcpu + 1); > we could just use cpu = cpu + 1 here But won't this get you double the occurrences of CPU 0 compared to the other non-tick_do_timer_cpu CPUs? You might get CPU 0 directly from torture_random(), or torture_random() might have given you CPU 7, which then wraps to CPU 0. What am I missing here? > > > + } else > > > +#else > > > cpu = (torture_random(&rand) >> 4) % (maxcpu + 1); > > > +#endif > > > > What happens if the value of tick_do_timer_cpu changes between the time of > > the check above and the call to torture_offline() below? Alternatively, > > how is such a change in value prevented? > I did a preliminary research about the above question, this is quite > complicated for me > (because I think I must not bring locks to kernel just because our > test frame need them), Agreed, it would be good to avoid added locks. > Please give me some days to perform intensive research.
Re: [PATCH 2/4] fs: define a firmware security filesystem named fwsecurityfs
On 11/23/22 10:57, Greg Kroah-Hartman wrote: On Wed, Nov 23, 2022 at 10:05:49AM -0500, Nayna wrote: On 11/22/22 18:21, Nayna wrote: From the perspective of our use case, we need to expose firmware security objects to userspace for management. Not all of the objects pre-exist and we would like to allow root to create them from userspace. From a unification perspective, I have considered a common location at /sys/firmware/security for managing any platform's security objects. And I've proposed a generic filesystem, which could be used by any platform to represent firmware security objects via /sys/firmware/security. Here are some alternatives to generic filesystem in discussion: 1. Start with a platform-specific filesystem. If more platforms would like to use the approach, it can be made generic. We would still have a common location of /sys/firmware/security and new code would live in arch. This is my preference and would be the best fit for our use case. 2. Use securityfs. This would mean modifying it to satisfy other use cases, including supporting userspace file creation. I don't know if the securityfs maintainer would find that acceptable. I would also still want some way to expose variables at /sys/firmware/security. 3. Use a sysfs-based approach. This would be a platform-specific implementation. However, sysfs has a similar issue to securityfs for file creation. When I tried it in RFC v1[1], I had to implement a workaround to achieve that. [1] https://lore.kernel.org/linuxppc-dev/20220122005637.28199-3-na...@linux.ibm.com/ Hi Greg, Based on the discussions so far, is Option 1, described above, an acceptable next step? No, as I said almost a year ago, I do not want to see platform-only filesystems going and implementing stuff that should be shared by all platforms. Given there are no other exploiters for fwsecurityfs and there should be no platform-specific fs, would modifying sysfs now to let userspace create files cleanly be the way forward? Or, if we should strongly consider securityfs, which would result in updating securityfs to allow userspace creation of files and then expose variables via a more platform-specific directory /sys/kernel/security/pks? We want to pick the best available option and would find some hints on direction helpful before we develop the next patch. Thanks & Regards, - Nayna
Re: [PATCH 10/13] powerpc/rtas: improve function information lookups
On 11/22/22 20:51, Andrew Donnellan wrote: On Fri, 2022-11-18 at 09:07 -0600, Nathan Lynch wrote: +enum rtas_function_flags { + RTAS_FN_FLAG_BANNED_FOR_SYSCALL_ON_LE = (1 << 0), +}; This seems to be new, what's the justification? Seems to be a run-time replacement of: #ifdef CONFIG_CPU_BIG_ENDIAN { "ibm,suspend-me", -1, -1, -1, -1, -1 }, { "ibm,update-nodes", -1, 0, -1, -1, -1, 4096 }, { "ibm,update-properties", -1, 0, -1, -1, -1, 4096 }, #endif It looks to be handled logically: + if (IS_ENABLED(CONFIG_CPU_LITTLE_ENDIAN) && + (func->flags & RTAS_FN_FLAG_BANNED_FOR_SYSCALL_ON_LE)) + goto err; Perhaps, also allow the addition of any future special cases for rtas functions easier to maintain?
Re: [PATCH 10/13] powerpc/rtas: improve function information lookups
On 11/18/22 09:07, Nathan Lynch wrote: +static int __init rtas_token_to_function_xarray_init(void) +{ + int err = 0; + + for (size_t i = 0; i < ARRAY_SIZE(rtas_function_table); ++i) { + const struct rtas_function *func = &rtas_function_table[i]; + const s32 token = func->token; + + if (token == RTAS_UNKNOWN_SERVICE) + continue; + + err = xa_err(xa_store(&rtas_token_to_function_xarray, + token, (void *)func, GFP_KERNEL)); + if (err) + break; + } + + return err; +} +arch_initcall(rtas_token_to_function_xarray_init); + +static const struct rtas_function *rtas_token_to_function(s32 token) +{ + const struct rtas_function *func; + + if (WARN_ONCE(token < 0, "invalid token %d", token)) + return NULL; + + func = xa_load(&rtas_token_to_function_xarray, (unsigned long)token); + Why typecast token here and not in xa_store? +static void __init rtas_function_table_init(void) +{ + struct property *prop; + + for (size_t i = 0; i < ARRAY_SIZE(rtas_function_table); ++i) { + struct rtas_function *curr = &rtas_function_table[i]; + struct rtas_function *prior; + int cmp; + + curr->token = RTAS_UNKNOWN_SERVICE; + + if (i == 0) + continue; + /* +* Ensure table is sorted correctly for binary search +* on function names. +*/ + prior = &rtas_function_table[i - 1]; + + cmp = strcmp(prior->name, curr->name); + if (cmp < 0) + continue; + + if (cmp == 0) { + pr_err("'%s' has duplicate function table entries\n", + curr->name); + } else { + pr_err("function table unsorted: '%s' wrongly precedes '%s'\n", + prior->name, curr->name); + } + } Just a thought, would it be simpler to use sort()? you already have the cmp_func implemented for bsearch(). As for the series as a whole: I am no RTAS expert but was able to build, boot and mess around with new tracepoints without errors: Tested-by: Nick Child
Re: [PATCH linux-next][RFC]torture: avoid offline tick_do_timer_cpu
On Mon, Nov 21, 2022 at 05:37:54PM -0800, Paul E. McKenney wrote: > On Mon, Nov 21, 2022 at 11:51:40AM +0800, Zhouyi Zhou wrote: > > @@ -358,7 +359,16 @@ torture_onoff(void *arg) > > schedule_timeout_interruptible(HZ / 10); > > continue; > > } > > +#ifdef CONFIG_NO_HZ_FULL > > + /* do not offline tick do timer cpu */ > > + if (tick_nohz_full_running) { > > + cpu = (torture_random(&rand) >> 4) % maxcpu; > > + if (cpu >= tick_do_timer_cpu) > > Why is this ">=" instead of "=="? > > > + cpu = (cpu + 1) % (maxcpu + 1); > > + } else > > +#else > > cpu = (torture_random(&rand) >> 4) % (maxcpu + 1); > > +#endif > > What happens if the value of tick_do_timer_cpu changes between the time of > the check above and the call to torture_offline() below? Alternatively, > how is such a change in value prevented? It can't, currently tick_do_timer_cpu is fixed when nohz_full is running. It can however have special values at early boot such as TICK_DO_TIMER_NONE. But if rcutorture is initialized after smp, it should be ok. Thanks. > > Thanx, Paul > > > if (!torture_offline(cpu, > > &n_offline_attempts, &n_offline_successes, > > &sum_offline, &min_offline, &max_offline)) > > -- > > 2.34.1 > >
Re: [PATCH linux-next][RFC]torture: avoid offline tick_do_timer_cpu
On Mon, Nov 21, 2022 at 11:51:40AM +0800, Zhouyi Zhou wrote: > During CPU-hotplug torture (CONFIG_NO_HZ_FULL=y), if we try to > offline tick_do_timer_cpu, the operation will fail because in > function tick_nohz_cpu_down: > ``` > if (tick_nohz_full_running && tick_do_timer_cpu == cpu) > return -EBUSY; > ``` > Above bug was first discovered in torture tests performed in PPC VM > of Open Source Lab of Oregon State University, and reproducable in RISC-V > and X86-64 (with additional kernel commandline cpu0_hotplug). > > In this patch, we avoid offline tick_do_timer_cpu by distribute > the offlining cpu among remaining cpus. > > Signed-off-by: Zhouyi Zhou > --- > include/linux/tick.h| 1 + > kernel/time/tick-common.c | 1 + > kernel/time/tick-internal.h | 1 - > kernel/torture.c| 10 ++ > 4 files changed, 12 insertions(+), 1 deletion(-) > > diff --git a/include/linux/tick.h b/include/linux/tick.h > index bfd571f18cfd..23cc0b205853 100644 > --- a/include/linux/tick.h > +++ b/include/linux/tick.h > @@ -14,6 +14,7 @@ > #include > > #ifdef CONFIG_GENERIC_CLOCKEVENTS > +extern int tick_do_timer_cpu __read_mostly; > extern void __init tick_init(void); > /* Should be core only, but ARM BL switcher requires it */ > extern void tick_suspend_local(void); > diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c > index 46789356f856..87b9b9afa320 100644 > --- a/kernel/time/tick-common.c > +++ b/kernel/time/tick-common.c > @@ -48,6 +48,7 @@ ktime_t tick_next_period; > *procedure also covers cpu hotplug. > */ > int tick_do_timer_cpu __read_mostly = TICK_DO_TIMER_BOOT; > +EXPORT_SYMBOL_GPL(tick_do_timer_cpu); Please rather make a function for this. This is an internal value that we don't want to expose to modules. This can be: int tick_nohz_full_timekeeper(void) { if (tick_nohz_full_enabled() && tick_do_timer_cpu >= 0) return tick_do_timer_cpu; else return nr_cpu_ids; } And then just check if the value is below nr_cpu_ids. Thanks.
Re: [PATCH linux-next][RFC]torture: avoid offline tick_do_timer_cpu
On Wed, Nov 23, 2022 at 11:25:43PM +0100, Frederic Weisbecker wrote: > On Mon, Nov 21, 2022 at 05:37:54PM -0800, Paul E. McKenney wrote: > > On Mon, Nov 21, 2022 at 11:51:40AM +0800, Zhouyi Zhou wrote: > > > @@ -358,7 +359,16 @@ torture_onoff(void *arg) > > > schedule_timeout_interruptible(HZ / 10); > > > continue; > > > } > > > +#ifdef CONFIG_NO_HZ_FULL > > > + /* do not offline tick do timer cpu */ > > > + if (tick_nohz_full_running) { > > > + cpu = (torture_random(&rand) >> 4) % maxcpu; > > > + if (cpu >= tick_do_timer_cpu) > > > > Why is this ">=" instead of "=="? > > > > > + cpu = (cpu + 1) % (maxcpu + 1); > > > + } else > > > +#else > > > cpu = (torture_random(&rand) >> 4) % (maxcpu + 1); > > > +#endif > > > > What happens if the value of tick_do_timer_cpu changes between the time of > > the check above and the call to torture_offline() below? Alternatively, > > how is such a change in value prevented? > > It can't, currently tick_do_timer_cpu is fixed when nohz_full is running. > It can however have special values at early boot such as TICK_DO_TIMER_NONE. > But if rcutorture is initialized after smp, it should be ok. Ah, getting ahead of myself, thank you for the info! So the thing to do would be to generate only maxcpu-1 choices. Thanx, Paul > Thanks. > > > > > Thanx, Paul > > > > > if (!torture_offline(cpu, > > >&n_offline_attempts, &n_offline_successes, > > >&sum_offline, &min_offline, &max_offline)) > > > -- > > > 2.34.1 > > >
linux-next: manual merge of the tip tree with the powerpc-objtool tree
Hi all, Today's linux-next merge of the tip tree got a conflict in: tools/objtool/check.c between commit: efb11fdb3e1a ("objtool: Fix SEGFAULT") from the powerpc-objtool tree and commit: dbcdbdfdf137 ("objtool: Rework instruction -> symbol mapping") from the tip tree. I fixed it up (see below) and can carry the fix as necessary. This is now fixed as far as linux-next is concerned, but any non trivial conflicts should be mentioned to your upstream maintainer when your tree is submitted for merging. You may also want to consider cooperating with the maintainer of the conflicting tree to minimise any particularly complex conflicts. -- Cheers, Stephen Rothwell diff --cc tools/objtool/check.c index 7580c66ca5c8,4f1a7384426b.. --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@@ -207,7 -204,7 +204,7 @@@ static bool __dead_end_function(struct return false; insn = find_insn(file, func->sec, func->offset); - if (!insn || !insn->func) - if (!insn_func(insn)) ++ if (!insn || !insn_func(insn)) return false; func_for_each_insn(file, func, insn) { @@@ -850,11 -861,73 +861,73 @@@ static int create_ibt_endbr_seal_sectio return 0; } + static int create_cfi_sections(struct objtool_file *file) + { + struct section *sec, *s; + struct symbol *sym; + unsigned int *loc; + int idx; + + sec = find_section_by_name(file->elf, ".cfi_sites"); + if (sec) { + INIT_LIST_HEAD(&file->call_list); + WARN("file already has .cfi_sites section, skipping"); + return 0; + } + + idx = 0; + for_each_sec(file, s) { + if (!s->text) + continue; + + list_for_each_entry(sym, &s->symbol_list, list) { + if (sym->type != STT_FUNC) + continue; + + if (strncmp(sym->name, "__cfi_", 6)) + continue; + + idx++; + } + } + + sec = elf_create_section(file->elf, ".cfi_sites", 0, sizeof(unsigned int), idx); + if (!sec) + return -1; + + idx = 0; + for_each_sec(file, s) { + if (!s->text) + continue; + + list_for_each_entry(sym, &s->symbol_list, list) { + if (sym->type != STT_FUNC) + continue; + + if (strncmp(sym->name, "__cfi_", 6)) + continue; + + loc = (unsigned int *)sec->data->d_buf + idx; + memset(loc, 0, sizeof(unsigned int)); + + if (elf_add_reloc_to_insn(file->elf, sec, + idx * sizeof(unsigned int), + R_X86_64_PC32, + s, sym->offset)) + return -1; + + idx++; + } + } + + return 0; + } + static int create_mcount_loc_sections(struct objtool_file *file) { - struct section *sec; - unsigned long *loc; + int addrsize = elf_class_addrsize(file->elf); struct instruction *insn; + struct section *sec; int idx; sec = find_section_by_name(file->elf, "__mcount_loc"); pgplnzELKulsx.pgp Description: OpenPGP digital signature
Re: [PATCH mm-unstable v1 09/20] mm/gup: reliable R/O long-term pinning in COW mappings
On 11/16/22 02:26, David Hildenbrand wrote: ... With this change, the new R/O long-term pinning tests for non-anonymous memory succeed: # [RUN] R/O longterm GUP pin ... with shared zeropage ok 151 Longterm R/O pin is reliable # [RUN] R/O longterm GUP pin ... with memfd ok 152 Longterm R/O pin is reliable # [RUN] R/O longterm GUP pin ... with tmpfile ok 153 Longterm R/O pin is reliable # [RUN] R/O longterm GUP pin ... with huge zeropage ok 154 Longterm R/O pin is reliable # [RUN] R/O longterm GUP pin ... with memfd hugetlb (2048 kB) ok 155 Longterm R/O pin is reliable # [RUN] R/O longterm GUP pin ... with memfd hugetlb (1048576 kB) ok 156 Longterm R/O pin is reliable # [RUN] R/O longterm GUP-fast pin ... with shared zeropage ok 157 Longterm R/O pin is reliable # [RUN] R/O longterm GUP-fast pin ... with memfd ok 158 Longterm R/O pin is reliable # [RUN] R/O longterm GUP-fast pin ... with tmpfile ok 159 Longterm R/O pin is reliable # [RUN] R/O longterm GUP-fast pin ... with huge zeropage ok 160 Longterm R/O pin is reliable # [RUN] R/O longterm GUP-fast pin ... with memfd hugetlb (2048 kB) ok 161 Longterm R/O pin is reliable # [RUN] R/O longterm GUP-fast pin ... with memfd hugetlb (1048576 kB) ok 162 Longterm R/O pin is reliable Yes. I was able to reproduce these results, after some minor distractions involving huge pages, don't ask. :) Note 1: We don't care about short-term R/O-pinning, because they have snapshot semantics: they are not supposed to observe modifications that happen after pinning. As one example, assume we start direct I/O to read from a page and store page content into a file: modifications to page content after starting direct I/O are not guaranteed to end up in the file. So even if we'd pin the shared zeropage, the end result would be as expected -- getting zeroes stored to the file. Note 2: For shared mappings we'll now always fallback to the slow path to lookup the VMA when R/O long-term pining. While that's the necessary price we have to pay right now, it's actually not that bad in practice: most FOLL_LONGTERM users already specify FOLL_WRITE, for example, along with FOLL_FORCE because they tried dealing with COW mappings correctly ... Note 3: For users that use FOLL_LONGTERM right now without FOLL_WRITE, such as VFIO, we'd now no longer pin the shared zeropage. Instead, we'd populate exclusive anon pages that we can pin. There was a concern that this could affect the memlock limit of existing setups. For example, a VM running with VFIO could run into the memlock limit and fail to run. However, we essentially had the same behavior already in commit 17839856fd58 ("gup: document and work around "COW can break either way" issue") which got merged into some enterprise distros, and there were not any such complaints. So most probably, we're fine. Signed-off-by: David Hildenbrand --- include/linux/mm.h | 27 --- mm/gup.c | 10 +- mm/huge_memory.c | 2 +- mm/hugetlb.c | 7 --- 4 files changed, 34 insertions(+), 12 deletions(-) Looks good, Reviewed-by: John Hubbard thanks, -- John Hubbard NVIDIA diff --git a/include/linux/mm.h b/include/linux/mm.h index 6bd2ee5872dd..e8cc838f42f9 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -3095,8 +3095,12 @@ static inline int vm_fault_to_errno(vm_fault_t vm_fault, int foll_flags) * Must be called with the (sub)page that's actually referenced via the * page table entry, which might not necessarily be the head page for a * PTE-mapped THP. + * + * If the vma is NULL, we're coming from the GUP-fast path and might have + * to fallback to the slow path just to lookup the vma. */ -static inline bool gup_must_unshare(unsigned int flags, struct page *page) +static inline bool gup_must_unshare(struct vm_area_struct *vma, + unsigned int flags, struct page *page) { /* * FOLL_WRITE is implicitly handled correctly as the page table entry @@ -3109,8 +3113,25 @@ static inline bool gup_must_unshare(unsigned int flags, struct page *page) * Note: PageAnon(page) is stable until the page is actually getting * freed. */ - if (!PageAnon(page)) - return false; + if (!PageAnon(page)) { + /* +* We only care about R/O long-term pining: R/O short-term +* pinning does not have the semantics to observe successive +* changes through the process page tables. +*/ + if (!(flags & FOLL_LONGTERM)) + return false; + + /* We really need the vma ... */ + if (!vma) + return true; + + /* +* ... because we only care about writable private ("COW") +* mappings where we have to break COW early. +*/
[powerpc:merge] BUILD SUCCESS 409ecd2dbb26f16b778803e8ce148a61aa6418d3
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git merge branch HEAD: 409ecd2dbb26f16b778803e8ce148a61aa6418d3 Automatic merge of 'master' into merge (2022-11-23 21:27) elapsed time: 730m configs tested: 63 configs skipped: 2 The following configs have been built successfully. More configs may be tested in the coming days. gcc tested configs: powerpc allnoconfig arc defconfig alpha defconfig s390 allmodconfig um i386_defconfig s390defconfig um x86_64_defconfig sh allmodconfig s390 allyesconfig mips allyesconfig powerpc allmodconfig ia64 allmodconfig x86_64 rhel-8.3-func x86_64rhel-8.3-kselftests alphaallyesconfig m68k allmodconfig arc allyesconfig i386 randconfig-a014 i386 randconfig-a012 i386 randconfig-a016 m68k allyesconfig x86_64 rhel-8.3-syz x86_64 rhel-8.3-kunit x86_64 rhel-8.3-kvm x86_64 defconfig x86_64 rhel-8.3 x86_64 allyesconfig i386defconfig x86_64 randconfig-a012-20221121 x86_64 randconfig-a011-20221121 x86_64 randconfig-a015-20221121 x86_64 randconfig-a013-20221121 x86_64 randconfig-a014-20221121 x86_64 randconfig-a016-20221121 i386 allyesconfig arc randconfig-r043-20221120 arc randconfig-r043-20221121 s390 randconfig-r044-20221121 riscvrandconfig-r042-20221121 arm defconfig arm64allyesconfig arm allyesconfig clang tested configs: i386 randconfig-a013 i386 randconfig-a011 i386 randconfig-a015 x86_64 randconfig-a001-20221121 x86_64 randconfig-a003-20221121 x86_64 randconfig-a002-20221121 x86_64 randconfig-a004-20221121 x86_64 randconfig-a006-20221121 x86_64 randconfig-a005-20221121 hexagon randconfig-r041-20221120 hexagon randconfig-r041-20221121 hexagon randconfig-r045-20221120 hexagon randconfig-r045-20221121 i386 randconfig-a004-20221121 riscvrandconfig-r042-20221120 s390 randconfig-r044-20221120 i386 randconfig-a003-20221121 i386 randconfig-a001-20221121 i386 randconfig-a002-20221121 i386 randconfig-a005-20221121 i386 randconfig-a006-20221121 -- 0-DAY CI Kernel Test Service https://01.org/lkp
Re: [PATCH linux-next][RFC]torture: avoid offline tick_do_timer_cpu
On Thu, Nov 24, 2022 at 6:37 AM Frederic Weisbecker wrote: > > On Mon, Nov 21, 2022 at 11:51:40AM +0800, Zhouyi Zhou wrote: > > During CPU-hotplug torture (CONFIG_NO_HZ_FULL=y), if we try to > > offline tick_do_timer_cpu, the operation will fail because in > > function tick_nohz_cpu_down: > > ``` > > if (tick_nohz_full_running && tick_do_timer_cpu == cpu) > > return -EBUSY; > > ``` > > Above bug was first discovered in torture tests performed in PPC VM > > of Open Source Lab of Oregon State University, and reproducable in RISC-V > > and X86-64 (with additional kernel commandline cpu0_hotplug). > > > > In this patch, we avoid offline tick_do_timer_cpu by distribute > > the offlining cpu among remaining cpus. > > > > Signed-off-by: Zhouyi Zhou > > --- > > include/linux/tick.h| 1 + > > kernel/time/tick-common.c | 1 + > > kernel/time/tick-internal.h | 1 - > > kernel/torture.c| 10 ++ > > 4 files changed, 12 insertions(+), 1 deletion(-) > > > > diff --git a/include/linux/tick.h b/include/linux/tick.h > > index bfd571f18cfd..23cc0b205853 100644 > > --- a/include/linux/tick.h > > +++ b/include/linux/tick.h > > @@ -14,6 +14,7 @@ > > #include > > > > #ifdef CONFIG_GENERIC_CLOCKEVENTS > > +extern int tick_do_timer_cpu __read_mostly; > > extern void __init tick_init(void); > > /* Should be core only, but ARM BL switcher requires it */ > > extern void tick_suspend_local(void); > > diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c > > index 46789356f856..87b9b9afa320 100644 > > --- a/kernel/time/tick-common.c > > +++ b/kernel/time/tick-common.c > > @@ -48,6 +48,7 @@ ktime_t tick_next_period; > > *procedure also covers cpu hotplug. > > */ > > int tick_do_timer_cpu __read_mostly = TICK_DO_TIMER_BOOT; > > +EXPORT_SYMBOL_GPL(tick_do_timer_cpu); > > Please rather make a function for this. This is an internal value > that we don't want to expose to modules. > > This can be: > > int tick_nohz_full_timekeeper(void) > { > if (tick_nohz_full_enabled() && tick_do_timer_cpu >= 0) > return tick_do_timer_cpu; > else > return nr_cpu_ids; > } > > And then just check if the value is below nr_cpu_ids. Thank Paul and Frederic both for your guidance! Things are much easier;-) and I will do it. Cheers Zhouyi > > Thanks.
Re: [PATCH linux-next][RFC]torture: avoid offline tick_do_timer_cpu
On Thu, Nov 24, 2022 at 2:49 AM Paul E. McKenney wrote: > > On Wed, Nov 23, 2022 at 10:23:11AM +0800, Zhouyi Zhou wrote: > > On Tue, Nov 22, 2022 at 9:37 AM Paul E. McKenney wrote: > > > > > > On Mon, Nov 21, 2022 at 11:51:40AM +0800, Zhouyi Zhou wrote: > > > > During CPU-hotplug torture (CONFIG_NO_HZ_FULL=y), if we try to > > > > offline tick_do_timer_cpu, the operation will fail because in > > > > function tick_nohz_cpu_down: > > > > ``` > > > > if (tick_nohz_full_running && tick_do_timer_cpu == cpu) > > > > return -EBUSY; > > > > ``` > > > > Above bug was first discovered in torture tests performed in PPC VM > > > > of Open Source Lab of Oregon State University, and reproducable in > > > > RISC-V > > > > and X86-64 (with additional kernel commandline cpu0_hotplug). > > > > > > > > In this patch, we avoid offline tick_do_timer_cpu by distribute > > > > the offlining cpu among remaining cpus. > > > > > > > > Signed-off-by: Zhouyi Zhou > > > > > > Good show chasing this down! > > Thank Paul for your guidance and encouragement! > > > > > > A couple of questions below. > > The answers below. > > > > > > > --- > > > > include/linux/tick.h| 1 + > > > > kernel/time/tick-common.c | 1 + > > > > kernel/time/tick-internal.h | 1 - > > > > kernel/torture.c| 10 ++ > > > > 4 files changed, 12 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/include/linux/tick.h b/include/linux/tick.h > > > > index bfd571f18cfd..23cc0b205853 100644 > > > > --- a/include/linux/tick.h > > > > +++ b/include/linux/tick.h > > > > @@ -14,6 +14,7 @@ > > > > #include > > > > > > > > #ifdef CONFIG_GENERIC_CLOCKEVENTS > > > > +extern int tick_do_timer_cpu __read_mostly; > > > > extern void __init tick_init(void); > > > > /* Should be core only, but ARM BL switcher requires it */ > > > > extern void tick_suspend_local(void); > > > > diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c > > > > index 46789356f856..87b9b9afa320 100644 > > > > --- a/kernel/time/tick-common.c > > > > +++ b/kernel/time/tick-common.c > > > > @@ -48,6 +48,7 @@ ktime_t tick_next_period; > > > > *procedure also covers cpu hotplug. > > > > */ > > > > int tick_do_timer_cpu __read_mostly = TICK_DO_TIMER_BOOT; > > > > +EXPORT_SYMBOL_GPL(tick_do_timer_cpu); > > > > #ifdef CONFIG_NO_HZ_FULL > > > > /* > > > > * tick_do_timer_boot_cpu indicates the boot CPU temporarily owns > > > > diff --git a/kernel/time/tick-internal.h b/kernel/time/tick-internal.h > > > > index 649f2b48e8f0..8953dca10fdd 100644 > > > > --- a/kernel/time/tick-internal.h > > > > +++ b/kernel/time/tick-internal.h > > > > @@ -15,7 +15,6 @@ > > > > > > > > DECLARE_PER_CPU(struct tick_device, tick_cpu_device); > > > > extern ktime_t tick_next_period; > > > > -extern int tick_do_timer_cpu __read_mostly; > > > > > > > > extern void tick_setup_periodic(struct clock_event_device *dev, int > > > > broadcast); > > > > extern void tick_handle_periodic(struct clock_event_device *dev); > > > > diff --git a/kernel/torture.c b/kernel/torture.c > > > > index 789aeb0e1159..bccbdd33dda2 100644 > > > > --- a/kernel/torture.c > > > > +++ b/kernel/torture.c > > > > @@ -33,6 +33,7 @@ > > > > #include > > > > #include > > > > #include > > > > +#include > > > > #include > > > > #include > > > > #include > > > > @@ -358,7 +359,16 @@ torture_onoff(void *arg) > > > > schedule_timeout_interruptible(HZ / 10); > > > > continue; > > > > } > > > > +#ifdef CONFIG_NO_HZ_FULL > > > > + /* do not offline tick do timer cpu */ > > > > + if (tick_nohz_full_running) { > > > > + cpu = (torture_random(&rand) >> 4) % maxcpu; by examine the beginning code of torture_onoff, I see if we has 8 cpus, then maxcpu = 7 (not 8) here, then cpu is distributed evenly among 0, 1, 2, 3, 4, 5, 6 if we happens to get 6, then cpu+1 below results in 7 > > > > + if (cpu >= tick_do_timer_cpu) > > > > > > Why is this ">=" instead of "=="? > > I use probability theory here to let the remaining cpu distribute evenly. > > Example: > > we have cpus: 0 1 2 3 4 5 6 7 > > maxcpu = 7 > > tick_do_timer_cpu = 2 > > remaining cpus are: 0 1 3 4 5 6 7 > > if the offline cpu candidate is 2, then the result cpu is 2+1 > > else if the offline cpu candidate is 3, then the result cpu is 3+1 > > ... > > else if the offline cpu candidate is 6, then the result cpu is 6+1 > > > > > > > + cpu = (cpu + 1) % (maxcpu + 1); > > we could just use cpu = cpu + 1 here > > But won't this get you double the occurrences of CPU 0 compared to the > other non-tick_do_timer_cpu CPUs? You might get CPU 0 directly from > torture_random(), or torture_random() might have given you CPU 7, which > then wraps to CPU 0. I think torture_random won't give me CPU 7 as illustrated above, my code is a little tricky, please correct me if I am wrong. > > What am I missing
Re: [PATCH 10/13] powerpc/rtas: improve function information lookups
On Wed, 2022-11-23 at 13:32 -0600, Nick Child wrote: > On 11/22/22 20:51, Andrew Donnellan wrote: > > On Fri, 2022-11-18 at 09:07 -0600, Nathan Lynch wrote: > > > +enum rtas_function_flags { > > > + RTAS_FN_FLAG_BANNED_FOR_SYSCALL_ON_LE = (1 << 0), > > > +}; > > > > This seems to be new, what's the justification? > > > > Seems to be a run-time replacement of: > #ifdef CONFIG_CPU_BIG_ENDIAN > { "ibm,suspend-me", -1, -1, -1, -1, -1 }, > { "ibm,update-nodes", -1, 0, -1, -1, -1, 4096 }, > { "ibm,update-properties", -1, 0, -1, -1, -1, 4096 }, > #endif > > It looks to be handled logically: > + if (IS_ENABLED(CONFIG_CPU_LITTLE_ENDIAN) && > + (func->flags & RTAS_FN_FLAG_BANNED_FOR_SYSCALL_ON_LE)) > + goto err; > > Perhaps, also allow the addition of any future special cases > for rtas functions easier to maintain? Makes sense, though I'm slightly confused about the original rationale for the ifdef and why it's not being fixed in userspace. Slightly clunky name though, something like RTAS_FN_FLAG_SYSCALL_BE_ONLY might be less clunky? -- Andrew DonnellanOzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
[PATCH] powerpc/mpc52xx_lpbfifo: fix all kernel-doc warnings
Fix multiple kernel-doc warnings in mpc52xx_lpbfifo.c: arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c:377: warning: expecting prototype for mpc52xx_lpbfifo_bcom_poll(). Prototype was for mpc52xx_lpbfifo_poll() instead mpc52xx_lpbfifo.c:221: warning: No description found for return value of 'mpc52xx_lpbfifo_irq' mpc52xx_lpbfifo.c:327: warning: No description found for return value of 'mpc52xx_lpbfifo_bcom_irq' mpc52xx_lpbfifo.c:398: warning: No description found for return value of 'mpc52xx_lpbfifo_submit' mpc52xx_lpbfifo.c:64: warning: Function parameter or member 'req' not described in 'mpc52xx_lpbfifo_kick' mpc52xx_lpbfifo.c:220: warning: contents before sections mpc52xx_lpbfifo.c:223: warning: Function parameter or member 'irq' not described in 'mpc52xx_lpbfifo_irq' mpc52xx_lpbfifo.c:223: warning: Function parameter or member 'dev_id' not described in 'mpc52xx_lpbfifo_irq' mpc52xx_lpbfifo.c:328: warning: contents before sections mpc52xx_lpbfifo.c:331: warning: Function parameter or member 'irq' not described in 'mpc52xx_lpbfifo_bcom_irq' mpc52xx_lpbfifo.c:331: warning: Function parameter or member 'dev_id' not described in 'mpc52xx_lpbfifo_bcom_irq' Reported-by: kernel test robot Signed-off-by: Randy Dunlap Cc: Mauro Carvalho Chehab Cc: John Bonesio Cc: Anatolij Gustschin Cc: linuxppc-dev@lists.ozlabs.org Cc: Michael Ellerman --- arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c | 14 +- 1 file changed, 13 insertions(+), 1 deletion(-) diff -- a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c b/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c --- a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c +++ b/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c @@ -59,6 +59,8 @@ static struct mpc52xx_lpbfifo lpbfifo; /** * mpc52xx_lpbfifo_kick - Trigger the next block of data to be transferred + * + * @req: Pointer to request structure */ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfifo_request *req) { @@ -178,6 +180,8 @@ static void mpc52xx_lpbfifo_kick(struct /** * mpc52xx_lpbfifo_irq - IRQ handler for LPB FIFO + * @irq: IRQ number to be handled + * @dev_id: device ID cookie * * On transmit, the dma completion irq triggers before the fifo completion * triggers. Handle the dma completion here instead of the LPB FIFO Bestcomm @@ -216,6 +220,8 @@ static void mpc52xx_lpbfifo_kick(struct * or nested spinlock condition. The out path is non-trivial, so * extra fiddling is done to make sure all paths lead to the same * outbound code. + * + * Return: irqreturn code (%IRQ_HANDLED) */ static irqreturn_t mpc52xx_lpbfifo_irq(int irq, void *dev_id) { @@ -320,8 +326,12 @@ static irqreturn_t mpc52xx_lpbfifo_irq(i /** * mpc52xx_lpbfifo_bcom_irq - IRQ handler for LPB FIFO Bestcomm task + * @irq: IRQ number to be handled + * @dev_id: device ID cookie * * Only used when receiving data. + * + * Return: irqreturn code (%IRQ_HANDLED) */ static irqreturn_t mpc52xx_lpbfifo_bcom_irq(int irq, void *dev_id) { @@ -372,7 +382,7 @@ static irqreturn_t mpc52xx_lpbfifo_bcom_ } /** - * mpc52xx_lpbfifo_bcom_poll - Poll for DMA completion + * mpc52xx_lpbfifo_poll - Poll for DMA completion */ void mpc52xx_lpbfifo_poll(void) { @@ -393,6 +403,8 @@ EXPORT_SYMBOL(mpc52xx_lpbfifo_poll); /** * mpc52xx_lpbfifo_submit - Submit an LPB FIFO transfer request. * @req: Pointer to request structure + * + * Return: %0 on success, -errno code on error */ int mpc52xx_lpbfifo_submit(struct mpc52xx_lpbfifo_request *req) {
Re: linux-next: manual merge of the tip tree with the powerpc-objtool tree
Le 24/11/2022 à 02:29, Stephen Rothwell a écrit : > Hi all, > > Today's linux-next merge of the tip tree got a conflict in: > >tools/objtool/check.c > > between commit: > >efb11fdb3e1a ("objtool: Fix SEGFAULT") > > from the powerpc-objtool tree and commit: > >dbcdbdfdf137 ("objtool: Rework instruction -> symbol mapping") > > from the tip tree. > > I fixed it up (see below) and can carry the fix as necessary. This > is now fixed as far as linux-next is concerned, but any non trivial > conflicts should be mentioned to your upstream maintainer when your tree > is submitted for merging. You may also want to consider cooperating > with the maintainer of the conflicting tree to minimise any particularly > complex conflicts. > Maybe it would be better to perform the check of insn inside the new insn_func() then ? Christophe