Re: [BUG] Build failure and alleged fix for next-20240523

[Paul E. McKenney]

On Thu, May 30, 2024 at 08:28:17PM +0200, Thierry Reding wrote:
> On Thu May 30, 2024 at 6:55 PM CEST, Paul E. McKenney wrote:
> > On Fri, May 24, 2024 at 12:57:58PM -0700, Abhinav Kumar wrote:
> > > Hello
> > > 
> > > On 5/24/2024 12:55 PM, Paul E. McKenney wrote:
> > > > Hello!
> > > > 
> > > > I get the following allmodconfig build error on x86 in next-20240523:
> > > > 
> > > > Traceback (most recent call last):
> > > >    File "drivers/gpu/drm/msm/registers/gen_header.py", line 970, in 
> > > > 
> > > >  main()
> > > >    File "drivers/gpu/drm/msm/registers/gen_header.py", line 951, in main
> > > >  parser.add_argument('--validate', 
> > > > action=argparse.BooleanOptionalAction)
> > > > AttributeError: module 'argparse' has no attribute 
> > > > 'BooleanOptionalAction'
> > > > 
> > > > The following patch allows the build to complete successfully:
> > > > 
> > > > https://patchwork.kernel.org/project/dri-devel/patch/20240508091751.336654-1-jonath...@nvidia.com/#25842751
> > > > 
> > > > As to whether this is a proper fix, I must defer to the DRM folks on CC.
> > > 
> > > Thanks for the report.
> > > 
> > > I have raised a merge request for
> > > https://patchwork.freedesktop.org/patch/593057/ to make it available for 
> > > the
> > > next fixes release for msm.
> >
> > Thank you!
> >
> > This still is not in -next, so I am putting it into -rcu just to silence
> > the diagnostic.  Or should I push this to mainline via -rcu?
> 
> I pushed this to drm-misc-fixes earlier today, so should show up in
> linux-next soon and hopefully in v6.10-rc2.

Thank you, Thierry!

Thanx, Paul

Re: [BUG] Build failure and alleged fix for next-20240523

[Paul E. McKenney]

On Fri, May 24, 2024 at 12:57:58PM -0700, Abhinav Kumar wrote:
> Hello
> 
> On 5/24/2024 12:55 PM, Paul E. McKenney wrote:
> > Hello!
> > 
> > I get the following allmodconfig build error on x86 in next-20240523:
> > 
> > Traceback (most recent call last):
> >    File "drivers/gpu/drm/msm/registers/gen_header.py", line 970, in 
> >  main()
> >    File "drivers/gpu/drm/msm/registers/gen_header.py", line 951, in main
> >  parser.add_argument('--validate', 
> > action=argparse.BooleanOptionalAction)
> > AttributeError: module 'argparse' has no attribute 'BooleanOptionalAction'
> > 
> > The following patch allows the build to complete successfully:
> > 
> > https://patchwork.kernel.org/project/dri-devel/patch/20240508091751.336654-1-jonath...@nvidia.com/#25842751
> > 
> > As to whether this is a proper fix, I must defer to the DRM folks on CC.
> 
> Thanks for the report.
> 
> I have raised a merge request for
> https://patchwork.freedesktop.org/patch/593057/ to make it available for the
> next fixes release for msm.

Thank you!

This still is not in -next, so I am putting it into -rcu just to silence
the diagnostic.  Or should I push this to mainline via -rcu?

Thanx, Paul

[BUG] Build failure and alleged fix for next-20240523

[Paul E. McKenney]

Hello!

I get the following allmodconfig build error on x86 in next-20240523:

Traceback (most recent call last):
  File "drivers/gpu/drm/msm/registers/gen_header.py", line 970, in 
main()
  File "drivers/gpu/drm/msm/registers/gen_header.py", line 951, in main
parser.add_argument('--validate', action=argparse.BooleanOptionalAction)
AttributeError: module 'argparse' has no attribute 'BooleanOptionalAction'

The following patch allows the build to complete successfully:

https://patchwork.kernel.org/project/dri-devel/patch/20240508091751.336654-1-jonath...@nvidia.com/#25842751

As to whether this is a proper fix, I must defer to the DRM folks on CC.

Thanx, Paul

Re: [PATCH 24/29] mm: vmscan: make global slab shrink lockless

[Paul E. McKenney]

On Fri, Jun 23, 2023 at 04:29:39PM +1000, Dave Chinner wrote:
> On Thu, Jun 22, 2023 at 05:12:02PM +0200, Vlastimil Babka wrote:
> > On 6/22/23 10:53, Qi Zheng wrote:
> > > @@ -1067,33 +1068,27 @@ static unsigned long shrink_slab(gfp_t gfp_mask, 
> > > int nid,
> > >   if (!mem_cgroup_disabled() && !mem_cgroup_is_root(memcg))
> > >   return shrink_slab_memcg(gfp_mask, nid, memcg, priority);
> > >  
> > > - if (!down_read_trylock(_rwsem))
> > > - goto out;
> > > -
> > > - list_for_each_entry(shrinker, _list, list) {
> > > + rcu_read_lock();
> > > + list_for_each_entry_rcu(shrinker, _list, list) {
> > >   struct shrink_control sc = {
> > >   .gfp_mask = gfp_mask,
> > >   .nid = nid,
> > >   .memcg = memcg,
> > >   };
> > >  
> > > + if (!shrinker_try_get(shrinker))
> > > + continue;
> > > + rcu_read_unlock();
> > 
> > I don't think you can do this unlock?

Sorry to be slow to respond here, this one fell through the cracks.
And thank you to Qi for reminding me!

If you do this unlock, you had jolly well better nail down the current
element (the one referenced by shrinker), for example, by acquiring an
explicit reference count on the object.  And presumably this is exactly
what shrinker_try_get() is doing.  And a look at your 24/29 confirms this,
at least assuming that shrinker->refcount is set to zero before the call
to synchronize_rcu() in free_module() *and* that synchronize_rcu() doesn't
start until *after* shrinker_put() calls complete().  Plus, as always,
the object must be removed from the list before the synchronize_rcu()
starts.  (On these parts of the puzzle, I defer to those more familiar
with this code path.  And I strongly suggest carefully commenting this
type of action-at-a-distance design pattern.)

Why is this important?  Because otherwise that object might be freed
before you get to the call to rcu_read_lock() at the end of this loop.
And if that happens, list_for_each_entry_rcu() will be walking the
freelist, which is quite bad for the health and well-being of your kernel.

There are a few other ways to make this sort of thing work:

1.  Defer the shrinker_put() to the beginning of the loop.
You would need a flag initially set to zero, and then set to
one just before (or just after) the rcu_read_lock() above.
You would also need another shrinker_old pointer to track the
old pointer.  Then at the top of the loop, if the flag is set,
invoke shrinker_put() on shrinker_old.  This ensures that the
previous shrinker structure stays around long enough to allow
the loop to find the next shrinker structure in the list.

This approach is attractive when the removal code path
can invoke shrinker_put() after the grace period ends.

2.  Make shrinker_put() invoke call_rcu() when ->refcount reaches
zero, and have the callback function free the object.  This of
course requires adding an rcu_head structure to the shrinker
structure, which might or might not be a reasonable course of
action.  If adding that rcu_head is reasonable, this simplifies
the logic quite a bit.

3.  For the shrinker-structure-removal code path, remove the shrinker
structure, then remove the initial count from ->refcount,
and then keep doing grace periods until ->refcount is zero,
then do one more.  Of course, if the result of removing the
initial count was zero, then only a single additional grace
period is required.

This would need to be carefully commented, as it is a bit
unconventional.

There are probably many other ways, but just to give an idea of a few
other ways to do this.

> > > +
> > >   ret = do_shrink_slab(, shrinker, priority);
> > >   if (ret == SHRINK_EMPTY)
> > >   ret = 0;
> > >   freed += ret;
> > > - /*
> > > -  * Bail out if someone want to register a new shrinker to
> > > -  * prevent the registration from being stalled for long periods
> > > -  * by parallel ongoing shrinking.
> > > -  */
> > > - if (rwsem_is_contended(_rwsem)) {
> > > - freed = freed ? : 1;
> > > - break;
> > > - }
> > > - }
> > >  
> > > - up_read(_rwsem);
> > > -out:
> > > + rcu_read_lock();
> > 
> > That new rcu_read_lock() won't help AFAIK, the whole
> > list_for_each_entry_rcu() needs to be under the single rcu_read_lock() to be
> > safe.
> 
> Yeah, that's the pattern we've been taught and the one we can look
> at and immediately say "this is safe".
> 
> This is a different pattern, as has been explained bi Qi, and I
> think it *might* be safe.
> 
> *However.*
> 
> Right now I don't have time to go through a novel RCU list iteration
> pattern it one step at to determine the correctness of the
> algorithm. I'm mostly 

Re: [BUG?] INFO: rcu_sched detected expedited stalls on CPUs/tasks: { 0-.... } 3 jiffies s: 309 root: 0x1/.

[Paul E. McKenney]

On Thu, Apr 13, 2023 at 04:32:32PM +0100, Rui Salvaterra wrote:
> Hi, Paul,
> 
> On Thu, 13 Apr 2023 at 15:43, Paul E. McKenney  wrote:
> >
> > My guess would be that you have CONFIG_RCU_EXP_CPU_STALL_TIMEOUT set to
> > some small non-zero number, for example, you might have set up a recent
> > Android .config or some such.  The default of zero would give you about
> > 21 seconds rather than the three jiffies that you are seeing.
> >
> > Could you please check your .config?
> 
> Well, this is embarrassing. I can't fathom why/how, but I had it set
> to 20, on this machine. That is, 20 millisseconds. I guess its a
> miracle I haven't seen *more* expedited RCU traces. Sorry for the
> noise, everyone.

Been there, done that!

And actually, it is kind of reassuring that the Linux kernel avoids
tens-of-milliseocnds latency blows in the common case on your system.

Thanx, Paul

Re: [BUG?] INFO: rcu_sched detected expedited stalls on CPUs/tasks: { 0-.... } 3 jiffies s: 309 root: 0x1/.

[Paul E. McKenney]

On Thu, Apr 13, 2023 at 08:30:02AM +0100, Rui Salvaterra wrote:
> Hi again, everyone.
> 
> So, while preparing to file the bug report with the requested
> information, I got a trace completely unrelated to DRM (on a swapon
> call, it seems).
> 
> [4.868340] rcu: INFO: rcu_sched detected expedited stalls on
> CPUs/tasks: { 4- } 3 jiffies s: 265 root: 0x10/.
> [4.868349] rcu: blocking rcu_node structures (internal RCU debug):
> [4.868351] Sending NMI from CPU 3 to CPUs 4:
> [4.868355] NMI backtrace for cpu 4
> [4.868357] CPU: 4 PID: 462 Comm: swapon Not tainted 6.3.0-rc6-debug+ #57
> [4.868359] Hardware name: Apple Inc.
> Macmini6,2/Mac-F65AE981FFA204ED, BIOS 429.0.0.0.0 03/18/2022
> [4.868360] RIP: 0010:zram_submit_bio+0x57c/0x940
> [4.868365] Code: 04 4c 01 f0 48 8d 48 08 f0 48 0f ba 68 08 0d 0f
> 82 80 00 00 00 4c 89 ef e8 01 eb ff ff 49 8b 45 00 4a 8d 44 30 09 f0
> 80 20 df  48 ff 45 00 48 81 eb 00 10 00 00 41 83 c4 01 48 81 fb ff
> 0f 00
> [4.868366] RSP: 0018:8881057dbcd8 EFLAGS: 0246
> [4.868368] RAX: c90001c186d9 RBX: 3e893000 RCX: 
> c90001c186d8
> [4.868369] RDX: c90001c186d0 RSI:  RDI: 
> 88810083b400
> [4.868369] RBP: 88810083b470 R08: 00027e40 R09: 
> 00025850
> [4.868370] R10: 0014b212 R11: 88810ba03180 R12: 
> 000c176d
> [4.868371] R13: 88810083b400 R14: 00c176d0 R15: 
> 
> [4.868372] FS:  7fbd8f8ce800() GS:88826610()
> knlGS:
> [4.868373] CS:  0010 DS:  ES:  CR0: 80050033
> [4.868374] CR2: 563005371000 CR3: 00010355c003 CR4: 
> 001706e0
> [4.868375] Call Trace:
> [4.868377]  
> [4.868378]  ? block_read_full_folio+0x23e/0x2e0
> [4.868383]  ? kmem_cache_alloc+0x1b/0x110
> [4.868385]  ? mempool_alloc+0x37/0x140
> [4.868388]  ? pcpu_block_update_hint_alloc+0xce/0x2f0
> [4.868390]  __submit_bio+0x41/0xd0
> [4.868394]  submit_bio_noacct_nocheck+0xc4/0x2b0
> [4.868396]  blk_next_bio+0x55/0x70
> [4.868398]  __blkdev_issue_discard+0xc8/0x180
> [4.868401]  blkdev_issue_discard+0x3c/0x80
> [4.868403]  __x64_sys_swapon+0xb71/0x1120
> [4.868407]  do_syscall_64+0x2b/0x50
> [4.868410]  entry_SYSCALL_64_after_hwframe+0x46/0xb0
> [4.868414] RIP: 0033:0x7fbd8f712d5b
> [4.868416] Code: 73 01 c3 48 8b 0d bd 30 0e 00 f7 d8 64 89 01 48
> 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa b8 a7 00 00
> 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 8d 30 0e 00 f7 d8 64 89
> 01 48
> [4.868417] RSP: 002b:7ffcaf9a3448 EFLAGS: 0246 ORIG_RAX:
> 00a7
> [4.868418] RAX: ffda RBX: 00018064 RCX: 
> 7fbd8f712d5b
> [4.868419] RDX: 00018064 RSI: 00018064 RDI: 
> 56300535fb10
> [4.868420] RBP: 7ffcaf9a3530 R08: 00014b213000 R09: 
> 7fbd8f7f70f0
> [4.868420] R10: 1000 R11: 0246 R12: 
> 56300535fb10
> [4.868421] R13: 0064 R14: 7ffcaf9a3530 R15: 
> 
> [4.868423]  
> 
> Could it be that RCU is reporting expedited stalls too eagerly? And,
> if so, why only on this machine?

My guess would be that you have CONFIG_RCU_EXP_CPU_STALL_TIMEOUT set to
some small non-zero number, for example, you might have set up a recent
Android .config or some such.  The default of zero would give you about
21 seconds rather than the three jiffies that you are seeing.

Could you please check your .config?

Thanx, Paul

Re: Stack-frame warnings in display_mode_vba_32.c

[Paul E. McKenney]

On Sat, Jul 30, 2022 at 02:06:10AM -0700, Guenter Roeck wrote:
> On 7/29/22 22:12, Paul E. McKenney wrote:
> > On Fri, Jul 29, 2022 at 11:41:55PM -0300, André Almeida wrote:
> > > Hi Paul,
> > > 
> > > Às 23:25 de 29/07/22, Paul E. McKenney escreveu:
> > > > Hello!
> > > > 
> > > > I am seeing the following in allmodconfig builds of recent -next on x86:
> > > > 
> > > > drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c:
> > > >  In function 
> > > > ‘DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation’:
> > > > drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c:1659:1:
> > > >  error: the frame size of 2144 bytes is larger than 2048 bytes 
> > > > [-Werror=frame-larger-than=]
> > > >   1659 | }
> > > >| ^
> > > > drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c:
> > > >  In function ‘dml32_ModeSupportAndSystemConfigurationFull’:
> > > > drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c:3799:1:
> > > >  error: the frame size of 2480 bytes is larger than 2048 bytes 
> > > > [-Werror=frame-larger-than=]
> > > >   3799 | } // ModeSupportAndSystemConfigurationFull
> > > >| ^
> > > 
> > > I think they are fixed at amd-staging-drm-next:
> > > 
> > > git log --oneline amd/amd-staging-drm-next
> > > drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_32.c
> > > 953daa61981b drm/amd/display: Reduce stack size in the mode support 
> > > function
> > > 361e705e712d drm/amd/display: reduce stack for
> > > dml32_CalculatePrefetchSchedule
> > > f2dbf5a4dd1e drm/amd/display: reduce stack for
> > > dml32_CalculateWatermarksMALLUseAndDRAMSpeedChangeSupport
> > > a0a68cda2ef8 drm/amd/display: reduce stack for 
> > > dml32_CalculateVMRowAndSwath
> > > ca6730ca0f01 drm/amd/display: reduce stack for
> > > dml32_CalculateSwathAndDETConfiguration
> > > 593eef8c1a5e drm/amd/display: reduce stack size in dcn32 dml (v2)
> > > 
> > > https://gitlab.freedesktop.org/agd5f/linux/-/commits/amd-staging-drm-next/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_32.c
> > 
> > Very good, thank you!  I will test again on the next -next.
> > 
> 
> Did you try next-20220728 ?
> 
> groeck@server:~/src/linux-next$ git describe
> next-20220728
> groeck@server:~/src/linux-next$ git log --oneline 
> drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c
> 1b54a0121dba drm/amd/display: Reduce stack size in the mode support function
> 86e4863e67a9 drm/amd/display: reduce stack for dml32_CalculatePrefetchSchedule
> 3c3abac60117 drm/amd/display: reduce stack for 
> dml32_CalculateWatermarksMALLUseAndDRAMSpeedChangeSupport
> c3b3f9ba25e6 drm/amd/display: reduce stack for dml32_CalculateVMRowAndSwath
> bac4b41d917a drm/amd/display: reduce stack for 
> dml32_CalculateSwathAndDETConfiguration
> 7acc487ab57e drm/amd/display: reduce stack size in dcn32 dml (v2)

Indeed, next-20220728 does avoid the problem, thank you!

Thanx, Paul

Re: Stack-frame warnings in display_mode_vba_32.c

[Paul E. McKenney]

On Fri, Jul 29, 2022 at 11:41:55PM -0300, André Almeida wrote:
> Hi Paul,
> 
> Às 23:25 de 29/07/22, Paul E. McKenney escreveu:
> > Hello!
> > 
> > I am seeing the following in allmodconfig builds of recent -next on x86:
> > 
> > drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c: 
> > In function 
> > ‘DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation’:
> > drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c:1659:1:
> >  error: the frame size of 2144 bytes is larger than 2048 bytes 
> > [-Werror=frame-larger-than=]
> >  1659 | }
> >   | ^
> > drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c: 
> > In function ‘dml32_ModeSupportAndSystemConfigurationFull’:
> > drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c:3799:1:
> >  error: the frame size of 2480 bytes is larger than 2048 bytes 
> > [-Werror=frame-larger-than=]
> >  3799 | } // ModeSupportAndSystemConfigurationFull
> >   | ^
> 
> I think they are fixed at amd-staging-drm-next:
> 
> git log --oneline amd/amd-staging-drm-next
> drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_32.c
> 953daa61981b drm/amd/display: Reduce stack size in the mode support function
> 361e705e712d drm/amd/display: reduce stack for
> dml32_CalculatePrefetchSchedule
> f2dbf5a4dd1e drm/amd/display: reduce stack for
> dml32_CalculateWatermarksMALLUseAndDRAMSpeedChangeSupport
> a0a68cda2ef8 drm/amd/display: reduce stack for dml32_CalculateVMRowAndSwath
> ca6730ca0f01 drm/amd/display: reduce stack for
> dml32_CalculateSwathAndDETConfiguration
> 593eef8c1a5e drm/amd/display: reduce stack size in dcn32 dml (v2)
> 
> https://gitlab.freedesktop.org/agd5f/linux/-/commits/amd-staging-drm-next/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_32.c

Very good, thank you!  I will test again on the next -next.

Thanx, Paul

> > Bisection located the commit shown below.  Doing an allmodconfig build
> > on this commit reproduces the error, its parent builds fine.
> > 
> > Thoughts?
> > 
> > Thanx, Paul
> > 
> > 
> > 
> > commit 3876a8b5e241081b2a519f848a65c00d8e6cd124
> > Author: Guenter Roeck 
> > Date:   Tue Jul 12 15:42:47 2022 -0700
> > 
> > drm/amd/display: Enable building new display engine with KCOV enabled
> > 
> > The new display engine uses floating point math, which is not supported
> > by KCOV. Commit 9d1d02ff3678 ("drm/amd/display: Don't build DCN1 when 
> > kcov
> > is enabled") tried to work around the problem by disabling
> > CONFIG_DRM_AMD_DC_DCN if KCOV_INSTRUMENT_ALL and KCOV_ENABLE_COMPARISONS
> > are enabled. The result is that KCOV can not be enabled on systems which
> > require this display engine. A much simpler and less invasive solution 
> > is
> > to disable KCOV selectively when compiling the display enagine while
> > keeping it enabled for the rest of the kernel.
> > 
> > Fixes: 9d1d02ff3678 ("drm/amd/display: Don't build DCN1 when kcov is 
> > enabled")
> > Cc: Arnd Bergmann 
> > Cc: Leo Li 
> > Reviewed-by: Harry Wentland 
> > Signed-off-by: Guenter Roeck 
> > Signed-off-by: Alex Deucher 
> > 
> > diff --git a/drivers/gpu/drm/amd/display/Kconfig 
> > b/drivers/gpu/drm/amd/display/Kconfig
> > index b4029c0d5d8c5..96cbc87f7b6b8 100644
> > --- a/drivers/gpu/drm/amd/display/Kconfig
> > +++ b/drivers/gpu/drm/amd/display/Kconfig
> > @@ -6,7 +6,7 @@ config DRM_AMD_DC
> > bool "AMD DC - Enable new display engine"
> > default y
> > select SND_HDA_COMPONENT if SND_HDA_CORE
> > -   select DRM_AMD_DC_DCN if (X86 || PPC64) && !(KCOV_INSTRUMENT_ALL && 
> > KCOV_ENABLE_COMPARISONS)
> > +   select DRM_AMD_DC_DCN if (X86 || PPC64)
> > help
> >   Choose this option if you want to use the new display engine
> >   support for AMDGPU. This adds required support for Vega and
> > diff --git a/drivers/gpu/drm/amd/display/dc/Makefile 
> > b/drivers/gpu/drm/amd/display/dc/Makefile
> > index 273f8f2c8e020..b9effadfc4bb7 100644
> > --- a/drivers/gpu/drm/amd/display/dc/Makefile
> > +++ b/drivers/gpu/drm/amd/display/dc/Makefile
> > @@ -25,6 +25,9 @@
> >  DC_LIBS = basics bios dml clk_mgr dce gpio irq link virtual
> >  
> >  ifdef CONFIG_DRM_AMD_DC_DCN
> > +
> > +KCOV_INSTRUMENT := n
> > +
> >  DC_LIBS += dcn20
> >  DC_LIBS += dsc
> >  DC_LIBS += dcn10

Stack-frame warnings in display_mode_vba_32.c

[Paul E. McKenney]

Hello!

I am seeing the following in allmodconfig builds of recent -next on x86:

drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c: In 
function 
‘DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation’:
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c:1659:1:
 error: the frame size of 2144 bytes is larger than 2048 bytes 
[-Werror=frame-larger-than=]
 1659 | }
  | ^
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c: In 
function ‘dml32_ModeSupportAndSystemConfigurationFull’:
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c:3799:1:
 error: the frame size of 2480 bytes is larger than 2048 bytes 
[-Werror=frame-larger-than=]
 3799 | } // ModeSupportAndSystemConfigurationFull
  | ^

Bisection located the commit shown below.  Doing an allmodconfig build
on this commit reproduces the error, its parent builds fine.

Thoughts?

Thanx, Paul



commit 3876a8b5e241081b2a519f848a65c00d8e6cd124
Author: Guenter Roeck 
Date:   Tue Jul 12 15:42:47 2022 -0700

drm/amd/display: Enable building new display engine with KCOV enabled

The new display engine uses floating point math, which is not supported
by KCOV. Commit 9d1d02ff3678 ("drm/amd/display: Don't build DCN1 when kcov
is enabled") tried to work around the problem by disabling
CONFIG_DRM_AMD_DC_DCN if KCOV_INSTRUMENT_ALL and KCOV_ENABLE_COMPARISONS
are enabled. The result is that KCOV can not be enabled on systems which
require this display engine. A much simpler and less invasive solution is
to disable KCOV selectively when compiling the display enagine while
keeping it enabled for the rest of the kernel.

Fixes: 9d1d02ff3678 ("drm/amd/display: Don't build DCN1 when kcov is 
enabled")
Cc: Arnd Bergmann 
Cc: Leo Li 
Reviewed-by: Harry Wentland 
Signed-off-by: Guenter Roeck 
Signed-off-by: Alex Deucher 

diff --git a/drivers/gpu/drm/amd/display/Kconfig 
b/drivers/gpu/drm/amd/display/Kconfig
index b4029c0d5d8c5..96cbc87f7b6b8 100644
--- a/drivers/gpu/drm/amd/display/Kconfig
+++ b/drivers/gpu/drm/amd/display/Kconfig
@@ -6,7 +6,7 @@ config DRM_AMD_DC
bool "AMD DC - Enable new display engine"
default y
select SND_HDA_COMPONENT if SND_HDA_CORE
-   select DRM_AMD_DC_DCN if (X86 || PPC64) && !(KCOV_INSTRUMENT_ALL && 
KCOV_ENABLE_COMPARISONS)
+   select DRM_AMD_DC_DCN if (X86 || PPC64)
help
  Choose this option if you want to use the new display engine
  support for AMDGPU. This adds required support for Vega and
diff --git a/drivers/gpu/drm/amd/display/dc/Makefile 
b/drivers/gpu/drm/amd/display/dc/Makefile
index 273f8f2c8e020..b9effadfc4bb7 100644
--- a/drivers/gpu/drm/amd/display/dc/Makefile
+++ b/drivers/gpu/drm/amd/display/dc/Makefile
@@ -25,6 +25,9 @@
 DC_LIBS = basics bios dml clk_mgr dce gpio irq link virtual
 
 ifdef CONFIG_DRM_AMD_DC_DCN
+
+KCOV_INSTRUMENT := n
+
 DC_LIBS += dcn20
 DC_LIBS += dsc
 DC_LIBS += dcn10

Re: [PATCH linux-next] video: fbdev: fbmem: fix pointer reference to null device field

[Paul E. McKenney]

On Thu, Feb 10, 2022 at 02:58:24PM +0800, Zhouyi Zhou wrote:
> In function do_remove_conflicting_framebuffers, if device is NULL, there
> will be null pointer reference. The patch add a check to the if expression.
> 
> Signed-off-by: Zhouyi Zhou 
> ---
> Dear Linux folks
> 
> I discover this bug in the PowerPC VM provided by
> Open source lab of Oregon State University:
> 
> https://lkml.org/lkml/2022/2/8/1145
> 
> I found that the root cause of null device field is in offb_init_fb:
> info = framebuffer_alloc(sizeof(u32) * 16, NULL);
> 
> I have tested the patch in the PowerPC VM. Hope my patch can be correct.

This looks plausible to me, but I am quite unfamiliar with this code.

Thanx, Paul

> Many Thanks
> Zhouyi
> --
>  drivers/video/fbdev/core/fbmem.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/video/fbdev/core/fbmem.c 
> b/drivers/video/fbdev/core/fbmem.c
> index 34d6bb1bf82e..422b1fc01722 100644
> --- a/drivers/video/fbdev/core/fbmem.c
> +++ b/drivers/video/fbdev/core/fbmem.c
> @@ -1579,7 +1579,7 @@ static void do_remove_conflicting_framebuffers(struct 
> apertures_struct *a,
>* If it's not a platform device, at least print a 
> warning. A
>* fix would add code to remove the device from the 
> system.
>*/
> - if (dev_is_platform(device)) {
> + if (device && dev_is_platform(device)) {
>   registered_fb[i]->forced_out = true;
>   
> platform_device_unregister(to_platform_device(device));
>   } else {
> -- 
> 2.25.1
> 

Re: WARNING: suspicious RCU usage in modeset_lock

[Paul E. McKenney]

On Thu, Dec 17, 2020 at 11:03:20AM +0100, Daniel Vetter wrote:
> On Wed, Dec 16, 2020 at 5:16 PM Paul E. McKenney  wrote:
> >
> > On Wed, Dec 16, 2020 at 10:52:06AM +0100, Daniel Vetter wrote:
> > > On Wed, Dec 16, 2020 at 2:14 AM syzbot
> > >  wrote:
> > > >
> > > > Hello,
> > > >
> > > > syzbot found the following issue on:
> > > >
> > > > HEAD commit:94801e5c Merge tag 'pinctrl-v5.10-3' of 
> > > > git://git.kernel.o..
> > > > git tree:   upstream
> > > > console output: https://syzkaller.appspot.com/x/log.txt?x=130558c550
> > > > kernel config:  
> > > > https://syzkaller.appspot.com/x/.config?x=ee8a1012a5314210
> > > > dashboard link: 
> > > > https://syzkaller.appspot.com/bug?extid=972b924c988834e868b2
> > > > compiler:   gcc (GCC) 10.1.0-syz 20200507
> > > > userspace arch: i386
> > > >
> > > > Unfortunately, I don't have any reproducer for this issue yet.
> > > >
> > > > IMPORTANT: if you fix the issue, please add the following tag to the 
> > > > commit:
> > > > Reported-by: syzbot+972b924c988834e86...@syzkaller.appspotmail.com
> > > >
> > > > =
> > > > WARNING: suspicious RCU usage
> > > > 5.10.0-rc7-syzkaller #0 Not tainted
> > > > -
> > > > kernel/sched/core.c:7270 Illegal context switch in RCU-sched read-side 
> > > > critical section!
> > > >
> > > > other info that might help us debug this:
> > > >
> > > >
> > > > rcu_scheduler_active = 2, debug_locks = 0
> > > > 7 locks held by syz-executor.1/9232:
> > > >  #0: 8b328c60 (console_lock){+.+.}-{0:0}, at: 
> > > > do_fb_ioctl+0x2e4/0x690 drivers/video/fbdev/core/fbmem.c:1106
> > > >  #1: 888041bd4078 (_info->lock){+.+.}-{3:3}, at: lock_fb_info 
> > > > include/linux/fb.h:636 [inline]
> > > >  #1: 888041bd4078 (_info->lock){+.+.}-{3:3}, at: 
> > > > do_fb_ioctl+0x2ee/0x690 drivers/video/fbdev/core/fbmem.c:1107
> > > >  #2: 888041adca78 (>lock){+.+.}-{3:3}, at: 
> > > > drm_fb_helper_pan_display+0xce/0x970 
> > > > drivers/gpu/drm/drm_fb_helper.c:1448
> > > >  #3: 8880159f01b8 (>master_mutex){+.+.}-{3:3}, at: 
> > > > drm_master_internal_acquire+0x1d/0x70 drivers/gpu/drm/drm_auth.c:407
> > > >  #4: 888041adc898 (>modeset_mutex){+.+.}-{3:3}, at: 
> > > > drm_client_modeset_commit_locked+0x44/0x580 
> > > > drivers/gpu/drm/drm_client_modeset.c:1143
> > > >  #5: c90001c07730 (crtc_ww_class_acquire){+.+.}-{0:0}, at: 
> > > > drm_client_modeset_commit_atomic+0xb7/0x7c0 
> > > > drivers/gpu/drm/drm_client_modeset.c:981
> > > >  #6: 888015986108 (crtc_ww_class_mutex){+.+.}-{3:3}, at: 
> > > > ww_mutex_lock_slow include/linux/ww_mutex.h:287 [inline]
> > > >  #6: 888015986108 (crtc_ww_class_mutex){+.+.}-{3:3}, at: 
> > > > modeset_lock+0x31c/0x650 drivers/gpu/drm/drm_modeset_lock.c:260
> > >
> > > Given that we managed to take all these locks without upsetting anyone
> > > the rcu section is very deep down. And looking at the backtrace below
> > > I just couldn't find anything.
> > >
> > > Best I can think of is that an interrupt of some sort leaked an rcu
> > > section, and we got shot here. But I'd assume the rcu debugging would
> > > catch this? Backtrace of the start of that rcu read side section would
> > > be really useful here, but I'm not seeing that in the logs. There's
> > > more stuff there, but it's just the usual "everything falls apart"
> > > stuff of little value to understanding how we got there.
> >
> > In my experience, lockdep will indeed complain if an interrupt handler
> > returns while in an RCU read-side critical section.
> >
> > > Adding some rcu people for more insights on what could have gone wrong 
> > > here.
> > > -Daniel
> > >
> > > > stack backtrace:
> > > > CPU: 1 PID: 9232 Comm: syz-executor.1 Not tainted 5.10.0-rc7-syzkaller 
> > > > #0
> > > > Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 
> > > > rel-1.12.0-59-gc9ba5276e321-prebuilt.qemu.org 04/01/2014
> > > > Call Trace:
> > > >  __dump_stack lib/dump_stack.c:77 [inline]
> 

Re: WARNING: suspicious RCU usage in modeset_lock

[Paul E. McKenney]

On Wed, Dec 16, 2020 at 10:52:06AM +0100, Daniel Vetter wrote:
> On Wed, Dec 16, 2020 at 2:14 AM syzbot
>  wrote:
> >
> > Hello,
> >
> > syzbot found the following issue on:
> >
> > HEAD commit:94801e5c Merge tag 'pinctrl-v5.10-3' of git://git.kernel.o..
> > git tree:   upstream
> > console output: https://syzkaller.appspot.com/x/log.txt?x=130558c550
> > kernel config:  https://syzkaller.appspot.com/x/.config?x=ee8a1012a5314210
> > dashboard link: https://syzkaller.appspot.com/bug?extid=972b924c988834e868b2
> > compiler:   gcc (GCC) 10.1.0-syz 20200507
> > userspace arch: i386
> >
> > Unfortunately, I don't have any reproducer for this issue yet.
> >
> > IMPORTANT: if you fix the issue, please add the following tag to the commit:
> > Reported-by: syzbot+972b924c988834e86...@syzkaller.appspotmail.com
> >
> > =
> > WARNING: suspicious RCU usage
> > 5.10.0-rc7-syzkaller #0 Not tainted
> > -
> > kernel/sched/core.c:7270 Illegal context switch in RCU-sched read-side 
> > critical section!
> >
> > other info that might help us debug this:
> >
> >
> > rcu_scheduler_active = 2, debug_locks = 0
> > 7 locks held by syz-executor.1/9232:
> >  #0: 8b328c60 (console_lock){+.+.}-{0:0}, at: 
> > do_fb_ioctl+0x2e4/0x690 drivers/video/fbdev/core/fbmem.c:1106
> >  #1: 888041bd4078 (_info->lock){+.+.}-{3:3}, at: lock_fb_info 
> > include/linux/fb.h:636 [inline]
> >  #1: 888041bd4078 (_info->lock){+.+.}-{3:3}, at: 
> > do_fb_ioctl+0x2ee/0x690 drivers/video/fbdev/core/fbmem.c:1107
> >  #2: 888041adca78 (>lock){+.+.}-{3:3}, at: 
> > drm_fb_helper_pan_display+0xce/0x970 drivers/gpu/drm/drm_fb_helper.c:1448
> >  #3: 8880159f01b8 (>master_mutex){+.+.}-{3:3}, at: 
> > drm_master_internal_acquire+0x1d/0x70 drivers/gpu/drm/drm_auth.c:407
> >  #4: 888041adc898 (>modeset_mutex){+.+.}-{3:3}, at: 
> > drm_client_modeset_commit_locked+0x44/0x580 
> > drivers/gpu/drm/drm_client_modeset.c:1143
> >  #5: c90001c07730 (crtc_ww_class_acquire){+.+.}-{0:0}, at: 
> > drm_client_modeset_commit_atomic+0xb7/0x7c0 
> > drivers/gpu/drm/drm_client_modeset.c:981
> >  #6: 888015986108 (crtc_ww_class_mutex){+.+.}-{3:3}, at: 
> > ww_mutex_lock_slow include/linux/ww_mutex.h:287 [inline]
> >  #6: 888015986108 (crtc_ww_class_mutex){+.+.}-{3:3}, at: 
> > modeset_lock+0x31c/0x650 drivers/gpu/drm/drm_modeset_lock.c:260
> 
> Given that we managed to take all these locks without upsetting anyone
> the rcu section is very deep down. And looking at the backtrace below
> I just couldn't find anything.
> 
> Best I can think of is that an interrupt of some sort leaked an rcu
> section, and we got shot here. But I'd assume the rcu debugging would
> catch this? Backtrace of the start of that rcu read side section would
> be really useful here, but I'm not seeing that in the logs. There's
> more stuff there, but it's just the usual "everything falls apart"
> stuff of little value to understanding how we got there.

In my experience, lockdep will indeed complain if an interrupt handler
returns while in an RCU read-side critical section.

> Adding some rcu people for more insights on what could have gone wrong here.
> -Daniel
> 
> > stack backtrace:
> > CPU: 1 PID: 9232 Comm: syz-executor.1 Not tainted 5.10.0-rc7-syzkaller #0
> > Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 
> > rel-1.12.0-59-gc9ba5276e321-prebuilt.qemu.org 04/01/2014
> > Call Trace:
> >  __dump_stack lib/dump_stack.c:77 [inline]
> >  dump_stack+0x107/0x163 lib/dump_stack.c:118
> >  ___might_sleep+0x25d/0x2b0 kernel/sched/core.c:7270
> >  __mutex_lock_common kernel/locking/mutex.c:935 [inline]
> >  __ww_mutex_lock.constprop.0+0xa9/0x2cc0 kernel/locking/mutex.c:
> >  ww_mutex_lock+0x3d/0x170 kernel/locking/mutex.c:1190

Acquiring a mutex while under the influence of rcu_read_lock() will
definitely get you this lockdep complaint, and rightfully so.

If you need to acquire a mutex with RCU-like protection, one approach
is to use SRCU.  But usually this indicates (as you suspected) that
someone forgot to invoke rcu_read_unlock().

One way to locate this is to enlist the aid of lockdep.  You can do this
by putting something like this in the callers:

RCU_LOCKDEP_WARN(lock_is_held(_bh_lock_map) ||
 lock_is_held(_lock_map) ||
 lock_is_held(_sched_lock_map),
 "We are in an RCU read-side critical section");

This will get you a lockdep complaint much like the one above if the
caller is in any sort of RCU read-side critical section.  You can push
this up the call stack one level at a time or just sprinkle it up the
stack in one go.

The complaint is specifically about RCU-sched, so you could focus on
that using this instead:

RCU_LOCKDEP_WARN(lock_is_held(_sched_lock_map),
 "We are in an RCU-sched read-side critical section");

This of course assumes that this is reproducible.  :-/

Re: [PATCH 04/65] mm: Extract might_alloc() debug check

[Paul E. McKenney]

On Fri, Oct 23, 2020 at 02:21:15PM +0200, Daniel Vetter wrote:
> Extracted from slab.h, which seems to have the most complete version
> including the correct might_sleep() check. Roll it out to slob.c.
> 
> Motivated by a discussion with Paul about possibly changing call_rcu
> behaviour to allocate memory, but only roughly every 500th call.
> 
> There are a lot fewer places in the kernel that care about whether
> allocating memory is allowed or not (due to deadlocks with reclaim
> code) than places that care whether sleeping is allowed. But debugging
> these also tends to be a lot harder, so nice descriptive checks could
> come in handy. I might have some use eventually for annotations in
> drivers/gpu.
> 
> Note that unlike fs_reclaim_acquire/release gfpflags_allow_blocking
> does not consult the PF_MEMALLOC flags. But there is no flag
> equivalent for GFP_NOWAIT, hence this check can't go wrong due to
> memalloc_no*_save/restore contexts.
> 
> Cc: Paul E. McKenney 
> Cc: Christoph Lameter 
> Cc: Pekka Enberg 
> Cc: David Rientjes 
> Cc: Joonsoo Kim 
> Cc: Andrew Morton 
> Cc: Peter Zijlstra 
> Cc: Ingo Molnar 
> Cc: Vlastimil Babka 
> Cc: Mathieu Desnoyers 
> Cc: Sebastian Andrzej Siewior 
> Cc: Michel Lespinasse 
> Cc: Daniel Vetter 
> Cc: Waiman Long 
> Cc: Thomas Gleixner 
> Cc: Randy Dunlap 
> Cc: linux...@kvack.org
> Cc: linux-fsde...@vger.kernel.org
> Cc: Dave Chinner 
> Cc: Qian Cai 
> Cc: linux-...@vger.kernel.org
> Signed-off-by: Daniel Vetter 

Nice!!!

Acked-by: Paul E. McKenney 

> ---
>  include/linux/sched/mm.h | 16 
>  mm/slab.h|  5 +
>  mm/slob.c|  6 ++
>  3 files changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h
> index f889e332912f..2b0037abac0b 100644
> --- a/include/linux/sched/mm.h
> +++ b/include/linux/sched/mm.h
> @@ -205,6 +205,22 @@ static inline void fs_reclaim_acquire(gfp_t gfp_mask) { }
>  static inline void fs_reclaim_release(gfp_t gfp_mask) { }
>  #endif
>  
> +/**
> + * might_alloc - Marks possible allocation sites
> + * @gfp_mask: gfp_t flags that would be use to allocate
> + *
> + * Similar to might_sleep() and other annotations this can be used in 
> functions
> + * that might allocate, but often dont. Compiles to nothing without
> + * CONFIG_LOCKDEP. Includes a conditional might_sleep() if @gfp allows 
> blocking.
> + */
> +static inline void might_alloc(gfp_t gfp_mask)
> +{
> + fs_reclaim_acquire(gfp_mask);
> + fs_reclaim_release(gfp_mask);
> +
> + might_sleep_if(gfpflags_allow_blocking(gfp_mask));
> +}
> +
>  /**
>   * memalloc_noio_save - Marks implicit GFP_NOIO allocation scope.
>   *
> diff --git a/mm/slab.h b/mm/slab.h
> index 6cc323f1313a..fedd789b2270 100644
> --- a/mm/slab.h
> +++ b/mm/slab.h
> @@ -492,10 +492,7 @@ static inline struct kmem_cache 
> *slab_pre_alloc_hook(struct kmem_cache *s,
>  {
>   flags &= gfp_allowed_mask;
>  
> - fs_reclaim_acquire(flags);
> - fs_reclaim_release(flags);
> -
> - might_sleep_if(gfpflags_allow_blocking(flags));
> + might_alloc(flags);
>  
>   if (should_failslab(s, flags))
>   return NULL;
> diff --git a/mm/slob.c b/mm/slob.c
> index 7cc9805c8091..8d4bfa46247f 100644
> --- a/mm/slob.c
> +++ b/mm/slob.c
> @@ -474,8 +474,7 @@ __do_kmalloc_node(size_t size, gfp_t gfp, int node, 
> unsigned long caller)
>  
>   gfp &= gfp_allowed_mask;
>  
> - fs_reclaim_acquire(gfp);
> - fs_reclaim_release(gfp);
> + might_alloc(gfp);
>  
>   if (size < PAGE_SIZE - minalign) {
>   int align = minalign;
> @@ -597,8 +596,7 @@ static void *slob_alloc_node(struct kmem_cache *c, gfp_t 
> flags, int node)
>  
>   flags &= gfp_allowed_mask;
>  
> - fs_reclaim_acquire(flags);
> - fs_reclaim_release(flags);
> + might_alloc(flags);
>  
>   if (c->size < PAGE_SIZE) {
>   b = slob_alloc(c->size, flags, c->align, node, 0);
> -- 
> 2.28.0
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH tip/core/rcu 11/15] drm/i915: Cleanup PREEMPT_COUNT leftovers

[Paul E. McKenney]

On Thu, Oct 01, 2020 at 10:25:06AM +0200, Thomas Gleixner wrote:
> On Thu, Oct 01 2020 at 10:17, Joonas Lahtinen wrote:
> > Quoting paul...@kernel.org (2020-09-29 02:30:58)
> >> CONFIG_PREEMPT_COUNT is now unconditionally enabled and will be
> >> removed. Cleanup the leftovers before doing so.
> >
> > Change looks fine:
> >
> > Reviewed-by: Joonas Lahtinen 

Applied, thank you!

> > Are you looking for us to merge or merge through another tree?
> >
> > If us, did the base patch always enabling PREEMPT_COUNT go into 5.9 or is
> > it heading to 5.10? We can queue this earliest for 5.11 as drm-next closed
> > for 5.10 at week of -rc5.
> 
> If at all it goes through rcu/tip because it depends on the earlier patches.

I was figuring on sending a pull request later today, Pacific Time.

Thanx, Paul
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [patch 00/13] preempt: Make preempt count unconditional

[Paul E. McKenney]

On Thu, Sep 17, 2020 at 09:52:30AM +0200, Daniel Vetter wrote:
> On Thu, Sep 17, 2020 at 12:39 AM Paul E. McKenney  wrote:
> >
> > On Wed, Sep 16, 2020 at 11:43:02PM +0200, Daniel Vetter wrote:
> > > On Wed, Sep 16, 2020 at 10:58 PM Paul E. McKenney  
> > > wrote:
> > > >
> > > > On Wed, Sep 16, 2020 at 10:29:06PM +0200, Daniel Vetter wrote:
> > > > > On Wed, Sep 16, 2020 at 5:29 PM Paul E. McKenney  
> > > > > wrote:
> > > > > >
> > > > > > On Wed, Sep 16, 2020 at 09:37:17AM +0200, Daniel Vetter wrote:
> > > > > > > On Tue, Sep 15, 2020 at 7:35 PM Linus Torvalds
> > > > > > >  wrote:
> > > > > > > >
> > > > > > > > On Tue, Sep 15, 2020 at 1:39 AM Thomas Gleixner 
> > > > > > > >  wrote:
> > > > > > > > >
> > > > > > > > > OTOH, having a working 'preemptible()' or maybe better named
> > > > > > > > > 'can_schedule()' check makes tons of sense to make decisions 
> > > > > > > > > about
> > > > > > > > > allocation modes or other things.
> > > > > > > >
> > > > > > > > No. I think that those kinds of decisions about actual behavior 
> > > > > > > > are
> > > > > > > > always simply fundamentally wrong.
> > > > > > > >
> > > > > > > > Note that this is very different from having warnings about 
> > > > > > > > invalid
> > > > > > > > use. THAT is correct. It may not warn in all configurations, 
> > > > > > > > but that
> > > > > > > > doesn't matter: what matters is that it warns in common enough
> > > > > > > > configurations that developers will catch it.
> > > > > > > >
> > > > > > > > So having a warning in "might_sleep()" that doesn't always 
> > > > > > > > trigger,
> > > > > > > > because you have a limited configuration that can't even detect 
> > > > > > > > the
> > > > > > > > situation, that's fine and dandy and intentional.
> > > > > > > >
> > > > > > > > But having code like
> > > > > > > >
> > > > > > > >if (can_schedule())
> > > > > > > >.. do something different ..
> > > > > > > >
> > > > > > > > is fundamentally complete and utter garbage.
> > > > > > > >
> > > > > > > > It's one thing if you test for "am I in hardware interrupt 
> > > > > > > > context".
> > > > > > > > Those tests aren't great either, but at least they make sense.
> > > > > > > >
> > > > > > > > But a driver - or some library routine - making a difference 
> > > > > > > > based on
> > > > > > > > some nebulous "can I schedule" is fundamentally and basically 
> > > > > > > > WRONG.
> > > > > > > >
> > > > > > > > If some code changes behavior, it needs to be explicit to the 
> > > > > > > > *caller*
> > > > > > > > of that code.
> > > > > > > >
> > > > > > > > So this is why GFP_ATOMIC is fine, but "if (!can_schedule())
> > > > > > > > do_something_atomic()" is pure shite.
> > > > > > > >
> > > > > > > > And I am not IN THE LEAST interested in trying to help people 
> > > > > > > > doing
> > > > > > > > pure shite. We need to fix them. Like the crypto code is getting
> > > > > > > > fixed.
> > > > > > >
> > > > > > > Just figured I'll throw my +1 in from reading too many (gpu) 
> > > > > > > drivers.
> > > > > > > Code that tries to cleverly adjust its behaviour depending upon 
> > > > > > > the
> > > > > > > context it's running in is harder to understand and blows up in 
> > > > > > > more
> > > > > > > interesting ways. We still have drm_can_sleep() and it's mo

Re: [patch 00/13] preempt: Make preempt count unconditional

[Paul E. McKenney]

On Wed, Sep 16, 2020 at 11:43:02PM +0200, Daniel Vetter wrote:
> On Wed, Sep 16, 2020 at 10:58 PM Paul E. McKenney  wrote:
> >
> > On Wed, Sep 16, 2020 at 10:29:06PM +0200, Daniel Vetter wrote:
> > > On Wed, Sep 16, 2020 at 5:29 PM Paul E. McKenney  
> > > wrote:
> > > >
> > > > On Wed, Sep 16, 2020 at 09:37:17AM +0200, Daniel Vetter wrote:
> > > > > On Tue, Sep 15, 2020 at 7:35 PM Linus Torvalds
> > > > >  wrote:
> > > > > >
> > > > > > On Tue, Sep 15, 2020 at 1:39 AM Thomas Gleixner 
> > > > > >  wrote:
> > > > > > >
> > > > > > > OTOH, having a working 'preemptible()' or maybe better named
> > > > > > > 'can_schedule()' check makes tons of sense to make decisions about
> > > > > > > allocation modes or other things.
> > > > > >
> > > > > > No. I think that those kinds of decisions about actual behavior are
> > > > > > always simply fundamentally wrong.
> > > > > >
> > > > > > Note that this is very different from having warnings about invalid
> > > > > > use. THAT is correct. It may not warn in all configurations, but 
> > > > > > that
> > > > > > doesn't matter: what matters is that it warns in common enough
> > > > > > configurations that developers will catch it.
> > > > > >
> > > > > > So having a warning in "might_sleep()" that doesn't always trigger,
> > > > > > because you have a limited configuration that can't even detect the
> > > > > > situation, that's fine and dandy and intentional.
> > > > > >
> > > > > > But having code like
> > > > > >
> > > > > >if (can_schedule())
> > > > > >.. do something different ..
> > > > > >
> > > > > > is fundamentally complete and utter garbage.
> > > > > >
> > > > > > It's one thing if you test for "am I in hardware interrupt context".
> > > > > > Those tests aren't great either, but at least they make sense.
> > > > > >
> > > > > > But a driver - or some library routine - making a difference based 
> > > > > > on
> > > > > > some nebulous "can I schedule" is fundamentally and basically WRONG.
> > > > > >
> > > > > > If some code changes behavior, it needs to be explicit to the 
> > > > > > *caller*
> > > > > > of that code.
> > > > > >
> > > > > > So this is why GFP_ATOMIC is fine, but "if (!can_schedule())
> > > > > > do_something_atomic()" is pure shite.
> > > > > >
> > > > > > And I am not IN THE LEAST interested in trying to help people doing
> > > > > > pure shite. We need to fix them. Like the crypto code is getting
> > > > > > fixed.
> > > > >
> > > > > Just figured I'll throw my +1 in from reading too many (gpu) drivers.
> > > > > Code that tries to cleverly adjust its behaviour depending upon the
> > > > > context it's running in is harder to understand and blows up in more
> > > > > interesting ways. We still have drm_can_sleep() and it's mostly just
> > > > > used for debug code, and I've largely ended up just deleting
> > > > > everything that used it because when you're driver is blowing up the
> > > > > last thing you want is to realize your debug code and output can't be
> > > > > relied upon. Or worse, that the only Oops you have is the one in the
> > > > > debug code, because the real one scrolled away - the original idea
> > > > > behind drm_can_sleep was to make all the modeset code work
> > > > > automagically both in normal ioctl/kworker context and in the panic
> > > > > handlers or kgdb callbacks. Wishful thinking at best.
> > > > >
> > > > > Also at least for me that extends to everything, e.g. I much prefer
> > > > > explicit spin_lock and spin_lock_irq vs magic spin_lock_irqsave for
> > > > > locks shared with interrupt handlers, since the former two gives me
> > > > > clear information from which contexts such function can be called.
> > > > > Other end is the memalloc_no*_save/restore functions, w

Re: [patch 00/13] preempt: Make preempt count unconditional

[Paul E. McKenney]

On Wed, Sep 16, 2020 at 10:29:06PM +0200, Daniel Vetter wrote:
> On Wed, Sep 16, 2020 at 5:29 PM Paul E. McKenney  wrote:
> >
> > On Wed, Sep 16, 2020 at 09:37:17AM +0200, Daniel Vetter wrote:
> > > On Tue, Sep 15, 2020 at 7:35 PM Linus Torvalds
> > >  wrote:
> > > >
> > > > On Tue, Sep 15, 2020 at 1:39 AM Thomas Gleixner  
> > > > wrote:
> > > > >
> > > > > OTOH, having a working 'preemptible()' or maybe better named
> > > > > 'can_schedule()' check makes tons of sense to make decisions about
> > > > > allocation modes or other things.
> > > >
> > > > No. I think that those kinds of decisions about actual behavior are
> > > > always simply fundamentally wrong.
> > > >
> > > > Note that this is very different from having warnings about invalid
> > > > use. THAT is correct. It may not warn in all configurations, but that
> > > > doesn't matter: what matters is that it warns in common enough
> > > > configurations that developers will catch it.
> > > >
> > > > So having a warning in "might_sleep()" that doesn't always trigger,
> > > > because you have a limited configuration that can't even detect the
> > > > situation, that's fine and dandy and intentional.
> > > >
> > > > But having code like
> > > >
> > > >if (can_schedule())
> > > >.. do something different ..
> > > >
> > > > is fundamentally complete and utter garbage.
> > > >
> > > > It's one thing if you test for "am I in hardware interrupt context".
> > > > Those tests aren't great either, but at least they make sense.
> > > >
> > > > But a driver - or some library routine - making a difference based on
> > > > some nebulous "can I schedule" is fundamentally and basically WRONG.
> > > >
> > > > If some code changes behavior, it needs to be explicit to the *caller*
> > > > of that code.
> > > >
> > > > So this is why GFP_ATOMIC is fine, but "if (!can_schedule())
> > > > do_something_atomic()" is pure shite.
> > > >
> > > > And I am not IN THE LEAST interested in trying to help people doing
> > > > pure shite. We need to fix them. Like the crypto code is getting
> > > > fixed.
> > >
> > > Just figured I'll throw my +1 in from reading too many (gpu) drivers.
> > > Code that tries to cleverly adjust its behaviour depending upon the
> > > context it's running in is harder to understand and blows up in more
> > > interesting ways. We still have drm_can_sleep() and it's mostly just
> > > used for debug code, and I've largely ended up just deleting
> > > everything that used it because when you're driver is blowing up the
> > > last thing you want is to realize your debug code and output can't be
> > > relied upon. Or worse, that the only Oops you have is the one in the
> > > debug code, because the real one scrolled away - the original idea
> > > behind drm_can_sleep was to make all the modeset code work
> > > automagically both in normal ioctl/kworker context and in the panic
> > > handlers or kgdb callbacks. Wishful thinking at best.
> > >
> > > Also at least for me that extends to everything, e.g. I much prefer
> > > explicit spin_lock and spin_lock_irq vs magic spin_lock_irqsave for
> > > locks shared with interrupt handlers, since the former two gives me
> > > clear information from which contexts such function can be called.
> > > Other end is the memalloc_no*_save/restore functions, where I recently
> > > made a real big fool of myself because I didn't realize how much that
> > > impacts everything that's run within - suddenly "GFP_KERNEL for small
> > > stuff never fails" is wrong everywhere.
> > >
> > > It's all great for debugging and sanity checks (and we run with all
> > > that stuff enabled in our CI), but really semantic changes depending
> > > upon magic context checks freak my out :-)
> >
> > All fair, but some of us need to write code that must handle being
> > invoked from a wide variety of contexts.  Now perhaps you like the idea of
> > call_rcu() for schedulable contexts, call_rcu_nosched() when preemption
> > is disabled, call_rcu_irqs_are_disabled() when interrupts are disabled,
> > call_rcu_raw_atomic() from contexts where (for example) raw spinlocks
> >

Re: [patch 00/13] preempt: Make preempt count unconditional

[Paul E. McKenney]

On Wed, Sep 16, 2020 at 08:23:52PM +0100, Matthew Wilcox wrote:
> On Mon, Sep 14, 2020 at 11:55:24PM +0200, Thomas Gleixner wrote:
> > But just look at any check which uses preemptible(), especially those
> > which check !preemptible():
> 
> hmm.
> 
> +++ b/include/linux/preempt.h
> @@ -180,7 +180,9 @@ do { \
>  
>  #define preempt_enable_no_resched() sched_preempt_enable_no_resched()
>  
> +#ifndef MODULE
>  #define preemptible()  (preempt_count() == 0 && !irqs_disabled())
> +#endif
>  
>  #ifdef CONFIG_PREEMPTION
>  #define preempt_enable() \
> 
> 
> $ git grep -w preemptible drivers
> (slightly trimmed by hand to remove, eg, comments)
> drivers/firmware/arm_sdei.c:WARN_ON_ONCE(preemptible());
> drivers/firmware/arm_sdei.c:WARN_ON_ONCE(preemptible());
> drivers/firmware/arm_sdei.c:WARN_ON_ONCE(preemptible());
> drivers/firmware/arm_sdei.c:WARN_ON_ONCE(preemptible());
> drivers/firmware/arm_sdei.c:WARN_ON(preemptible());
> drivers/firmware/efi/efi-pstore.c:preemptible(), 
> record->size, record->psi->buf);
> drivers/irqchip/irq-gic-v4.c:   WARN_ON(preemptible());
> drivers/irqchip/irq-gic-v4.c:   WARN_ON(preemptible());
> drivers/scsi/hisi_sas/hisi_sas_main.c:  if (!preemptible())
> drivers/xen/time.c: BUG_ON(preemptible());
> 
> That only looks like two drivers that need more than WARNectomies.

I could easily imagine someone thinking that these did something in
CONFIG_PREEMPT_NONE=y kernels.  In fact, I could easily imagine myself
making that mistake.  :-/

> Although maybe rcu_read_load_sched_held() or rcu_read_lock_any_held()
> might get called from a module ...

But yes, from the rcutorture module for certain and also from any other
RCU-using module that includes the usual RCU debug checks.

Thanx, Paul
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [patch 00/13] preempt: Make preempt count unconditional

[Paul E. McKenney]

On Wed, Sep 16, 2020 at 11:32:00AM -0700, Linus Torvalds wrote:
> On Wed, Sep 16, 2020 at 8:29 AM Paul E. McKenney  wrote:
> >
> > All fair, but some of us need to write code that must handle being
> > invoked from a wide variety of contexts.
> 
> Note that I think that core functionality is different from random drivers.
> 
> Of course core code can (and will) look at things like
> 
> if (in_interrupt())
> .. schedule work asynchronously ..
> 
> because core code ends up being called from odd places, and code like
> that is expected to have understanding of the rules it plays with.
> 
> But something like RCU is a very different beast from some "walk the
> scatter-gather list" code.
> 
> RCU does its work in the background, and works with lots of different
> things. And it's so core and used everywhere that it knows about these
> things. I mean, we literally have special code explicitly to let RCU
> know "we entered kernel context now".
> 
> But something like a driver list walking thing should not be doing
> different things behind peoples back depending on whether they hold
> spinlocks or not. It should either just work regardless, or there
> should be a flag (or special interface) for the "you're being called
> in a crtitical region".
> 
> Because dynamically changing behavior really is very confusing.

Whew!  I feel much better now.  ;-)

Thanx, Paul
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [patch 00/13] preempt: Make preempt count unconditional

[Paul E. McKenney]

On Wed, Sep 16, 2020 at 09:37:17AM +0200, Daniel Vetter wrote:
> On Tue, Sep 15, 2020 at 7:35 PM Linus Torvalds
>  wrote:
> >
> > On Tue, Sep 15, 2020 at 1:39 AM Thomas Gleixner  wrote:
> > >
> > > OTOH, having a working 'preemptible()' or maybe better named
> > > 'can_schedule()' check makes tons of sense to make decisions about
> > > allocation modes or other things.
> >
> > No. I think that those kinds of decisions about actual behavior are
> > always simply fundamentally wrong.
> >
> > Note that this is very different from having warnings about invalid
> > use. THAT is correct. It may not warn in all configurations, but that
> > doesn't matter: what matters is that it warns in common enough
> > configurations that developers will catch it.
> >
> > So having a warning in "might_sleep()" that doesn't always trigger,
> > because you have a limited configuration that can't even detect the
> > situation, that's fine and dandy and intentional.
> >
> > But having code like
> >
> >if (can_schedule())
> >.. do something different ..
> >
> > is fundamentally complete and utter garbage.
> >
> > It's one thing if you test for "am I in hardware interrupt context".
> > Those tests aren't great either, but at least they make sense.
> >
> > But a driver - or some library routine - making a difference based on
> > some nebulous "can I schedule" is fundamentally and basically WRONG.
> >
> > If some code changes behavior, it needs to be explicit to the *caller*
> > of that code.
> >
> > So this is why GFP_ATOMIC is fine, but "if (!can_schedule())
> > do_something_atomic()" is pure shite.
> >
> > And I am not IN THE LEAST interested in trying to help people doing
> > pure shite. We need to fix them. Like the crypto code is getting
> > fixed.
> 
> Just figured I'll throw my +1 in from reading too many (gpu) drivers.
> Code that tries to cleverly adjust its behaviour depending upon the
> context it's running in is harder to understand and blows up in more
> interesting ways. We still have drm_can_sleep() and it's mostly just
> used for debug code, and I've largely ended up just deleting
> everything that used it because when you're driver is blowing up the
> last thing you want is to realize your debug code and output can't be
> relied upon. Or worse, that the only Oops you have is the one in the
> debug code, because the real one scrolled away - the original idea
> behind drm_can_sleep was to make all the modeset code work
> automagically both in normal ioctl/kworker context and in the panic
> handlers or kgdb callbacks. Wishful thinking at best.
> 
> Also at least for me that extends to everything, e.g. I much prefer
> explicit spin_lock and spin_lock_irq vs magic spin_lock_irqsave for
> locks shared with interrupt handlers, since the former two gives me
> clear information from which contexts such function can be called.
> Other end is the memalloc_no*_save/restore functions, where I recently
> made a real big fool of myself because I didn't realize how much that
> impacts everything that's run within - suddenly "GFP_KERNEL for small
> stuff never fails" is wrong everywhere.
> 
> It's all great for debugging and sanity checks (and we run with all
> that stuff enabled in our CI), but really semantic changes depending
> upon magic context checks freak my out :-)

All fair, but some of us need to write code that must handle being
invoked from a wide variety of contexts.  Now perhaps you like the idea of
call_rcu() for schedulable contexts, call_rcu_nosched() when preemption
is disabled, call_rcu_irqs_are_disabled() when interrupts are disabled,
call_rcu_raw_atomic() from contexts where (for example) raw spinlocks
are held, and so on.  However, from what I can see, most people instead
consistently prefer that the RCU API instead be consolidated.

Some in-flight cache-efficiency work for kvfree_rcu() and call_rcu()
needs to be able to allocate memory occasionally.  It can do that when
invoked from some contexts, but not when invoked from others.  Right now,
in !PREEMPT kernels, it cannot tell, and must either do things to the
memory allocators that some of the MM hate or must unnecessarily invoke
workqueues.  Thomas's patches would allow the code to just allocate in
the common case when these primitives are invoked from contexts where
allocation is permitted.

If we want to restrict access to the can_schedule() or whatever primitive,
fine and good.  We can add a check to checkpatch.pl, for example.  Maybe
we can go back to the old brlock approach of requiring certain people's
review for each addition to the kernel.

But there really are use cases that it would greatly help.

Thanx, Paul
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [patch 00/13] preempt: Make preempt count unconditional

[Paul E. McKenney]

On Mon, Sep 14, 2020 at 01:59:15PM -0700, Linus Torvalds wrote:
> On Mon, Sep 14, 2020 at 1:45 PM Thomas Gleixner  wrote:
> >
> > Recently merged code does:
> >
> >  gfp = preemptible() ? GFP_KERNEL : GFP_ATOMIC;
> >
> > Looks obviously correct, except for the fact that preemptible() is
> > unconditionally false for CONFIF_PREEMPT_COUNT=n, i.e. all allocations in
> > that code use GFP_ATOMIC on such kernels.
> 
> I don't think this is a good reason to entirely get rid of the no-preempt 
> thing.
> 
> The above is just garbage. It's bogus. You can't do it.
> 
> Blaming the no-preempt code for this bug is extremely unfair, imho.
> 
> And the no-preempt code does help make for much better code generation
> for simple spinlocks.
> 
> Where is that horribly buggy recent code? It's not in that exact
> format, certainly, since 'grep' doesn't find it.

It would be convenient for that "gfp =" code to work, as this would
allow better cache locality while invoking RCU callbacks, and would
further provide better robustness to callback floods.  The full story
is quite long, but here are alternatives have not yet been proven to be
abject failures:

1.  Use workqueues to do the allocations in a clean context.
While waiting for the allocations, the callbacks are queued
in the old cache-busting manner.  This functions correctly,
but in the meantime (which on busy systems can be some time)
the cache locality and robustness are lost.

2.  Provide the ability to allocate memory in raw atomic context.
This is extremely effective, especially when used in combination
with #1 above, but as you might suspect, the MM guys don't like
it much.

In contrast, with Thomas's patch series, call_rcu() and kvfree_rcu()
could just look at preemptible() to see whether or not it was safe to
allocate memory, even in !PREEMPT kernels -- and in the common case,
it almost always would be safe.  It is quite possible that this approach
would work in isolation, or failing that, that adding #1 above would do
the trick.

I understand that this is all very hand-wavy, and I do apologize for that.
If you really want the full sad story with performance numbers and the
works, let me know!

Thanx, Paul
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 11/11] rcu: constify sysrq_key_op

[Paul E. McKenney]

On Wed, May 13, 2020 at 10:43:51PM +0100, Emil Velikov wrote:
> With earlier commits, the API no longer discards the const-ness of the
> sysrq_key_op. As such we can add the notation.
> 
> Cc: Greg Kroah-Hartman 
> Cc: Jiri Slaby 
> Cc: linux-ker...@vger.kernel.org
> Cc: "Paul E. McKenney" 
> Cc: Josh Triplett 
> Cc: r...@vger.kernel.org
> Signed-off-by: Emil Velikov 
> ---
> Please keep me in the CC list, as I'm not subscribed to the list.
> 
> IMHO it would be better if this gets merged this via the tty tree.

Works for me:

Reviewed-by: Paul E. McKenney 

> ---
>  kernel/rcu/tree_stall.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/rcu/tree_stall.h b/kernel/rcu/tree_stall.h
> index 119ed6afd20f..4e6ed7b91f59 100644
> --- a/kernel/rcu/tree_stall.h
> +++ b/kernel/rcu/tree_stall.h
> @@ -729,7 +729,7 @@ static void sysrq_show_rcu(int key)
>   show_rcu_gp_kthreads();
>  }
>  
> -static struct sysrq_key_op sysrq_rcudump_op = {
> +static const struct sysrq_key_op sysrq_rcudump_op = {
>   .handler = sysrq_show_rcu,
>   .help_msg = "show-rcu(y)",
>   .action_msg = "Show RCU tree",
> -- 
> 2.25.1
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 03/35] docs: fix broken references to text files

[Paul E. McKenney]

On Wed, Apr 08, 2020 at 05:45:55PM +0200, Mauro Carvalho Chehab wrote:
> Several references got broken due to txt to ReST conversion.
> 
> Several of them can be automatically fixed with:
> 
>   scripts/documentation-file-ref-check --fix
> 
> Reviewed-by: Mathieu Poirier  # 
> hwtracing/coresight/Kconfig
> Signed-off-by: Mauro Carvalho Chehab 

For the memory-barrier.txt portions:

Reviewed-by: Paul E. McKenney 

> ---
>  Documentation/memory-barriers.txt|  2 +-
>  Documentation/process/submit-checklist.rst   |  2 +-
>  .../translations/it_IT/process/submit-checklist.rst  |  2 +-
>  Documentation/translations/ko_KR/memory-barriers.txt |  2 +-
>  .../translations/zh_CN/filesystems/sysfs.txt |  2 +-
>  .../translations/zh_CN/process/submit-checklist.rst  |  2 +-
>  Documentation/virt/kvm/arm/pvtime.rst|  2 +-
>  Documentation/virt/kvm/devices/vcpu.rst  |  2 +-
>  Documentation/virt/kvm/hypercalls.rst|  4 ++--
>  arch/powerpc/include/uapi/asm/kvm_para.h |  2 +-
>  drivers/gpu/drm/Kconfig  |  2 +-
>  drivers/gpu/drm/drm_ioctl.c  |  2 +-
>  drivers/hwtracing/coresight/Kconfig  |  2 +-
>  fs/fat/Kconfig   |  8 
>  fs/fuse/Kconfig  |  2 +-
>  fs/fuse/dev.c|  2 +-
>  fs/overlayfs/Kconfig |  6 +++---
>  include/linux/mm.h   |  4 ++--
>  include/uapi/linux/ethtool_netlink.h |  2 +-
>  include/uapi/rdma/rdma_user_ioctl_cmds.h |  2 +-
>  mm/gup.c | 12 ++--
>  virt/kvm/arm/vgic/vgic-mmio-v3.c |  2 +-
>  virt/kvm/arm/vgic/vgic.h |  4 ++--
>  23 files changed, 36 insertions(+), 36 deletions(-)
> 
> diff --git a/Documentation/memory-barriers.txt 
> b/Documentation/memory-barriers.txt
> index e1c355e84edd..eaabc3134294 100644
> --- a/Documentation/memory-barriers.txt
> +++ b/Documentation/memory-barriers.txt
> @@ -620,7 +620,7 @@ because the CPUs that the Linux kernel supports don't do 
> writes
>  until they are certain (1) that the write will actually happen, (2)
>  of the location of the write, and (3) of the value to be written.
>  But please carefully read the "CONTROL DEPENDENCIES" section and the
> -Documentation/RCU/rcu_dereference.txt file:  The compiler can and does
> +Documentation/RCU/rcu_dereference.rst file:  The compiler can and does
>  break dependencies in a great many highly creative ways.
>  
>   CPU 1 CPU 2
> diff --git a/Documentation/process/submit-checklist.rst 
> b/Documentation/process/submit-checklist.rst
> index 8e56337d422d..3f8e9d5d95c2 100644
> --- a/Documentation/process/submit-checklist.rst
> +++ b/Documentation/process/submit-checklist.rst
> @@ -107,7 +107,7 @@ and elsewhere regarding submitting Linux kernel patches.
>  and why.
>  
>  26) If any ioctl's are added by the patch, then also update
> -``Documentation/ioctl/ioctl-number.rst``.
> +``Documentation/userspace-api/ioctl/ioctl-number.rst``.
>  
>  27) If your modified source code depends on or uses any of the kernel
>  APIs or features that are related to the following ``Kconfig`` symbols,
> diff --git a/Documentation/translations/it_IT/process/submit-checklist.rst 
> b/Documentation/translations/it_IT/process/submit-checklist.rst
> index 995ee69fab11..3e575502690f 100644
> --- a/Documentation/translations/it_IT/process/submit-checklist.rst
> +++ b/Documentation/translations/it_IT/process/submit-checklist.rst
> @@ -117,7 +117,7 @@ sottomissione delle patch, in particolare
>  sorgenti che ne spieghi la logica: cosa fanno e perché.
>  
>  25) Se la patch aggiunge nuove chiamate ioctl, allora aggiornate
> -``Documentation/ioctl/ioctl-number.rst``.
> +``Documentation/userspace-api/ioctl/ioctl-number.rst``.
>  
>  26) Se il codice che avete modificato dipende o usa una qualsiasi 
> interfaccia o
>  funzionalità del kernel che è associata a uno dei seguenti simboli
> diff --git a/Documentation/translations/ko_KR/memory-barriers.txt 
> b/Documentation/translations/ko_KR/memory-barriers.txt
> index 2e831ece6e26..e50fe6541335 100644
> --- a/Documentation/translations/ko_KR/memory-barriers.txt
> +++ b/Documentation/translations/ko_KR/memory-barriers.txt
> @@ -641,7 +641,7 @@ P 는 짝수 번호 캐시 라인에 저장되어 있고, 변수 B 는 홀수 
>  리눅스 커널이 지원하는 CPU 들은 (1) 쓰기가 정말로 일어날지, (2) 쓰기가 어디에
>  이루어질지, 그리고 (3) 쓰여질 값을 확실히 알기 전까지는 쓰기를 수행하지 않기
>  때문입니다.  하지만 "컨트롤 의존성" 섹션

Re: rcu_barrier() no longer allowed within mmap_sem?

[Paul E. McKenney]

On Mon, Mar 30, 2020 at 03:00:35PM +0200, Daniel Vetter wrote:
> Hi all, for all = rcu, cpuhotplug and perf maintainers
> 
> We've hit an interesting new lockdep splat in our drm/i915 CI:
> 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17096/shard-tglb7/igt@kms_frontbuffer_track...@fbcpsr-rgb101010-draw-mmap-gtt.html#dmesg-warnings861
> 
> Summarizing away the driver parts we have
> 
> < gpu locks which are held within mm->mmap_sem in various gpu fault handlers >
> 
> -> #4 (>mmap_sem#2){}:
> <4> [604.892615] __might_fault+0x63/0x90
> <4> [604.892617] _copy_to_user+0x1e/0x80
> <4> [604.892619] perf_read+0x200/0x2b0
> <4> [604.892621] vfs_read+0x96/0x160
> <4> [604.892622] ksys_read+0x9f/0xe0
> <4> [604.892623] do_syscall_64+0x4f/0x220
> <4> [604.892624] entry_SYSCALL_64_after_hwframe+0x49/0xbe
> <4> [604.892625]
> -> #3 (_mutex){+.+.}:
> <4> [604.892626] __mutex_lock+0x9a/0x9c0
> <4> [604.892627] perf_event_init_cpu+0xa4/0x140
> <4> [604.892629] perf_event_init+0x19d/0x1cd
> <4> [604.892630] start_kernel+0x362/0x4e4
> <4> [604.892631] secondary_startup_64+0xa4/0xb0
> <4> [604.892631]
> -> #2 (pmus_lock){+.+.}:
> <4> [604.892633] __mutex_lock+0x9a/0x9c0
> <4> [604.892633] perf_event_init_cpu+0x6b/0x140
> <4> [604.892635] cpuhp_invoke_callback+0x9b/0x9d0
> <4> [604.892636] _cpu_up+0xa2/0x140
> <4> [604.892637] do_cpu_up+0x61/0xa0
> <4> [604.892639] smp_init+0x57/0x96
> <4> [604.892639] kernel_init_freeable+0x87/0x1dc
> <4> [604.892640] kernel_init+0x5/0x100
> <4> [604.892642] ret_from_fork+0x24/0x50
> <4> [604.892642]
> -> #1 (cpu_hotplug_lock.rw_sem){}:
> <4> [604.892643] cpus_read_lock+0x34/0xd0
> <4> [604.892644] rcu_barrier+0xaa/0x190
> <4> [604.892645] kernel_init+0x21/0x100
> <4> [604.892647] ret_from_fork+0x24/0x50
> <4> [604.892647]
> -> #0 (rcu_state.barrier_mutex){+.+.}:
> <4> [604.892649] __lock_acquire+0x1328/0x15d0
> <4> [604.892650] lock_acquire+0xa7/0x1c0
> <4> [604.892651] __mutex_lock+0x9a/0x9c0
> <4> [604.892652] rcu_barrier+0x23/0x190
> <4> [604.892680] i915_gem_object_unbind+0x29d/0x3f0 [i915]
> <4> [604.892707] i915_gem_object_pin_to_display_plane+0x141/0x270 [i915]
> <4> [604.892737] intel_pin_and_fence_fb_obj+0xec/0x1f0 [i915]
> <4> [604.892767] intel_plane_pin_fb+0x3f/0xd0 [i915]
> <4> [604.892797] intel_prepare_plane_fb+0x13b/0x5c0 [i915]
> <4> [604.892798] drm_atomic_helper_prepare_planes+0x85/0x110
> <4> [604.892827] intel_atomic_commit+0xda/0x390 [i915]
> <4> [604.892828] drm_atomic_helper_set_config+0x57/0xa0
> <4> [604.892830] drm_mode_setcrtc+0x1c4/0x720
> <4> [604.892830] drm_ioctl_kernel+0xb0/0xf0
> <4> [604.892831] drm_ioctl+0x2e1/0x390
> <4> [604.892833] ksys_ioctl+0x7b/0x90
> <4> [604.892835] __x64_sys_ioctl+0x11/0x20
> <4> [604.892835] do_syscall_64+0x4f/0x220
> <4> [604.892836] entry_SYSCALL_64_after_hwframe+0x49/0xbe
> 
> The last backtrace boils down to i915 driver code which holds the same
> locks we are holding within mm->mmap_sem, and then ends up calling
> rcu_barrier(). From what I can see i915 is just the messenger here,
> any driver with this pattern of a lock held within mmap_sem which also
> has a path of calling rcu_barrier while holding that lock should be
> hitting this splat.
> 
> Two questions:
> - This suggests that calling rcu_barrier() isn't ok anymore while
> holding mmap_sem, or anything that has a dependency upon mmap_sem. I
> guess that's not the idea, please confirm.
> - Assuming this depedency is indeed not intended, where should the
> loop be broken? It goes through perf, cpuhotplug and rcu subsystems,
> and I don't have a clue about any of those.

Indeed, rcu_barrier() excludes CPU hotplug in order to eliminate a number
of interesting races.

Am I interpreting the above trace correctly in thinking that the various
calls to cpus_read_lock() are with mmap_sem held?  If so, can the calls
to rcu_barrier() be moved out from under the regions of code protected
by cpus_read_lock()?  Invoking rcu_barrier() with cpus_read_lock() held
is an immediate self-deadlock.

Or is rcu_barrier() somehow indirectly sometimes acquiring mmap_sem
or pmus_lock?  (Not seeing it myself, but...)

Thanx, Paul
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] Make drm_dp_mst_dsc_aux_for_port() safe for old compilers

[Paul E. McKenney]

Older compilers either want two extra pairs of curly braces around the
initializer for local variable "desc", or they want a single pair of
curly braces with nothing inside.  Current Linux-kernel practice favors
the latter, so this commit makes it so.

This is a fix for a regression introduced into v5.6-rc1.

Fixes: 5b03f9d86880 ("drm/dp_mst: Add new quirk for Synaptics MST hubs")
Suggested-by: Chris Wilson 
Suggested-by: Joe Perches 
Suggested-by: Christoph Hellwig 
Signed-off-by: Paul E. McKenney 
Cc: Maxime Ripard 
Cc: David Airlie 
Cc: Daniel Vetter 

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 20cdaf3..b123f60 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -5396,7 +5396,7 @@ struct drm_dp_aux *drm_dp_mst_dsc_aux_for_port(struct 
drm_dp_mst_port *port)
 {
struct drm_dp_mst_port *immediate_upstream_port;
struct drm_dp_mst_port *fec_port;
-   struct drm_dp_desc desc = { 0 };
+   struct drm_dp_desc desc = { };
u8 endpoint_fec;
u8 endpoint_dsc;
 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: drm_dp_mst_topology.c and old compilers

[Paul E. McKenney]

On Thu, Feb 20, 2020 at 07:58:58AM +, Chris Wilson wrote:
> Quoting Alex Deucher (2020-02-20 02:52:32)
> > On Wed, Feb 19, 2020 at 7:42 PM Paul E. McKenney  wrote:
> > >
> > > Hello!
> > >
> > > A box with GCC 4.8.3 compiler didn't like drm_dp_mst_topology.c.  The
> > > following (lightly tested) patch makes it happy and seems OK for newer
> > > compilers as well.
> > >
> > > Is this of interest?
> > 
> > How about a memset instead?  That should be consistent across compilers.
> 
> The kernel has adopted the gccism: struct drm_dp_desc desc = {};
> git grep '= {}' | wc -l: 2046
> git grep '= { }' | wc -l: 694
> -Chris

And this works well, a big "thank you!" to all three of you!

Please see below for the updated patch.

Thanx, Paul

----

commit 78c0e53a98a9772a99e46806f8fcbe1140d667a4
Author: Paul E. McKenney 
Date:   Wed Feb 19 16:42:47 2020 -0800

EXP drm: Make drm_dp_mst_dsc_aux_for_port() safe for old compilers

Older compilers either want two extra pairs of curly braces around the
initializer for local variable desc, or they want a single pair of curly
braces with nothing inside.  Current Linux-kernel practice favors the
latter, so this commit makes it so.

    Suggested-by: Chris Wilson 
Suggested-by: Joe Perches 
Suggested-by: Christoph Hellwig 
Signed-off-by: Paul E. McKenney 

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 20cdaf3..b123f60 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -5396,7 +5396,7 @@ struct drm_dp_aux *drm_dp_mst_dsc_aux_for_port(struct 
drm_dp_mst_port *port)
 {
struct drm_dp_mst_port *immediate_upstream_port;
struct drm_dp_mst_port *fec_port;
-   struct drm_dp_desc desc = { 0 };
+   struct drm_dp_desc desc = { };
u8 endpoint_fec;
u8 endpoint_dsc;
 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

drm_dp_mst_topology.c and old compilers

[Paul E. McKenney]

Hello!

A box with GCC 4.8.3 compiler didn't like drm_dp_mst_topology.c.  The
following (lightly tested) patch makes it happy and seems OK for newer
compilers as well.

Is this of interest?

Thanx, Paul

---

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 20cdaf3..232408a 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -5396,7 +5396,7 @@ struct drm_dp_aux *drm_dp_mst_dsc_aux_for_port(struct 
drm_dp_mst_port *port)
 {
struct drm_dp_mst_port *immediate_upstream_port;
struct drm_dp_mst_port *fec_port;
-   struct drm_dp_desc desc = { 0 };
+   struct drm_dp_desc desc = {{{ 0 }}};
u8 endpoint_fec;
u8 endpoint_dsc;
 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[tip: core/rcu] drm/i915: Replace rcu_swap_protected() with rcu_replace_pointer()

[tip-bot2 for Paul E. McKenney]

The following commit has been merged into the core/rcu branch of tip:

Commit-ID: 1feace5d6a4a1acf44dde2bfb5c36cc0b1cf559c
Gitweb:
https://git.kernel.org/tip/1feace5d6a4a1acf44dde2bfb5c36cc0b1cf559c
Author:Paul E. McKenney 
AuthorDate:Mon, 23 Sep 2019 15:22:15 -07:00
Committer: Paul E. McKenney 
CommitterDate: Wed, 30 Oct 2019 08:44:04 -07:00

drm/i915: Replace rcu_swap_protected() with rcu_replace_pointer()

This commit replaces the use of rcu_swap_protected() with the more
intuitively appealing rcu_replace_pointer() as a step towards removing
rcu_swap_protected().

Link: 
https://lore.kernel.org/lkml/CAHk-=wiAsJLw1egFEE=z7-ggtm6wcvtyytxza1+bhqta4gg...@mail.gmail.com/
Reported-by: Linus Torvalds 
[ paulmck: From rcu_replace() to rcu_replace_pointer() per Ingo Molnar. ]
Signed-off-by: Paul E. McKenney 
Reviewed-by: Joonas Lahtinen 
Cc: Jani Nikula 
Cc: Rodrigo Vivi 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Chris Wilson 
Cc: Tvrtko Ursulin 
Cc: 
Cc: 
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c 
b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 1cdfe05..3f3e803 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -1629,7 +1629,7 @@ replace:
i915_gem_context_set_user_engines(ctx);
else
i915_gem_context_clear_user_engines(ctx);
-   rcu_swap_protected(ctx->engines, set.engines, 1);
+   set.engines = rcu_replace_pointer(ctx->engines, set.engines, 1);
mutex_unlock(>engines_mutex);
 
call_rcu(>rcu, free_engines_rcu);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH tip/core/rcu 03/10] drivers/gpu: Replace rcu_swap_protected() with rcu_replace()

[Paul E. McKenney]

On Mon, Oct 28, 2019 at 02:57:26PM +0200, Joonas Lahtinen wrote:
> Quoting paul...@kernel.org (2019-10-22 22:12:08)
> > From: "Paul E. McKenney" 
> > 
> > This commit replaces the use of rcu_swap_protected() with the more
> > intuitively appealing rcu_replace() as a step towards removing
> > rcu_swap_protected().
> > 
> > Link: 
> > https://lore.kernel.org/lkml/CAHk-=wiAsJLw1egFEE=z7-ggtm6wcvtyytxza1+bhqta4gg...@mail.gmail.com/
> > Reported-by: Linus Torvalds 
> > [ paulmck: From rcu_replace() to rcu_replace_pointer() per Ingo Molnar. ]
> > Signed-off-by: Paul E. McKenney 
> > Cc: Jani Nikula 
> > Cc: Joonas Lahtinen 
> > Cc: Rodrigo Vivi 
> > Cc: David Airlie 
> > Cc: Daniel Vetter 
> > Cc: Chris Wilson 
> > Cc: Tvrtko Ursulin 
> > Cc: 
> > Cc: 
> 
> "drm/i915:" preferred as the subject prefix for increased specificity.

"drm/i915" it is!

> Let me know which tree you end up merging with.

I expect to be sending a pull request for inclusion into the -tip
tree in a day or three.

> Reviewed-by: Joonas Lahtinen 

Applied, thank you!

Thanx, Paul
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/7] docs: fix broken doc references due to renames

[Paul E. McKenney]

On Fri, Jul 26, 2019 at 08:47:21AM -0300, Mauro Carvalho Chehab wrote:
> Some files got renamed but probably due to some merge conflicts,
> a few references still point to the old locations.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> Acked-by: Wolfram Sang  # I2C part
> Reviewed-by: Jerry Hoemann  # hpwdt.rst

Acked-by: Paul E. McKenney 

> ---
>  Documentation/RCU/rculist_nulls.txt   |  2 +-
>  Documentation/devicetree/bindings/arm/idle-states.txt |  2 +-
>  Documentation/locking/spinlocks.rst   |  4 ++--
>  Documentation/memory-barriers.txt |  2 +-
>  Documentation/translations/ko_KR/memory-barriers.txt  |  2 +-
>  Documentation/watchdog/hpwdt.rst  |  2 +-
>  MAINTAINERS   | 10 +-
>  drivers/gpu/drm/drm_modes.c   |  2 +-
>  drivers/i2c/busses/i2c-nvidia-gpu.c   |  2 +-
>  drivers/scsi/hpsa.c   |  4 ++--
>  10 files changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/Documentation/RCU/rculist_nulls.txt 
> b/Documentation/RCU/rculist_nulls.txt
> index 8151f0195f76..23f115dc87cf 100644
> --- a/Documentation/RCU/rculist_nulls.txt
> +++ b/Documentation/RCU/rculist_nulls.txt
> @@ -1,7 +1,7 @@
>  Using hlist_nulls to protect read-mostly linked lists and
>  objects using SLAB_TYPESAFE_BY_RCU allocations.
>  
> -Please read the basics in Documentation/RCU/listRCU.txt
> +Please read the basics in Documentation/RCU/listRCU.rst
>  
>  Using special makers (called 'nulls') is a convenient way
>  to solve following problem :
> diff --git a/Documentation/devicetree/bindings/arm/idle-states.txt 
> b/Documentation/devicetree/bindings/arm/idle-states.txt
> index 326f29b270ad..2d325bed37e5 100644
> --- a/Documentation/devicetree/bindings/arm/idle-states.txt
> +++ b/Documentation/devicetree/bindings/arm/idle-states.txt
> @@ -703,4 +703,4 @@ cpus {
>  https://www.devicetree.org/specifications/
>  
>  [6] ARM Linux Kernel documentation - Booting AArch64 Linux
> -Documentation/arm64/booting.txt
> +Documentation/arm64/booting.rst
> diff --git a/Documentation/locking/spinlocks.rst 
> b/Documentation/locking/spinlocks.rst
> index 098107fb7d86..e93ec6645238 100644
> --- a/Documentation/locking/spinlocks.rst
> +++ b/Documentation/locking/spinlocks.rst
> @@ -82,7 +82,7 @@ itself.  The read lock allows many concurrent readers.  
> Anything that
>  **changes** the list will have to get the write lock.
>  
> NOTE! RCU is better for list traversal, but requires careful
> -   attention to design detail (see Documentation/RCU/listRCU.txt).
> +   attention to design detail (see Documentation/RCU/listRCU.rst).
>  
>  Also, you cannot "upgrade" a read-lock to a write-lock, so if you at _any_
>  time need to do any changes (even if you don't do it every time), you have
> @@ -90,7 +90,7 @@ to get the write-lock at the very beginning.
>  
> NOTE! We are working hard to remove reader-writer spinlocks in most
> cases, so please don't add a new one without consensus.  (Instead, see
> -   Documentation/RCU/rcu.txt for complete information.)
> +   Documentation/RCU/rcu.rst for complete information.)
>  
>  
>  
> diff --git a/Documentation/memory-barriers.txt 
> b/Documentation/memory-barriers.txt
> index 045bb8148fe9..1adbb8a371c7 100644
> --- a/Documentation/memory-barriers.txt
> +++ b/Documentation/memory-barriers.txt
> @@ -548,7 +548,7 @@ There are certain things that the Linux kernel memory 
> barriers do not guarantee:
>  
>   [*] For information on bus mastering DMA and coherency please read:
>  
> - Documentation/PCI/pci.rst
> + Documentation/driver-api/pci/pci.rst
>   Documentation/DMA-API-HOWTO.txt
>   Documentation/DMA-API.txt
>  
> diff --git a/Documentation/translations/ko_KR/memory-barriers.txt 
> b/Documentation/translations/ko_KR/memory-barriers.txt
> index a33c2a536542..2774624ee843 100644
> --- a/Documentation/translations/ko_KR/memory-barriers.txt
> +++ b/Documentation/translations/ko_KR/memory-barriers.txt
> @@ -569,7 +569,7 @@ ACQUIRE 는 해당 오퍼레이션의 로드 부분에만 적용되고 RELEASE 
>  
>   [*] 버스 마스터링 DMA 와 일관성에 대해서는 다음을 참고하시기 바랍니다:
>  
> - Documentation/PCI/pci.rst
> + Documentation/driver-api/pci/pci.rst
>   Documentation/DMA-API-HOWTO.txt
>   Documentation/DMA-API.txt
>  
> diff --git a/Documentation/watchdog/hpwdt.rst 
> b/Documentation/watchdog/hpwdt.rst
> index c165d92cfd12..c824cd7f6e32 100644
> --- a/Documentation/watchdog/hpwdt.rst
> +++ b/Documentation/watchdog/hpwdt.rst
> @@ -63,7 +63,7 @@ Last reviewed: 08/20/2018

Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules

[Paul E. McKenney]

On Tue, Apr 09, 2019 at 02:04:11PM -0400, Mathieu Desnoyers wrote:
> - On Apr 9, 2019, at 1:55 PM, paulmck paul...@linux.ibm.com wrote:
> [...]
> > The current state is not horrible, so my thought would be to give it
> > some time to see if better thoughts arise.
> > 
> > Either way, cleanup_srcu_struct() keeps its current checks for callbacks
> > still being in flight, which is why I believe that the current state is
> > not horrible.  ;-)
> 
> In that case, I think the comment above cleanup_srcu_struct_quiesced() in
> include/linux/srcu.h needs to be updated to cover situations where API
> users statically define a SRCU domain in a module and intend to unload
> that module.
> 
> Given that we end up doing the allocation/cleanup under the hood, the
> API users don't interact with init_srcu_struct() nor cleanup_srcu_struct(),
> so it's not obvious that this comment also applies to them.

Actually, it turned out that cleanup_srcu_struct_quiesced() is extremely
hard to use correctly, and maybe even impossible to use correctly.
So cleanup_srcu_struct_quiesced has been eliminated in current -rcu.

Thanx, Paul

> Thanks,
> 
> Mathieu
> 
> -- 
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com
> 

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules

[Paul E. McKenney]

On Tue, Apr 09, 2019 at 11:56:03AM -0400, Mathieu Desnoyers wrote:
> - On Apr 9, 2019, at 11:40 AM, Joel Fernandes, Google 
> j...@joelfernandes.org wrote:
> 
> > On Mon, Apr 08, 2019 at 01:24:47PM -0400, Mathieu Desnoyers wrote:
> >> - On Apr 8, 2019, at 11:46 AM, paulmck paul...@linux.ibm.com wrote:
> >> 
> >> > On Mon, Apr 08, 2019 at 10:49:32AM -0400, Mathieu Desnoyers wrote:
> >> >> - On Apr 8, 2019, at 10:22 AM, paulmck paul...@linux.ibm.com wrote:
> >> >> 
> >> >> > On Mon, Apr 08, 2019 at 09:05:34AM -0400, Mathieu Desnoyers wrote:
> >> >> >> - On Apr 7, 2019, at 10:27 PM, paulmck paul...@linux.ibm.com 
> >> >> >> wrote:
> >> >> >> 
> >> >> >> > On Sun, Apr 07, 2019 at 09:07:18PM +, Joel Fernandes wrote:
> >> >> >> >> On Sun, Apr 07, 2019 at 04:41:36PM -0400, Mathieu Desnoyers wrote:
> >> >> >> >> > 
> >> >> >> >> > - On Apr 7, 2019, at 3:32 PM, Joel Fernandes, Google 
> >> >> >> >> > j...@joelfernandes.org
> >> >> >> >> > wrote:
> >> >> >> >> > 
> >> >> >> >> > > On Sun, Apr 07, 2019 at 03:26:16PM -0400, Mathieu Desnoyers 
> >> >> >> >> > > wrote:
> >> >> >> >> > >> - On Apr 7, 2019, at 9:59 AM, paulmck 
> >> >> >> >> > >> paul...@linux.ibm.com wrote:
> >> >> >> >> > >> 
> >> >> >> >> > >> > On Sun, Apr 07, 2019 at 06:39:41AM -0700, Paul E. McKenney 
> >> >> >> >> > >> > wrote:
> >> >> >> >> > >> >> On Sat, Apr 06, 2019 at 07:06:13PM -0400, Joel Fernandes 
> >> >> >> >> > >> >> wrote:
> >> >> >> >> > >> > 
> >> >> >> >> > >> > [ . . . ]
> >> >> >> >> > >> > 
> >> >> >> >> > >> >> > > diff --git a/include/asm-generic/vmlinux.lds.h
> >> >> >> >> > >> >> > > b/include/asm-generic/vmlinux.lds.h
> >> >> >> >> > >> >> > > index f8f6f04c4453..c2d919a1566e 100644
> >> >> >> >> > >> >> > > --- a/include/asm-generic/vmlinux.lds.h
> >> >> >> >> > >> >> > > +++ b/include/asm-generic/vmlinux.lds.h
> >> >> >> >> > >> >> > > @@ -338,6 +338,10 @@
> >> >> >> >> > >> >> > >   KEEP(*(__tracepoints_ptrs)) /* 
> >> >> >> >> > >> >> > > Tracepoints: pointer array */ \
> >> >> >> >> > >> >> > >   __stop___tracepoints_ptrs = .;  
> >> >> >> >> > >> >> > > \
> >> >> >> >> > >> >> > >   *(__tracepoints_strings)/* Tracepoints: 
> >> >> >> >> > >> >> > > strings */  \
> >> >> >> >> > >> >> > > + . = ALIGN(8);   
> >> >> >> >> > >> >> > > \
> >> >> >> >> > >> >> > > + __start___srcu_struct = .;  
> >> >> >> >> > >> >> > > \
> >> >> >> >> > >> >> > > + *(___srcu_struct_ptrs)  
> >> >> >> >> > >> >> > > \
> >> >> >> >> > >> >> > > + __end___srcu_struct = .;
> >> >> >> >> > >> >> > > \
> >> >> >> >> > >> >> > >   }   
> >> >> >> >> > >> >> > > \
> >> >> >> >> > >> >> > 
> >> >> >> >> > >> >> > This vmlinux linker modification is not needed. I 
> >> >> >> 

Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules

[Paul E. McKenney]

On Tue, Apr 09, 2019 at 12:45:25PM -0400, Mathieu Desnoyers wrote:
> - On Apr 9, 2019, at 12:40 PM, paulmck paul...@linux.ibm.com wrote:
> 
> > On Tue, Apr 09, 2019 at 11:56:03AM -0400, Mathieu Desnoyers wrote:
> >> - On Apr 9, 2019, at 11:40 AM, Joel Fernandes, Google 
> >> j...@joelfernandes.org
> >> wrote:
> >> 
> >> > On Mon, Apr 08, 2019 at 01:24:47PM -0400, Mathieu Desnoyers wrote:
> >> >> - On Apr 8, 2019, at 11:46 AM, paulmck paul...@linux.ibm.com wrote:
> >> >> 
> >> >> > On Mon, Apr 08, 2019 at 10:49:32AM -0400, Mathieu Desnoyers wrote:
> >> >> >> - On Apr 8, 2019, at 10:22 AM, paulmck paul...@linux.ibm.com 
> >> >> >> wrote:
> >> >> >> 
> >> >> >> > On Mon, Apr 08, 2019 at 09:05:34AM -0400, Mathieu Desnoyers wrote:
> >> >> >> >> - On Apr 7, 2019, at 10:27 PM, paulmck paul...@linux.ibm.com 
> >> >> >> >> wrote:
> >> >> >> >> 
> >> >> >> >> > On Sun, Apr 07, 2019 at 09:07:18PM +, Joel Fernandes wrote:
> >> >> >> >> >> On Sun, Apr 07, 2019 at 04:41:36PM -0400, Mathieu Desnoyers 
> >> >> >> >> >> wrote:
> >> >> >> >> >> > 
> >> >> >> >> >> > - On Apr 7, 2019, at 3:32 PM, Joel Fernandes, Google 
> >> >> >> >> >> > j...@joelfernandes.org
> >> >> >> >> >> > wrote:
> >> >> >> >> >> > 
> >> >> >> >> >> > > On Sun, Apr 07, 2019 at 03:26:16PM -0400, Mathieu 
> >> >> >> >> >> > > Desnoyers wrote:
> >> >> >> >> >> > >> - On Apr 7, 2019, at 9:59 AM, paulmck 
> >> >> >> >> >> > >> paul...@linux.ibm.com wrote:
> >> >> >> >> >> > >> 
> >> >> >> >> >> > >> > On Sun, Apr 07, 2019 at 06:39:41AM -0700, Paul E. 
> >> >> >> >> >> > >> > McKenney wrote:
> >> >> >> >> >> > >> >> On Sat, Apr 06, 2019 at 07:06:13PM -0400, Joel 
> >> >> >> >> >> > >> >> Fernandes wrote:
> >> >> >> >> >> > >> > 
> >> >> >> >> >> > >> > [ . . . ]
> >> >> >> >> >> > >> > 
> >> >> >> >> >> > >> >> > > diff --git a/include/asm-generic/vmlinux.lds.h
> >> >> >> >> >> > >> >> > > b/include/asm-generic/vmlinux.lds.h
> >> >> >> >> >> > >> >> > > index f8f6f04c4453..c2d919a1566e 100644
> >> >> >> >> >> > >> >> > > --- a/include/asm-generic/vmlinux.lds.h
> >> >> >> >> >> > >> >> > > +++ b/include/asm-generic/vmlinux.lds.h
> >> >> >> >> >> > >> >> > > @@ -338,6 +338,10 @@
> >> >> >> >> >> > >> >> > >KEEP(*(__tracepoints_ptrs)) /* 
> >> >> >> >> >> > >> >> > > Tracepoints: pointer array */ \
> >> >> >> >> >> > >> >> > >__stop___tracepoints_ptrs = .;  
> >> >> >> >> >> > >> >> > > \
> >> >> >> >> >> > >> >> > >*(__tracepoints_strings)/* Tracepoints: 
> >> >> >> >> >> > >> >> > > strings */  \
> >> >> >> >> >> > >> >> > > +  . = ALIGN(8);   
> >> >> >> >> >> > >> >> > > \
> >> >> >> >> >> > >> >> > > +  __start___srcu_struct = .;  
> >> >> >> >> >> > >> >> > > \
> >> >> >> >> >> > >> >> > > +  *(___srcu_struct_ptrs)  
> >> >> >> >> >> &g

Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules

[Paul E. McKenney]

On Mon, Apr 08, 2019 at 10:49:32AM -0400, Mathieu Desnoyers wrote:
> - On Apr 8, 2019, at 10:22 AM, paulmck paul...@linux.ibm.com wrote:
> 
> > On Mon, Apr 08, 2019 at 09:05:34AM -0400, Mathieu Desnoyers wrote:
> >> - On Apr 7, 2019, at 10:27 PM, paulmck paul...@linux.ibm.com wrote:
> >> 
> >> > On Sun, Apr 07, 2019 at 09:07:18PM +, Joel Fernandes wrote:
> >> >> On Sun, Apr 07, 2019 at 04:41:36PM -0400, Mathieu Desnoyers wrote:
> >> >> > 
> >> >> > - On Apr 7, 2019, at 3:32 PM, Joel Fernandes, Google 
> >> >> > j...@joelfernandes.org
> >> >> > wrote:
> >> >> > 
> >> >> > > On Sun, Apr 07, 2019 at 03:26:16PM -0400, Mathieu Desnoyers wrote:
> >> >> > >> - On Apr 7, 2019, at 9:59 AM, paulmck paul...@linux.ibm.com 
> >> >> > >> wrote:
> >> >> > >> 
> >> >> > >> > On Sun, Apr 07, 2019 at 06:39:41AM -0700, Paul E. McKenney wrote:
> >> >> > >> >> On Sat, Apr 06, 2019 at 07:06:13PM -0400, Joel Fernandes wrote:
> >> >> > >> > 
> >> >> > >> > [ . . . ]
> >> >> > >> > 
> >> >> > >> >> > > diff --git a/include/asm-generic/vmlinux.lds.h
> >> >> > >> >> > > b/include/asm-generic/vmlinux.lds.h
> >> >> > >> >> > > index f8f6f04c4453..c2d919a1566e 100644
> >> >> > >> >> > > --- a/include/asm-generic/vmlinux.lds.h
> >> >> > >> >> > > +++ b/include/asm-generic/vmlinux.lds.h
> >> >> > >> >> > > @@ -338,6 +338,10 @@
> >> >> > >> >> > > KEEP(*(__tracepoints_ptrs)) /* Tracepoints: 
> >> >> > >> >> > > pointer array */ \
> >> >> > >> >> > > __stop___tracepoints_ptrs = .;  
> >> >> > >> >> > > \
> >> >> > >> >> > > *(__tracepoints_strings)/* Tracepoints: strings 
> >> >> > >> >> > > */  \
> >> >> > >> >> > > +   . = ALIGN(8);   
> >> >> > >> >> > > \
> >> >> > >> >> > > +   __start___srcu_struct = .;  
> >> >> > >> >> > > \
> >> >> > >> >> > > +   *(___srcu_struct_ptrs)  
> >> >> > >> >> > > \
> >> >> > >> >> > > +   __end___srcu_struct = .;
> >> >> > >> >> > > \
> >> >> > >> >> > > }   
> >> >> > >> >> > > \
> >> >> > >> >> > 
> >> >> > >> >> > This vmlinux linker modification is not needed. I tested 
> >> >> > >> >> > without it and srcu
> >> >> > >> >> > torture works fine with rcutorture built as a module. Putting 
> >> >> > >> >> > further prints
> >> >> > >> >> > in kernel/module.c verified that the kernel is able to find 
> >> >> > >> >> > the srcu structs
> >> >> > >> >> > just fine. You could squash the below patch into this one or 
> >> >> > >> >> > apply it on top
> >> >> > >> >> > of the dev branch.
> >> >> > >> >> 
> >> >> > >> >> Good point, given that otherwise FORTRAN named common blocks 
> >> >> > >> >> would not
> >> >> > >> >> work.
> >> >> > >> >> 
> >> >> > >> >> But isn't one advantage of leaving that stuff in the 
> >> >> > >> >> RO_DATA_SECTION()
> >> >> > >> >> macro that it can be mapped read-only?  Or am I suffering from 
> >> >> > >> >> excessive
> >> >> > >> >> optimism?
> >> >> > >> > 
> >

Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules

[Paul E. McKenney]

On Mon, Apr 08, 2019 at 09:05:34AM -0400, Mathieu Desnoyers wrote:
> - On Apr 7, 2019, at 10:27 PM, paulmck paul...@linux.ibm.com wrote:
> 
> > On Sun, Apr 07, 2019 at 09:07:18PM +, Joel Fernandes wrote:
> >> On Sun, Apr 07, 2019 at 04:41:36PM -0400, Mathieu Desnoyers wrote:
> >> > 
> >> > - On Apr 7, 2019, at 3:32 PM, Joel Fernandes, Google 
> >> > j...@joelfernandes.org
> >> > wrote:
> >> > 
> >> > > On Sun, Apr 07, 2019 at 03:26:16PM -0400, Mathieu Desnoyers wrote:
> >> > >> - On Apr 7, 2019, at 9:59 AM, paulmck paul...@linux.ibm.com wrote:
> >> > >> 
> >> > >> > On Sun, Apr 07, 2019 at 06:39:41AM -0700, Paul E. McKenney wrote:
> >> > >> >> On Sat, Apr 06, 2019 at 07:06:13PM -0400, Joel Fernandes wrote:
> >> > >> > 
> >> > >> > [ . . . ]
> >> > >> > 
> >> > >> >> > > diff --git a/include/asm-generic/vmlinux.lds.h
> >> > >> >> > > b/include/asm-generic/vmlinux.lds.h
> >> > >> >> > > index f8f6f04c4453..c2d919a1566e 100644
> >> > >> >> > > --- a/include/asm-generic/vmlinux.lds.h
> >> > >> >> > > +++ b/include/asm-generic/vmlinux.lds.h
> >> > >> >> > > @@ -338,6 +338,10 @@
> >> > >> >> > >KEEP(*(__tracepoints_ptrs)) /* Tracepoints: 
> >> > >> >> > > pointer array */ \
> >> > >> >> > >__stop___tracepoints_ptrs = .;  
> >> > >> >> > > \
> >> > >> >> > >*(__tracepoints_strings)/* Tracepoints: strings 
> >> > >> >> > > */  \
> >> > >> >> > > +  . = ALIGN(8);   
> >> > >> >> > > \
> >> > >> >> > > +  __start___srcu_struct = .;  
> >> > >> >> > > \
> >> > >> >> > > +  *(___srcu_struct_ptrs)  
> >> > >> >> > > \
> >> > >> >> > > +  __end___srcu_struct = .;
> >> > >> >> > > \
> >> > >> >> > >}   
> >> > >> >> > > \
> >> > >> >> > 
> >> > >> >> > This vmlinux linker modification is not needed. I tested without 
> >> > >> >> > it and srcu
> >> > >> >> > torture works fine with rcutorture built as a module. Putting 
> >> > >> >> > further prints
> >> > >> >> > in kernel/module.c verified that the kernel is able to find the 
> >> > >> >> > srcu structs
> >> > >> >> > just fine. You could squash the below patch into this one or 
> >> > >> >> > apply it on top
> >> > >> >> > of the dev branch.
> >> > >> >> 
> >> > >> >> Good point, given that otherwise FORTRAN named common blocks would 
> >> > >> >> not
> >> > >> >> work.
> >> > >> >> 
> >> > >> >> But isn't one advantage of leaving that stuff in the 
> >> > >> >> RO_DATA_SECTION()
> >> > >> >> macro that it can be mapped read-only?  Or am I suffering from 
> >> > >> >> excessive
> >> > >> >> optimism?
> >> > >> > 
> >> > >> > And to answer the other question, in the case where I am suffering 
> >> > >> > from
> >> > >> > excessive optimism, it should be a separate commit.  Please see 
> >> > >> > below
> >> > >> > for the updated original commit thus far.
> >> > >> > 
> >> > >> > And may I have your Tested-by?
> >> > >> 
> >> > >> Just to confirm: does the cleanup performed in the modules going
> >> > >> notifier end up acting as a barrier first before freeing the memory ?
> >> > >> If not, is it explicitly stated t

Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules

[Paul E. McKenney]

On Sat, Apr 06, 2019 at 01:33:27PM +, Joel Fernandes wrote:
> On Fri, Apr 05, 2019 at 04:28:35PM -0700, Paul E. McKenney wrote:
> > On Wed, Apr 03, 2019 at 09:20:39AM -0700, Paul E. McKenney wrote:
> > > On Wed, Apr 03, 2019 at 10:27:42AM -0400, Mathieu Desnoyers wrote:
> > > > - On Apr 3, 2019, at 9:32 AM, paulmck paul...@linux.ibm.com wrote:
> > > > 
> > > > > On Tue, Apr 02, 2019 at 11:34:07AM -0400, Mathieu Desnoyers wrote:
> > > > >> - On Apr 2, 2019, at 11:23 AM, paulmck paul...@linux.ibm.com 
> > > > >> wrote:
> > > > >> 
> > > > >> > On Tue, Apr 02, 2019 at 11:14:40AM -0400, Mathieu Desnoyers wrote:
> > > > >> >> - On Apr 2, 2019, at 10:28 AM, paulmck paul...@linux.ibm.com 
> > > > >> >> wrote:
> > > > >> >> 
> > > > >> >> > Hello!
> > > > >> >> > 
> > > > >> >> > This series prohibits use of DEFINE_SRCU() and 
> > > > >> >> > DEFINE_STATIC_SRCU()
> > > > >> >> > by loadable modules.  The reason for this prohibition is the 
> > > > >> >> > fact
> > > > >> >> > that using these two macros within modules requires that the 
> > > > >> >> > size of
> > > > >> >> > the reserved region be increased, which is not something we 
> > > > >> >> > want to
> > > > >> >> > be doing all that often.  Instead, loadable modules should 
> > > > >> >> > define an
> > > > >> >> > srcu_struct and invoke init_srcu_struct() from their 
> > > > >> >> > module_init function
> > > > >> >> > and cleanup_srcu_struct() from their module_exit function.  
> > > > >> >> > Note that
> > > > >> >> > modules using call_srcu() will also need to invoke 
> > > > >> >> > srcu_barrier() from
> > > > >> >> > their module_exit function.
> > > > >> >> 
> > > > >> >> This arbitrary API limitation seems weird.
> > > > >> >> 
> > > > >> >> Isn't there a way to allow modules to use DEFINE_SRCU and 
> > > > >> >> DEFINE_STATIC_SRCU
> > > > >> >> while implementing them with dynamic allocation under the hood ?
> > > > >> > 
> > > > >> > Although call_srcu() already has initialization hooks, some would
> > > > >> > also be required in srcu_read_lock(), and I am concerned about 
> > > > >> > adding
> > > > >> > memory allocation at that point, especially given the possibility
> > > > >> > of memory-allocation failure.  And the possibility that the first
> > > > >> > srcu_read_lock() happens in an interrupt handler or similar.
> > > > >> > 
> > > > >> > Or am I missing a trick here?
> > > > >> 
> > > > >> I was more thinking that under #ifdef MODULE, both DEFINE_SRCU and
> > > > >> DEFINE_STATIC_SRCU could append data in a dedicated section. module.c
> > > > >> would additionally lookup that section on module load, and deal with
> > > > >> those statically defined SRCU entries as if they were dynamically
> > > > >> allocated ones. It would of course cleanup those resources on module
> > > > >> unload.
> > > > >> 
> > > > >> Am I missing some subtlety there ?
> > > > > 
> > > > > If I understand you correctly, that is actually what is already done. 
> > > > >  The
> > > > > size of this dedicated section is currently set by 
> > > > > PERCPU_MODULE_RESERVE,
> > > > > and the additions of DEFINE{_STATIC}_SRCU() in modules was requiring 
> > > > > that
> > > > > this to be increased frequently.  That led to a request that something
> > > > > be done, in turn leading to this patch series.
> > > > 
> > > > I think we are not expressing quite the same idea.
> > > > 
> > > > AFAIU, yours is to have DEFINE*_SRCU directly define per-cpu data 
> > > > within modules,
> > > > which ends up using percpu module reserved memory.
> > > > 
> 

Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules

[Paul E. McKenney]

On Sun, Apr 07, 2019 at 08:36:46PM -0400, Joel Fernandes wrote:
> On Sun, Apr 07, 2019 at 10:05:14AM -0700, Paul E. McKenney wrote:
> > On Sun, Apr 07, 2019 at 03:46:13PM +, Joel Fernandes wrote:
> > > On Sun, Apr 07, 2019 at 06:59:37AM -0700, Paul E. McKenney wrote:
> > > > On Sun, Apr 07, 2019 at 06:39:41AM -0700, Paul E. McKenney wrote:
> > > > > On Sat, Apr 06, 2019 at 07:06:13PM -0400, Joel Fernandes wrote:
> > > > 
> > > > [ . . . ]
> > > > 
> > > > > > > diff --git a/include/asm-generic/vmlinux.lds.h 
> > > > > > > b/include/asm-generic/vmlinux.lds.h
> > > > > > > index f8f6f04c4453..c2d919a1566e 100644
> > > > > > > --- a/include/asm-generic/vmlinux.lds.h
> > > > > > > +++ b/include/asm-generic/vmlinux.lds.h
> > > > > > > @@ -338,6 +338,10 @@
> > > > > > >   KEEP(*(__tracepoints_ptrs)) /* Tracepoints: pointer 
> > > > > > > array */ \
> > > > > > >   __stop___tracepoints_ptrs = .;  
> > > > > > > \
> > > > > > >   *(__tracepoints_strings)/* Tracepoints: strings */  
> > > > > > > \
> > > > > > > + . = ALIGN(8);   
> > > > > > > \
> > > > > > > + __start___srcu_struct = .;  
> > > > > > > \
> > > > > > > + *(___srcu_struct_ptrs)  
> > > > > > > \
> > > > > > > + __end___srcu_struct = .;
> > > > > > > \
> > > > > > >   }   
> > > > > > > \
> > > > > > 
> > > > > > This vmlinux linker modification is not needed. I tested without it 
> > > > > > and srcu
> > > > > > torture works fine with rcutorture built as a module. Putting 
> > > > > > further prints
> > > > > > in kernel/module.c verified that the kernel is able to find the 
> > > > > > srcu structs
> > > > > > just fine. You could squash the below patch into this one or apply 
> > > > > > it on top
> > > > > > of the dev branch.
> > > > > 
> > > > > Good point, given that otherwise FORTRAN named common blocks would not
> > > > > work.
> > > > > 
> > > > > But isn't one advantage of leaving that stuff in the RO_DATA_SECTION()
> > > > > macro that it can be mapped read-only?  Or am I suffering from 
> > > > > excessive
> > > > > optimism?
> > > > 
> > > > And to answer the other question, in the case where I am suffering from
> > > > excessive optimism, it should be a separate commit.  Please see below
> > > > for the updated original commit thus far.
> > > 
> > > Actually the vmlinux.lds.h file is unused for module building. For ex, if 
> > > you
> > > delete include/asm-generic/vmlinux.lds.h , then you can still build
> > > rcutorture.ko. Did I miss something obvious? In that case the 
> > > vmlinux.lds.h
> > > are not needed since the __section annotations automatically place the 
> > > srcu
> > > structs in a separate section.
> > 
> > Hard to argue given that I just deleted include/asm-generic/vmlinux.lds.h,
> > touched kernel/rcu/rcutorture.c, and rebuilt the corresponding .ko
> > without errors.  ;-)
> > 
> > Hmmm...  Is there some way to place a section into a read-only page,
> > for example, tagged onto the text section for that module?  That would
> > get rid of a class of bugs, if nothing else.
> 
> Strictly speaking, the array of pointers in the new srcu section are fixed up
> at runtime because the srcu_struct(s) they point to can be loaded at a
> dynamic location in memory. The srcu_struct(s) are themselves in the .bss
> section of the module and their locations depend on where the .bss section of
> the module is loaded in memory at load time.
> 
> I agree that after such relocation fixups are done, there is no reason to keep
> the array-of-pointers section readable but unfortunately I couldn't figure a
> way out to make them read-only post the relocations.
> 
> I copied Jessica Yu who maintains module loading for any in

Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules

[Paul E. McKenney]

On Wed, Apr 03, 2019 at 09:20:39AM -0700, Paul E. McKenney wrote:
> On Wed, Apr 03, 2019 at 10:27:42AM -0400, Mathieu Desnoyers wrote:
> > - On Apr 3, 2019, at 9:32 AM, paulmck paul...@linux.ibm.com wrote:
> > 
> > > On Tue, Apr 02, 2019 at 11:34:07AM -0400, Mathieu Desnoyers wrote:
> > >> - On Apr 2, 2019, at 11:23 AM, paulmck paul...@linux.ibm.com wrote:
> > >> 
> > >> > On Tue, Apr 02, 2019 at 11:14:40AM -0400, Mathieu Desnoyers wrote:
> > >> >> - On Apr 2, 2019, at 10:28 AM, paulmck paul...@linux.ibm.com 
> > >> >> wrote:
> > >> >> 
> > >> >> > Hello!
> > >> >> > 
> > >> >> > This series prohibits use of DEFINE_SRCU() and DEFINE_STATIC_SRCU()
> > >> >> > by loadable modules.  The reason for this prohibition is the fact
> > >> >> > that using these two macros within modules requires that the size of
> > >> >> > the reserved region be increased, which is not something we want to
> > >> >> > be doing all that often.  Instead, loadable modules should define an
> > >> >> > srcu_struct and invoke init_srcu_struct() from their module_init 
> > >> >> > function
> > >> >> > and cleanup_srcu_struct() from their module_exit function.  Note 
> > >> >> > that
> > >> >> > modules using call_srcu() will also need to invoke srcu_barrier() 
> > >> >> > from
> > >> >> > their module_exit function.
> > >> >> 
> > >> >> This arbitrary API limitation seems weird.
> > >> >> 
> > >> >> Isn't there a way to allow modules to use DEFINE_SRCU and 
> > >> >> DEFINE_STATIC_SRCU
> > >> >> while implementing them with dynamic allocation under the hood ?
> > >> > 
> > >> > Although call_srcu() already has initialization hooks, some would
> > >> > also be required in srcu_read_lock(), and I am concerned about adding
> > >> > memory allocation at that point, especially given the possibility
> > >> > of memory-allocation failure.  And the possibility that the first
> > >> > srcu_read_lock() happens in an interrupt handler or similar.
> > >> > 
> > >> > Or am I missing a trick here?
> > >> 
> > >> I was more thinking that under #ifdef MODULE, both DEFINE_SRCU and
> > >> DEFINE_STATIC_SRCU could append data in a dedicated section. module.c
> > >> would additionally lookup that section on module load, and deal with
> > >> those statically defined SRCU entries as if they were dynamically
> > >> allocated ones. It would of course cleanup those resources on module
> > >> unload.
> > >> 
> > >> Am I missing some subtlety there ?
> > > 
> > > If I understand you correctly, that is actually what is already done.  The
> > > size of this dedicated section is currently set by PERCPU_MODULE_RESERVE,
> > > and the additions of DEFINE{_STATIC}_SRCU() in modules was requiring that
> > > this to be increased frequently.  That led to a request that something
> > > be done, in turn leading to this patch series.
> > 
> > I think we are not expressing quite the same idea.
> > 
> > AFAIU, yours is to have DEFINE*_SRCU directly define per-cpu data within 
> > modules,
> > which ends up using percpu module reserved memory.
> > 
> > My idea is to make DEFINE*_SRCU have a different behavior under #ifdef 
> > MODULE.
> > It could emit a _global variable_ (_not_ per-cpu) within a new section. That
> > section would then be used by module init/exit code to figure out what "srcu
> > descriptors" are present in the modules. It would therefore rely on dynamic
> > allocation for those, therefore removing the need to involve the percpu 
> > module
> > reserved pool at all.
> > 
> > > 
> > > I don't see a way around this short of changing module loading to do
> > > alloc_percpu() and then updating the relocation based on this result.
> > > Which would admittedly be far more convenient.  I was assuming that
> > > this would be difficult due to varying CPU offsets or the like.
> > > 
> > > But if it can be done reasonably, it would be quite a bit nicer than
> > > forcing dynamic allocation in cases where it is not otherwise needed.
> > 
> > Hopefully my explanation above helps clear out

Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules

[Paul E. McKenney]

On Sun, Apr 07, 2019 at 09:07:18PM +, Joel Fernandes wrote:
> On Sun, Apr 07, 2019 at 04:41:36PM -0400, Mathieu Desnoyers wrote:
> > 
> > - On Apr 7, 2019, at 3:32 PM, Joel Fernandes, Google 
> > j...@joelfernandes.org wrote:
> > 
> > > On Sun, Apr 07, 2019 at 03:26:16PM -0400, Mathieu Desnoyers wrote:
> > >> - On Apr 7, 2019, at 9:59 AM, paulmck paul...@linux.ibm.com wrote:
> > >> 
> > >> > On Sun, Apr 07, 2019 at 06:39:41AM -0700, Paul E. McKenney wrote:
> > >> >> On Sat, Apr 06, 2019 at 07:06:13PM -0400, Joel Fernandes wrote:
> > >> > 
> > >> > [ . . . ]
> > >> > 
> > >> >> > > diff --git a/include/asm-generic/vmlinux.lds.h
> > >> >> > > b/include/asm-generic/vmlinux.lds.h
> > >> >> > > index f8f6f04c4453..c2d919a1566e 100644
> > >> >> > > --- a/include/asm-generic/vmlinux.lds.h
> > >> >> > > +++ b/include/asm-generic/vmlinux.lds.h
> > >> >> > > @@ -338,6 +338,10 @@
> > >> >> > >   KEEP(*(__tracepoints_ptrs)) /* Tracepoints: pointer 
> > >> >> > > array */ \
> > >> >> > >   __stop___tracepoints_ptrs = .;  
> > >> >> > > \
> > >> >> > >   *(__tracepoints_strings)/* Tracepoints: strings */  
> > >> >> > > \
> > >> >> > > + . = ALIGN(8);   
> > >> >> > > \
> > >> >> > > + __start___srcu_struct = .;  
> > >> >> > > \
> > >> >> > > + *(___srcu_struct_ptrs)  
> > >> >> > > \
> > >> >> > > + __end___srcu_struct = .;
> > >> >> > > \
> > >> >> > >   }   
> > >> >> > > \
> > >> >> > 
> > >> >> > This vmlinux linker modification is not needed. I tested without it 
> > >> >> > and srcu
> > >> >> > torture works fine with rcutorture built as a module. Putting 
> > >> >> > further prints
> > >> >> > in kernel/module.c verified that the kernel is able to find the 
> > >> >> > srcu structs
> > >> >> > just fine. You could squash the below patch into this one or apply 
> > >> >> > it on top
> > >> >> > of the dev branch.
> > >> >> 
> > >> >> Good point, given that otherwise FORTRAN named common blocks would not
> > >> >> work.
> > >> >> 
> > >> >> But isn't one advantage of leaving that stuff in the RO_DATA_SECTION()
> > >> >> macro that it can be mapped read-only?  Or am I suffering from 
> > >> >> excessive
> > >> >> optimism?
> > >> > 
> > >> > And to answer the other question, in the case where I am suffering from
> > >> > excessive optimism, it should be a separate commit.  Please see below
> > >> > for the updated original commit thus far.
> > >> > 
> > >> > And may I have your Tested-by?
> > >> 
> > >> Just to confirm: does the cleanup performed in the modules going
> > >> notifier end up acting as a barrier first before freeing the memory ?
> > >> If not, is it explicitly stated that a barrier must be issued before
> > >> module unload ?
> > >> 
> > > 
> > > You mean rcu_barrier? It is mentioned in the documentation that this is 
> > > the
> > > responsibility of the module writer to prevent delays for all modules.
> > 
> > It's a srcu barrier yes. Considering it would be a barrier specific to the
> > srcu domain within that module, I don't see how it would cause delays for
> > "all" modules if we implicitly issue the barrier on module unload. What
> > am I missing ?
> 
> Yes you are right. I thought of this after I just sent my email. I think it
> makes sense for srcu case to do and could avoid a class of bugs.

If there are call_srcu() callbacks outstanding, the module writer still
needs the srcu_barrier() because otherwise callbacks arrive after
the module text has gone, which will be disappoint the CPU when it
tries fetching instructions that are no longer mapped.  If there are
no call_srcu() callbacks from that module, then there is no need for
srcu_barrier() either way.

So if an srcu_barrier() is needed, the module developer needs to
supply it.

Thanx, Paul

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules

[Paul E. McKenney]

On Sun, Apr 07, 2019 at 03:46:13PM +, Joel Fernandes wrote:
> On Sun, Apr 07, 2019 at 06:59:37AM -0700, Paul E. McKenney wrote:
> > On Sun, Apr 07, 2019 at 06:39:41AM -0700, Paul E. McKenney wrote:
> > > On Sat, Apr 06, 2019 at 07:06:13PM -0400, Joel Fernandes wrote:
> > 
> > [ . . . ]
> > 
> > > > > diff --git a/include/asm-generic/vmlinux.lds.h 
> > > > > b/include/asm-generic/vmlinux.lds.h
> > > > > index f8f6f04c4453..c2d919a1566e 100644
> > > > > --- a/include/asm-generic/vmlinux.lds.h
> > > > > +++ b/include/asm-generic/vmlinux.lds.h
> > > > > @@ -338,6 +338,10 @@
> > > > >   KEEP(*(__tracepoints_ptrs)) /* Tracepoints: pointer 
> > > > > array */ \
> > > > >   __stop___tracepoints_ptrs = .;  
> > > > > \
> > > > >   *(__tracepoints_strings)/* Tracepoints: strings */  
> > > > > \
> > > > > + . = ALIGN(8);   
> > > > > \
> > > > > + __start___srcu_struct = .;  
> > > > > \
> > > > > + *(___srcu_struct_ptrs)  
> > > > > \
> > > > > + __end___srcu_struct = .;
> > > > > \
> > > > >   }   
> > > > > \
> > > > 
> > > > This vmlinux linker modification is not needed. I tested without it and 
> > > > srcu
> > > > torture works fine with rcutorture built as a module. Putting further 
> > > > prints
> > > > in kernel/module.c verified that the kernel is able to find the srcu 
> > > > structs
> > > > just fine. You could squash the below patch into this one or apply it 
> > > > on top
> > > > of the dev branch.
> > > 
> > > Good point, given that otherwise FORTRAN named common blocks would not
> > > work.
> > > 
> > > But isn't one advantage of leaving that stuff in the RO_DATA_SECTION()
> > > macro that it can be mapped read-only?  Or am I suffering from excessive
> > > optimism?
> > 
> > And to answer the other question, in the case where I am suffering from
> > excessive optimism, it should be a separate commit.  Please see below
> > for the updated original commit thus far.
> 
> Actually the vmlinux.lds.h file is unused for module building. For ex, if you
> delete include/asm-generic/vmlinux.lds.h , then you can still build
> rcutorture.ko. Did I miss something obvious? In that case the vmlinux.lds.h
> are not needed since the __section annotations automatically place the srcu
> structs in a separate section.

Hard to argue given that I just deleted include/asm-generic/vmlinux.lds.h,
touched kernel/rcu/rcutorture.c, and rebuilt the corresponding .ko
without errors.  ;-)

Hmmm...  Is there some way to place a section into a read-only page,
for example, tagged onto the text section for that module?  That would
get rid of a class of bugs, if nothing else.

> Let me know if you would like me to send a patch separately, or if the
> appended patch for the same in my previous email suffices.

Please do resend as a formal patch with the above in the commit log.
I doubt that I am the only one needing a bit of module-build education!
And thank you for providing that education, by the way!

> > And may I have your Tested-by?
> 
> Absolutely, please do and thanks!

Done, and thank you for giving it a go!

Thanx, Paul

>  - Joel
> 
> 
> > Thanx, Paul
> > 
> > 
> > 
> > commit a365bb5f6eafb220a1448674054b05c250829313
> > Author: Paul E. McKenney 
> > Date:   Fri Apr 5 16:15:00 2019 -0700
> > 
> > srcu: Allocate per-CPU data for DEFINE_SRCU() in modules
> > 
> > Adding DEFINE_SRCU() or DEFINE_STATIC_SRCU() to a loadable module 
> > requires
> > that the size of the reserved region be increased, which is not 
> > something
> > we want to be doing all that often.  One approach would be to require
> > that loadable modules define an srcu_struct and invoke 
> > init_srcu_struct()
> > from their module_init function and cleanup_srcu_struct() from their
> > module_exit function.  However, th

Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules

[Paul E. McKenney]

On Sun, Apr 07, 2019 at 06:39:41AM -0700, Paul E. McKenney wrote:
> On Sat, Apr 06, 2019 at 07:06:13PM -0400, Joel Fernandes wrote:

[ . . . ]

> > > diff --git a/include/asm-generic/vmlinux.lds.h 
> > > b/include/asm-generic/vmlinux.lds.h
> > > index f8f6f04c4453..c2d919a1566e 100644
> > > --- a/include/asm-generic/vmlinux.lds.h
> > > +++ b/include/asm-generic/vmlinux.lds.h
> > > @@ -338,6 +338,10 @@
> > >   KEEP(*(__tracepoints_ptrs)) /* Tracepoints: pointer array */ \
> > >   __stop___tracepoints_ptrs = .;  \
> > >   *(__tracepoints_strings)/* Tracepoints: strings */  \
> > > + . = ALIGN(8);   \
> > > + __start___srcu_struct = .;  \
> > > + *(___srcu_struct_ptrs)  \
> > > + __end___srcu_struct = .;\
> > >   }   \
> > 
> > This vmlinux linker modification is not needed. I tested without it and srcu
> > torture works fine with rcutorture built as a module. Putting further prints
> > in kernel/module.c verified that the kernel is able to find the srcu structs
> > just fine. You could squash the below patch into this one or apply it on top
> > of the dev branch.
> 
> Good point, given that otherwise FORTRAN named common blocks would not
> work.
> 
> But isn't one advantage of leaving that stuff in the RO_DATA_SECTION()
> macro that it can be mapped read-only?  Or am I suffering from excessive
> optimism?

And to answer the other question, in the case where I am suffering from
excessive optimism, it should be a separate commit.  Please see below
for the updated original commit thus far.

And may I have your Tested-by?

        Thanx, Paul



commit a365bb5f6eafb220a1448674054b05c250829313
Author: Paul E. McKenney 
Date:   Fri Apr 5 16:15:00 2019 -0700

srcu: Allocate per-CPU data for DEFINE_SRCU() in modules

Adding DEFINE_SRCU() or DEFINE_STATIC_SRCU() to a loadable module requires
that the size of the reserved region be increased, which is not something
we want to be doing all that often.  One approach would be to require
that loadable modules define an srcu_struct and invoke init_srcu_struct()
from their module_init function and cleanup_srcu_struct() from their
module_exit function.  However, this is more than a bit user unfriendly.

This commit therefore creates an ___srcu_struct_ptrs linker section,
and pointers to srcu_struct structures created by DEFINE_SRCU() and
DEFINE_STATIC_SRCU() within a module are placed into that module's
___srcu_struct_ptrs section.  The required init_srcu_struct() and
cleanup_srcu_struct() functions are then automatically invoked as needed
when that module is loaded and unloaded, thus allowing modules to continue
to use DEFINE_SRCU() and DEFINE_STATIC_SRCU() while avoiding the need
to increase the size of the reserved region.

Many of the algorithms and some of the code was cheerfully cherry-picked
from other code making use of linker sections, perhaps most notably from
tracepoints.  All bugs are nevertheless the sole property of the author.

Suggested-by: Mathieu Desnoyers 
[ paulmck: Use __section() and use "default" in srcu_module_notify()'s
  "switch" statement as suggested by Joel Fernandes. ]
Signed-off-by: Paul E. McKenney 

diff --git a/include/asm-generic/vmlinux.lds.h 
b/include/asm-generic/vmlinux.lds.h
index f8f6f04c4453..c2d919a1566e 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -338,6 +338,10 @@
KEEP(*(__tracepoints_ptrs)) /* Tracepoints: pointer array */ \
__stop___tracepoints_ptrs = .;  \
*(__tracepoints_strings)/* Tracepoints: strings */  \
+   . = ALIGN(8);   \
+   __start___srcu_struct = .;  \
+   *(___srcu_struct_ptrs)  \
+   __end___srcu_struct = .;\
}   \
\
.rodata1  : AT(ADDR(.rodata1) - LOAD_OFFSET) {  \
diff --git a/include/linux/module.h b/include/linux/module.h
index 5bf5dcd91009..921443a026dd 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -21,6 +21,7 @@
 #include 
 #

Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules

[Paul E. McKenney]

On Sat, Apr 06, 2019 at 07:06:13PM -0400, Joel Fernandes wrote:
> On Fri, Apr 05, 2019 at 04:28:35PM -0700, Paul E. McKenney wrote:
> > On Wed, Apr 03, 2019 at 09:20:39AM -0700, Paul E. McKenney wrote:
> > > On Wed, Apr 03, 2019 at 10:27:42AM -0400, Mathieu Desnoyers wrote:
> > > > - On Apr 3, 2019, at 9:32 AM, paulmck paul...@linux.ibm.com wrote:
> > > > 
> > > > > On Tue, Apr 02, 2019 at 11:34:07AM -0400, Mathieu Desnoyers wrote:
> > > > >> - On Apr 2, 2019, at 11:23 AM, paulmck paul...@linux.ibm.com 
> > > > >> wrote:
> > > > >> 
> > > > >> > On Tue, Apr 02, 2019 at 11:14:40AM -0400, Mathieu Desnoyers wrote:
> > > > >> >> - On Apr 2, 2019, at 10:28 AM, paulmck paul...@linux.ibm.com 
> > > > >> >> wrote:
> > > > >> >> 
> > > > >> >> > Hello!
> > > > >> >> > 
> > > > >> >> > This series prohibits use of DEFINE_SRCU() and 
> > > > >> >> > DEFINE_STATIC_SRCU()
> > > > >> >> > by loadable modules.  The reason for this prohibition is the 
> > > > >> >> > fact
> > > > >> >> > that using these two macros within modules requires that the 
> > > > >> >> > size of
> > > > >> >> > the reserved region be increased, which is not something we 
> > > > >> >> > want to
> > > > >> >> > be doing all that often.  Instead, loadable modules should 
> > > > >> >> > define an
> > > > >> >> > srcu_struct and invoke init_srcu_struct() from their 
> > > > >> >> > module_init function
> > > > >> >> > and cleanup_srcu_struct() from their module_exit function.  
> > > > >> >> > Note that
> > > > >> >> > modules using call_srcu() will also need to invoke 
> > > > >> >> > srcu_barrier() from
> > > > >> >> > their module_exit function.
> > > > >> >> 
> > > > >> >> This arbitrary API limitation seems weird.
> > > > >> >> 
> > > > >> >> Isn't there a way to allow modules to use DEFINE_SRCU and 
> > > > >> >> DEFINE_STATIC_SRCU
> > > > >> >> while implementing them with dynamic allocation under the hood ?
> > > > >> > 
> > > > >> > Although call_srcu() already has initialization hooks, some would
> > > > >> > also be required in srcu_read_lock(), and I am concerned about 
> > > > >> > adding
> > > > >> > memory allocation at that point, especially given the possibility
> > > > >> > of memory-allocation failure.  And the possibility that the first
> > > > >> > srcu_read_lock() happens in an interrupt handler or similar.
> > > > >> > 
> > > > >> > Or am I missing a trick here?
> > > > >> 
> > > > >> I was more thinking that under #ifdef MODULE, both DEFINE_SRCU and
> > > > >> DEFINE_STATIC_SRCU could append data in a dedicated section. module.c
> > > > >> would additionally lookup that section on module load, and deal with
> > > > >> those statically defined SRCU entries as if they were dynamically
> > > > >> allocated ones. It would of course cleanup those resources on module
> > > > >> unload.
> > > > >> 
> > > > >> Am I missing some subtlety there ?
> > > > > 
> > > > > If I understand you correctly, that is actually what is already done. 
> > > > >  The
> > > > > size of this dedicated section is currently set by 
> > > > > PERCPU_MODULE_RESERVE,
> > > > > and the additions of DEFINE{_STATIC}_SRCU() in modules was requiring 
> > > > > that
> > > > > this to be increased frequently.  That led to a request that something
> > > > > be done, in turn leading to this patch series.
> > > > 
> > > > I think we are not expressing quite the same idea.
> > > > 
> > > > AFAIU, yours is to have DEFINE*_SRCU directly define per-cpu data 
> > > > within modules,
> > > > which ends up using percpu module reserved memory.
> > > > 
> 

Re: [PATCH RFC tip/core/rcu 3/4] drivers/gpu/drm/amd: Dynamically allocate kfd_processes_srcu

[Paul E. McKenney]

On Tue, Apr 02, 2019 at 05:40:54PM +, Kuehling, Felix wrote:
> On 2019-04-02 10:29 a.m., Paul E. McKenney wrote:
> > Having DEFINE_SRCU() or DEFINE_STATIC_SRCU() in a loadable module
> > requires that the size of the reserved region be increased, which is
> > not something we really want to be doing.  This commit therefore removes
> > the DEFINE_STATIC_SRCU() from drivers/gpu/drm/amd/amdkfd/kfd_process.c in
> > favor of defining kfd_processes_srcu as a simple srcu_struct, initializing
> > it in amdgpu_amdkfd_init(), and cleaning it up in amdgpu_amdkfd_fini().
> >
> > Reported-by: kbuild test robot 
> > Signed-off-by: Paul E. McKenney 
> > Tested-by: kbuild test robot 
> > Cc: Oded Gabbay 
> > Cc: Alex Deucher 
> > Cc: "Christian König"  > Cc: "David (ChunMing) Zhou" 
> > Cc: David Airlie 
> > Cc: Daniel Vetter 
> > Cc: Tejun Heo 
> > Cc: 
> > Cc: 
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 5 +
> >   drivers/gpu/drm/amd/amdkfd/kfd_process.c   | 2 +-
> >   2 files changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> > index fe1d7368c1e6..eadb20dee867 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> > @@ -28,6 +28,8 @@
> >   #include 
> >   #include 
> >   
> > +extern struct srcu_struct kfd_processes_srcu;
> > +
> >   static const unsigned int compute_vmid_bitmap = 0xFF00;
> >   
> >   /* Total memory size in system memory and all GPU VRAM. Used to
> > @@ -40,6 +42,8 @@ int amdgpu_amdkfd_init(void)
> > struct sysinfo si;
> > int ret;
> >   
> > +   ret = init_srcu_struct(_processes_srcu);
> > +   WARN_ON(ret);
> 
> kfd_processes_srcu only exists if kfd_process.c is compiled in. That 
> depends on CONFIG_HSA_AMD. So this should at least move into #ifdef a 
> few lines below.
> 
> However, it would be cleaner to move this initialization into kfd_init 
> in kfd_module.c, or better yet, into kfd_process_create_wq in 
> kfd_process.c. Then kfd_process_create_wq should be renamed to something 
> more generic, such as kfd_process_init.
> 
> 
> > si_meminfo();
> > amdgpu_amdkfd_total_mem_size = si.totalram - si.totalhigh;
> > amdgpu_amdkfd_total_mem_size *= si.mem_unit;
> > @@ -57,6 +61,7 @@ int amdgpu_amdkfd_init(void)
> >   void amdgpu_amdkfd_fini(void)
> >   {
> > kgd2kfd_exit();
> > +   cleanup_srcu_struct(_processes_srcu);
> 
> Similarly, this would be cleaner in kfd_exit in kfd_module.c or 
> kfd_process_destroy_wq in kfd_process.c, with that function similarly 
> renamed to kfd_process_fini.
> 
> I'm attaching a revised patch. It's only compile tested.

Thank you, Felix!  I have reverted my original and applied your revised
version.

Thanx, Paul

> Regards,
>    Felix
> 
> 
> >   }
> >   
> >   void amdgpu_amdkfd_device_probe(struct amdgpu_device *adev)
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c 
> > b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> > index 4bdae78bab8e..98b694068b8a 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> > @@ -47,7 +47,7 @@ struct mm_struct;
> >   DEFINE_HASHTABLE(kfd_processes_table, KFD_PROCESS_TABLE_SIZE);
> >   static DEFINE_MUTEX(kfd_processes_mutex);
> >   
> > -DEFINE_SRCU(kfd_processes_srcu);
> > +struct srcu_struct kfd_processes_srcu;
> >   
> >   /* For process termination handling */
> >   static struct workqueue_struct *kfd_process_wq;

> From 5857a9aa63957a5755ff81ae5c46533bca408c12 Mon Sep 17 00:00:00 2001
> From: "Paul E. McKenney" 
> Date: Tue, 2 Apr 2019 07:29:32 -0700
> Subject: [PATCH 1/1] drivers/gpu/drm/amd: Dynamically allocate
>  kfd_processes_srcu v2
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> Having DEFINE_SRCU() or DEFINE_STATIC_SRCU() in a loadable module
> requires that the size of the reserved region be increased, which is
> not something we really want to be doing.  This commit therefore removes
> the DEFINE_STATIC_SRCU() from drivers/gpu/drm/amd/amdkfd/kfd_process.c in
> favor of defining kfd_processes_srcu as a simple srcu_struct, initializing
> it in kfd_process_init(), and cleaning it up in kfd_process_fini().
> 
> v2 (Felix Kuehling): Move srcu init and cleanup into kfd_process.c
> 
> Reported-by

Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules

[Paul E. McKenney]

On Wed, Apr 03, 2019 at 10:27:42AM -0400, Mathieu Desnoyers wrote:
> - On Apr 3, 2019, at 9:32 AM, paulmck paul...@linux.ibm.com wrote:
> 
> > On Tue, Apr 02, 2019 at 11:34:07AM -0400, Mathieu Desnoyers wrote:
> >> - On Apr 2, 2019, at 11:23 AM, paulmck paul...@linux.ibm.com wrote:
> >> 
> >> > On Tue, Apr 02, 2019 at 11:14:40AM -0400, Mathieu Desnoyers wrote:
> >> >> - On Apr 2, 2019, at 10:28 AM, paulmck paul...@linux.ibm.com wrote:
> >> >> 
> >> >> > Hello!
> >> >> > 
> >> >> > This series prohibits use of DEFINE_SRCU() and DEFINE_STATIC_SRCU()
> >> >> > by loadable modules.  The reason for this prohibition is the fact
> >> >> > that using these two macros within modules requires that the size of
> >> >> > the reserved region be increased, which is not something we want to
> >> >> > be doing all that often.  Instead, loadable modules should define an
> >> >> > srcu_struct and invoke init_srcu_struct() from their module_init 
> >> >> > function
> >> >> > and cleanup_srcu_struct() from their module_exit function.  Note that
> >> >> > modules using call_srcu() will also need to invoke srcu_barrier() from
> >> >> > their module_exit function.
> >> >> 
> >> >> This arbitrary API limitation seems weird.
> >> >> 
> >> >> Isn't there a way to allow modules to use DEFINE_SRCU and 
> >> >> DEFINE_STATIC_SRCU
> >> >> while implementing them with dynamic allocation under the hood ?
> >> > 
> >> > Although call_srcu() already has initialization hooks, some would
> >> > also be required in srcu_read_lock(), and I am concerned about adding
> >> > memory allocation at that point, especially given the possibility
> >> > of memory-allocation failure.  And the possibility that the first
> >> > srcu_read_lock() happens in an interrupt handler or similar.
> >> > 
> >> > Or am I missing a trick here?
> >> 
> >> I was more thinking that under #ifdef MODULE, both DEFINE_SRCU and
> >> DEFINE_STATIC_SRCU could append data in a dedicated section. module.c
> >> would additionally lookup that section on module load, and deal with
> >> those statically defined SRCU entries as if they were dynamically
> >> allocated ones. It would of course cleanup those resources on module
> >> unload.
> >> 
> >> Am I missing some subtlety there ?
> > 
> > If I understand you correctly, that is actually what is already done.  The
> > size of this dedicated section is currently set by PERCPU_MODULE_RESERVE,
> > and the additions of DEFINE{_STATIC}_SRCU() in modules was requiring that
> > this to be increased frequently.  That led to a request that something
> > be done, in turn leading to this patch series.
> 
> I think we are not expressing quite the same idea.
> 
> AFAIU, yours is to have DEFINE*_SRCU directly define per-cpu data within 
> modules,
> which ends up using percpu module reserved memory.
> 
> My idea is to make DEFINE*_SRCU have a different behavior under #ifdef MODULE.
> It could emit a _global variable_ (_not_ per-cpu) within a new section. That
> section would then be used by module init/exit code to figure out what "srcu
> descriptors" are present in the modules. It would therefore rely on dynamic
> allocation for those, therefore removing the need to involve the percpu module
> reserved pool at all.
> 
> > 
> > I don't see a way around this short of changing module loading to do
> > alloc_percpu() and then updating the relocation based on this result.
> > Which would admittedly be far more convenient.  I was assuming that
> > this would be difficult due to varying CPU offsets or the like.
> > 
> > But if it can be done reasonably, it would be quite a bit nicer than
> > forcing dynamic allocation in cases where it is not otherwise needed.
> 
> Hopefully my explanation above helps clear out what I have in mind.
> 
> You can find similar tricks performed by include/linux/tracepoint.h:
> 
> #ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
> static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
> {
> return offset_to_ptr(p);
> }
> 
> #define __TRACEPOINT_ENTRY(name)\
> asm("   .section \"__tracepoints_ptrs\", \"a\"  \n" \
> "   .balign 4   \n" \
> "   .long   __tracepoint_" #name " - .  \n" \
> "   .previous   \n")
> #else
> static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
> {
> return *p;
> }
> 
> #define __TRACEPOINT_ENTRY(name) \
> static tracepoint_ptr_t __tracepoint_ptr_##name __used   \
> __attribute__((section("__tracepoints_ptrs"))) = \
> &__tracepoint_##name
> #endif
> 
> [...]
> 
> #define DEFINE_TRACE_FN(name, reg, unreg)\
> static const char __tpstrtab_##name[]\
> 

Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules

[Paul E. McKenney]

On Tue, Apr 02, 2019 at 11:34:07AM -0400, Mathieu Desnoyers wrote:
> - On Apr 2, 2019, at 11:23 AM, paulmck paul...@linux.ibm.com wrote:
> 
> > On Tue, Apr 02, 2019 at 11:14:40AM -0400, Mathieu Desnoyers wrote:
> >> - On Apr 2, 2019, at 10:28 AM, paulmck paul...@linux.ibm.com wrote:
> >> 
> >> > Hello!
> >> > 
> >> > This series prohibits use of DEFINE_SRCU() and DEFINE_STATIC_SRCU()
> >> > by loadable modules.  The reason for this prohibition is the fact
> >> > that using these two macros within modules requires that the size of
> >> > the reserved region be increased, which is not something we want to
> >> > be doing all that often.  Instead, loadable modules should define an
> >> > srcu_struct and invoke init_srcu_struct() from their module_init function
> >> > and cleanup_srcu_struct() from their module_exit function.  Note that
> >> > modules using call_srcu() will also need to invoke srcu_barrier() from
> >> > their module_exit function.
> >> 
> >> This arbitrary API limitation seems weird.
> >> 
> >> Isn't there a way to allow modules to use DEFINE_SRCU and 
> >> DEFINE_STATIC_SRCU
> >> while implementing them with dynamic allocation under the hood ?
> > 
> > Although call_srcu() already has initialization hooks, some would
> > also be required in srcu_read_lock(), and I am concerned about adding
> > memory allocation at that point, especially given the possibility
> > of memory-allocation failure.  And the possibility that the first
> > srcu_read_lock() happens in an interrupt handler or similar.
> > 
> > Or am I missing a trick here?
> 
> I was more thinking that under #ifdef MODULE, both DEFINE_SRCU and
> DEFINE_STATIC_SRCU could append data in a dedicated section. module.c
> would additionally lookup that section on module load, and deal with
> those statically defined SRCU entries as if they were dynamically
> allocated ones. It would of course cleanup those resources on module
> unload.
> 
> Am I missing some subtlety there ?

If I understand you correctly, that is actually what is already done.  The
size of this dedicated section is currently set by PERCPU_MODULE_RESERVE,
and the additions of DEFINE{_STATIC}_SRCU() in modules was requiring that
this to be increased frequently.  That led to a request that something
be done, in turn leading to this patch series.

I don't see a way around this short of changing module loading to do
alloc_percpu() and then updating the relocation based on this result.
Which would admittedly be far more convenient.  I was assuming that
this would be difficult due to varying CPU offsets or the like.

But if it can be done reasonably, it would be quite a bit nicer than
forcing dynamic allocation in cases where it is not otherwise needed.

Thanx, Paul

> Thanks,
> 
> Mathieu
> 
> 
> > 
> > Thanx, Paul
> > 
> >> Thanks,
> >> 
> >> Mathieu
> >> 
> >> 
> >> > 
> >> > This series consist of the following:
> >> > 
> >> > 1.   Dynamically allocate dax_srcu.
> >> > 
> >> > 2.   Dynamically allocate drm_unplug_srcu.
> >> > 
> >> > 3.   Dynamically allocate kfd_processes_srcu.
> >> > 
> >> > These build and have been subjected to 0day testing, but might also need
> >> > testing by someone having the requisite hardware.
> >> > 
> >> >  Thanx, Paul
> >> > 
> >> > 
> >> > 
> >> > drivers/dax/super.c|   10 +-
> >> > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c |5 +++
> >> > drivers/gpu/drm/amd/amdkfd/kfd_process.c   |2 -
> >> > drivers/gpu/drm/drm_drv.c  |8 
> >> > include/linux/srcutree.h   |   19 +--
> >> > kernel/rcu/rcuperf.c   |   40 
> >> > +++-
> >> > kernel/rcu/rcutorture.c|   48 
> >> > +
> >> >  7 files changed, 105 insertions(+), 27 deletions(-)
> >> 
> >> --
> >> Mathieu Desnoyers
> >> EfficiOS Inc.
> >> http://www.efficios.com
> 
> -- 
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com
> 

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules

[Paul E. McKenney]

On Tue, Apr 02, 2019 at 02:40:54PM -0400, Joel Fernandes wrote:
> On Tue, Apr 02, 2019 at 08:23:34AM -0700, Paul E. McKenney wrote:
> > On Tue, Apr 02, 2019 at 11:14:40AM -0400, Mathieu Desnoyers wrote:
> > > - On Apr 2, 2019, at 10:28 AM, paulmck paul...@linux.ibm.com wrote:
> > > 
> > > > Hello!
> > > > 
> > > > This series prohibits use of DEFINE_SRCU() and DEFINE_STATIC_SRCU()
> > > > by loadable modules.  The reason for this prohibition is the fact
> > > > that using these two macros within modules requires that the size of
> > > > the reserved region be increased, which is not something we want to
> > > > be doing all that often.  Instead, loadable modules should define an
> > > > srcu_struct and invoke init_srcu_struct() from their module_init 
> > > > function
> > > > and cleanup_srcu_struct() from their module_exit function.  Note that
> > > > modules using call_srcu() will also need to invoke srcu_barrier() from
> > > > their module_exit function.
> > > 
> > > This arbitrary API limitation seems weird.
> > > 
> > > Isn't there a way to allow modules to use DEFINE_SRCU and 
> > > DEFINE_STATIC_SRCU
> > > while implementing them with dynamic allocation under the hood ?
> > 
> > Although call_srcu() already has initialization hooks, some would
> > also be required in srcu_read_lock(), and I am concerned about adding
> > memory allocation at that point, especially given the possibility
> > of memory-allocation failure.  And the possibility that the first
> > srcu_read_lock() happens in an interrupt handler or similar.
> > 
> > Or am I missing a trick here?
> 
> Hi Paul,
> 
> Which 'reserved region' are you referring to? Isn't this region also
> used for non-module cases in which case the same problem applies to
> non-modules?

The percpu/module reservation discussed in this thread:

https://lore.kernel.org/lkml/c72402f2-967e-cd56-99d8-9139c9e7f...@google.com/T/#mbcacf60ddc0b3fd6e831a3ea71efc90da359a3bf

For non-modules, global per-CPU variables are statically allocated.
For modules, they must be dynamically allocated at modprobe time, and
their size is set by PERCPU_MODULE_RESERVE.

Thanx, Paul

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH RFC tip/core/rcu 2/4] drivers/gpu/drm: Dynamically allocate drm_unplug_srcu

[Paul E. McKenney]

Having DEFINE_SRCU() or DEFINE_STATIC_SRCU() in a loadable module
requires that the size of the reserved region be increased, which is not
something we really want to be doing.  This commit therefore removes
the DEFINE_STATIC_SRCU() from drivers/gpu/drm/drm_drv.c in favor of
defining drm_unplug_srcu as a simple srcu_struct, initializing it in
drm_core_init(), and cleaning it up in drm_core_exit().

In this particular case, drm_unplug_srcu is statically allocated and thus
guaranteed to be initially zero.  Unfortunately, cleanup_srcu_struct()
cannot rely on this in the general case (which includes dynamically
allocated srcu_struct structures), and therefore cannot tell whether it
is being called on something that has been passed to init_srcu_struct().
Thus the code added to drm_core_init() is a bit non-standard.

Reported-by: kbuild test robot 
Signed-off-by: Paul E. McKenney 
Cc: Maarten Lankhorst 
Cc: Maxime Ripard 
Cc: Sean Paul 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Tejun Heo 
Cc: 
---
 drivers/gpu/drm/drm_drv.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 381581b01d48..ce4582af814d 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -77,7 +77,7 @@ static bool drm_core_init_complete = false;
 
 static struct dentry *drm_debugfs_root;
 
-DEFINE_STATIC_SRCU(drm_unplug_srcu);
+static struct srcu_struct drm_unplug_srcu;
 
 /*
  * DRM Minors
@@ -958,12 +958,18 @@ static void drm_core_exit(void)
drm_sysfs_destroy();
idr_destroy(_minors_idr);
drm_connector_ida_destroy();
+   cleanup_srcu_struct(_unplug_srcu);
 }
 
 static int __init drm_core_init(void)
 {
int ret;
 
+   ret = init_srcu_struct(_unplug_srcu);
+   if (ret) {
+   DRM_ERROR("Cannot create DRM class: %d\n", ret);
+   return ret; /* cannot cleanup after failed srcu_struct init. */
+   }
drm_connector_ida_init();
idr_init(_minors_idr);
 
-- 
2.17.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH RFC tip/core/rcu 3/4] drivers/gpu/drm/amd: Dynamically allocate kfd_processes_srcu

[Paul E. McKenney]

Having DEFINE_SRCU() or DEFINE_STATIC_SRCU() in a loadable module
requires that the size of the reserved region be increased, which is
not something we really want to be doing.  This commit therefore removes
the DEFINE_STATIC_SRCU() from drivers/gpu/drm/amd/amdkfd/kfd_process.c in
favor of defining kfd_processes_srcu as a simple srcu_struct, initializing
it in amdgpu_amdkfd_init(), and cleaning it up in amdgpu_amdkfd_fini().

Reported-by: kbuild test robot 
Signed-off-by: Paul E. McKenney 
Tested-by: kbuild test robot 
Cc: Oded Gabbay 
Cc: Alex Deucher 
Cc: "Christian König" 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Tejun Heo 
Cc: 
Cc: 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 5 +
 drivers/gpu/drm/amd/amdkfd/kfd_process.c   | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index fe1d7368c1e6..eadb20dee867 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -28,6 +28,8 @@
 #include 
 #include 
 
+extern struct srcu_struct kfd_processes_srcu;
+
 static const unsigned int compute_vmid_bitmap = 0xFF00;
 
 /* Total memory size in system memory and all GPU VRAM. Used to
@@ -40,6 +42,8 @@ int amdgpu_amdkfd_init(void)
struct sysinfo si;
int ret;
 
+   ret = init_srcu_struct(_processes_srcu);
+   WARN_ON(ret);
si_meminfo();
amdgpu_amdkfd_total_mem_size = si.totalram - si.totalhigh;
amdgpu_amdkfd_total_mem_size *= si.mem_unit;
@@ -57,6 +61,7 @@ int amdgpu_amdkfd_init(void)
 void amdgpu_amdkfd_fini(void)
 {
kgd2kfd_exit();
+   cleanup_srcu_struct(_processes_srcu);
 }
 
 void amdgpu_amdkfd_device_probe(struct amdgpu_device *adev)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index 4bdae78bab8e..98b694068b8a 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -47,7 +47,7 @@ struct mm_struct;
 DEFINE_HASHTABLE(kfd_processes_table, KFD_PROCESS_TABLE_SIZE);
 static DEFINE_MUTEX(kfd_processes_mutex);
 
-DEFINE_SRCU(kfd_processes_srcu);
+struct srcu_struct kfd_processes_srcu;
 
 /* For process termination handling */
 static struct workqueue_struct *kfd_process_wq;
-- 
2.17.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules

[Paul E. McKenney]

On Tue, Apr 02, 2019 at 11:14:40AM -0400, Mathieu Desnoyers wrote:
> - On Apr 2, 2019, at 10:28 AM, paulmck paul...@linux.ibm.com wrote:
> 
> > Hello!
> > 
> > This series prohibits use of DEFINE_SRCU() and DEFINE_STATIC_SRCU()
> > by loadable modules.  The reason for this prohibition is the fact
> > that using these two macros within modules requires that the size of
> > the reserved region be increased, which is not something we want to
> > be doing all that often.  Instead, loadable modules should define an
> > srcu_struct and invoke init_srcu_struct() from their module_init function
> > and cleanup_srcu_struct() from their module_exit function.  Note that
> > modules using call_srcu() will also need to invoke srcu_barrier() from
> > their module_exit function.
> 
> This arbitrary API limitation seems weird.
> 
> Isn't there a way to allow modules to use DEFINE_SRCU and DEFINE_STATIC_SRCU
> while implementing them with dynamic allocation under the hood ?

Although call_srcu() already has initialization hooks, some would
also be required in srcu_read_lock(), and I am concerned about adding
memory allocation at that point, especially given the possibility
of memory-allocation failure.  And the possibility that the first
srcu_read_lock() happens in an interrupt handler or similar.

Or am I missing a trick here?

Thanx, Paul

> Thanks,
> 
> Mathieu
> 
> 
> > 
> > This series consist of the following:
> > 
> > 1.  Dynamically allocate dax_srcu.
> > 
> > 2.  Dynamically allocate drm_unplug_srcu.
> > 
> > 3.  Dynamically allocate kfd_processes_srcu.
> > 
> > These build and have been subjected to 0day testing, but might also need
> > testing by someone having the requisite hardware.
> > 
> > Thanx, Paul
> > 
> > 
> > 
> > drivers/dax/super.c|   10 +-
> > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c |5 +++
> > drivers/gpu/drm/amd/amdkfd/kfd_process.c   |2 -
> > drivers/gpu/drm/drm_drv.c  |8 
> > include/linux/srcutree.h   |   19 +--
> > kernel/rcu/rcuperf.c   |   40 +++-
> > kernel/rcu/rcutorture.c|   48 
> > +
> >  7 files changed, 105 insertions(+), 27 deletions(-)
> 
> -- 
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com
> 

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules

[Paul E. McKenney]

Hello!

This series prohibits use of DEFINE_SRCU() and DEFINE_STATIC_SRCU()
by loadable modules.  The reason for this prohibition is the fact
that using these two macros within modules requires that the size of
the reserved region be increased, which is not something we want to
be doing all that often.  Instead, loadable modules should define an
srcu_struct and invoke init_srcu_struct() from their module_init function
and cleanup_srcu_struct() from their module_exit function.  Note that
modules using call_srcu() will also need to invoke srcu_barrier() from
their module_exit function.

This series consist of the following:

1.  Dynamically allocate dax_srcu.

2.  Dynamically allocate drm_unplug_srcu.

3.  Dynamically allocate kfd_processes_srcu.

These build and have been subjected to 0day testing, but might also need
testing by someone having the requisite hardware.

Thanx, Paul



 drivers/dax/super.c|   10 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c |5 +++
 drivers/gpu/drm/amd/amdkfd/kfd_process.c   |2 -
 drivers/gpu/drm/drm_drv.c  |8 
 include/linux/srcutree.h   |   19 +--
 kernel/rcu/rcuperf.c   |   40 +++-
 kernel/rcu/rcutorture.c|   48 +
 7 files changed, 105 insertions(+), 27 deletions(-)

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] [RFC v2] Drop all 00-INDEX files from Documentation/

[Paul E. McKenney]

On Tue, Sep 04, 2018 at 12:15:23AM +0200, Henrik Austad wrote:
> This is a respin with a wider audience (all that get_maintainer returned)
> and I know this spams a *lot* of people. Not sure what would be the correct
> way, so my apologies for ruining your inbox.
> 
> The 00-INDEX files are supposed to give a summary of all files present
> in a directory, but these files are horribly out of date and their
> usefulness is brought into question. Often a simple "ls" would reveal
> the same information as the filenames are generally quite descriptive as
> a short introduction to what the file covers (it should not surprise
> anyone what Documentation/sched/sched-design-CFS.txt covers)
> 
> A few years back it was mentioned that these files were no longer really
> needed, and they have since then grown further out of date, so perhaps
> it is time to just throw them out.
> 
> A short status yields the following _outdated_ 00-INDEX files, first
> counter is files listed in 00-INDEX but missing in the directory, last
> is files present but not listed in 00-INDEX.
> 
> List of outdated 00-INDEX:
> Documentation: (4/10)
> Documentation/sysctl: (0/1)
> Documentation/timers: (1/0)
> Documentation/blockdev: (3/1)
> Documentation/w1/slaves: (0/1)
> Documentation/locking: (0/1)
> Documentation/devicetree: (0/5)
> Documentation/power: (1/1)
> Documentation/powerpc: (0/5)
> Documentation/arm: (1/0)
> Documentation/x86: (0/9)
> Documentation/x86/x86_64: (1/1)
> Documentation/scsi: (4/4)
> Documentation/filesystems: (2/9)
> Documentation/filesystems/nfs: (0/2)
> Documentation/cgroup-v1: (0/2)
> Documentation/kbuild: (0/4)
> Documentation/spi: (1/0)
> Documentation/virtual/kvm: (1/0)
> Documentation/scheduler: (0/2)
> Documentation/fb: (0/1)
> Documentation/block: (0/1)
> Documentation/networking: (6/37)
> Documentation/vm: (1/3)
> 
> Then there are 364 subdirectories in Documentation/ with several files that
> are missing 00-INDEX alltogether (and another 120 with a single file and no
> 00-INDEX).
> 
> I don't really have an opinion to whether or not we /should/ have 00-INDEX,
> but the above 00-INDEX should either be removed or be kept up to date. If
> we should keep the files, I can try to keep them updated, but I rather not
> if we just want to delete them anyway.
> 
> As a starting point, remove all index-files and references to 00-INDEX and
> see where the discussion is going.

For the RCU portions:

Acked-by: Paul E. McKenney 

> Again, sorry for the insanely wide distribution.
> 
> Signed-off-by: Henrik Austad 
> Cc: Jonathan Corbet 
> Cc: Bjorn Helgaas 
> Cc: "Paul E. McKenney" 
> Cc: Josh Triplett 
> Cc: Steven Rostedt 
> Cc: Mathieu Desnoyers 
> Cc: Lai Jiangshan 
> Cc: Jens Axboe 
> Cc: Rob Herring 
> Cc: Mark Rutland 
> Cc: Bartlomiej Zolnierkiewicz 
> Cc: Linus Walleij 
> Cc: "David S. Miller" 
> Cc: Karsten Keil 
> Cc: Masahiro Yamada 
> Cc: Michal Marek 
> Cc: Peter Zijlstra 
> Cc: Ingo Molnar 
> Cc: Will Deacon 
> Cc: Ralf Baechle 
> Cc: Paul Burton 
> Cc: James Hogan 
> Cc: Paul Moore 
> Cc: "James E.J. Bottomley" 
> Cc: Helge Deller 
> Cc: "Rafael J. Wysocki" 
> Cc: Len Brown 
> Cc: Pavel Machek 
> Cc: Benjamin Herrenschmidt 
> Cc: Paul Mackerras 
> Cc: Michael Ellerman 
> Cc: Martin Schwidefsky 
> Cc: Heiko Carstens 
> Cc: Greg Kroah-Hartman 
> Cc: Jiri Slaby 
> Cc: Mark Brown 
> Cc: Thomas Gleixner 
> Cc: Paolo Bonzini 
> Cc: "Radim Krčmář" 
> Cc: Evgeniy Polyakov 
> Cc: "H. Peter Anvin" 
> Cc: x...@kernel.org
> Cc: Henrik Austad 
> Cc: Andrew Morton 
> Cc: Ian Kent 
> Cc: Jacek Anaszewski 
> Cc: Mike Rapoport 
> Cc: Jan Kandziora 
> Cc: linux-...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> Cc: linux-...@vger.kernel.org
> Cc: devicet...@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-fb...@vger.kernel.org
> Cc: linux-g...@vger.kernel.org
> Cc: linux-...@vger.kernel.org
> Cc: net...@vger.kernel.org
> Cc: linux-kbu...@vger.kernel.org
> Cc: linux-m...@linux-mips.org
> Cc: linux-security-mod...@vger.kernel.org
> Cc: linux-par...@vger.kernel.org
> Cc: linux...@vger.kernel.org
> Cc: linuxppc-...@lists.ozlabs.org
> Cc: linux-s...@vger.kernel.org
> Cc: linux-...@vger.kernel.org
> Cc: k...@vger.kernel.org
> Signed-off-by: Henrik Austad 
> ---
>  Documentation/00-INDEX  | 428 
> 
>  Documentation/PCI/00-INDEX  |  26 --
>  Documentation/RCU/00-INDEX  |  34 ---
>  Documentation/RCU/rcu.txt   |   4 -
>  Documentation/admin-guide/README.rst|  

Re: linux-next: build failure after merge of the rcu tree

[Paul E. McKenney]

On Wed, Mar 08, 2017 at 11:13:38AM +0100, Daniel Vetter wrote:
> On Wed, Mar 08, 2017 at 12:16:45PM +1100, Stephen Rothwell wrote:
> > Hi Paul,
> > 
> > After merging the rcu tree, today's linux-next build (x86_64 allmodconfig)
> > failed like this:
> > 
> > In file included from include/linux/resource_ext.h:19:0,
> >  from include/linux/pci.h:32,
> >  from include/drm/drmP.h:50,
> >  from drivers/gpu/drm/i915/i915_gem.c:28:
> > drivers/gpu/drm/i915/selftests/mock_gem_device.c: In function 
> > 'mock_gem_device':
> > drivers/gpu/drm/i915/selftests/mock_gem_device.c:177:9: error: 
> > 'SLAB_DESTROY_BY_RCU' undeclared (first use in this function)
> >  SLAB_DESTROY_BY_RCU);
> >  ^
> > include/linux/slab.h:149:4: note: in definition of macro 'KMEM_CACHE'
> >(__flags), NULL)
> > ^
> > drivers/gpu/drm/i915/selftests/mock_gem_device.c:177:9: note: each 
> > undeclared identifier is reported only once for each function it appears in
> >  SLAB_DESTROY_BY_RCU);
> >  ^
> > include/linux/slab.h:149:4: note: in definition of macro 'KMEM_CACHE'
> >(__flags), NULL)
> > ^
> > /
> > 
> > Caused by commit
> > 
> >   24b7cb25b8d1 ("mm: Rename SLAB_DESTROY_BY_RCU to SLAB_TYPESAFE_BY_RCU")
> 
> Awesome rename. Count us in among the people who first thought this
> provides more guarantees than it does.

Glad you like it!  ;-)

> > interacting with commit
> > 
> >   0daf0113cff6 ("drm/i915: Mock infrastructure for request emission")
> > 
> > from the drm-intel tree.
> > 
> > I added the following merge fix patch:
> > 
> > From: Stephen Rothwell 
> > Date: Wed, 8 Mar 2017 12:09:49 +1100
> > Subject: [PATCH] drm/i915: merge fix for "mm: Rename SLAB_DESTROY_BY_RCU to
> >  SLAB_TYPESAFE_BY_RCU"
> > 
> > Signed-off-by: Stephen Rothwell 
> 
> Should we handle this with a topic branch? It's trivial to resolve, but I
> fear the note that this conflict exists might get lost somewhere between
> now and when the drm pull lands in Linus' inbox in 2 months ...
> 
> Otoh he's probably going to compile test drm extra carefully and will
> notice :-)

If it gets too ugly, I can always allow both SLAB_TYPESAFE_BY_RCU
and SLAB_DESTROY_BY_RCU as synonyms in 4.12, and then remove
SLAB_DESTROY_BY_RCU in 4.13.

Thanx, Paul

> -Daniel
> 
> > ---
> >  drivers/gpu/drm/i915/selftests/mock_gem_device.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c 
> > b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> > index 6a8258eacdcb..9f24c5da3f8d 100644
> > --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> > +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> > @@ -174,7 +174,7 @@ struct drm_i915_private *mock_gem_device(void)
> > i915->requests = KMEM_CACHE(mock_request,
> > SLAB_HWCACHE_ALIGN |
> > SLAB_RECLAIM_ACCOUNT |
> > -   SLAB_DESTROY_BY_RCU);
> > +   SLAB_TYPESAFE_BY_RCU);
> > if (!i915->requests)
> > goto err_vmas;
> >  
> > -- 
> > 2.11.0
> > 
> > -- 
> > Cheers,
> > Stephen Rothwell
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> 

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/2] Revert "drm/fb-helper: Reduce READ_ONCE(master) to lockless_dereference"

[Paul E. McKenney]

On Fri, Aug 12, 2016 at 08:25:43PM +0200, Peter Zijlstra wrote:
> On Thu, Aug 11, 2016 at 11:26:47AM -0700, Paul E. McKenney wrote:
> > If my upcoming testing of the two changes together pans out, I will
> > give you a Tested-by -- I am guessing that you don't want to wait
> > until the next merge window for these changes.
> 
> I was planning to stuff them in tip/locking/urgent, so they'd end up in
> this release.

They seem to work fine for me, so for both:

Tested-by: Paul E. McKenney 

Thanx, Paul

[PATCH 1/2] Revert "drm/fb-helper: Reduce READ_ONCE(master) to lockless_dereference"

[Paul E. McKenney]

On Thu, Aug 11, 2016 at 12:38:59PM +0200, Daniel Vetter wrote:
> On Thu, Aug 11, 2016 at 11:50:21AM +0200, Johannes Berg wrote:
> > From: Johannes Berg 
> > 
> > This reverts commit fa7d81bb3c269a2ee38b6e4d569d9eb8be1a78ad.
> > 
> > As Peter explained:
> >   [...] lockless_dereference() is _stronger_ than READ_ONCE(), not weaker.
> > 
> >   [...]
> > 
> >   Also, clue is in the name: 'dereference', you don't actually dereference
> >   the pointer here, only load it.
> > 
> > My next patch breaks compile on this, because it assumes you want to
> > deference and thus also need the struct type visible (which it isn't
> > here), so revert it.
> > 
> > Cc: Peter Zijlstra 
> > Signed-off-by: Johannes Berg 
> 
> Reviewed-by: Daniel Vetter 
> 
> And ack-by: me for merging through whatever tree this makes sense for.
> -Daniel

Initial testing says that the change below must precede the change
to the definition of lockless_dereference(), so the two should go
together.

If my upcoming testing of the two changes together pans out, I will
give you a Tested-by -- I am guessing that you don't want to wait
until the next merge window for these changes.

Thanx, Paul

> > ---
> >  drivers/gpu/drm/drm_fb_helper.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_fb_helper.c 
> > b/drivers/gpu/drm/drm_fb_helper.c
> > index ce54e985d91b..0a06f9120b5a 100644
> > --- a/drivers/gpu/drm/drm_fb_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > @@ -464,7 +464,7 @@ static bool drm_fb_helper_is_bound(struct drm_fb_helper 
> > *fb_helper)
> >  
> > /* Sometimes user space wants everything disabled, so don't steal the
> >  * display if there's a master. */
> > -   if (lockless_dereference(dev->master))
> > +   if (READ_ONCE(dev->master))
> > return false;
> >  
> > drm_for_each_crtc(crtc, dev) {
> > -- 
> > 2.8.1
> > 
> > ___
> > dri-devel mailing list
> > dri-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> 

drm/fb-helper: Reduce READ_ONCE(master) to lockless_dereference

[Paul E. McKenney]

On Thu, Aug 11, 2016 at 11:44:08AM +0200, Peter Zijlstra wrote:
> On Wed, Jun 22, 2016 at 08:46:12AM +0100, Chris Wilson wrote:
> > We are only documenting that the read is outside of the lock, and do not
> > require strict ordering on the operation. In this case the more relaxed
> > lockless_dereference() will suffice.
> 
> No, no, no... This is 'broken'. lockless_dereference() is _stronger_
> than READ_ONCE(), not weaker.
> 
> lockless_dereference() is a wrapper around smp_read_barrier_depends()
> and is used to form read dependencies. There is no read dependency here,
> therefore using lockless_dereference() is entirely pointless.
> 
> Look at the definition of lockless_dereference(), it does a READ_ONCE()
> and then smp_read_barrier_depends().
> 
> Also, clue is in the name: 'dereference', you don't actually dereference
> the pointer here, only load it.

What Peter said!

If you are just checking the value of an RCU-protected pointer,
that is, one on which you would use rcu_dereference() rather than
lockless_dereference(), rcu_access_pointer() does the job of READ_ONCE()
while keeping sparse happy.

Thanx, Paul

> > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > @@ -464,7 +464,7 @@ static bool drm_fb_helper_is_bound(struct drm_fb_helper 
> > *fb_helper)
> >  
> > /* Sometimes user space wants everything disabled, so don't steal the
> >  * display if there's a master. */
> > -   if (READ_ONCE(dev->master))
> > +   if (lockless_dereference(dev->master))
> > return false;
> >  
> > drm_for_each_crtc(crtc, dev) {
> 

linux-next: build failure after merge of the tip tree (from the drm-intel tree)

[Paul E. McKenney]

On Tue, Jul 05, 2016 at 10:25:12AM +0200, Peter Zijlstra wrote:
> On Tue, Jul 05, 2016 at 01:53:03PM +1000, Stephen Rothwell wrote:
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c 
> > b/drivers/gpu/drm/i915/i915_gem.c
> > index d3502c0603e5..1f91f187b2a8 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -3290,7 +3290,7 @@ i915_gem_retire_work_handler(struct work_struct *work)
> >  * We do not need to do this test under locking as in the worst-case
> >  * we queue the retire worker once too often.
> >  */
> > -   if (lockless_dereference(dev_priv->gt.awake))
> > +   if (/*lockless_dereference*/(dev_priv->gt.awake))
> > queue_delayed_work(dev_priv->wq,
> >_priv->gt.retire_work,
> >round_jiffies_up_relative(HZ));
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c 
> > b/drivers/gpu/drm/i915/i915_irq.c
> > index f6de8dd567a2..2c1926418691 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -3095,7 +3095,7 @@ static void i915_hangcheck_elapsed(struct work_struct 
> > *work)
> > if (!i915.enable_hangcheck)
> > return;
> >  
> > -   if (!lockless_dereference(dev_priv->gt.awake))
> > +   if (!/*lockless_dereference*/(dev_priv->gt.awake))
> > return;
> >  
> > /* As enabling the GPU requires fairly extensive mmio access,
> 
> Right, neither case appears to include a data dependency and thus
> lockless_dereference() seems misguided.

Agreed!  At first glance, this code wants READ_ONCE() rather than
lockless_dereference().

Thanx, Paul

reservation.h: build error with lockdep disabled

[Paul E. McKenney]

On Thu, Nov 26, 2015 at 01:43:40PM +, Russell King - ARM Linux wrote:
> As of 3c3b177a9369 ("reservation: add suppport for read-only access
> using rcu") linux/reservation.h uses lockdep macros:
> 
> +#define reservation_object_held(obj) lockdep_is_held(&(obj)->lock.base)
> 
> This results in build errors when lockdep is disabled as lockdep_is_held()
> is only available when lockdep is enabled.  This has been reported today
> to break the etnaviv kernel driver, which we're hoping to submit for 4.5.
> 
> As this gets used with rcu_dereference_protected(), eg:
> 
> static inline struct reservation_object_list *
> reservation_object_get_list(struct reservation_object *obj)
> {
> return rcu_dereference_protected(obj->fence,
>  reservation_object_held(obj));
> }
> 
> I'm guessing that it's not going to be a simple case of making it always
> return true or always return false.
> 
> Any ideas how to solve this?

The usual approach is something like this:

#ifdef CONFIG_PROVE_LOCKING
#define reservation_object_held(obj) lockdep_is_held(&(obj)->lock.base)
#else
#define reservation_object_held(obj) true
#endif

Thanx, Paul

[RESUBMIT] [PATCH] Replace mentions of "list_struct" to "list_head"

[Paul E. McKenney]

On Fri, Nov 14, 2014 at 05:09:55AM +0400, Andrey Utkin wrote:
> There's no such thing as "list_struct".
> 
> Signed-off-by: Andrey Utkin 

May as well get group rates on the acks...  ;-)

Acked-by: Paul E. McKenney 

> ---
>  drivers/gpu/drm/radeon/mkregtable.c  | 24 
>  drivers/media/pci/cx18/cx18-driver.h |  2 +-
>  include/linux/list.h | 34 +-
>  include/linux/plist.h| 10 +-
>  include/linux/rculist.h  |  8 
>  scripts/kconfig/list.h   |  6 +++---
>  tools/usb/usbip/libsrc/list.h|  2 +-
>  7 files changed, 43 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/gpu/drm/radeon/mkregtable.c 
> b/drivers/gpu/drm/radeon/mkregtable.c
> index 4a85bb6..b928c17 100644
> --- a/drivers/gpu/drm/radeon/mkregtable.c
> +++ b/drivers/gpu/drm/radeon/mkregtable.c
> @@ -347,7 +347,7 @@ static inline void list_splice_tail_init(struct list_head 
> *list,
>   * list_entry - get the struct for this entry
>   * @ptr: the  list_head pointer.
>   * @type:the type of the struct this is embedded in.
> - * @member:  the name of the list_struct within the struct.
> + * @member:  the name of the list_head within the struct.
>   */
>  #define list_entry(ptr, type, member) \
>   container_of(ptr, type, member)
> @@ -356,7 +356,7 @@ static inline void list_splice_tail_init(struct list_head 
> *list,
>   * list_first_entry - get the first element from a list
>   * @ptr: the list head to take the element from.
>   * @type:the type of the struct this is embedded in.
> - * @member:  the name of the list_struct within the struct.
> + * @member:  the name of the list_head within the struct.
>   *
>   * Note, that list is expected to be not empty.
>   */
> @@ -406,7 +406,7 @@ static inline void list_splice_tail_init(struct list_head 
> *list,
>   * list_for_each_entry   -   iterate over list of given type
>   * @pos: the type * to use as a loop cursor.
>   * @head:the head for your list.
> - * @member:  the name of the list_struct within the struct.
> + * @member:  the name of the list_head within the struct.
>   */
>  #define list_for_each_entry(pos, head, member)   
> \
>   for (pos = list_entry((head)->next, typeof(*pos), member);  \
> @@ -417,7 +417,7 @@ static inline void list_splice_tail_init(struct list_head 
> *list,
>   * list_for_each_entry_reverse - iterate backwards over list of given type.
>   * @pos: the type * to use as a loop cursor.
>   * @head:the head for your list.
> - * @member:  the name of the list_struct within the struct.
> + * @member:  the name of the list_head within the struct.
>   */
>  #define list_for_each_entry_reverse(pos, head, member)   
> \
>   for (pos = list_entry((head)->prev, typeof(*pos), member);  \
> @@ -428,7 +428,7 @@ static inline void list_splice_tail_init(struct list_head 
> *list,
>   * list_prepare_entry - prepare a pos entry for use in 
> list_for_each_entry_continue()
>   * @pos: the type * to use as a start point
>   * @head:the head of the list
> - * @member:  the name of the list_struct within the struct.
> + * @member:  the name of the list_head within the struct.
>   *
>   * Prepares a pos entry for use as a start point in 
> list_for_each_entry_continue().
>   */
> @@ -439,7 +439,7 @@ static inline void list_splice_tail_init(struct list_head 
> *list,
>   * list_for_each_entry_continue - continue iteration over list of given type
>   * @pos: the type * to use as a loop cursor.
>   * @head:the head for your list.
> - * @member:  the name of the list_struct within the struct.
> + * @member:  the name of the list_head within the struct.
>   *
>   * Continue to iterate over list of given type, continuing after
>   * the current position.
> @@ -453,7 +453,7 @@ static inline void list_splice_tail_init(struct list_head 
> *list,
>   * list_for_each_entry_continue_reverse - iterate backwards from the given 
> point
>   * @pos: the type * to use as a loop cursor.
>   * @head:the head for your list.
> - * @member:  the name of the list_struct within the struct.
> + * @member:  the name of the list_head within the struct.
>   *
>   * Start to iterate over list of given type backwards, continuing after
>   * the current position.
> @@ -467,7 +467,7 @@ static inline void list_splice_tail_init(struct list_head 
> *list,
>   * list_for_each_entry_from - iterate over list of given type from the 
> current point
>   * @pos: the type * to use as a loop cursor.
>   * @head:the head for your list.
> - * @member:  t

[PATCH] list: Fix order of arguments for hlist_add_after(_rcu)

[Paul E. McKenney]

On Fri, Jun 06, 2014 at 03:56:52PM +, David Laight wrote:
> From: Behalf Of Ken Helias
> > All other add functions for lists have the new item as first argument and 
> > the
> > position where it is added as second argument. This was changed for no good
> > reason in this function and makes using it unnecessary confusing.
> > 
> > Also the naming of the arguments in hlist_add_after was confusing. It was
> > changed to use the same names as hlist_add_after_rcu.
> ...
> > -static inline void hlist_add_after_rcu(struct hlist_node *prev,
> > -  struct hlist_node *n)
> > +static inline void hlist_add_after_rcu(struct hlist_node *n,
> > +  struct hlist_node *prev)
> 
> It is rather a shame that the change doesn't generate a compilation
> error for old source files.

I am also a bit concerned by this.

Thanx, Paul

Re: 3.1-rc1-git9: Reported regressions 2.6.39 - 3.0

[Paul E. McKenney]

On Sun, Aug 14, 2011 at 09:02:27PM +0200, Rafael J. Wysocki wrote:
 [NOTE:
  We already have a bug entry for tracking regressions from 3.0:
 
  http://bugzilla.kernel.org/show_bug.cgi?id=40982
 
  but there are no reports linked to it, mostly because Maciej is on vacation,
  but also because the frequency of reporting regressions has been low
  recently.  So, if you're seeing a regression from 3.0, please let us know.]
 
 This message contains a list of some post-2.6.39 regressions introduced before
 3.0, for which there are no fixes in the mainline known to the tracking team.
 If any of them have been fixed already, please let us know.
 
 If you know of any other unresolved post-2.6.39 regressions, please let us 
 know
 either and we'll add them to the list.  Also, please let us know if any
 of the entries below are invalid.
 
 Each entry from the list will be sent additionally in an automatic reply to
 this message with CCs to the people involved in reporting and handling the
 issue.
 
 
 Listed regressions statistics:
 
   Date  Total  Pending  Unresolved
   
   2011-08-14   63   22  19
 
 
 Unresolved regressions
 --
 
 Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=40282
 Subject   : IP forwarding regression since 3.0-rc6
 Submitter : Stephan Seitz stse+l...@fsing.rootsland.net
 Date  : 2011-07-25 12:51 (21 days old)
 Message-ID: 20110725t141145.ga.2ae38.s...@fsing.rootsland.net
 References: http://marc.info/?l=linux-kernelm=131159880004908w=2
 
 
 Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=40172
 Subject   : lockdep trace from post 3.0.
 Submitter : Dave Jones da...@redhat.com
 Date  : 2011-07-24 1:32 (22 days old)
 Message-ID: 20110724013257.ga6...@redhat.com
 References: http://marc.info/?l=linux-kernelm=131147120206819w=2
 
 
 Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=40092
 Subject   : RCU stall in linux-3.0.0
 Submitter : Philip Armstrong p...@kantaka.co.uk
 Date  : 2011-07-25 21:44 (21 days old)

This one was due the the CPU's idea of the current time getting way
out of sync (as in more than a minute's worth of disagreement, which is
pretty impressive when you think about it).  So what happened is that
one CPU decided that the current grace period was a full minute old
immediately, and thus started screaming piteously.  John Stultz provided
a configuration workaround, and he and Thomas Gleixner would be working
out the long-term fix.

Thanx, Paul
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

3.1-rc1-git9: Reported regressions 2.6.39 -> 3.0

[Paul E. McKenney]

On Sun, Aug 14, 2011 at 09:02:27PM +0200, Rafael J. Wysocki wrote:
> [NOTE:
>  We already have a bug entry for tracking regressions from 3.0:
> 
>  http://bugzilla.kernel.org/show_bug.cgi?id=40982
> 
>  but there are no reports linked to it, mostly because Maciej is on vacation,
>  but also because the frequency of reporting regressions has been low
>  recently.  So, if you're seeing a regression from 3.0, please let us know.]
> 
> This message contains a list of some post-2.6.39 regressions introduced before
> 3.0, for which there are no fixes in the mainline known to the tracking team.
> If any of them have been fixed already, please let us know.
> 
> If you know of any other unresolved post-2.6.39 regressions, please let us 
> know
> either and we'll add them to the list.  Also, please let us know if any
> of the entries below are invalid.
> 
> Each entry from the list will be sent additionally in an automatic reply to
> this message with CCs to the people involved in reporting and handling the
> issue.
> 
> 
> Listed regressions statistics:
> 
>   Date  Total  Pending  Unresolved
>   
>   2011-08-14   63   22  19
> 
> 
> Unresolved regressions
> --
> 
> Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=40282
> Subject   : IP forwarding regression since 3.0-rc6
> Submitter : Stephan Seitz 
> Date  : 2011-07-25 12:51 (21 days old)
> Message-ID: <20110725T141145.GA.2ae38.stse at fsing.rootsland.net>
> References: http://marc.info/?l=linux-kernel=131159880004908=2
> 
> 
> Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=40172
> Subject   : lockdep trace from post 3.0.
> Submitter : Dave Jones 
> Date  : 2011-07-24 1:32 (22 days old)
> Message-ID: <20110724013257.GA6777 at redhat.com>
> References: http://marc.info/?l=linux-kernel=131147120206819=2
> 
> 
> Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=40092
> Subject   : RCU stall in linux-3.0.0
> Submitter : Philip Armstrong 
> Date  : 2011-07-25 21:44 (21 days old)

This one was due the the CPU's idea of the current time getting way
out of sync (as in more than a minute's worth of disagreement, which is
pretty impressive when you think about it).  So what happened is that
one CPU decided that the current grace period was a full minute old
immediately, and thus started screaming piteously.  John Stultz provided
a configuration workaround, and he and Thomas Gleixner would be working
out the long-term fix.

Thanx, Paul