The UP.rst file calls for locks acquired within RCU callback functions
to use _irq variants (spin_lock_irqsave() or similar), which does work,
but can be overkill. This commit therefore instead calls for _bh variants
(spin_lock_bh() or similar), while noting that _irq does work.
Signed-off-by: Paul E. McKenney <[email protected]>
Signed-off-by: Jiunn Chang <[email protected]>
---
Documentation/RCU/UP.rst | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/Documentation/RCU/UP.rst b/Documentation/RCU/UP.rst
index 67715a47ae89..e26dda27430c 100644
--- a/Documentation/RCU/UP.rst
+++ b/Documentation/RCU/UP.rst
@@ -113,12 +113,13 @@ Answer to Quick Quiz #1:
Answer to Quick Quiz #2:
What locking restriction must RCU callbacks respect?
- Any lock that is acquired within an RCU callback must be
- acquired elsewhere using an _irq variant of the spinlock
- primitive. For example, if "mylock" is acquired by an
- RCU callback, then a process-context acquisition of this
- lock must use something like spin_lock_irqsave() to
- acquire the lock.
+ Any lock that is acquired within an RCU callback must be acquired
+ elsewhere using an _bh variant of the spinlock primitive.
+ For example, if "mylock" is acquired by an RCU callback, then
+ a process-context acquisition of this lock must use something
+ like spin_lock_bh() to acquire the lock. Please note that
+ it is also OK to use _irq variants of spinlocks, for example,
+ spin_lock_irqsave().
If the process-context code were to simply use spin_lock(),
then, since RCU callbacks can be invoked from softirq context,
--
2.22.0
On 6/27/19 3:01 PM, Jiunn Chang wrote:
> The UP.rst file calls for locks acquired within RCU callback functions
> to use _irq variants (spin_lock_irqsave() or similar), which does work,
> but can be overkill. This commit therefore instead calls for _bh variants
> (spin_lock_bh() or similar), while noting that _irq does work.
>
> Signed-off-by: Paul E. McKenney <[email protected]>
Should this by Suggested-by?
> Signed-off-by: Jiunn Chang <[email protected]>
> ---
> Documentation/RCU/UP.rst | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/RCU/UP.rst b/Documentation/RCU/UP.rst
> index 67715a47ae89..e26dda27430c 100644
> --- a/Documentation/RCU/UP.rst
> +++ b/Documentation/RCU/UP.rst
> @@ -113,12 +113,13 @@ Answer to Quick Quiz #1:
> Answer to Quick Quiz #2:
> What locking restriction must RCU callbacks respect?
>
> - Any lock that is acquired within an RCU callback must be
> - acquired elsewhere using an _irq variant of the spinlock
> - primitive. For example, if "mylock" is acquired by an
> - RCU callback, then a process-context acquisition of this
> - lock must use something like spin_lock_irqsave() to
> - acquire the lock.
> + Any lock that is acquired within an RCU callback must be acquired
> + elsewhere using an _bh variant of the spinlock primitive.
> + For example, if "mylock" is acquired by an RCU callback, then
> + a process-context acquisition of this lock must use something
> + like spin_lock_bh() to acquire the lock. Please note that
> + it is also OK to use _irq variants of spinlocks, for example,
> + spin_lock_irqsave().
>
> If the process-context code were to simply use spin_lock(),
> then, since RCU callbacks can be invoked from softirq context,
>
thanks,
-- Shuah
On Thu, Jun 27, 2019 at 04:01:35PM -0600, Shuah Khan wrote:
> On 6/27/19 3:01 PM, Jiunn Chang wrote:
> >The UP.rst file calls for locks acquired within RCU callback functions
> >to use _irq variants (spin_lock_irqsave() or similar), which does work,
> >but can be overkill. This commit therefore instead calls for _bh variants
> >(spin_lock_bh() or similar), while noting that _irq does work.
> >
> >Signed-off-by: Paul E. McKenney <[email protected]>
>
> Should this by Suggested-by?
I wrote it and Jiunn converted my change to .rst, so I believe that
this is correct as is.
Thanx, Paul
> >Signed-off-by: Jiunn Chang <[email protected]>
> >---
> > Documentation/RCU/UP.rst | 13 +++++++------
> > 1 file changed, 7 insertions(+), 6 deletions(-)
> >
> >diff --git a/Documentation/RCU/UP.rst b/Documentation/RCU/UP.rst
> >index 67715a47ae89..e26dda27430c 100644
> >--- a/Documentation/RCU/UP.rst
> >+++ b/Documentation/RCU/UP.rst
> >@@ -113,12 +113,13 @@ Answer to Quick Quiz #1:
> > Answer to Quick Quiz #2:
> > What locking restriction must RCU callbacks respect?
> >- Any lock that is acquired within an RCU callback must be
> >- acquired elsewhere using an _irq variant of the spinlock
> >- primitive. For example, if "mylock" is acquired by an
> >- RCU callback, then a process-context acquisition of this
> >- lock must use something like spin_lock_irqsave() to
> >- acquire the lock.
> >+ Any lock that is acquired within an RCU callback must be acquired
> >+ elsewhere using an _bh variant of the spinlock primitive.
> >+ For example, if "mylock" is acquired by an RCU callback, then
> >+ a process-context acquisition of this lock must use something
> >+ like spin_lock_bh() to acquire the lock. Please note that
> >+ it is also OK to use _irq variants of spinlocks, for example,
> >+ spin_lock_irqsave().
> > If the process-context code were to simply use spin_lock(),
> > then, since RCU callbacks can be invoked from softirq context,
> >
>
> thanks,
> -- Shuah
>
On 6/27/19 4:10 PM, Paul E. McKenney wrote:
> On Thu, Jun 27, 2019 at 04:01:35PM -0600, Shuah Khan wrote:
>> On 6/27/19 3:01 PM, Jiunn Chang wrote:
>>> The UP.rst file calls for locks acquired within RCU callback functions
>>> to use _irq variants (spin_lock_irqsave() or similar), which does work,
>>> but can be overkill. This commit therefore instead calls for _bh variants
>>> (spin_lock_bh() or similar), while noting that _irq does work.
>>>
>>> Signed-off-by: Paul E. McKenney <[email protected]>
>>
>> Should this by Suggested-by?
>
> I wrote it and Jiunn converted my change to .rst, so I believe that
> this is correct as is.
>
Great.
thanks,
-- Shuah
On Thu, 27 Jun 2019 15:10:45 -0700
"Paul E. McKenney" <[email protected]> wrote:
> On Thu, Jun 27, 2019 at 04:01:35PM -0600, Shuah Khan wrote:
> > On 6/27/19 3:01 PM, Jiunn Chang wrote:
> > >The UP.rst file calls for locks acquired within RCU callback functions
> > >to use _irq variants (spin_lock_irqsave() or similar), which does work,
> > >but can be overkill. This commit therefore instead calls for _bh variants
> > >(spin_lock_bh() or similar), while noting that _irq does work.
> > >
> > >Signed-off-by: Paul E. McKenney <[email protected]>
> >
> > Should this by Suggested-by?
>
> I wrote it and Jiunn converted my change to .rst, so I believe that
> this is correct as is.
Note, you did send Jiunn an explicit Signed-off-by when you wrote it,
correct? As Signed-off-by is equivalent to a signature.
-- Steve
On Thu, Jun 27, 2019 at 06:29:38PM -0400, Steven Rostedt wrote:
> On Thu, 27 Jun 2019 15:10:45 -0700
> "Paul E. McKenney" <[email protected]> wrote:
>
> > On Thu, Jun 27, 2019 at 04:01:35PM -0600, Shuah Khan wrote:
> > > On 6/27/19 3:01 PM, Jiunn Chang wrote:
> > > >The UP.rst file calls for locks acquired within RCU callback functions
> > > >to use _irq variants (spin_lock_irqsave() or similar), which does work,
> > > >but can be overkill. This commit therefore instead calls for _bh variants
> > > >(spin_lock_bh() or similar), while noting that _irq does work.
> > > >
> > > >Signed-off-by: Paul E. McKenney <[email protected]>
> > >
> > > Should this by Suggested-by?
> >
> > I wrote it and Jiunn converted my change to .rst, so I believe that
> > this is correct as is.
>
> Note, you did send Jiunn an explicit Signed-off-by when you wrote it,
> correct? As Signed-off-by is equivalent to a signature.
Indeed I did, but I now see that it was via private email. Here it is
again for public consumption, and Jiunn's patch is based on this one,
just translated to .rst. I once again verified that the Jiunn's version
is word-for-word identical to mine, so Jiunn's patch should be good. ;-)
Thanx, Paul
------------------------------------------------------------------------
commit a293734a310b463a0dba68409943a4e6065cd39d
Author: Paul E. McKenney <[email protected]>
Date: Wed Jun 26 10:16:19 2019 -0700
doc: RCU callback locks need only _bh, not necessarily _irq
The UP.txt file calls for locks acquired within RCU callback functions
to use _irq variants (spin_lock_irqsave() or similar), which does work,
but can be overkill. This commit therefore instead calls for _bh variants
(spin_lock_bh() or similar), while noting that _irq does work.
Signed-off-by: Paul E. McKenney <[email protected]>
diff --git a/Documentation/RCU/UP.txt b/Documentation/RCU/UP.txt
index 53bde717017b..0edd8c5af0b5 100644
--- a/Documentation/RCU/UP.txt
+++ b/Documentation/RCU/UP.txt
@@ -104,12 +104,13 @@ Answer to Quick Quiz #1:
Answer to Quick Quiz #2:
What locking restriction must RCU callbacks respect?
- Any lock that is acquired within an RCU callback must be
- acquired elsewhere using an _irq variant of the spinlock
- primitive. For example, if "mylock" is acquired by an
- RCU callback, then a process-context acquisition of this
- lock must use something like spin_lock_irqsave() to
- acquire the lock.
+ Any lock that is acquired within an RCU callback must be acquired
+ elsewhere using an _bh variant of the spinlock primitive.
+ For example, if "mylock" is acquired by an RCU callback, then
+ a process-context acquisition of this lock must use something
+ like spin_lock_bh() to acquire the lock. Please note that
+ it is also OK to use _irq variants of spinlocks, for example,
+ spin_lock_irqsave().
If the process-context code were to simply use spin_lock(),
then, since RCU callbacks can be invoked from softirq context,
On Thu, 27 Jun 2019 16:01:47 -0500
Jiunn Chang <[email protected]> wrote:
> The UP.rst file calls for locks acquired within RCU callback functions
> to use _irq variants (spin_lock_irqsave() or similar), which does work,
> but can be overkill. This commit therefore instead calls for _bh variants
> (spin_lock_bh() or similar), while noting that _irq does work.
>
> Signed-off-by: Paul E. McKenney <[email protected]>
> Signed-off-by: Jiunn Chang <[email protected]>
> ---
> Documentation/RCU/UP.rst | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
Applied, thanks.
jon