lkml.org 
[lkml]   [2006]   [Aug]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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/

\
 
 \ /
  Last update: 2006-08-30 00:29    [W:0.038 / U:1.396 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site