On 12/12/2012 11:32 PM, Oleg Nesterov wrote:

On 12/12, Srivatsa S. Bhat wrote:

quoted

On 12/12/2012 10:47 PM, Oleg Nesterov wrote:

quoted

Why it needs to be per-cpu? It can be global and __read_mostly to avoid
the false-sharing. OK, perhaps to put reader_percpu_refcnt/writer_signal
into a single cacheline...

Even I realized this (that we could use a global) after posting out the
series.. But do you think that it would be better to retain the per-cpu
variant itself, due to the cache effects?

I don't really know, up to you. This was the question ;)

OK :-)

quoted

quoted

Do we really need local_irq_save/restore in put_ ?

Hmm.. good point! I don't think we need it.

And _perhaps_ get_ can avoid it too?

I didn't really try to think, probably this is not right, but can't
something like this work?

	#define XXXX	(1 << 16)
	#define MASK	(XXXX -1)

	void get_online_cpus_atomic(void)
	{
		preempt_disable();

		// only for writer
		__this_cpu_add(reader_percpu_refcnt, XXXX);

		if (__this_cpu_read(reader_percpu_refcnt) & MASK) {
			__this_cpu_inc(reader_percpu_refcnt);
		} else {
			smp_wmb();
			if (writer_active()) {
				...
			}
		}

		__this_cpu_dec(reader_percpu_refcnt, XXXX);
	}

Sorry, may be I'm too blind to see, but I didn't understand the logic
of how the mask helps us avoid disabling interrupts.. Can you kindly
elaborate?

	void put_online_cpus_atomic(void)
	{
		if (__this_cpu_read(reader_percpu_refcnt) & MASK)
			__this_cpu_dec(reader_percpu_refcnt);
		else
			read_unlock(&hotplug_rwlock);
		preempt_enable();
	}

Regards,
Srivatsa S. Bhat