Hi,
I have a piece of code that is always called under a spinlock with
interrups disabled. Within that piece of code I iterate through a
list. I have another piece of code that wants to modify that list. I
have 2 options:
1) Have the other piece of code acquire the same spinlock
2) Use RCU
I don't want to do 1) because the otheir piece of code does not really
care about object owning the spinlock and so acquiring the spinlock is
"not nice". However it is guaranteed that the piece of code that
accesses lock runs atomically with interrupts disabled. So
rcu_read_lock() would be superfluos there.
Is it possible to still use list_for_each_rcu() and friends to access
that list without rcu_read_lock()? Or it is betteruse complete RCU
interface and eat cost of couple of extra instrctions?
--
Dmitry
On Fri, 2007-06-15 at 15:00 -0400, Dmitry Torokhov wrote:
> Hi,
>
> I have a piece of code that is always called under a spinlock with
> interrups disabled. Within that piece of code I iterate through a
> list. I have another piece of code that wants to modify that list. I
> have 2 options:
>
> 1) Have the other piece of code acquire the same spinlock
> 2) Use RCU
>
> I don't want to do 1) because the otheir piece of code does not really
> care about object owning the spinlock and so acquiring the spinlock is
> "not nice". However it is guaranteed that the piece of code that
> accesses lock runs atomically with interrupts disabled. So
> rcu_read_lock() would be superfluos there.
>
> Is it possible to still use list_for_each_rcu() and friends to access
> that list without rcu_read_lock()? Or it is betteruse complete RCU
> interface and eat cost of couple of extra instrctions?
Yes, preemptible rcu requires that you use the full interface, also, it
more clearly documents the code. Trying to find code that breaks these
assumptions is very tedious work after the fact.
Please do use the RCU interface in full.
On Fri, Jun 15, 2007 at 09:04:19PM +0200, Peter Zijlstra wrote:
> On Fri, 2007-06-15 at 15:00 -0400, Dmitry Torokhov wrote:
> > Hi,
> >
> > I have a piece of code that is always called under a spinlock with
> > interrups disabled. Within that piece of code I iterate through a
> > list. I have another piece of code that wants to modify that list. I
> > have 2 options:
> >
> > I don't want to do 1) because the otheir piece of code does not really
> > care about object owning the spinlock and so acquiring the spinlock is
> > "not nice". However it is guaranteed that the piece of code that
> > accesses lock runs atomically with interrupts disabled. So
> > rcu_read_lock() would be superfluos there.
> >
> > Is it possible to still use list_for_each_rcu() and friends to access
> > that list without rcu_read_lock()? Or it is betteruse complete RCU
> > interface and eat cost of couple of extra instrctions?
>
> Yes, preemptible rcu requires that you use the full interface, also, it
> more clearly documents the code. Trying to find code that breaks these
> assumptions is very tedious work after the fact.
>
> Please do use the RCU interface in full.
As Peter said, you should use the strict RCU APIs and not rely
on the current implementation of RCU to optimize. Things change.
Plus static/dynamic checking becomes easier that way.
Thanks
Dipankar
On Sat, Jun 16, 2007 at 12:59:40AM +0530, Dipankar Sarma wrote:
> On Fri, Jun 15, 2007 at 09:04:19PM +0200, Peter Zijlstra wrote:
> > On Fri, 2007-06-15 at 15:00 -0400, Dmitry Torokhov wrote:
> > > Hi,
> > >
> > > I have a piece of code that is always called under a spinlock with
> > > interrups disabled. Within that piece of code I iterate through a
> > > list. I have another piece of code that wants to modify that list. I
> > > have 2 options:
> > >
> > > I don't want to do 1) because the otheir piece of code does not really
> > > care about object owning the spinlock and so acquiring the spinlock is
> > > "not nice". However it is guaranteed that the piece of code that
> > > accesses lock runs atomically with interrupts disabled. So
> > > rcu_read_lock() would be superfluos there.
> > >
> > > Is it possible to still use list_for_each_rcu() and friends to access
> > > that list without rcu_read_lock()? Or it is betteruse complete RCU
> > > interface and eat cost of couple of extra instrctions?
> >
> > Yes, preemptible rcu requires that you use the full interface, also, it
> > more clearly documents the code. Trying to find code that breaks these
> > assumptions is very tedious work after the fact.
> >
> > Please do use the RCU interface in full.
>
> As Peter said, you should use the strict RCU APIs and not rely
> on the current implementation of RCU to optimize. Things change.
> Plus static/dynamic checking becomes easier that way.
What they said!!!
There are a couple of other options, however:
1. Use preempt_disable() and preempt_enable() on the read side,
and synchronize_sched() on the update side.
2. Use local_irq_save() and local_irq_restore() on the read side,
and synchronize_sched() on the update side. Usually not
competitive -- unless interrupts needed to be disabled for some
other reason anyway. Which you in fact say that you do.
I believe that #2 might do what you want. But please, PLEASE carefully
comment this usage!!!
Thanx, Paul
On 6/15/07, Paul E. McKenney <[email protected]> wrote:
> On Sat, Jun 16, 2007 at 12:59:40AM +0530, Dipankar Sarma wrote:
> > On Fri, Jun 15, 2007 at 09:04:19PM +0200, Peter Zijlstra wrote:
> > > On Fri, 2007-06-15 at 15:00 -0400, Dmitry Torokhov wrote:
> > > > Hi,
> > > >
> > > > I have a piece of code that is always called under a spinlock with
> > > > interrups disabled. Within that piece of code I iterate through a
> > > > list. I have another piece of code that wants to modify that list. I
> > > > have 2 options:
> > > >
> > > > I don't want to do 1) because the otheir piece of code does not really
> > > > care about object owning the spinlock and so acquiring the spinlock is
> > > > "not nice". However it is guaranteed that the piece of code that
> > > > accesses lock runs atomically with interrupts disabled. So
> > > > rcu_read_lock() would be superfluos there.
> > > >
> > > > Is it possible to still use list_for_each_rcu() and friends to access
> > > > that list without rcu_read_lock()? Or it is betteruse complete RCU
> > > > interface and eat cost of couple of extra instrctions?
> > >
> > > Yes, preemptible rcu requires that you use the full interface, also, it
> > > more clearly documents the code. Trying to find code that breaks these
> > > assumptions is very tedious work after the fact.
> > >
> > > Please do use the RCU interface in full.
> >
> > As Peter said, you should use the strict RCU APIs and not rely
> > on the current implementation of RCU to optimize. Things change.
> > Plus static/dynamic checking becomes easier that way.
>
> What they said!!!
>
> There are a couple of other options, however:
>
> 1. Use preempt_disable() and preempt_enable() on the read side,
> and synchronize_sched() on the update side.
>
> 2. Use local_irq_save() and local_irq_restore() on the read side,
> and synchronize_sched() on the update side. Usually not
> competitive -- unless interrupts needed to be disabled for some
> other reason anyway. Which you in fact say that you do.
Right. The callsite that iterates through the list is essentially
protected by spin_lock_irqsave()/spin_unlock_irqrestore() - needed for
other reasons (such as updating internal state of a device - and that
can happen from different contexts).
>
> I believe that #2 might do what you want. But please, PLEASE carefully
> comment this usage!!!
>
Would there be a reson not to use #2 but rather full RCU with
rcu_read_lock()/synchronize_rcu()?
Thank you.
--
Dmitry
On Fri, Jun 15, 2007 at 04:25:02PM -0400, Dmitry Torokhov wrote:
> On 6/15/07, Paul E. McKenney <[email protected]> wrote:
> >On Sat, Jun 16, 2007 at 12:59:40AM +0530, Dipankar Sarma wrote:
> >> On Fri, Jun 15, 2007 at 09:04:19PM +0200, Peter Zijlstra wrote:
> >> > On Fri, 2007-06-15 at 15:00 -0400, Dmitry Torokhov wrote:
> >> > > Hi,
> >> > >
> >> > > I have a piece of code that is always called under a spinlock with
> >> > > interrups disabled. Within that piece of code I iterate through a
> >> > > list. I have another piece of code that wants to modify that list. I
> >> > > have 2 options:
> >> > >
> >> > > I don't want to do 1) because the otheir piece of code does not
> >really
> >> > > care about object owning the spinlock and so acquiring the spinlock
> >is
> >> > > "not nice". However it is guaranteed that the piece of code that
> >> > > accesses lock runs atomically with interrupts disabled. So
> >> > > rcu_read_lock() would be superfluos there.
> >> > >
> >> > > Is it possible to still use list_for_each_rcu() and friends to access
> >> > > that list without rcu_read_lock()? Or it is betteruse complete RCU
> >> > > interface and eat cost of couple of extra instrctions?
> >> >
> >> > Yes, preemptible rcu requires that you use the full interface, also, it
> >> > more clearly documents the code. Trying to find code that breaks these
> >> > assumptions is very tedious work after the fact.
> >> >
> >> > Please do use the RCU interface in full.
> >>
> >> As Peter said, you should use the strict RCU APIs and not rely
> >> on the current implementation of RCU to optimize. Things change.
> >> Plus static/dynamic checking becomes easier that way.
> >
> >What they said!!!
> >
> >There are a couple of other options, however:
> >
> >1. Use preempt_disable() and preempt_enable() on the read side,
> > and synchronize_sched() on the update side.
> >
> >2. Use local_irq_save() and local_irq_restore() on the read side,
> > and synchronize_sched() on the update side. Usually not
> > competitive -- unless interrupts needed to be disabled for some
> > other reason anyway. Which you in fact say that you do.
>
> Right. The callsite that iterates through the list is essentially
> protected by spin_lock_irqsave()/spin_unlock_irqrestore() - needed for
> other reasons (such as updating internal state of a device - and that
> can happen from different contexts).
That will work!
> >I believe that #2 might do what you want. But please, PLEASE carefully
> >comment this usage!!!
>
> Would there be a reson not to use #2 but rather full RCU with
> rcu_read_lock()/synchronize_rcu()?
Probably not, but here are a couple of situations where the full RCU
might be preferred:
1. If you were relying on interrupts being disabled within an
interrupt handler (which they are -not- in -rt), then you would
either need to add some form of local_irq_save() or, as you say,
go to the rcu_read_lock() and synchronize_rcu() interfaces.
2. If updates needed to use callbacks rather than synchronous waits
for grace periods, in other words, if you needed call_rcu()
instead of synchronize_rcu(). Of course, a callback API for
_sched (call_rcu_sched() or some such) could be added if needed,
though it would be better to avoid the API proliferation unless
really badly needed.
Thanx, Paul
On 6/15/07, Paul E. McKenney <[email protected]> wrote:
> On Fri, Jun 15, 2007 at 04:25:02PM -0400, Dmitry Torokhov wrote:
> > On 6/15/07, Paul E. McKenney <[email protected]> wrote:
> > >On Sat, Jun 16, 2007 at 12:59:40AM +0530, Dipankar Sarma wrote:
> > >> On Fri, Jun 15, 2007 at 09:04:19PM +0200, Peter Zijlstra wrote:
> > >> > On Fri, 2007-06-15 at 15:00 -0400, Dmitry Torokhov wrote:
> > >> > > Hi,
> > >> > >
> > >> > > I have a piece of code that is always called under a spinlock with
> > >> > > interrups disabled. Within that piece of code I iterate through a
> > >> > > list. I have another piece of code that wants to modify that list. I
> > >> > > have 2 options:
> > >> > >
> > >> > > I don't want to do 1) because the otheir piece of code does not
> > >really
> > >> > > care about object owning the spinlock and so acquiring the spinlock
> > >is
> > >> > > "not nice". However it is guaranteed that the piece of code that
> > >> > > accesses lock runs atomically with interrupts disabled. So
> > >> > > rcu_read_lock() would be superfluos there.
> > >> > >
> > >> > > Is it possible to still use list_for_each_rcu() and friends to access
> > >> > > that list without rcu_read_lock()? Or it is betteruse complete RCU
> > >> > > interface and eat cost of couple of extra instrctions?
> > >> >
> > >> > Yes, preemptible rcu requires that you use the full interface, also, it
> > >> > more clearly documents the code. Trying to find code that breaks these
> > >> > assumptions is very tedious work after the fact.
> > >> >
> > >> > Please do use the RCU interface in full.
> > >>
> > >> As Peter said, you should use the strict RCU APIs and not rely
> > >> on the current implementation of RCU to optimize. Things change.
> > >> Plus static/dynamic checking becomes easier that way.
> > >
> > >What they said!!!
> > >
> > >There are a couple of other options, however:
> > >
> > >1. Use preempt_disable() and preempt_enable() on the read side,
> > > and synchronize_sched() on the update side.
> > >
> > >2. Use local_irq_save() and local_irq_restore() on the read side,
> > > and synchronize_sched() on the update side. Usually not
> > > competitive -- unless interrupts needed to be disabled for some
> > > other reason anyway. Which you in fact say that you do.
> >
> > Right. The callsite that iterates through the list is essentially
> > protected by spin_lock_irqsave()/spin_unlock_irqrestore() - needed for
> > other reasons (such as updating internal state of a device - and that
> > can happen from different contexts).
>
> That will work!
>
> > >I believe that #2 might do what you want. But please, PLEASE carefully
> > >comment this usage!!!
> >
> > Would there be a reson not to use #2 but rather full RCU with
> > rcu_read_lock()/synchronize_rcu()?
>
> Probably not, but here are a couple of situations where the full RCU
> might be preferred:
>
> 1. If you were relying on interrupts being disabled within an
> interrupt handler (which they are -not- in -rt), then you would
> either need to add some form of local_irq_save() or, as you say,
> go to the rcu_read_lock() and synchronize_rcu() interfaces.
>
> 2. If updates needed to use callbacks rather than synchronous waits
> for grace periods, in other words, if you needed call_rcu()
> instead of synchronize_rcu(). Of course, a callback API for
> _sched (call_rcu_sched() or some such) could be added if needed,
> though it would be better to avoid the API proliferation unless
> really badly needed.
>
OK, then I will got with route #2 and just comment it properly.
Thank you for your answers.
--
Dmitry