On 12/19/2012 01:13 AM, Oleg Nesterov wrote:

On 12/18, Srivatsa S. Bhat wrote:

quoted

So now that we can't avoid disabling and enabling interrupts,

Still I think it would be better to not use local_irq_save/restore
directly.

Sure, we can use this_cpu_add() itself. I explicitly used
local_irq_save/restore here just to explain my question.

And,

quoted

I was
wondering if we could exploit this to avoid the smp_mb()..

Maybe this is a stupid question, but I'll shoot it anyway...
Does local_irq_disable()/enable provide any ordering guarantees by any chance?

Oh, I do not know.

But please look at the comment above prepare_to_wait(). It assumes
that even spin_unlock_irqrestore() is not the full barrier.

Semi-permeable barrier.. Hmm.. 

In any case. get_online_cpus_atomic() has to use irq_restore, not
irq_enable. And _restore does nothing "special" if irqs were already
disabled, so I think we can't rely on sti.

Right, I forgot about the _restore part.

quoted

I tried thinking about other ways to avoid that smp_mb() in the reader,

Just in case, I think there is no way to avoid mb() in _get (although
perhaps it can be "implicit").

Actually, I was trying to avoid mb() in the _fastpath_, when there is no
active writer. I missed stating that clearly, sorry.

The writer changes cpu_online_mask and drops the lock. We need to ensure
that the reader sees the change in cpu_online_mask after _get returns.

The write_unlock() will ensure the completion of the update to cpu_online_mask,
using the same semi-permeable logic that you pointed above. So readers will
see the update as soon as the writer releases the lock, right?

quoted

but was unsuccessful. So if the above assumption is wrong, I guess we'll
just have to go with the version that uses synchronize_sched() at the
writer-side.

In this case we can also convert get_online_cpus() to use percpu_rwsem
and avoid mutex_lock(&cpu_hotplug.lock), but this is minor I guess.
I do not think get_online_cpus() is called too often.

Yes, we could do that as well. I remember you saying that you had some
patches for percpu_rwsem to help use it in cpu hotplug code (to make it
recursive, IIRC).

So, I guess we'll go with the synchronize_sched() approach for percpu rwlocks
then. Tejun, it is still worthwhile to expose this as a generic percpu rwlock
and then use it inside cpu hotplug code, right?


Regards,
Srivatsa S. Bhat