Messages in this thread | | | Date | Tue, 29 Aug 2006 15:15:18 -0700 | From | Stephane Eranian <> | Subject | Re: [PATCH 4/18] 2.6.17.9 perfmon2 patch for review: new system calls support |
| |
On Tue, Aug 29, 2006 at 10:26:27AM -0700, Andrew Morton wrote: > On Tue, 29 Aug 2006 09:59:57 -0700 > Stephane Eranian <eranian@hpl.hp.com> wrote: > > > > > + > > > > + PFM_DBG("going wait_inactive for [%d] state=%ld flags=0x%lx", > > > > + task->pid, > > > > + task->state, > > > > + local_flags); > > > > + > > > > + spin_unlock_irqrestore(&ctx->lock, local_flags); > > > > + > > > > + wait_task_inactive(task); > > > > + > > > > + spin_lock_irqsave(&ctx->lock, new_flags); > > > > > > This sort of thing.. > > > > We need to wait until the task is effectively off the CPU, i.e., with its > > machine state (incl PMU) saved. When we come back we re-run the series of tests. > > This applies only to per-thread, therefore it is not affected by smp_processor_id(). > > > > Generally, if a reviewer asks a question and the developer answers that > question, this is a sign that a code comment is needed. Because others are > likely to ask themselves the same question ;)
Understood. I have added a comment about what is going there.
Now, I don't particularly like what we have to do here. You may have a better solution to my problem: - how inside the kernel, can I make sure a thread is stopped and off any CPU?
- if the thread is not stopped, how do I go about forcing it to stop and how can I get notified when it is eventually off any CPU?
- how do I make sure it cannot wake up until I am done with it?
> > > > > > When copying a struct from kernel to userspace we must beware of > > > compiler-inserted padding. Because that can cause the kernel to leak > > > a few bytes of uninitialised kernel memory. > > > > We are copying out exactly the same amount of data that was passed in. > > > > Are you suggesting that copy_from/copy_to may copy more data? > > No, that's OK. I was pointing out the problem in situations like this: > > struct foo { > u32 a; > u64 b; > }; > > { > struct foo f; > > f.a = 0; > f.b = 0; > copy_to_user(p, &f, sizeof(f)); > } > > which exposes kernel memory. As you appear to be confident that the > perfmon code can't do this, all is OK.
Ok, now I understand. I don't think I have that but I will write a test program and run on all architectures to ensure this is indeed the case.
>
--
-Stephane - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |