On Thu, Jul 24, 2008 at 02:13:42PM -0700, Linus Torvalds wrote:

On Thu, 24 Jul 2008, Patrick McHardy wrote:

quoted

To fix this I think we need a __krealloc() that doesn't
free the old memory, especially since it must not be
freed immediately because it may still be used in a RCU
read side (see the last part in the patch attached to
this mail (based on a kernel without your patch)).

Hmm. Don't you need to fix some of the ordering of the initialization too?

If there are possible readers that happen in parallel with changing this 
thing, don't you need to protect the update of "ext->len" against the 
actual changes? And the readers should probably have a read barrier 
between checking "len" and actually looking at the values? Finally, why do 
the "ct->ext" dereference thing, when we know it has to be equal to "new"?

ie something like this on the writing side (in _addition_ to both the 
patches already seen), but I didn't do the reading side (ie there are no 
"smp_rmb()"'s on the reading side)

And no, I don't know the code, so I don't know who/what can read those 
things with RCU, so maybe there is some reason why the actual data doesn't 
need protecting. But I somehow doubt it.

There certainly needs to be an rcu_assign_pointer() or a smp_wmb()
in there somewhere -- whenever it is that the new structure becomes
accessible to RCU readers.  I looked around a bit, but couldn't tell
when this stuff becomes accessible to readers...

							Thanx, Paul