lkml.org 
[lkml]   [2007]   [Mar]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH 2/2] Replace pid_t in autofs with struct pid reference
Date
"Serge E. Hallyn" <serue@us.ibm.com> writes:

> True, current->pid can probably always be legitimately taken as the pid
> number in the current task's cloning namespace. But task->pid is
> wrong.

Agreed.

> So if as you say it's worth caching (not saying I doubt you, just that I
> haven't verified), then ideally we could cache current->pid but only
> access it using current_pid(). Does that seem worth doing?

Doing current_pid() and friends certainly seem to be worth doing.
Mostly for the secondary effect that we can then breaking any code we
miss in our conversion. I don't know if the caching is worth it but
it is so easy to implement I don't see a reason to avoid it.

> In any case, certainly adding a task_pid_nr() helper which for starters
> returns pid_nr(task_pid(task)) seems reasonable. Note that Suka's about
> ready to send a new iteration of the pidns patchset, so I'd like this to
> be considered something to clean up on top of that patchset.

I will keep it in mind. As long a we keep patches that actually change the
code separate from patches that change the set of helper functions I think
we will be ok. Hopefully Suka has learned enough by now that reviewing
his patches will be easier than just writing them myself. There were
so many silly little issues to clean up in the last round of review
by the time the patch was clean enough to look at the meat I missed the
deep issues :( At least things were clear enough that Oleg managed to
catch some of the deeper issues.

Eric
-
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: 2007-03-19 22:59    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site