On Fri, Oct 04, 2013 at 03:27:15PM +0000, Christoph Lameter wrote: > On Fri, 4 Oct 2013, Peter Zijlstra wrote: > > > > -# define __this_cpu_read(pcp) > > > __pcpu_size_call_return(__this_cpu_read_, (pcp)) > > > +# define __this_cpu_read(pcp) \ > > > + (__this_cpu_preempt_check(),__pcpu_size_call_return(__this_cpu_read_, > > > (pcp))) > > > #endif > > > > Would it not be move convenient to implement it in terms of the > > raw_this_cpu*() thingies? That way you're sure they actually do the same > > thing and there's only 1 site to change when changing the > > implementation. > > The __this_cpu_read_xxx() are asm primitives provided by various arches. > __this_cpu_read() is currently not overriden by any arches. That is why > the approach here of replicating only the higher level for raw_cpu_ops > works. Renaming the __this_cpu_xxx primitives would be a significant > change.
This isn't about read; this is about all of them. Make sure the raw_* implementation is the actual real implementation; then implement the checking variant in terms of those. > > > if (!printk_ratelimit()) > > > goto out_enable; > > > > > > - printk(KERN_ERR "BUG: using smp_processor_id() in preemptible [%08x] " > > > - "code: %s/%d\n", > > > + printk(KERN_ERR "%s in preemptible [%08x] " > > > + "code: %s/%d\n", what, > > > preempt_count() - 1, current->comm, current->pid); > > > > I would argue for keeping the "BUG" string intact and in front of the > > %s. > > Most of the place that I have seen are not bugs but there was a > reason for the code to run a __this_cpu op without preemption disabled. No; it is an actual BUG; it means that whoemever wrote the code didn't think straight and forgot to use the right primitive and comments. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
