Messages in this thread | | | Date | Mon, 19 Jan 2009 13:35:53 -0600 | From | "Serge E. Hallyn" <> | Subject | Re: [PATCH] autofs: fix the wrong usage of the deprecated task_pgrp_nr() |
| |
Quoting Oleg Nesterov (oleg@redhat.com): > On 01/19, Serge E. Hallyn wrote: > > > > Quoting Oleg Nesterov (oleg@redhat.com): > > > On 01/19, Serge E. Hallyn wrote: > > > > > > > > But so there does still need to be a patch modifying parse_options() > > > > to return an error if pgrp= was not specified, right? > > > > > > Why? In that case we should use the caller's pgrp. This is what the > > > current tries to do, why should the patch change this behaviour? > > > > Well, because Ian said that not specifying it is supposed to > > be an error :) I didn't quite understand why, so am fishing > > for more info... > > I think you misunderstood him. Or I am totally confused ;) > > In any case. Both autofs and autofs4 use current's pgrp if this > option was not specified, and these patches doesn't change this > behaviour. > > > Actually, I am very much surprized this one-liner patch has so > many questions. Isn't it "obiously correct" ?
I'm not sure which one-liner you're talking about. In fact, the patch I'm looking at right now isn't the one i looked at before my last response. Dangit.
The patch turning the cached pid_t into a struct pid is certainly mostly right. It shouldn't store a pid_t.
But as for passing pid_t's in from userspace and especially printing them out in error messages, I believe what Ian was trying to do before, which seemed sensible, was to always use values in the init_pid_ns. After all, if you do a DPRINTK with pid_vnr(somepid), then by the time a human reads the logs the subjective pidns might no longer exist. So for logs I'd tend to agree with printing out the pid_t in the init_pid_ns.
-serge
| |