Re: [PATCH -next] fbdev: offb: allow build when DRM_OFDRM=m

2022-11-23 Thread Thomas Zimmermann


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

2022-11-23 Thread kajoljain



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

2022-11-23 Thread Michal Suchánek
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

2022-11-23 Thread Arnd Bergmann
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

2022-11-23 Thread Greg Kroah-Hartman
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

2022-11-23 Thread 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?

> 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

2022-11-23 Thread Thomas Zimmermann

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

2022-11-23 Thread zhang.songyi
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

2022-11-23 Thread Mark Brown
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

2022-11-23 Thread Hans Verkuil
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

2022-11-23 Thread Hans Verkuil
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

2022-11-23 Thread Hans Verkuil
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

2022-11-23 Thread Arnaldo Carvalho de Melo
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

2022-11-23 Thread Christophe Leroy
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

2022-11-23 Thread Anshuman Khandual



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

2022-11-23 Thread Hans Verkuil
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

2022-11-23 Thread Nayna



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

2022-11-23 Thread Sourabh Jain



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

2022-11-23 Thread Greg Kroah-Hartman
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

2022-11-23 Thread Randy Dunlap



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

2022-11-23 Thread Mark Brown
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

2022-11-23 Thread Paul E. McKenney
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

2022-11-23 Thread Nayna



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

2022-11-23 Thread Nick Child

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

2022-11-23 Thread Nick Child




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

2022-11-23 Thread Frederic Weisbecker
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

2022-11-23 Thread Frederic Weisbecker
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

2022-11-23 Thread Paul E. McKenney
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

2022-11-23 Thread Stephen Rothwell
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

2022-11-23 Thread John Hubbard

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

2022-11-23 Thread kernel test robot
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

2022-11-23 Thread Zhouyi Zhou
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

2022-11-23 Thread Zhouyi Zhou
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

2022-11-23 Thread Andrew Donnellan
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

2022-11-23 Thread Randy Dunlap
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

2022-11-23 Thread Christophe Leroy


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