On Mon, 2009-08-03 at 19:16 +0200, Oleg Nesterov wrote:
> > + filp->f_owner.task_only =
> > + (type == PIDTYPE_PID && (flags & FF_SETOWN_TID));
>
> Do we need type == PIDTYPE_PID check? FF_SETOWN_TID must imply
> PIDTYPE_PID, it is only used by f_setown_tid().
Paranoia I guess.
> > +static int f_setown_tid(struct file *filp, unsigned long arg)
> > +{
> > + int flags = FF_SETOWN_FORCE;
> > + struct pid *pid;
> > + int who = arg;
> > + int ret = 0;
> > +
> > + if (who < 0)
> > + who = -who;
> > + else
> > + flags |= FF_SETOWN_TID;
>
> Hmm, OK. so fcntl(F_SETOWN_TID, -666) <=> fcntl(F_SETOWN, +666).
>
> Not that I disagree, but I think this should be discussed. Perhaps
> F_SETOWN_TID can just reject who < 0.
Yeah, I considered that. Opinions?
> > +static pid_t f_getown_tid(struct file *filp)
> > +{
> > + pid_t tid;
> > +
> > + read_lock(&filp->f_owner.lock);
> > + tid = pid_vnr(filp->f_owner.pid);
> > + if (filp->f_owner.pid_type == PIDTYPE_PGID)
> > + tid = 0;
> > + if (!filp->f_owner.task_only)
> > + tid = -tid;
>
> I didn't think about this before... What should F_GETOWN_TID return
> if we did F_GETOWN ? (and vice versa). f_getown_tid() returns < 0
> if !task_only and ->piD != 0, this helps.
>
> but the caller of F_GETOWN can't know what the returned value actually
> means if F_GETOWN_TID was used.
Ah, I made GETOWN_TID deal with !PID but forgot the TID case in GETOWN.
Yeah, icky, esp since there is no room for errors in the return value :/
I guess I could make it return 0.
> Do we really need fown->task_only? Not only this enlarges fown_struct,
> we have to modify f_modown() and f_setown().
>
> Perhaps we can just add
>
> #define F_PIDTYPE_THREAD PIDTYPE_MAX
>
> into fcntl.c ? Then,
>
> static int f_setown_xxx(struct file *filp, unsigned long arg, int
> force, bool group)
> {
> enum pid_type type;
> struct pid *pid;
> int who = arg;
> int result;
>
> type = PIDTYPE_PID;
> if (!group)
> type = F_PIDTYPE_THREAD
> else if (who < 0) {
> type = PIDTYPE_PGID;
> who = -who;
> }
>
> rcu_read_lock();
> pid = find_vpid(who);
> result = __f_setown(filp, pid, type, force);
> rcu_read_unlock();
> return result;
> }
>
> int f_setown(struct file *filp, unsigned long arg, int force)
> {
> return f_setown_xxx(..., true);
> }
>
> Now we should also change send_sigio/send_sigurg, but this is trivial
>
> type = fown->pid_type;
> + if (type == F_PIDTYPE_THREAD)
> type = PIDTYPE_PID;
>
> send_sigio_to_task() is trivial too.
>
> What do you think? I agree, this is a bit hackish, but otoh this lessens
> the changes outside of fcntl.h.
Right, I considered adding PIDTYPE_TID, but then I'd have to go through
the kernel and make everything consistent, which is where I gave up ;-)
You hack above makes sense, dunno if people will go for it though..
------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus on
what you do best, core application coding. Discover what's new with
Crystal Reports now. http://p.sf.net/sfu/bobj-july
_______________________________________________
perfmon2-devel mailing list
perfmon2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/perfmon2-devel
