Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

[2.6.32-rc5-git5] synchronize_sched() inside spin_lock()?

4 views
Skip to first unread message

Tetsuo Handa

unread,
Nov 2, 2009, 7:10:01 AM11/2/09
to
Commit: 4ea7e38696c7e798c47ebbecadfd392f23f814f9

tracepoint_synchronize_unregister() calls synchronize_sched(), but it is
between spin_lock() and spin_unlock(). Is it OK?

static int set_all_monitor_traces(int state)
{
int rc = 0;
struct dm_hw_stat_delta *new_stat = NULL;
struct dm_hw_stat_delta *temp;

spin_lock(&trace_state_lock);

switch (state) {
case TRACE_ON:
rc |= register_trace_kfree_skb(trace_kfree_skb_hit);
rc |= register_trace_napi_poll(trace_napi_poll_hit);
break;
case TRACE_OFF:
rc |= unregister_trace_kfree_skb(trace_kfree_skb_hit);
rc |= unregister_trace_napi_poll(trace_napi_poll_hit);

tracepoint_synchronize_unregister();

/*
* Clean the device list
*/
list_for_each_entry_safe(new_stat, temp, &hw_stats_list, list) {
if (new_stat->dev == NULL) {
list_del_rcu(&new_stat->list);
call_rcu(&new_stat->rcu, free_dm_hw_stat);
}
}
break;
default:
rc = 1;
break;
}

if (!rc)
trace_state = state;

spin_unlock(&trace_state_lock);

if (rc)
return -EINPROGRESS;
return rc;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Paul E. McKenney

unread,
Nov 2, 2009, 5:40:02 PM11/2/09
to
On Mon, Nov 02, 2009 at 09:00:06PM +0900, Tetsuo Handa wrote:
> Commit: 4ea7e38696c7e798c47ebbecadfd392f23f814f9
>
> tracepoint_synchronize_unregister() calls synchronize_sched(), but it is
> between spin_lock() and spin_unlock(). Is it OK?

Calling synchronize_sched() while holding a spinlock would indeed be
very bad, but the code below seems to instead be invoking call_rcu(),
which is no problem.

Or am I missing something here?

Thanx, Paul

Dmitry Torokhov

unread,
Nov 2, 2009, 6:40:02 PM11/2/09
to
On Monday 02 November 2009 02:30:28 pm Paul E. McKenney wrote:
> On Mon, Nov 02, 2009 at 09:00:06PM +0900, Tetsuo Handa wrote:
> > Commit: 4ea7e38696c7e798c47ebbecadfd392f23f814f9
> >
> > tracepoint_synchronize_unregister() calls synchronize_sched(), but it is
> > between spin_lock() and spin_unlock(). Is it OK?
>
> Calling synchronize_sched() while holding a spinlock would indeed be
> very bad, but the code below seems to instead be invoking call_rcu(),
> which is no problem.
>
> Or am I missing something here?
>
> Thanx, Paul
>
> > static int set_all_monitor_traces(int state)
> > {
> > int rc = 0;
> > struct dm_hw_stat_delta *new_stat = NULL;
> > struct dm_hw_stat_delta *temp;
> >
> > spin_lock(&trace_state_lock);
> >
> > switch (state) {
> > case TRACE_ON:
> > rc |= register_trace_kfree_skb(trace_kfree_skb_hit);
> > rc |= register_trace_napi_poll(trace_napi_poll_hit);
> > break;
> > case TRACE_OFF:
> > rc |= unregister_trace_kfree_skb(trace_kfree_skb_hit);
> > rc |= unregister_trace_napi_poll(trace_napi_poll_hit);
> >
> > tracepoint_synchronize_unregister();

This has synchronize_sched() inside.


--
Dmitry

Paul E. McKenney

unread,
Nov 2, 2009, 8:00:01 PM11/2/09
to

Ah! That would indeed be bad.

Thanx, Paul

Tetsuo Handa

unread,
Nov 22, 2009, 8:50:01 AM11/22/09
to
Commit: 4ea7e38696c7e798c47ebbecadfd392f23f814f9

set_all_monitor_traces() calls tracepoint_synchronize_unregister()
with a spinlock held, but tracepoint_synchronize_unregister() may sleep.

0 new messages