Re: Proposed task.stp additions

[Mike Mason <mmlnx at us dot ibm dot com>]

Stone, Joshua I wrote:
> Mike Mason wrote:
> > A while back we discussed submitting changes to the list for review
> and
> > ACKS.  I realize this patch is relatively minor, but thought I'd get
> the
> > ball rolling.
>
> Internally we've been playing with Review Board for stuff like this,
> which works nicely.
> http://reviewboard.chipx86.com/
>
> > I'd like to add these functions to task.stp.  I've found them useful
> in
> > scripts I've written.
>
> task_cputime() and is_kthread() look good, but I have similar concerns
> as Frank with the other two.
>
> It's unsafe to use d_path() because it takes locks.  Since we can call
> from arbitrary contexts, this has the possibility to create a deadlock.
> It also dereferences the pointers you give it, so you have to be 100%
> sure that those pointers are valid.

I understand that now and should have earlier.  What are my options?  If I implement the d_path() functionality without locks and protected by kreads, is that sufficient?  Also, if I limit cwd() to the current task (put in context.stp) does that help?  I expect cwd() to be needed in the context of current most of the time anyway.  I've used it to construct a full pathname in a sys_open probe.

> Using pid2task() also has the possibility of the task disappearing from
> under you later.  The proper method is to use (get|put)_task_struct(),
> but put_task_struct() can't be used by modules.  Even if it could, we
> still don't have a way to *guarantee* that the put() is called
> eventually.
>
> BUT, as long as *everything* you do with the result of pid2task() is
> protected by kread(), this is probably OK.  In the rare case that the
> task disappears, you may get corrupt data or an error from an invalid
> dereference, but this won't impact system stability.

That's true of all the task_* functions, isn't it?  If we cache task_struct pointers and pass them to the task_* functions, there's never a guarantee you'll get what you expect.

Mike

> Josh