Re: [pchecks v2 2/2] percpu: Add preemption checks to __this_cpuops

[Christoph Lameter]

On Fri, 4 Oct 2013, Peter Zijlstra wrote:

> > 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.

Yes this is valid for all of them. The __this_cpu op is the low level
implementation.

> > > >  	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.

??? The assumption needs to be that they used the function as it was
documented. No checks because they knew they were not needed in a
particular context.



--
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/