Re: [patch 2/13] signal/timer/event fds v6 - signalfd core ...

[ebiederm@xmission.com (Eric W. Biederman)]

Davide Libenzi <davidel@xmailserver.org> writes:

> On Mon, 19 Mar 2007, Eric W. Biederman wrote:
>
>> Davide Libenzi <davidel@xmailserver.org> writes:
>> 
>> > struct signalfd_siginfo {
>> > 	__u32 signo;	/* si_signo */
>> > 	__s32 err;	/* si_errno */
>> > 	__s32 code;	/* si_code */
>> > 	__u32 pid;	/* si_pid */
>> > 	__u32 uid;	/* si_uid */
>> > 	__s32 fd;	/* si_fd */
>> > 	__u32 tid;	/* si_fd */  
>> > 	__u32 band;	/* si_band */
>> > 	__u32 overrun;	/* si_overrun */
>> > 	__u32 trapno;	/* si_trapno */
>> > 	__s32 status;	/* si_status */
>> > 	__s32 svint;	/* si_int */
>> > 	__u64 svptr;	/* si_ptr */
>> > 	__u64 utime;	/* si_utime */
>> > 	__u64 stime;	/* si_stime */
>> > 	__u64 addr;	/* si_addr */
>> > };
>> 
>> Shouldn't we pad this to 128 bytes like we do siginfo in case there are
>> more fields we need to include, or we need to extend the size of some
>> field?
>
> Yes, I guess we can.

I'm just a little paranoid about ABI's.  There is always something
that crops up.  And while we can probably cope by simply having another
version of the signalfd or whatever your syscall is, but having to do
that at the first sign of trouble sucks.  Especially since we would have
to maintain two versions indefinitely.

>> > +
>> > +struct signalfd_ctx {
>> > +	struct list_head lnk;
>> > +	wait_queue_head_t wqh;
>> > +	sigset_t sigmask;
>> > +	struct task_struct *tsk;
>> > +};
>> 
>> I think you want to use a struct pid *pid instead of a pointer to the
>> task struct here.  It is slightly less efficient (one more
>> dereference) but it means that we won't pin the task struct in memory
>> indefinitely.  Pinning the task_struct like this makes for a very
>> interesting way to get around the limits on the number of processes a
>> user can have.
>
> Hmm, when the task is detached from the sighand, we get a notify, so I 
> could do a put from there. This would avoid the extra de-reference. I need 
> to verify locking though ...

Ok.  That sounds more efficient than playing with struct pid pointers,
if it works.

Eric