lkml.org 
[lkml]   [2009]   [Aug]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 3/2] fcntl: F_[SG]ETOWN_TID
    On 08/03, Peter Zijlstra wrote:
    >
    > --- linux-2.6.orig/fs/fcntl.c
    > +++ linux-2.6/fs/fcntl.c
    > @@ -197,13 +197,15 @@ static int setfl(int fd, struct file * f
    > }
    >
    > static void f_modown(struct file *filp, struct pid *pid, enum pid_type type,
    > - int force)
    > + int flags)
    > {
    > write_lock_irq(&filp->f_owner.lock);
    > - if (force || !filp->f_owner.pid) {
    > + if ((flags & FF_SETOWN_FORCE) || !filp->f_owner.pid) {
    > put_pid(filp->f_owner.pid);
    > filp->f_owner.pid = get_pid(pid);
    > filp->f_owner.pid_type = type;
    > + 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().

    Hmm. Actually I am not sure we should change f_modown() at all. I was
    going to suggest this as a subsequent cleanup, but now I am starting
    to think it is better to do from the very beginning. Please see below.

    > +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.

    > +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.


    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.

    Oleg.



    \
     
     \ /
      Last update: 2009-08-03 19:23    [from the cache]
    ©2003-2014 Jasper Spaans. Advertise on this site