Re: [RFC,PATCH] RCU: clean up a few remaining synchronize_kernel() calls

[Paul E. McKenney]

On Tue, Jun 28, 2005 at 08:22:24AM -0600, Zwane Mwaikambo wrote:
> Hello Paul,
> 
> On Sun, 26 Jun 2005, Paul E. McKenney wrote:
> 
> > How does the following look for NMI-RCU documentation?
> 
> That looks good, there is just one bit i'm not entirely sure about and i'd 
> appreciate it if you could entertain me for a bit;

I will see what entertainment I can provide.  ;-)

> Answer to Quick Quiz
> 
>         Why might the rcu_dereference() be necessary on Alpha, given
>         that the code reference by the pointer is read-only?
> 
>         Answer: The caller to set_nmi_callback() might well have
>                 initialized some data that is to be used by the
>                 new NMI handler.  In this case, the rcu_dereference()
>                 would be needed, because otherwise a CPU that received
>                 an NMI just after the new handler was set might see
>                 the pointer to the new NMI handler, but the old
>                 pre-initialized version of the handler's data.
> 
> Reading that i would think the general programming model for this would 
> be;
> 
> setup data
> write barrier
> setup callback

Agreed!  I have updated the document to make this requirement clear.

> Isn't that still required considering the following scenario;
> 
> CPU0			CPU1
> setup data		<NMI>
> setup callback		...
> ...			call callback
> 
> on i386, interrupts are data synchronising events, however if we happen to 
> take an interrupt right when the data is being setup we won't synchronise 
> with respect to that data. This could be achieved via the explicit write 
> barrier after data setup or rcu_dereference in the NMI handler. Or perhaps 
> i'm missing something?

On i386, writes are ordered.  So if CPU1 sees the callback, it is
guaranteed to also see the data.

However, you do have a good point -- weakly ordered CPUs would need to
have an explicit memory barrier.  This might well already be taken care
of by the memory barriers in the locking primitives used by the up()
operation invoked at the end of oprofile_start(), but I did not check
all the possible ways that these functions can be called.

Given that set_nmi_callback isn't invoked all that often, seems like
it might be preferable to insert an smp_wmb() at the beginning of
set_nmi_callback(), so that it reads as follows:

	void set_nmi_callback(nmi_callback_t callback)
	{
		smp_wmb();
		nmi_callback = callback;
	}

Thoughts?

						Thanx, Paul
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/