2005-04-03 06:21:51

by Paul E. McKenney

[permalink] [raw]
Subject: [RFC,PATCH 2/4] Deprecate synchronize_kernel, GPL replacement

The synchronize_kernel() primitive is used for quite a few different
purposes: waiting for RCU readers, waiting for NMIs, waiting for interrupts,
and so on. This makes RCU code harder to read, since synchronize_kernel()
might or might not have matching rcu_read_lock()s. This patch creates
a new synchronize_rcu() that is to be used for RCU readers and a new
synchronize_sched() that is used for the rest. These two new primitives
currently have the same implementation, but this is might well change
with additional real-time support. Both new primitives are GPL-only,
the old primitive is deprecated.

Signed-off-by: <[email protected]>
---
Depends on earlier "Add deprecated_for_modules" patch.

include/linux/rcupdate.h | 24 +++++++++++++++++++++---
kernel/rcupdate.c | 16 ++++++++++++++--
2 files changed, 35 insertions(+), 5 deletions(-)

diff -urpN -X dontdiff linux-2.6.12-rc1/include/linux/rcupdate.h linux-2.6.12-rc1-bettersk/include/linux/rcupdate.h
--- linux-2.6.12-rc1/include/linux/rcupdate.h Tue Mar 1 23:37:50 2005
+++ linux-2.6.12-rc1-bettersk/include/linux/rcupdate.h Sat Apr 2 13:06:15 2005
@@ -41,6 +41,7 @@
#include <linux/percpu.h>
#include <linux/cpumask.h>
#include <linux/seqlock.h>
+#include <linux/module.h>

/**
* struct rcu_head - callback structure for use with RCU
@@ -157,9 +158,9 @@ static inline int rcu_pending(int cpu)
/**
* rcu_read_lock - mark the beginning of an RCU read-side critical section.
*
- * When synchronize_kernel() is invoked on one CPU while other CPUs
+ * When synchronize_rcu() is invoked on one CPU while other CPUs
* are within RCU read-side critical sections, then the
- * synchronize_kernel() is guaranteed to block until after all the other
+ * synchronize_rcu() is guaranteed to block until after all the other
* CPUs exit their critical sections. Similarly, if call_rcu() is invoked
* on one CPU while other CPUs are within RCU read-side critical
* sections, invocation of the corresponding RCU callback is deferred
@@ -256,6 +257,21 @@ static inline int rcu_pending(int cpu)
(p) = (v); \
})

+/**
+ * synchronize_sched - block until all CPUs have exited any non-preemptive
+ * kernel code sequences.
+ *
+ * This means that all preempt_disable code sequences, including NMI and
+ * hardware-interrupt handlers, in progress on entry will have completed
+ * before this primitive returns. However, this does not guarantee that
+ * softirq handlers will have completed, since in some kernels
+ *
+ * This primitive provides the guarantees made by the (deprecated)
+ * synchronize_kernel() API. In contrast, synchronize_rcu() only
+ * guarantees that rcu_read_lock() sections will have completed.
+ */
+#define synchronize_sched() synchronize_rcu()
+
extern void rcu_init(void);
extern void rcu_check_callbacks(int cpu, int user);
extern void rcu_restart_cpu(int cpu);
@@ -265,7 +281,9 @@ extern void FASTCALL(call_rcu(struct rcu
void (*func)(struct rcu_head *head)));
extern void FASTCALL(call_rcu_bh(struct rcu_head *head,
void (*func)(struct rcu_head *head)));
-extern void synchronize_kernel(void);
+extern deprecated_for_modules void synchronize_kernel(void);
+extern void synchronize_rcu(void);
+void synchronize_idle(void);

#endif /* __KERNEL__ */
#endif /* __LINUX_RCUPDATE_H */
diff -urpN -X dontdiff linux-2.6.12-rc1/kernel/rcupdate.c linux-2.6.12-rc1-bettersk/kernel/rcupdate.c
--- linux-2.6.12-rc1/kernel/rcupdate.c Tue Mar 1 23:37:30 2005
+++ linux-2.6.12-rc1-bettersk/kernel/rcupdate.c Sat Apr 2 13:10:09 2005
@@ -444,15 +444,18 @@ static void wakeme_after_rcu(struct rcu_
}

/**
- * synchronize_kernel - wait until a grace period has elapsed.
+ * synchronize_rcu - wait until a grace period has elapsed.
*
* Control will return to the caller some time after a full grace
* period has elapsed, in other words after all currently executing RCU
* read-side critical sections have completed. RCU read-side critical
* sections are delimited by rcu_read_lock() and rcu_read_unlock(),
* and may be nested.
+ *
+ * If your read-side code is not protected by rcu_read_lock(), do -not-
+ * use synchronize_rcu().
*/
-void synchronize_kernel(void)
+void synchronize_rcu(void)
{
struct rcu_synchronize rcu;

@@ -464,7 +467,16 @@ void synchronize_kernel(void)
wait_for_completion(&rcu.completion);
}

+/*
+ * Deprecated, use synchronize_rcu() or synchronize_sched() instead.
+ */
+void synchronize_kernel(void)
+{
+ synchronize_rcu();
+}
+
module_param(maxbatch, int, 0);
EXPORT_SYMBOL(call_rcu);
EXPORT_SYMBOL(call_rcu_bh);
+EXPORT_SYMBOL_GPL(synchronize_rcu);
EXPORT_SYMBOL(synchronize_kernel);


2005-04-03 08:56:57

by Dipankar Sarma

[permalink] [raw]
Subject: Re: [RFC,PATCH 2/4] Deprecate synchronize_kernel, GPL replacement

On Sat, Apr 02, 2005 at 10:21:50PM -0800, Paul E. McKenney wrote:
> The synchronize_kernel() primitive is used for quite a few different
> purposes: waiting for RCU readers, waiting for NMIs, waiting for interrupts,
> and so on. This makes RCU code harder to read, since synchronize_kernel()
> might or might not have matching rcu_read_lock()s. This patch creates
> a new synchronize_rcu() that is to be used for RCU readers and a new
> synchronize_sched() that is used for the rest. These two new primitives
> currently have the same implementation, but this is might well change
> with additional real-time support. Both new primitives are GPL-only,
> the old primitive is deprecated.
>
> Signed-off-by: <[email protected]>
> ---
> Depends on earlier "Add deprecated_for_modules" patch.
>
> +/*
> + * Deprecated, use synchronize_rcu() or synchronize_sched() instead.
> + */
> +void synchronize_kernel(void)
> +{
> + synchronize_rcu();
> +}
> +

We should probably mark it deprecated -

void __deprecated synchronize_kernel(void)
{
synchronize_rcu();
}

Thanks
Dipankar

2005-04-03 14:14:40

by Michael Ellerman

[permalink] [raw]
Subject: Re: [RFC,PATCH 2/4] Deprecate synchronize_kernel, GPL replacement

Hi Paul,

I'm not quite clear on the difference between the two synchronize functions ,
the comment for synchronize_sched() seems to have a bit missing? (see below)

cheers

On Sun, 3 Apr 2005 16:21, Paul E. McKenney wrote:
> +/**
> + * synchronize_sched - block until all CPUs have exited any non-preemptive
> + * kernel code sequences.
> + *
> + * This means that all preempt_disable code sequences, including NMI and
> + * hardware-interrupt handlers, in progress on entry will have completed
> + * before this primitive returns. However, this does not guarantee that
> + * softirq handlers will have completed, since in some kernels

??

> + *
> + * This primitive provides the guarantees made by the (deprecated)
> + * synchronize_kernel() API. In contrast, synchronize_rcu() only
> + * guarantees that rcu_read_lock() sections will have completed.
> + */
> +#define synchronize_sched() synchronize_rcu()


Attachments:
(No filename) (913.00 B)
(No filename) (189.00 B)
Download all attachments

2005-04-03 18:50:20

by Paul E. McKenney

[permalink] [raw]
Subject: Re: [RFC,PATCH 2/4] Deprecate synchronize_kernel, GPL replacement

On Sun, Apr 03, 2005 at 02:26:50PM +0530, Dipankar Sarma wrote:
> On Sat, Apr 02, 2005 at 10:21:50PM -0800, Paul E. McKenney wrote:
> > The synchronize_kernel() primitive is used for quite a few different
> > purposes: waiting for RCU readers, waiting for NMIs, waiting for interrupts,
> > and so on. This makes RCU code harder to read, since synchronize_kernel()
> > might or might not have matching rcu_read_lock()s. This patch creates
> > a new synchronize_rcu() that is to be used for RCU readers and a new
> > synchronize_sched() that is used for the rest. These two new primitives
> > currently have the same implementation, but this is might well change
> > with additional real-time support. Both new primitives are GPL-only,
> > the old primitive is deprecated.
> >
> > Signed-off-by: <[email protected]>
> > ---
> > Depends on earlier "Add deprecated_for_modules" patch.
> >
> > +/*
> > + * Deprecated, use synchronize_rcu() or synchronize_sched() instead.
> > + */
> > +void synchronize_kernel(void)
> > +{
> > + synchronize_rcu();
> > +}
> > +
>
> We should probably mark it deprecated -
>
> void __deprecated synchronize_kernel(void)
> {
> synchronize_rcu();
> }

Hello, Dipankar,

That was the first thing I tried. ;-)

When I did that, I got a "deprecated" warning from gcc on the
EXPORT_SYMBOL() later in that same file. After futzing around a
bit, I hit on the compromise of marking the rcupdate.h declaration
of synchronize_kernel() as "__deprecated_for_modules".

That said, you are quite right that it would be better if gcc also gave
the "deprecated" warning for use of synchronize_kernel() within in-tree
non-module code. I suppose I could do something like the following
before the #includes in rcupdate.c:

#define SUPPRESS_DEPRECATION_OF_SYNCHRONIZE_KERNEL

and then something like this in rcupdate.h:

#ifdef SUPPRESS_DEPRECATION_OF_SYNCHRONIZE_KERNEL
extern void synchronize_kernel(void);
#else /* #ifdef SUPPRESS_DEPRECATION_OF_SYNCHRONIZE_KERNEL */
extern __deprecated void synchronize_kernel(void);
#endif /* #else #ifdef SUPPRESS_DEPRECATION_OF_SYNCHRONIZE_KERNEL */

but this seemed a bit ugly at the time. Maybe it is worthwhile.

I couldn't find any way to suppress the "deprecated" warning that is
generated by the "&sym" in the last line of the __EXPORT_SYMBOL()
macro. Anyone know a way of doing this? There doesn't seem to me
to be any point to giving the warning on the EXPORT_SYMBOL() -- and
it does clutter up compiler output with useless "deprecated" warnings.

Thanx, Paul

2005-04-03 18:54:55

by Paul E. McKenney

[permalink] [raw]
Subject: Re: [RFC,PATCH 2/4] Deprecate synchronize_kernel, GPL replacement

On Mon, Apr 04, 2005 at 12:11:56AM +1000, Michael Ellerman wrote:
> Hi Paul,
>
> I'm not quite clear on the difference between the two synchronize functions ,
> the comment for synchronize_sched() seems to have a bit missing? (see below)
>
> cheers
>
> On Sun, 3 Apr 2005 16:21, Paul E. McKenney wrote:
> > +/**
> > + * synchronize_sched - block until all CPUs have exited any non-preemptive
> > + * kernel code sequences.
> > + *
> > + * This means that all preempt_disable code sequences, including NMI and
> > + * hardware-interrupt handlers, in progress on entry will have completed
> > + * before this primitive returns. However, this does not guarantee that
> > + * softirq handlers will have completed, since in some kernels
>
> ??
>
> > + *
> > + * This primitive provides the guarantees made by the (deprecated)
> > + * synchronize_kernel() API. In contrast, synchronize_rcu() only
> > + * guarantees that rcu_read_lock() sections will have completed.
> > + */
> > +#define synchronize_sched() synchronize_rcu()

Good catch! How about the following?

+ * This means that all preempt_disable code sequences, including NMI and
+ * hardware-interrupt handlers, in progress on entry will have completed
+ * before this primitive returns. However, this does not guarantee softirq
+ * handlers have completed, since some configs run them in process context.

Updated patch below.

Thanx, Paul

Signed-off-by: <[email protected]>


diff -urpN -X dontdiff linux-2.6.12-rc1/include/linux/rcupdate.h linux-2.6.12-rc1-bettersk/include/linux/rcupdate.h
--- linux-2.6.12-rc1/include/linux/rcupdate.h Tue Mar 1 23:37:50 2005
+++ linux-2.6.12-rc1-bettersk/include/linux/rcupdate.h Sat Apr 2 13:06:15 2005
@@ -41,6 +41,7 @@
#include <linux/percpu.h>
#include <linux/cpumask.h>
#include <linux/seqlock.h>
+#include <linux/module.h>

/**
* struct rcu_head - callback structure for use with RCU
@@ -157,9 +158,9 @@ static inline int rcu_pending(int cpu)
/**
* rcu_read_lock - mark the beginning of an RCU read-side critical section.
*
- * When synchronize_kernel() is invoked on one CPU while other CPUs
+ * When synchronize_rcu() is invoked on one CPU while other CPUs
* are within RCU read-side critical sections, then the
- * synchronize_kernel() is guaranteed to block until after all the other
+ * synchronize_rcu() is guaranteed to block until after all the other
* CPUs exit their critical sections. Similarly, if call_rcu() is invoked
* on one CPU while other CPUs are within RCU read-side critical
* sections, invocation of the corresponding RCU callback is deferred
@@ -256,6 +257,21 @@ static inline int rcu_pending(int cpu)
(p) = (v); \
})

+/**
+ * synchronize_sched - block until all CPUs have exited any non-preemptive
+ * kernel code sequences.
+ *
+ * This means that all preempt_disable code sequences, including NMI and
+ * hardware-interrupt handlers, in progress on entry will have completed
+ * before this primitive returns. However, this does not guarantee softirq
+ * handlers have completed, since some configs run them in process context.
+ *
+ * This primitive provides the guarantees made by the (deprecated)
+ * synchronize_kernel() API. In contrast, synchronize_rcu() only
+ * guarantees that rcu_read_lock() sections will have completed.
+ */
+#define synchronize_sched() synchronize_rcu()
+
extern void rcu_init(void);
extern void rcu_check_callbacks(int cpu, int user);
extern void rcu_restart_cpu(int cpu);
@@ -265,7 +281,9 @@ extern void FASTCALL(call_rcu(struct rcu
void (*func)(struct rcu_head *head)));
extern void FASTCALL(call_rcu_bh(struct rcu_head *head,
void (*func)(struct rcu_head *head)));
-extern void synchronize_kernel(void);
+extern __deprecated_for_modules void synchronize_kernel(void);
+extern void synchronize_rcu(void);
+void synchronize_idle(void);

#endif /* __KERNEL__ */
#endif /* __LINUX_RCUPDATE_H */
diff -urpN -X dontdiff linux-2.6.12-rc1/kernel/rcupdate.c linux-2.6.12-rc1-bettersk/kernel/rcupdate.c
--- linux-2.6.12-rc1/kernel/rcupdate.c Tue Mar 1 23:37:30 2005
+++ linux-2.6.12-rc1-bettersk/kernel/rcupdate.c Sat Apr 2 13:10:09 2005
@@ -444,15 +444,18 @@ static void wakeme_after_rcu(struct rcu_
}

/**
- * synchronize_kernel - wait until a grace period has elapsed.
+ * synchronize_rcu - wait until a grace period has elapsed.
*
* Control will return to the caller some time after a full grace
* period has elapsed, in other words after all currently executing RCU
* read-side critical sections have completed. RCU read-side critical
* sections are delimited by rcu_read_lock() and rcu_read_unlock(),
* and may be nested.
+ *
+ * If your read-side code is not protected by rcu_read_lock(), do -not-
+ * use synchronize_rcu().
*/
-void synchronize_kernel(void)
+void synchronize_rcu(void)
{
struct rcu_synchronize rcu;

@@ -464,7 +467,16 @@ void synchronize_kernel(void)
wait_for_completion(&rcu.completion);
}

+/*
+ * Deprecated, use synchronize_rcu() or synchronize_sched() instead.
+ */
+void synchronize_kernel(void)
+{
+ synchronize_rcu();
+}
+
module_param(maxbatch, int, 0);
EXPORT_SYMBOL(call_rcu);
EXPORT_SYMBOL(call_rcu_bh);
+EXPORT_SYMBOL_GPL(synchronize_rcu);
EXPORT_SYMBOL(synchronize_kernel);

2005-04-03 22:31:19

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [RFC,PATCH 2/4] Deprecate synchronize_kernel, GPL replacement

On S?nndag 03 April 2005 20:50, Paul E. McKenney wrote:
> I couldn't find any way to suppress the "deprecated" warning that is
> generated by the "&sym" in the last line of the __EXPORT_SYMBOL()
> macro. ?Anyone know a way of doing this? ?There doesn't seem to me
> to be any point to giving the warning on the EXPORT_SYMBOL() -- and
> it does clutter up compiler output with useless "deprecated" warnings.

You can define an inline function that is marked __deprecated and calls
the exported function:

extern void __synchronize_kernel(void);
static inline __deprecated synchronize_kernel(void)
{
__synchronize_kernel();
}
===
void __synchronize_kernel(void)
{
synchronize_rcu();
}
EXPORT_SYMBOL(__synchronize_kernel);

You could even make __synchronize_kernel() static to let it only be used
by modules, but that might create some confusion about the interface.

Arnd <><

2005-04-04 21:30:01

by Paul E. McKenney

[permalink] [raw]
Subject: Re: [RFC,PATCH 2/4] Deprecate synchronize_kernel, GPL replacement

On Mon, Apr 04, 2005 at 12:26:50AM +0200, Arnd Bergmann wrote:
> On S?nndag 03 April 2005 20:50, Paul E. McKenney wrote:
> > I couldn't find any way to suppress the "deprecated" warning that is
> > generated by the "&sym" in the last line of the __EXPORT_SYMBOL()
> > macro. ?Anyone know a way of doing this? ?There doesn't seem to me
> > to be any point to giving the warning on the EXPORT_SYMBOL() -- and
> > it does clutter up compiler output with useless "deprecated" warnings.
>
> You can define an inline function that is marked __deprecated and calls
> the exported function:
>
> extern void __synchronize_kernel(void);
> static inline __deprecated synchronize_kernel(void)
> {
> __synchronize_kernel();
> }
> ===
> void __synchronize_kernel(void)
> {
> synchronize_rcu();
> }
> EXPORT_SYMBOL(__synchronize_kernel);

Cute! This would indeed deprecate synchronize_kernel() everywhere, while
at the same time avoiding a "deprecated" warning on any EXPORT_SYMBOL().
However, it would provide a new symbol, __synchronize_kernel(), that
could be called from any module, but with no "deprecated" warning.

> You could even make __synchronize_kernel() static to let it only be used
> by modules, but that might create some confusion about the interface.

-That- is an interesting trick!

Thinking about this more, it seems like getting the warning in modules
but not in mainline is OK. If uses creep into the mainline, we can always
just change them when they are noticed. In the worst case, it is still
easy to simply update them when we remove synchronize_kernel() next April,
right?

Thanx, Paul