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