lkml.org 
[lkml]   [2008]   [Feb]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [RFC] mmiotrace full patch, preview 1
From
Date
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


\
 
 \ /
  Last update: 2008-02-26 18:23    [W:0.083 / U:0.640 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site