Américo Wang writes: > On Tue, Jan 26, 2010 at 3:45 PM, KOSAKI Motohiro > wrote: >> Hi >> >>> On Tue, Jan 26, 2010 at 02:01:12PM +0800, Am??rico Wang wrote: >>> >>> > I agree, it seems that patch is useless, since we already >>> > do lock_kernel() before calling __f_setown()... >>> >>> What's to prevent pid from being freed under us?  BKL won't... >> >> I don't understand this issue at all. so, this is stupid dumb question. >> Why can't we write following code? >> >> >>                enum pid_type type; >>                struct pid *pid; >>                if (!waitqueue_active(&tty->read_wait)) >>                        tty->minimum_to_wake = 1; >>                spin_lock_irqsave(&tty->ctrl_lock, flags); >>                if (tty->pgrp) { >>                        pid = tty->pgrp; >>                        type = PIDTYPE_PGID; >>                } else { >>                        pid = task_pid(current); >>                        type = PIDTYPE_PID; >>                } >>                get_pid(pid)                                    // insert here >>                spin_unlock_irqrestore(&tty->ctrl_lock, flags); >>                retval = __f_setown(filp, pid, type, 0); >>                put_pid(pid)                                    // insert here >> > > Yeah, this seems reasonable for me, but not sure if this is the best fix. That or tweak __f_setown to use irqsave/irqrestore variants for it's locks, __f_setown is already atomic. I prefer that direction because the code is just a little simpler. Eric {.n++%ݶw{.n+{G{ayʇڙ,jfhz_(階ݢj"mG?&~iOzv^m ?I