2006-09-10 16:18:06

by Oleg Nesterov

Subject: [PATCH] force_quiescent_state: factor out duplicated code

On top of rcu-simplify-improve-batch-tuning.patch

Cleanup. Move '#ifdef CONFIG_SMP' check and rdp->bhlimit setting
into force_quiescent_state().

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

--- 18-rc6/kernel/rcupdate.c~2_fold 2006-09-10 16:56:10.000000000 +0400
+++ 18-rc6/kernel/rcupdate.c 2006-09-10 20:00:31.000000000 +0400
@@ -76,14 +76,17 @@ static atomic_t rcu_barrier_cpu_count;
static DEFINE_MUTEX(rcu_barrier_mutex);
static struct completion rcu_barrier_completion;

-#ifdef CONFIG_SMP
static void force_quiescent_state(struct rcu_data *rdp,
- struct rcu_ctrlblk *rcp)
+ struct rcu_ctrlblk *rcp)
{
- int cpu;
- cpumask_t cpumask;
+ rdp->blimit = INT_MAX;
set_need_resched();
+
+#ifdef CONFIG_SMP
if (unlikely(!rcp->signaled)) {
+ cpumask_t cpumask;
+ int cpu;
+
rcp->signaled = 1;
/*
* Don't send IPI to itself. With irqs disabled,
@@ -94,14 +97,8 @@ static void force_quiescent_state(struct
for_each_cpu_mask(cpu, cpumask)
smp_send_reschedule(cpu);
}
-}
-#else
-static inline void force_quiescent_state(struct rcu_data *rdp,
- struct rcu_ctrlblk *rcp)
-{
- set_need_resched();
-}
#endif
+}

/**
* call_rcu - Queue an RCU callback for invocation after a grace period.
@@ -126,10 +123,9 @@ void fastcall call_rcu(struct rcu_head *
rdp = &__get_cpu_var(rcu_data);
*rdp->nxttail = head;
rdp->nxttail = &head->next;
- if (unlikely(++rdp->qlen > qhimark)) {
- rdp->blimit = INT_MAX;
+
+ if (unlikely(++rdp->qlen > qhimark))
force_quiescent_state(rdp, &rcu_ctrlblk);
- }
local_irq_restore(flags);
}

@@ -162,11 +158,8 @@ void fastcall call_rcu_bh(struct rcu_hea
*rdp->nxttail = head;
rdp->nxttail = &head->next;

- if (unlikely(++rdp->qlen > qhimark)) {
- rdp->blimit = INT_MAX;
+ if (unlikely(++rdp->qlen > qhimark))
force_quiescent_state(rdp, &rcu_bh_ctrlblk);
- }
-
local_irq_restore(flags);
}

2006-09-10 21:03:55

by Dipankar Sarma

Subject: Re: [PATCH] force_quiescent_state: factor out duplicated code

On Sun, Sep 10, 2006 at 08:18:10PM +0400, Oleg Nesterov wrote:
> On top of rcu-simplify-improve-batch-tuning.patch
>
> Cleanup. Move '#ifdef CONFIG_SMP' check and rdp->bhlimit setting
> into force_quiescent_state().


Pushing the rdp->blimit setting into force_quiescent_state is fine,
but I see no need to insert #ifdef CONFIG_SMP into a routine.
Let us continue to have separate versions of force_quiescent_state()
for CONFIG_SMP and !CONFIG_SMP. It looks cleaner with #ifdefs inserted
in between.

Thanks
Dipankar

2006-09-10 21:13:55

by Oleg Nesterov

Subject: Re: [PATCH] force_quiescent_state: factor out duplicated code

On 09/11, Dipankar Sarma wrote:
>
> On Sun, Sep 10, 2006 at 08:18:10PM +0400, Oleg Nesterov wrote:
> > On top of rcu-simplify-improve-batch-tuning.patch
> >
> > Cleanup. Move '#ifdef CONFIG_SMP' check and rdp->bhlimit setting
> > into force_quiescent_state().
>
>
> Pushing the rdp->blimit setting into force_quiescent_state is fine,
> but I see no need to insert #ifdef CONFIG_SMP into a routine.

Otherwise we need to push the rdp->blimit setting into both versions
of force_quiescent_state(), for CONFIG_SMP and !CONFIG_SMP.

> Let us continue to have separate versions of force_quiescent_state()
> for CONFIG_SMP and !CONFIG_SMP. It looks cleaner with #ifdefs inserted
> in between.

I personally think exactly opposite :)

Ok, let's forget about this patch, it was only cleanup.

Oleg.