Re: PATCH: EDAC - clean up atomic stuff

[Andrew Morton]

ebiederm@xxxxxxxxxxxx (Eric W. Biederman) wrote:
>
> Ok.  If I recall correctly atomic kmaps have the rule that they are
>  per cpu, and you can't be interrupted when the map is taken, by
>  something else that will use the map.  But they are safe to use from
>  interrupt context.  As I recall the code needs to ensure that an
>  interrupt handler doesn't use the same buffer and that it isn't
>  preempted where another thread will use the buffer.  The preemption
>  angle is new since that piece of code was written.

Yes, a particular atomic kmap slot is simply a static, per-cpu scalar. 
It's just like

int foo[NR_CPUS];

	...
	foo(smp_processor_id());

and all the same rules apply.

The use of KM_BOUNCE_READ does appear to be incorrect.  bounce_copy_vec()
will use KM_BOUNCE_READ from interrupt context, so if the EDAC code is
interrupted by the block layer while it holds that kmap, it will find that
it's suddenly diddling with a different physical page.

So to use KM_BOUNCE_READ, the EDAC code nees to disable local interrupts,
or to use a different (or new) slot.

In what contexts is edac_mc_scrub_block() called?  If process context, then
KM_USER0 would suit.

Ah, edac_mc_scrub_block() is passing the pageframe address to
kunmap_atomic() - that's a common bug.  It needs to pass in the virtual
address which kmap_atomic() returned.

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