Hey, Pekka,
A couple of little things I noticed...
> +static int post_kmmio_handler(unsigned long condition, struct pt_regs *regs)
> +{
> + int ret = 0;
> + struct kmmio_probe *probe;
> + struct kmmio_fault_page *faultpage;
> + struct kmmio_context *ctx = &get_cpu_var(kmmio_ctx);
> +
> + if (!ctx->active)
> + goto out;
Should that text read something like:
if (condition != DIE_TRAP || !ctx->active)
Presumably you won't be active if something else is going wrong, but one
never knows.
> +int register_kmmio_probe(struct kmmio_probe *p)
> +{
> + int ret = 0;
> + unsigned long size = 0;
> +
> + spin_lock_irq(&kmmio_lock);
> + kmmio_count++;
> + if (get_kmmio_probe(p->addr)) {
> + ret = -EEXIST;
> + goto out;
> + }
That only checks the first page; if the probed region partially overlaps
another one found later in memory, the registration will succeed.
Maybe you want to decrement kmmio_count if you decide to return -EEXIST
(or hold off on the increment until after the test)?
In general, I worry about what happens if an interrupt handler generates
traced MMIO traffic while a fault handler is active. It looks a lot
like the "all hell breaks loose" scenario mentioned in the comments. Is
there some way of preventing that which I missed? Otherwise, maybe,
should the ioremap() wrappers take an additional argument, being an IRQ
to disable while trace handlers are active?
jon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
