Re: [RFC][PATCH 0/5] Introduce checks for preemptable code forthis_cpu_read/write()

[Christoph Lameter]

On Tue, 20 Sep 2011, Steven Rostedt wrote:

> BTW, Can you explain to me where the this_cpu_*() ops were designed to
> be used? The only places where "this_cpu_*()" is used in slub.c and
> page_alloc.c have irqs disabled on their use. I thought this was for
> slub and page_alloc?

Initially these were used for statistics that used per cpu counters. The
slub thing was an outgrow of this.

> Is this_cpu() made just for statistics? I see it used in the inode code
> for that, and some accounting in the namespace.c code.

That is the main use case yes.

> Note and there's places all over the kernel that uses this_cpu_read()
> and thinks preemption should be disabled. Just look at
> arch/x86/mm/tlb.c:
>
> 	/* Caller has disabled preemption */
> 	sender = this_cpu_read(tlb_vector_offset);
>
> Why the comment?
>
> My argument is that this_cpu_* is just confusing. Rename your use case
> and keep this_cpu_*() as what you want __this_cpu_*() to be.

Thought about this a bit last night. I think the main issue are these
this_cpu_read() and this_cpu_write() operations since people use those
irresponsibly. It usually does not make sense to read a value from a
random cpu nor does writing make sense. The situation is different for
per cpu counter increments where it does not matter which cpus counter is
incremented since we sum them up later anyways.

How about getting rid of this_cpu_read() and this_cpu_write() entirely?

Only allow __this_cpu_read and __this_cpu_write. There we check that the
caller has disabled preemption.

For the rare special cases (are there any?) that are legitimate use cases
for this_cpu_read/write we can use manual determination of per cpu
pointers and then just do a load via the pointer?

Or alternatively give this_cpu_read and write special names that make
their dangerousness clear.

In the case of slub there are only some this_cpu_write() things that can
be __this_cpu_write without a problem.

The __this_cpu_ptr() can become this_cpu_ptr as far as I can tell. This
should make it consistent so that we can check for disabled preemption for
all __this_cpu thingies.
--
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/