Ian Kent <[EMAIL PROTECTED]> writes: > On Fri, 2007-03-16 at 15:44 +0100, Cedric Le Goater wrote: >> > How about you send over the autofs4 bit and I'll have a look (the autofs >> > patch looked fine). That would save me a bit of time and if there are >> > any changes needed I can send an updated patch for you guys to review. I >> > don't think autofs4 uses pids differently, in principle, than autofs so >> > it "should" be straight forward. >> >> Here's the latest. > > That looks OK to me, assuming the "find_get_pid" and friends do what > they suggest, but I'll give it a closer look tomorrow. > > A ref count is used here, what affect does that have on a thread (or > process) that may go away (or be summarily killed) without umounting the > mount?
Nothing. The primary advantage is that you are pid wrap around safe as the struct pid will never point to another process after one of those events occurs. struct pid is a very small structure so not freeing it when the process it originally referred to goes away is no big deal. Although not leaking when you stop using it is still important. The other big use of struct pid is that to get the user space pid value you call pid_nr(). Depending on the pid namespace of the caller the return value of pid_nr() can be different. So when you store a pid or pass a pid between processes that should be done by passing a struct pid because those processes could be in different pid namespaces. >> Index: 2.6.20/fs/autofs4/waitq.c >> =================================================================== >> --- 2.6.20.orig/fs/autofs4/waitq.c >> +++ 2.6.20/fs/autofs4/waitq.c >> @@ -292,8 +292,8 @@ int autofs4_wait(struct autofs_sb_info * >> wq->ino = autofs4_get_ino(sbi); >> wq->uid = current->uid; >> wq->gid = current->gid; >> - wq->pid = current->pid; >> - wq->tgid = current->tgid; >> + wq->pid = pid_nr(task_pid(current)); >> + wq->tgid = pid_nr(task_tgid(current)); >> wq->status = -EINTR; /* Status return if interrupted */ >> atomic_set(&wq->wait_ctr, 2); >> mutex_unlock(&sbi->wq_mutex); I have a concern with this bit as I my quick review said the wait queue persists, and if so we should be cache the struct pid pointer, not the pid_t value. Heck the whol pid_nr(task_xxx(current)) idiom I find very suspicious. Eric - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
