Re: [PATCH -mm 4/5][AIO] - AIO completion signal notification
On Thu, 25 Jan 2007 05:42:42 + Christoph Hellwig <[EMAIL PROTECTED]> wrote: > On Wed, Jan 24, 2007 at 12:11:30PM +0100, S?bastien Dugu? wrote: > > > > + if (unlikely(!notify->sigq)) > > > > + return -EAGAIN; > > > > > > Did this just leak a ref on the task_struct? > > > > > > > No, the ref is released in really_put_req() when we dispose of > > the iocb. > > And the code really needs a comment explaining this. I tripped over > this before, and I think it's even already the second time Andrew > stumbled over it. > OK, will do. Thanks, Sébastien. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm 4/5][AIO] - AIO completion signal notification
On Thu, 25 Jan 2007 05:42:42 + Christoph Hellwig [EMAIL PROTECTED] wrote: On Wed, Jan 24, 2007 at 12:11:30PM +0100, S?bastien Dugu? wrote: + if (unlikely(!notify-sigq)) + return -EAGAIN; Did this just leak a ref on the task_struct? No, the ref is released in really_put_req() when we dispose of the iocb. And the code really needs a comment explaining this. I tripped over this before, and I think it's even already the second time Andrew stumbled over it. OK, will do. Thanks, Sébastien. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm 4/5][AIO] - AIO completion signal notification
On Wed, Jan 24, 2007 at 12:11:30PM +0100, S?bastien Dugu? wrote: > > > + if (unlikely(!notify->sigq)) > > > + return -EAGAIN; > > > > Did this just leak a ref on the task_struct? > > > > No, the ref is released in really_put_req() when we dispose of > the iocb. And the code really needs a comment explaining this. I tripped over this before, and I think it's even already the second time Andrew stumbled over it. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm 4/5][AIO] - AIO completion signal notification
On Tue, 23 Jan 2007 21:35:13 -0800 Andrew Morton <[EMAIL PROTECTED]> wrote:
> On Wed, 17 Jan 2007 10:50:18 +0100
> Sébastien Dugué <[EMAIL PROTECTED]> wrote:
>
> > +static long aio_setup_sigevent(struct aio_notify *notify,
> > + struct sigevent __user *user_event)
> > +{
> > + sigevent_t event;
> > + struct task_struct *target;
> > +
> > + if (copy_from_user(, user_event, sizeof (event)))
> > + return -EFAULT;
> > +
> > + if (event.sigev_notify == SIGEV_NONE)
> > + return 0;
> > +
> > + notify->notify = event.sigev_notify;
> > + notify->signo = event.sigev_signo;
> > + notify->value = event.sigev_value;
> > +
> > + read_lock(_lock);
> > + target = good_sigevent();
> > +
> > + if (unlikely(!target || (target->flags & PF_EXITING)))
> > + goto out_unlock;
> > +
> > + /*
> > +* At this point, we know that notify is either SIGEV_SIGNAL or
> > +* SIGEV_THREAD_ID and the target task is valid. So get a reference
> > +* on the task, it will be dropped in really_put_req() when
> > +* we're done with the request.
> > +*/
> > + get_task_struct(target);
> > + notify->target = target;
> > + read_unlock(_lock);
> > +
> > + /*
> > +* NOTE: we cannot free the sigqueue in the completion path as
> > +* the signal may not have been delivered to the target task.
> > +* Therefore it has to be freed in __sigqueue_free() when the
> > +* signal is collected if si_code is SI_ASYNCIO.
> > +*/
> > + notify->sigq = sigqueue_alloc();
> > +
> > + if (unlikely(!notify->sigq))
> > + return -EAGAIN;
>
> Did this just leak a ref on the task_struct?
>
No, the ref is released in really_put_req() when we dispose of
the iocb.
Thanks,
Sébastien.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm 4/5][AIO] - AIO completion signal notification
On Tue, 23 Jan 2007 21:35:13 -0800 Andrew Morton [EMAIL PROTECTED] wrote:
On Wed, 17 Jan 2007 10:50:18 +0100
Sébastien Dugué [EMAIL PROTECTED] wrote:
+static long aio_setup_sigevent(struct aio_notify *notify,
+ struct sigevent __user *user_event)
+{
+ sigevent_t event;
+ struct task_struct *target;
+
+ if (copy_from_user(event, user_event, sizeof (event)))
+ return -EFAULT;
+
+ if (event.sigev_notify == SIGEV_NONE)
+ return 0;
+
+ notify-notify = event.sigev_notify;
+ notify-signo = event.sigev_signo;
+ notify-value = event.sigev_value;
+
+ read_lock(tasklist_lock);
+ target = good_sigevent(event);
+
+ if (unlikely(!target || (target-flags PF_EXITING)))
+ goto out_unlock;
+
+ /*
+* At this point, we know that notify is either SIGEV_SIGNAL or
+* SIGEV_THREAD_ID and the target task is valid. So get a reference
+* on the task, it will be dropped in really_put_req() when
+* we're done with the request.
+*/
+ get_task_struct(target);
+ notify-target = target;
+ read_unlock(tasklist_lock);
+
+ /*
+* NOTE: we cannot free the sigqueue in the completion path as
+* the signal may not have been delivered to the target task.
+* Therefore it has to be freed in __sigqueue_free() when the
+* signal is collected if si_code is SI_ASYNCIO.
+*/
+ notify-sigq = sigqueue_alloc();
+
+ if (unlikely(!notify-sigq))
+ return -EAGAIN;
Did this just leak a ref on the task_struct?
No, the ref is released in really_put_req() when we dispose of
the iocb.
Thanks,
Sébastien.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm 4/5][AIO] - AIO completion signal notification
On Wed, Jan 24, 2007 at 12:11:30PM +0100, S?bastien Dugu? wrote: + if (unlikely(!notify-sigq)) + return -EAGAIN; Did this just leak a ref on the task_struct? No, the ref is released in really_put_req() when we dispose of the iocb. And the code really needs a comment explaining this. I tripped over this before, and I think it's even already the second time Andrew stumbled over it. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm 4/5][AIO] - AIO completion signal notification
On Wed, 17 Jan 2007 10:50:18 +0100
Sébastien Dugué <[EMAIL PROTECTED]> wrote:
> +static long aio_setup_sigevent(struct aio_notify *notify,
> +struct sigevent __user *user_event)
> +{
> + sigevent_t event;
> + struct task_struct *target;
> +
> + if (copy_from_user(, user_event, sizeof (event)))
> + return -EFAULT;
> +
> + if (event.sigev_notify == SIGEV_NONE)
> + return 0;
> +
> + notify->notify = event.sigev_notify;
> + notify->signo = event.sigev_signo;
> + notify->value = event.sigev_value;
> +
> + read_lock(_lock);
> + target = good_sigevent();
> +
> + if (unlikely(!target || (target->flags & PF_EXITING)))
> + goto out_unlock;
> +
> + /*
> + * At this point, we know that notify is either SIGEV_SIGNAL or
> + * SIGEV_THREAD_ID and the target task is valid. So get a reference
> + * on the task, it will be dropped in really_put_req() when
> + * we're done with the request.
> + */
> + get_task_struct(target);
> + notify->target = target;
> + read_unlock(_lock);
> +
> + /*
> + * NOTE: we cannot free the sigqueue in the completion path as
> + * the signal may not have been delivered to the target task.
> + * Therefore it has to be freed in __sigqueue_free() when the
> + * signal is collected if si_code is SI_ASYNCIO.
> + */
> + notify->sigq = sigqueue_alloc();
> +
> + if (unlikely(!notify->sigq))
> + return -EAGAIN;
Did this just leak a ref on the task_struct?
> +
> + return 0;
> +
> +out_unlock:
> + read_unlock(_lock);
> + return -EINVAL;
> +}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm 4/5][AIO] - AIO completion signal notification
On Wed, 17 Jan 2007 10:50:18 +0100
Sébastien Dugué [EMAIL PROTECTED] wrote:
+static long aio_setup_sigevent(struct aio_notify *notify,
+struct sigevent __user *user_event)
+{
+ sigevent_t event;
+ struct task_struct *target;
+
+ if (copy_from_user(event, user_event, sizeof (event)))
+ return -EFAULT;
+
+ if (event.sigev_notify == SIGEV_NONE)
+ return 0;
+
+ notify-notify = event.sigev_notify;
+ notify-signo = event.sigev_signo;
+ notify-value = event.sigev_value;
+
+ read_lock(tasklist_lock);
+ target = good_sigevent(event);
+
+ if (unlikely(!target || (target-flags PF_EXITING)))
+ goto out_unlock;
+
+ /*
+ * At this point, we know that notify is either SIGEV_SIGNAL or
+ * SIGEV_THREAD_ID and the target task is valid. So get a reference
+ * on the task, it will be dropped in really_put_req() when
+ * we're done with the request.
+ */
+ get_task_struct(target);
+ notify-target = target;
+ read_unlock(tasklist_lock);
+
+ /*
+ * NOTE: we cannot free the sigqueue in the completion path as
+ * the signal may not have been delivered to the target task.
+ * Therefore it has to be freed in __sigqueue_free() when the
+ * signal is collected if si_code is SI_ASYNCIO.
+ */
+ notify-sigq = sigqueue_alloc();
+
+ if (unlikely(!notify-sigq))
+ return -EAGAIN;
Did this just leak a ref on the task_struct?
+
+ return 0;
+
+out_unlock:
+ read_unlock(tasklist_lock);
+ return -EINVAL;
+}
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH -mm 4/5][AIO] - AIO completion signal notification
AIO completion signal notification
The current 2.6 kernel does not support notification of user space via
an RT signal upon an asynchronous IO completion. The POSIX specification
states that when an AIO request completes, a signal can be delivered to
the application as notification.
This patch adds a struct sigevent *aio_sigeventp to the iocb.
The relevant fields (pid, signal number and value) are stored in the kiocb
for use when the request completes.
That sigevent structure is filled by the application as part of the AIO
request preparation. Upon request completion, the kernel notifies the
application using those sigevent parameters. If SIGEV_NONE has been specified,
then the old behaviour is retained and the application must rely on polling
the completion queue using io_getevents().
A struct sigevent *aio_sigeventp field is added to struct iocb in
include/linux/aio_abi.h
A struct aio_notify containing the sigevent parameters is defined in aio.h:
struct aio_notify {
struct task_struct *target;
__u16 signo;
__u16 notify;
sigval_tvalue;
};
A struct aio_notify ki_notify is added to struct kiocb in include/linux/aio.h
In io_submit_one(), if the application provided a sigevent then
setup_sigevent() is called which does the following:
- check access to the user sigevent and make a local copy
- if the requested notification is SIGEV_NONE, then nothing to do
- fill in the kiocb->ki_notify fields (notify, signo, value)
- check sigevent consistency, get the signal target task and
save it in kiocb->ki_notify.target
- preallocate a sigqueue for this event using sigqueue_alloc()
Upon request completion, in aio_complete(), if notification is needed for
this request (iocb->ki_notify.notify != SIGEV_NONE), then aio_send_signal()
is called to signal the target task as follows:
- fill in the siginfo struct to be sent to the task
- if notify is SIGEV_THREAD_ID then send signal to specific task
using send_sigqueue()
- else send signal to task group using send_5group_sigqueue()
Notes concerning sigqueue preallocation:
To ensure reliable delivery of completion notification, the sigqueue is
preallocated in the submission path so that there is no chance it can fail
in the completion path.
Unlike the posix-timers case (currently the single other user of sigqueue
preallocation), where the sigqueue is allocated for the lifetime of the timer
and freed at timer destruction time, the aio case is a bit more tricky due to
the async nature of the whole thing.
In the aio case, the sigqueue exists for the lifetime of the request,
therefore
it must be freed only once the signal for the request completion has been
delivered. This involves changing __sigqueue_free() to free the sigqueue when
the
signal is collected if si_code is SI_ASYNCIO even if it was preallocated as well
as explicitly calling sigqueue_free() in submission and completion error paths.
fs/aio.c| 115 ++--
fs/compat.c | 18 +++
include/linux/aio.h | 12 +
include/linux/aio_abi.h |3 -
kernel/signal.c |2
5 files changed, 144 insertions(+), 6 deletions(-)
Signed-off-by: Sébastien Dugué <[EMAIL PROTECTED]>
Signed-off-by: Laurent Vivier <[EMAIL PROTECTED]>
Index: linux-2.6.20-rc4-mm1/fs/aio.c
===
--- linux-2.6.20-rc4-mm1.orig/fs/aio.c 2007-01-12 11:40:38.0 +0100
+++ linux-2.6.20-rc4-mm1/fs/aio.c 2007-01-12 12:32:55.0 +0100
@@ -419,6 +419,7 @@ static struct kiocb fastcall *__aio_get_
req->ki_dtor = NULL;
req->private = NULL;
req->ki_iovec = NULL;
+ req->ki_notify.sigq = NULL;
INIT_LIST_HEAD(>ki_run_list);
/* Check if the completion queue has enough free space to
@@ -465,6 +466,12 @@ static inline void really_put_req(struct
req->ki_dtor(req);
if (req->ki_iovec != >ki_inline_vec)
kfree(req->ki_iovec);
+
+ /* Release task ref */
+ if (req->ki_notify.notify == SIGEV_THREAD_ID ||
+ req->ki_notify.notify == SIGEV_SIGNAL)
+ put_task_struct(req->ki_notify.target);
+
kmem_cache_free(kiocb_cachep, req);
ctx->reqs_active--;
@@ -916,6 +923,79 @@ void fastcall kick_iocb(struct kiocb *io
}
EXPORT_SYMBOL(kick_iocb);
+static int aio_send_signal(struct aio_notify *notify)
+{
+ struct sigqueue *sigq = notify->sigq;
+ struct siginfo *info = >info;
+ int ret;
+
+ memset(info, 0, sizeof(struct siginfo));
+
+ info->si_signo = notify->signo;
+ info->si_errno = 0;
+ info->si_code = SI_ASYNCIO;
+ info->si_pid = 0;
+ info->si_uid = 0;
+ info->si_value =
[PATCH -mm 4/5][AIO] - AIO completion signal notification
AIO completion signal notification
The current 2.6 kernel does not support notification of user space via
an RT signal upon an asynchronous IO completion. The POSIX specification
states that when an AIO request completes, a signal can be delivered to
the application as notification.
This patch adds a struct sigevent *aio_sigeventp to the iocb.
The relevant fields (pid, signal number and value) are stored in the kiocb
for use when the request completes.
That sigevent structure is filled by the application as part of the AIO
request preparation. Upon request completion, the kernel notifies the
application using those sigevent parameters. If SIGEV_NONE has been specified,
then the old behaviour is retained and the application must rely on polling
the completion queue using io_getevents().
A struct sigevent *aio_sigeventp field is added to struct iocb in
include/linux/aio_abi.h
A struct aio_notify containing the sigevent parameters is defined in aio.h:
struct aio_notify {
struct task_struct *target;
__u16 signo;
__u16 notify;
sigval_tvalue;
};
A struct aio_notify ki_notify is added to struct kiocb in include/linux/aio.h
In io_submit_one(), if the application provided a sigevent then
setup_sigevent() is called which does the following:
- check access to the user sigevent and make a local copy
- if the requested notification is SIGEV_NONE, then nothing to do
- fill in the kiocb-ki_notify fields (notify, signo, value)
- check sigevent consistency, get the signal target task and
save it in kiocb-ki_notify.target
- preallocate a sigqueue for this event using sigqueue_alloc()
Upon request completion, in aio_complete(), if notification is needed for
this request (iocb-ki_notify.notify != SIGEV_NONE), then aio_send_signal()
is called to signal the target task as follows:
- fill in the siginfo struct to be sent to the task
- if notify is SIGEV_THREAD_ID then send signal to specific task
using send_sigqueue()
- else send signal to task group using send_5group_sigqueue()
Notes concerning sigqueue preallocation:
To ensure reliable delivery of completion notification, the sigqueue is
preallocated in the submission path so that there is no chance it can fail
in the completion path.
Unlike the posix-timers case (currently the single other user of sigqueue
preallocation), where the sigqueue is allocated for the lifetime of the timer
and freed at timer destruction time, the aio case is a bit more tricky due to
the async nature of the whole thing.
In the aio case, the sigqueue exists for the lifetime of the request,
therefore
it must be freed only once the signal for the request completion has been
delivered. This involves changing __sigqueue_free() to free the sigqueue when
the
signal is collected if si_code is SI_ASYNCIO even if it was preallocated as well
as explicitly calling sigqueue_free() in submission and completion error paths.
fs/aio.c| 115 ++--
fs/compat.c | 18 +++
include/linux/aio.h | 12 +
include/linux/aio_abi.h |3 -
kernel/signal.c |2
5 files changed, 144 insertions(+), 6 deletions(-)
Signed-off-by: Sébastien Dugué [EMAIL PROTECTED]
Signed-off-by: Laurent Vivier [EMAIL PROTECTED]
Index: linux-2.6.20-rc4-mm1/fs/aio.c
===
--- linux-2.6.20-rc4-mm1.orig/fs/aio.c 2007-01-12 11:40:38.0 +0100
+++ linux-2.6.20-rc4-mm1/fs/aio.c 2007-01-12 12:32:55.0 +0100
@@ -419,6 +419,7 @@ static struct kiocb fastcall *__aio_get_
req-ki_dtor = NULL;
req-private = NULL;
req-ki_iovec = NULL;
+ req-ki_notify.sigq = NULL;
INIT_LIST_HEAD(req-ki_run_list);
/* Check if the completion queue has enough free space to
@@ -465,6 +466,12 @@ static inline void really_put_req(struct
req-ki_dtor(req);
if (req-ki_iovec != req-ki_inline_vec)
kfree(req-ki_iovec);
+
+ /* Release task ref */
+ if (req-ki_notify.notify == SIGEV_THREAD_ID ||
+ req-ki_notify.notify == SIGEV_SIGNAL)
+ put_task_struct(req-ki_notify.target);
+
kmem_cache_free(kiocb_cachep, req);
ctx-reqs_active--;
@@ -916,6 +923,79 @@ void fastcall kick_iocb(struct kiocb *io
}
EXPORT_SYMBOL(kick_iocb);
+static int aio_send_signal(struct aio_notify *notify)
+{
+ struct sigqueue *sigq = notify-sigq;
+ struct siginfo *info = sigq-info;
+ int ret;
+
+ memset(info, 0, sizeof(struct siginfo));
+
+ info-si_signo = notify-signo;
+ info-si_errno = 0;
+ info-si_code = SI_ASYNCIO;
+ info-si_pid = 0;
+ info-si_uid = 0;
+ info-si_value = notify-value;
+
+ if
Re: [PATCH -mm 4/5][AIO] - AIO completion signal notification
AIO completion signal notification
The current 2.6 kernel does not support notification of user space via
an RT signal upon an asynchronous IO completion. The POSIX specification
states that when an AIO request completes, a signal can be delivered to
the application as notification.
This patch adds a struct sigevent *aio_sigeventp to the iocb.
The relevant fields (pid, signal number and value) are stored in the kiocb
for use when the request completes.
That sigevent structure is filled by the application as part of the AIO
request preparation. Upon request completion, the kernel notifies the
application using those sigevent parameters. If SIGEV_NONE has been specified,
then the old behaviour is retained and the application must rely on polling
the completion queue using io_getevents().
A struct sigevent *aio_sigeventp field is added to struct iocb in
include/linux/aio_abi.h
A struct aio_notify containing the sigevent parameters is defined in aio.h:
struct aio_notify {
struct task_struct *target;
__u16 signo;
__u16 notify;
sigval_tvalue;
};
A struct aio_notify ki_notify is added to struct kiocb in include/linux/aio.h
In io_submit_one(), if the application provided a sigevent then
setup_sigevent() is called which does the following:
- check access to the user sigevent and make a local copy
- if the requested notification is SIGEV_NONE, then nothing to do
- fill in the kiocb->ki_notify fields (notify, signo, value)
- check sigevent consistency, get the signal target task and
save it in kiocb->ki_notify.target
- preallocate a sigqueue for this event using sigqueue_alloc()
Upon request completion, in aio_complete(), if notification is needed for
this request (iocb->ki_notify.notify != SIGEV_NONE), then aio_send_signal()
is called to signal the target task as follows:
- fill in the siginfo struct to be sent to the task
- if notify is SIGEV_THREAD_ID then send signal to specific task
using send_sigqueue()
- else send signal to task group using send_5group_sigqueue()
Notes concerning sigqueue preallocation:
To ensure reliable delivery of completion notification, the sigqueue is
preallocated in the submission path so that there is no chance it can fail
in the completion path.
Unlike the posix-timers case (currently the single other user of sigqueue
preallocation), where the sigqueue is allocated for the lifetime of the timer
and freed at timer destruction time, the aio case is a bit more tricky due to
the async nature of the whole thing.
In the aio case, the sigqueue exists for the lifetime of the request,
therefore
it must be freed only once the signal for the request completion has been
delivered. This involves changing __sigqueue_free() to free the sigqueue when
the
signal is collected if si_code is SI_ASYNCIO even if it was preallocated as well
as explicitly calling sigqueue_free() in submission and completion error paths.
fs/aio.c| 115 ++--
fs/compat.c | 18 +++
include/linux/aio.h | 12 +
include/linux/aio_abi.h |3 -
kernel/signal.c |2
5 files changed, 144 insertions(+), 6 deletions(-)
Signed-off-by: Sébastien Dugué <[EMAIL PROTECTED]>
Signed-off-by: Laurent Vivier <[EMAIL PROTECTED]>
Index: linux-2.6.19-rc6-mm2/fs/aio.c
===
--- linux-2.6.19-rc6-mm2.orig/fs/aio.c 2006-11-30 10:54:16.0 +0100
+++ linux-2.6.19-rc6-mm2/fs/aio.c 2006-11-30 15:18:52.0 +0100
@@ -416,6 +416,7 @@ static struct kiocb fastcall *__aio_get_
req->ki_dtor = NULL;
req->private = NULL;
req->ki_iovec = NULL;
+ req->ki_notify.sigq = NULL;
INIT_LIST_HEAD(>ki_run_list);
/* Check if the completion queue has enough free space to
@@ -463,6 +464,12 @@ static inline void really_put_req(struct
req->ki_dtor(req);
if (req->ki_iovec != >ki_inline_vec)
kfree(req->ki_iovec);
+
+ /* Release task ref */
+ if (req->ki_notify.notify == SIGEV_THREAD_ID ||
+ req->ki_notify.notify == SIGEV_SIGNAL)
+ put_task_struct(req->ki_notify.target);
+
kmem_cache_free(kiocb_cachep, req);
ctx->reqs_active--;
@@ -929,6 +936,79 @@ void fastcall kick_iocb(struct kiocb *io
}
EXPORT_SYMBOL(kick_iocb);
+static int aio_send_signal(struct aio_notify *notify)
+{
+ struct sigqueue *sigq = notify->sigq;
+ struct siginfo *info = >info;
+ int ret;
+
+ memset(info, 0, sizeof(struct siginfo));
+
+ info->si_signo = notify->signo;
+ info->si_errno = 0;
+ info->si_code = SI_ASYNCIO;
+ info->si_pid = 0;
+ info->si_uid = 0;
+ info->si_value =
Re: [PATCH -mm 4/5][AIO] - AIO completion signal notification
AIO completion signal notification
The current 2.6 kernel does not support notification of user space via
an RT signal upon an asynchronous IO completion. The POSIX specification
states that when an AIO request completes, a signal can be delivered to
the application as notification.
This patch adds a struct sigevent *aio_sigeventp to the iocb.
The relevant fields (pid, signal number and value) are stored in the kiocb
for use when the request completes.
That sigevent structure is filled by the application as part of the AIO
request preparation. Upon request completion, the kernel notifies the
application using those sigevent parameters. If SIGEV_NONE has been specified,
then the old behaviour is retained and the application must rely on polling
the completion queue using io_getevents().
A struct sigevent *aio_sigeventp field is added to struct iocb in
include/linux/aio_abi.h
A struct aio_notify containing the sigevent parameters is defined in aio.h:
struct aio_notify {
struct task_struct *target;
__u16 signo;
__u16 notify;
sigval_tvalue;
};
A struct aio_notify ki_notify is added to struct kiocb in include/linux/aio.h
In io_submit_one(), if the application provided a sigevent then
setup_sigevent() is called which does the following:
- check access to the user sigevent and make a local copy
- if the requested notification is SIGEV_NONE, then nothing to do
- fill in the kiocb-ki_notify fields (notify, signo, value)
- check sigevent consistency, get the signal target task and
save it in kiocb-ki_notify.target
- preallocate a sigqueue for this event using sigqueue_alloc()
Upon request completion, in aio_complete(), if notification is needed for
this request (iocb-ki_notify.notify != SIGEV_NONE), then aio_send_signal()
is called to signal the target task as follows:
- fill in the siginfo struct to be sent to the task
- if notify is SIGEV_THREAD_ID then send signal to specific task
using send_sigqueue()
- else send signal to task group using send_5group_sigqueue()
Notes concerning sigqueue preallocation:
To ensure reliable delivery of completion notification, the sigqueue is
preallocated in the submission path so that there is no chance it can fail
in the completion path.
Unlike the posix-timers case (currently the single other user of sigqueue
preallocation), where the sigqueue is allocated for the lifetime of the timer
and freed at timer destruction time, the aio case is a bit more tricky due to
the async nature of the whole thing.
In the aio case, the sigqueue exists for the lifetime of the request,
therefore
it must be freed only once the signal for the request completion has been
delivered. This involves changing __sigqueue_free() to free the sigqueue when
the
signal is collected if si_code is SI_ASYNCIO even if it was preallocated as well
as explicitly calling sigqueue_free() in submission and completion error paths.
fs/aio.c| 115 ++--
fs/compat.c | 18 +++
include/linux/aio.h | 12 +
include/linux/aio_abi.h |3 -
kernel/signal.c |2
5 files changed, 144 insertions(+), 6 deletions(-)
Signed-off-by: Sébastien Dugué [EMAIL PROTECTED]
Signed-off-by: Laurent Vivier [EMAIL PROTECTED]
Index: linux-2.6.19-rc6-mm2/fs/aio.c
===
--- linux-2.6.19-rc6-mm2.orig/fs/aio.c 2006-11-30 10:54:16.0 +0100
+++ linux-2.6.19-rc6-mm2/fs/aio.c 2006-11-30 15:18:52.0 +0100
@@ -416,6 +416,7 @@ static struct kiocb fastcall *__aio_get_
req-ki_dtor = NULL;
req-private = NULL;
req-ki_iovec = NULL;
+ req-ki_notify.sigq = NULL;
INIT_LIST_HEAD(req-ki_run_list);
/* Check if the completion queue has enough free space to
@@ -463,6 +464,12 @@ static inline void really_put_req(struct
req-ki_dtor(req);
if (req-ki_iovec != req-ki_inline_vec)
kfree(req-ki_iovec);
+
+ /* Release task ref */
+ if (req-ki_notify.notify == SIGEV_THREAD_ID ||
+ req-ki_notify.notify == SIGEV_SIGNAL)
+ put_task_struct(req-ki_notify.target);
+
kmem_cache_free(kiocb_cachep, req);
ctx-reqs_active--;
@@ -929,6 +936,79 @@ void fastcall kick_iocb(struct kiocb *io
}
EXPORT_SYMBOL(kick_iocb);
+static int aio_send_signal(struct aio_notify *notify)
+{
+ struct sigqueue *sigq = notify-sigq;
+ struct siginfo *info = sigq-info;
+ int ret;
+
+ memset(info, 0, sizeof(struct siginfo));
+
+ info-si_signo = notify-signo;
+ info-si_errno = 0;
+ info-si_code = SI_ASYNCIO;
+ info-si_pid = 0;
+ info-si_uid = 0;
+ info-si_value = notify-value;
+
+ if
Re: [PATCH -mm 4/5][AIO] - AIO completion signal notification
On Wed, 29 Nov 2006 13:50:12 +, Christoph Hellwig <[EMAIL PROTECTED]> wrote:
> On Wed, Nov 29, 2006 at 02:08:01PM +0100, S?bastien Dugu? wrote:
> > On Wed, 29 Nov 2006 10:51:50 +, Christoph Hellwig <[EMAIL PROTECTED]>
> > wrote:
> >
> > > I'm a little bit unhappy about the usage of the notify flag. The usage
> > > seems correct but very confusing:
> >
> > Well, I followed the logic from posix-timers.c, but it may be a poor
> > choice ;-)
> >
> > For a start, the SIGEV_* flags are quite confusing (for me at least).
> > SIGEV_SIGNAL is defined as 0, SIGEV_NONE as 1 and SIGEV_THREAD_ID as 4. I
> > would rather have seen SIGEV_NONE defined as 0 to avoid all this.
> >
> > I also wish I knew why those SIGEV_* constants were defined that way.
>
> Ah, I missed that. It explains some of the more wierd bits. I suspect
> we should then use != SIGEV_NONE for the any kind of signal notification
> bit and == SIGEV_THREAD_ID for the case where we want to deliver to
> a particular thread.
Right, that would make things much cleaner. Will try for it.
>
> But this means we only get a thread reference for SIGEV_THREAD_ID
> here:
>
> > > > + if (notify->notify == (SIGEV_SIGNAL|SIGEV_THREAD_ID)) {
> > > > + /*
> > > > +* This reference will be dropped in really_put_req()
> > > > when
> > > > +* we're done with the request.
> > > > +*/
> > > > + get_task_struct(target);
> > > > + }
It's the way it is in posix-timers and I'm not sure I understand why. We take
a ref on the specific task if notify is SIGEV_THREAD_ID but not for
SIGEV_SIGNAL.
I'm wondering what I'm missing here, shouldn't we also take a ref on the task
group leader in the SIGEV_SIGNAL case in posix-timers?
>
> But even use it for SIGEV_SIGNAL without SIGEV_THREAD_ID here:
>
> > > > + if (notify->notify & SIGEV_THREAD_ID)
> > > > + ret = send_sigqueue(notify->signo, sigq,
> > > > notify->target);
> > > > + else
> > > > + ret = send_group_sigqueue(notify->signo, sigq,
> > > > notify->target);
>
> Or do I miss something?
I missing something too here ;-)
If someone cared to explain why there is no ref taken on the task for the
SIGEV_SIGNAL case, it would be much appreciated. Is this a bug in posix-timers?
Thanks,
Sébastien.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm 4/5][AIO] - AIO completion signal notification
On Wed, Nov 29, 2006 at 02:08:01PM +0100, S?bastien Dugu? wrote:
> On Wed, 29 Nov 2006 10:51:50 +, Christoph Hellwig <[EMAIL PROTECTED]>
> wrote:
>
> > I'm a little bit unhappy about the usage of the notify flag. The usage
> > seems correct but very confusing:
>
> Well, I followed the logic from posix-timers.c, but it may be a poor
> choice ;-)
>
> For a start, the SIGEV_* flags are quite confusing (for me at least).
> SIGEV_SIGNAL is defined as 0, SIGEV_NONE as 1 and SIGEV_THREAD_ID as 4. I
> would rather have seen SIGEV_NONE defined as 0 to avoid all this.
>
> I also wish I knew why those SIGEV_* constants were defined that way.
Ah, I missed that. It explains some of the more wierd bits. I suspect
we should then use != SIGEV_NONE for the any kind of signal notification
bit and == SIGEV_THREAD_ID for the case where we want to deliver to
a particular thread.
But this means we only get a thread reference for SIGEV_THREAD_ID
here:
> > > + if (notify->notify == (SIGEV_SIGNAL|SIGEV_THREAD_ID)) {
> > > + /*
> > > + * This reference will be dropped in really_put_req() when
> > > + * we're done with the request.
> > > + */
> > > + get_task_struct(target);
> > > + }
But even use it for SIGEV_SIGNAL without SIGEV_THREAD_ID here:
> > > + if (notify->notify & SIGEV_THREAD_ID)
> > > + ret = send_sigqueue(notify->signo, sigq, notify->target);
> > > + else
> > > + ret = send_group_sigqueue(notify->signo, sigq, notify->target);
Or do I miss something?
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm 4/5][AIO] - AIO completion signal notification
On Wed, 29 Nov 2006 06:33:35 -0500, Jakub Jelinek <[EMAIL PROTECTED]> wrote: > On Wed, Nov 29, 2006 at 11:33:01AM +0100, S?bastien Dugu? wrote: > > AIO completion signal notification > > > > The current 2.6 kernel does not support notification of user space via > > an RT signal upon an asynchronous IO completion. The POSIX specification > > states that when an AIO request completes, a signal can be delivered to > > the application as notification. > > > > This patch adds a struct sigevent *aio_sigeventp to the iocb. > > The relevant fields (pid, signal number and value) are stored in the kiocb > > for use when the request completes. > > > > That sigevent structure is filled by the application as part of the AIO > > request preparation. Upon request completion, the kernel notifies the > > application using those sigevent parameters. If SIGEV_NONE has been > > specified, > > then the old behaviour is retained and the application must rely on polling > > the completion queue using io_getevents(). > > Well, from what I see applications must rely on polling the completion > queue using io_getevents() in any case, isn't that the only way how to free > the kernel resources associated with the AIO request, even if it uses > SIGEV_SIGNAL or thread notification? Well, applications do not need to do any polling on the queue anymore. io_getevents() needs to be called only once when the signal has been received, either from a signal handler or from a thread blocking on the signal. > aio_error/aio_return/aio_suspend > will still need to io_getevents, Right, but only once. > the only important difference with this > patch is that a) the polling doesn't need to be asynchronous (i.e. have > a special thread which just loops doing io_getevents) Yes, no more polling loop and I think this makes a big difference. > b) it doesn't need to care about notification itself. > Uh! what do you mean here? Sébastien. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm 4/5][AIO] - AIO completion signal notification
On Wed, 29 Nov 2006 10:51:50 +, Christoph Hellwig <[EMAIL PROTECTED]> wrote:
> I'm a little bit unhappy about the usage of the notify flag. The usage
> seems correct but very confusing:
Well, I followed the logic from posix-timers.c, but it may be a poor
choice ;-)
For a start, the SIGEV_* flags are quite confusing (for me at least).
SIGEV_SIGNAL is defined as 0, SIGEV_NONE as 1 and SIGEV_THREAD_ID as 4. I
would rather have seen SIGEV_NONE defined as 0 to avoid all this.
I also wish I knew why those SIGEV_* constants were defined that way.
>
> In io_submit_one we set ki_notify.notify to SIGEV_NONE and possibly
> call aio_setup_sigevent:
>
> > + /* handle setting up the sigevent for POSIX AIO signals */
> > + req->ki_notify.notify = SIGEV_NONE;
> > +
> > + if (iocb->aio_sigeventp) {
> > + ret = aio_setup_sigevent(>ki_notify,
> > +(struct sigevent __user *)(unsigned
> > long)
> > +iocb->aio_sigeventp);
> > + if (ret)
> > + goto out_put_req;
> > + }
> > +
>
> aio_setup_sigevent then checks the user passed even for which notify type
> we have, and returns if it's none or otherwise sets notify->notify to it.
>
> > + if (event.sigev_notify == SIGEV_NONE)
> > + return 0;
> > +
> > + notify->notify = event.sigev_notify;
>
> Later aio_setup_sigevent gets a reference to the target task_structure
> if notify->notify is (SIGEV_SIGNAL|SIGEV_THREAD_ID) but _always_ stores
> the target pointer.
Yep, as SIGEV_SIGNAL is 0, this in fact checks that notify is SIGEV_THREAD_ID.
It could have been written as:
if (notify->notify == SIGEV_THREAD_ID)
And the target pointer is always store because at this point notify is either
SIGEV_SIGNAL or SIGEV_THREAD_ID.
>
> > + if (notify->notify == (SIGEV_SIGNAL|SIGEV_THREAD_ID)) {
> > + /*
> > +* This reference will be dropped in really_put_req() when
> > +* we're done with the request.
> > +*/
> > + get_task_struct(target);
> > + }
> > +
> > + notify->target = target;
>
>
> Once we're done with the iocb aio_complete aclls aio_send_signal if
> notify.notify is not SIGEV_NONE.
Again, if it's not SIGEV_NONE, then it's either SIGEV_SIGNAL or
SIGEV_THREAD_ID.
>
> > + if (iocb->ki_notify.notify != SIGEV_NONE) {
> > + ret = aio_send_signal(>ki_notify);
> > +
> > + /* If signal generation failed, release the sigqueue */
> > + if (ret)
> > + sigqueue_free(iocb->ki_notify.sigq);
> > + }
> > +
>
> Which then uses notify->target to send the signal:
> > + if (notify->notify & SIGEV_THREAD_ID)
> > + ret = send_sigqueue(notify->signo, sigq, notify->target);
> > + else
> > + ret = send_group_sigqueue(notify->signo, sigq, notify->target);
>
> And finally really_put_req puts the target if notify.notify contains
> either SIGEV_SIGNAL or SIGEV_THREAD_ID.
>
> > + /* Release task ref */
> > + if (req->ki_notify.notify == (SIGEV_SIGNAL|SIGEV_THREAD_ID))
> > + put_task_struct(req->ki_notify.target);
Could have been if (req->ki_notify.notify == SIGEV_THREAD_ID)
>
> Do you see the confusing? I think all the notify.notify != SIGEV_NONE
> in the above code should be replaces by the much more descriptive
> notify.notify == (SIGEV_SIGNAL|SIGEV_THREAD_ID). In addition we should
> only store the target pointer inside the (SIGEV_SIGNAL|SIGEV_THREAD_ID)
> if block that gets a reference to it.
No, I don't think so, notify == (SIGEV_SIGNAL|SIGEV_THREAD_ID) really means
notify == SIGEV_THREAD_ID which leaves out notify == SIGEV_SIGNAL. Or
am I grossly mistaken?
Thanks,
Sébastien.
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [EMAIL PROTECTED]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm 4/5][AIO] - AIO completion signal notification
On Wed, Nov 29, 2006 at 11:33:01AM +0100, S?bastien Dugu? wrote: > AIO completion signal notification > > The current 2.6 kernel does not support notification of user space via > an RT signal upon an asynchronous IO completion. The POSIX specification > states that when an AIO request completes, a signal can be delivered to > the application as notification. > > This patch adds a struct sigevent *aio_sigeventp to the iocb. > The relevant fields (pid, signal number and value) are stored in the kiocb > for use when the request completes. > > That sigevent structure is filled by the application as part of the AIO > request preparation. Upon request completion, the kernel notifies the > application using those sigevent parameters. If SIGEV_NONE has been specified, > then the old behaviour is retained and the application must rely on polling > the completion queue using io_getevents(). Well, from what I see applications must rely on polling the completion queue using io_getevents() in any case, isn't that the only way how to free the kernel resources associated with the AIO request, even if it uses SIGEV_SIGNAL or thread notification? aio_error/aio_return/aio_suspend will still need to io_getevents, the only important difference with this patch is that a) the polling doesn't need to be asynchronous (i.e. have a special thread which just loops doing io_getevents) b) it doesn't need to care about notification itself. Jakub - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm 4/5][AIO] - AIO completion signal notification
I'm a little bit unhappy about the usage of the notify flag. The usage
seems correct but very confusing:
In io_submit_one we set ki_notify.notify to SIGEV_NONE and possibly
call aio_setup_sigevent:
> + /* handle setting up the sigevent for POSIX AIO signals */
> + req->ki_notify.notify = SIGEV_NONE;
> +
> + if (iocb->aio_sigeventp) {
> + ret = aio_setup_sigevent(>ki_notify,
> + (struct sigevent __user *)(unsigned
> long)
> + iocb->aio_sigeventp);
> + if (ret)
> + goto out_put_req;
> + }
> +
aio_setup_sigevent then checks the user passed even for which notify type
we have, and returns if it's none or otherwise sets notify->notify to it.
> + if (event.sigev_notify == SIGEV_NONE)
> + return 0;
> +
> + notify->notify = event.sigev_notify;
Later aio_setup_sigevent gets a reference to the target task_structure
if notify->notify is (SIGEV_SIGNAL|SIGEV_THREAD_ID) but _always_ stores
the target pointer.
> + if (notify->notify == (SIGEV_SIGNAL|SIGEV_THREAD_ID)) {
> + /*
> + * This reference will be dropped in really_put_req() when
> + * we're done with the request.
> + */
> + get_task_struct(target);
> + }
> +
> + notify->target = target;
Once we're done with the iocb aio_complete aclls aio_send_signal if
notify.notify is not SIGEV_NONE.
> + if (iocb->ki_notify.notify != SIGEV_NONE) {
> + ret = aio_send_signal(>ki_notify);
> +
> + /* If signal generation failed, release the sigqueue */
> + if (ret)
> + sigqueue_free(iocb->ki_notify.sigq);
> + }
> +
Which then uses notify->target to send the signal:
> + if (notify->notify & SIGEV_THREAD_ID)
> + ret = send_sigqueue(notify->signo, sigq, notify->target);
> + else
> + ret = send_group_sigqueue(notify->signo, sigq, notify->target);
And finally really_put_req puts the target if notify.notify contains
either SIGEV_SIGNAL or SIGEV_THREAD_ID.
> + /* Release task ref */
> + if (req->ki_notify.notify == (SIGEV_SIGNAL|SIGEV_THREAD_ID))
> + put_task_struct(req->ki_notify.target);
Do you see the confusing? I think all the notify.notify != SIGEV_NONE
in the above code should be replaces by the much more descriptive
notify.notify == (SIGEV_SIGNAL|SIGEV_THREAD_ID). In addition we should
only store the target pointer inside the (SIGEV_SIGNAL|SIGEV_THREAD_ID)
if block that gets a reference to it.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH -mm 4/5][AIO] - AIO completion signal notification
AIO completion signal notification
The current 2.6 kernel does not support notification of user space via
an RT signal upon an asynchronous IO completion. The POSIX specification
states that when an AIO request completes, a signal can be delivered to
the application as notification.
This patch adds a struct sigevent *aio_sigeventp to the iocb.
The relevant fields (pid, signal number and value) are stored in the kiocb
for use when the request completes.
That sigevent structure is filled by the application as part of the AIO
request preparation. Upon request completion, the kernel notifies the
application using those sigevent parameters. If SIGEV_NONE has been specified,
then the old behaviour is retained and the application must rely on polling
the completion queue using io_getevents().
A struct sigevent *aio_sigeventp field is added to struct iocb in
include/linux/aio_abi.h
A struct aio_notify containing the sigevent parameters is defined in aio.h:
struct aio_notify {
struct task_struct *target;
__u16 signo;
__u16 notify;
sigval_tvalue;
};
A struct aio_notify ki_notify is added to struct kiocb in include/linux/aio.h
In io_submit_one(), if the application provided a sigevent then
setup_sigevent() is called which does the following:
- check access to the user sigevent and make a local copy
- if the requested notification is SIGEV_NONE, then nothing to do
- fill in the kiocb->ki_notify fields (notify, signo, value)
- check sigevent consistency, get the signal target task and
save it in kiocb->ki_notify.target
- preallocate a sigqueue for this event using sigqueue_alloc()
Upon request completion, in aio_complete(), if notification is needed for
this request (iocb->ki_notify.notify != SIGEV_NONE), then aio_send_signal()
is called to signal the target task as follows:
- fill in the siginfo struct to be sent to the task
- if notify is SIGEV_THREAD_ID then send signal to specific task
using send_sigqueue()
- else send signal to task group using send_5group_sigqueue()
Notes concerning sigqueue preallocation:
To ensure reliable delivery of completion notification, the sigqueue is
preallocated in the submission path so that there is no chance it can fail
in the completion path.
Unlike the posix-timers case (currently the single other user of sigqueue
preallocation), where the sigqueue is allocated for the lifetime of the timer
and freed at timer destruction time, the aio case is a bit more tricky due to
the async nature of the whole thing.
In the aio case, the sigqueue exists for the lifetime of the request,
therefore it must be freed only once the signal for the request completion has
been delivered. This involves changing __sigqueue_free() to free the sigqueue
when the signal is collected if si_code is SI_ASYNCIO even if it was
preallocated as well as explicitly calling sigqueue_free() in submission and
completion error paths.
fs/aio.c| 115 ++--
fs/compat.c | 18 +++
include/linux/aio.h | 12 +
include/linux/aio_abi.h |3 -
kernel/signal.c |2
5 files changed, 144 insertions(+), 6 deletions(-)
Signed-off-by: Sébastien Dugué <[EMAIL PROTECTED]>
Signed-off-by: Laurent Vivier <[EMAIL PROTECTED]>
Index: linux-2.6.19-rc6-mm2/fs/aio.c
===
--- linux-2.6.19-rc6-mm2.orig/fs/aio.c 2006-11-28 12:49:40.0
+0100 +++ linux-2.6.19-rc6-mm2/fs/aio.c 2006-11-29 10:14:47.0
+0100 @@ -416,6 +416,7 @@ static struct kiocb fastcall *__aio_get_
req->ki_dtor = NULL;
req->private = NULL;
req->ki_iovec = NULL;
+ req->ki_notify.sigq = NULL;
INIT_LIST_HEAD(>ki_run_list);
/* Check if the completion queue has enough free space to
@@ -463,6 +464,11 @@ static inline void really_put_req(struct
req->ki_dtor(req);
if (req->ki_iovec != >ki_inline_vec)
kfree(req->ki_iovec);
+
+ /* Release task ref */
+ if (req->ki_notify.notify == (SIGEV_SIGNAL|SIGEV_THREAD_ID))
+ put_task_struct(req->ki_notify.target);
+
kmem_cache_free(kiocb_cachep, req);
ctx->reqs_active--;
@@ -929,6 +935,80 @@ void fastcall kick_iocb(struct kiocb *io
}
EXPORT_SYMBOL(kick_iocb);
+static int aio_send_signal(struct aio_notify *notify)
+{
+ struct sigqueue *sigq = notify->sigq;
+ struct siginfo *info = >info;
+ int ret;
+
+ memset(info, 0, sizeof(struct siginfo));
+
+ info->si_signo = notify->signo;
+ info->si_errno = 0;
+ info->si_code = SI_ASYNCIO;
+ info->si_pid = 0;
+ info->si_uid = 0;
+ info->si_value = notify->value;
+
+ if (notify->notify &
[PATCH -mm 4/5][AIO] - AIO completion signal notification
AIO completion signal notification
The current 2.6 kernel does not support notification of user space via
an RT signal upon an asynchronous IO completion. The POSIX specification
states that when an AIO request completes, a signal can be delivered to
the application as notification.
This patch adds a struct sigevent *aio_sigeventp to the iocb.
The relevant fields (pid, signal number and value) are stored in the kiocb
for use when the request completes.
That sigevent structure is filled by the application as part of the AIO
request preparation. Upon request completion, the kernel notifies the
application using those sigevent parameters. If SIGEV_NONE has been specified,
then the old behaviour is retained and the application must rely on polling
the completion queue using io_getevents().
A struct sigevent *aio_sigeventp field is added to struct iocb in
include/linux/aio_abi.h
A struct aio_notify containing the sigevent parameters is defined in aio.h:
struct aio_notify {
struct task_struct *target;
__u16 signo;
__u16 notify;
sigval_tvalue;
};
A struct aio_notify ki_notify is added to struct kiocb in include/linux/aio.h
In io_submit_one(), if the application provided a sigevent then
setup_sigevent() is called which does the following:
- check access to the user sigevent and make a local copy
- if the requested notification is SIGEV_NONE, then nothing to do
- fill in the kiocb-ki_notify fields (notify, signo, value)
- check sigevent consistency, get the signal target task and
save it in kiocb-ki_notify.target
- preallocate a sigqueue for this event using sigqueue_alloc()
Upon request completion, in aio_complete(), if notification is needed for
this request (iocb-ki_notify.notify != SIGEV_NONE), then aio_send_signal()
is called to signal the target task as follows:
- fill in the siginfo struct to be sent to the task
- if notify is SIGEV_THREAD_ID then send signal to specific task
using send_sigqueue()
- else send signal to task group using send_5group_sigqueue()
Notes concerning sigqueue preallocation:
To ensure reliable delivery of completion notification, the sigqueue is
preallocated in the submission path so that there is no chance it can fail
in the completion path.
Unlike the posix-timers case (currently the single other user of sigqueue
preallocation), where the sigqueue is allocated for the lifetime of the timer
and freed at timer destruction time, the aio case is a bit more tricky due to
the async nature of the whole thing.
In the aio case, the sigqueue exists for the lifetime of the request,
therefore it must be freed only once the signal for the request completion has
been delivered. This involves changing __sigqueue_free() to free the sigqueue
when the signal is collected if si_code is SI_ASYNCIO even if it was
preallocated as well as explicitly calling sigqueue_free() in submission and
completion error paths.
fs/aio.c| 115 ++--
fs/compat.c | 18 +++
include/linux/aio.h | 12 +
include/linux/aio_abi.h |3 -
kernel/signal.c |2
5 files changed, 144 insertions(+), 6 deletions(-)
Signed-off-by: Sébastien Dugué [EMAIL PROTECTED]
Signed-off-by: Laurent Vivier [EMAIL PROTECTED]
Index: linux-2.6.19-rc6-mm2/fs/aio.c
===
--- linux-2.6.19-rc6-mm2.orig/fs/aio.c 2006-11-28 12:49:40.0
+0100 +++ linux-2.6.19-rc6-mm2/fs/aio.c 2006-11-29 10:14:47.0
+0100 @@ -416,6 +416,7 @@ static struct kiocb fastcall *__aio_get_
req-ki_dtor = NULL;
req-private = NULL;
req-ki_iovec = NULL;
+ req-ki_notify.sigq = NULL;
INIT_LIST_HEAD(req-ki_run_list);
/* Check if the completion queue has enough free space to
@@ -463,6 +464,11 @@ static inline void really_put_req(struct
req-ki_dtor(req);
if (req-ki_iovec != req-ki_inline_vec)
kfree(req-ki_iovec);
+
+ /* Release task ref */
+ if (req-ki_notify.notify == (SIGEV_SIGNAL|SIGEV_THREAD_ID))
+ put_task_struct(req-ki_notify.target);
+
kmem_cache_free(kiocb_cachep, req);
ctx-reqs_active--;
@@ -929,6 +935,80 @@ void fastcall kick_iocb(struct kiocb *io
}
EXPORT_SYMBOL(kick_iocb);
+static int aio_send_signal(struct aio_notify *notify)
+{
+ struct sigqueue *sigq = notify-sigq;
+ struct siginfo *info = sigq-info;
+ int ret;
+
+ memset(info, 0, sizeof(struct siginfo));
+
+ info-si_signo = notify-signo;
+ info-si_errno = 0;
+ info-si_code = SI_ASYNCIO;
+ info-si_pid = 0;
+ info-si_uid = 0;
+ info-si_value = notify-value;
+
+ if (notify-notify SIGEV_THREAD_ID)
+
Re: [PATCH -mm 4/5][AIO] - AIO completion signal notification
I'm a little bit unhappy about the usage of the notify flag. The usage
seems correct but very confusing:
In io_submit_one we set ki_notify.notify to SIGEV_NONE and possibly
call aio_setup_sigevent:
+ /* handle setting up the sigevent for POSIX AIO signals */
+ req-ki_notify.notify = SIGEV_NONE;
+
+ if (iocb-aio_sigeventp) {
+ ret = aio_setup_sigevent(req-ki_notify,
+ (struct sigevent __user *)(unsigned
long)
+ iocb-aio_sigeventp);
+ if (ret)
+ goto out_put_req;
+ }
+
aio_setup_sigevent then checks the user passed even for which notify type
we have, and returns if it's none or otherwise sets notify-notify to it.
+ if (event.sigev_notify == SIGEV_NONE)
+ return 0;
+
+ notify-notify = event.sigev_notify;
Later aio_setup_sigevent gets a reference to the target task_structure
if notify-notify is (SIGEV_SIGNAL|SIGEV_THREAD_ID) but _always_ stores
the target pointer.
+ if (notify-notify == (SIGEV_SIGNAL|SIGEV_THREAD_ID)) {
+ /*
+ * This reference will be dropped in really_put_req() when
+ * we're done with the request.
+ */
+ get_task_struct(target);
+ }
+
+ notify-target = target;
Once we're done with the iocb aio_complete aclls aio_send_signal if
notify.notify is not SIGEV_NONE.
+ if (iocb-ki_notify.notify != SIGEV_NONE) {
+ ret = aio_send_signal(iocb-ki_notify);
+
+ /* If signal generation failed, release the sigqueue */
+ if (ret)
+ sigqueue_free(iocb-ki_notify.sigq);
+ }
+
Which then uses notify-target to send the signal:
+ if (notify-notify SIGEV_THREAD_ID)
+ ret = send_sigqueue(notify-signo, sigq, notify-target);
+ else
+ ret = send_group_sigqueue(notify-signo, sigq, notify-target);
And finally really_put_req puts the target if notify.notify contains
either SIGEV_SIGNAL or SIGEV_THREAD_ID.
+ /* Release task ref */
+ if (req-ki_notify.notify == (SIGEV_SIGNAL|SIGEV_THREAD_ID))
+ put_task_struct(req-ki_notify.target);
Do you see the confusing? I think all the notify.notify != SIGEV_NONE
in the above code should be replaces by the much more descriptive
notify.notify == (SIGEV_SIGNAL|SIGEV_THREAD_ID). In addition we should
only store the target pointer inside the (SIGEV_SIGNAL|SIGEV_THREAD_ID)
if block that gets a reference to it.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm 4/5][AIO] - AIO completion signal notification
On Wed, Nov 29, 2006 at 11:33:01AM +0100, S?bastien Dugu? wrote: AIO completion signal notification The current 2.6 kernel does not support notification of user space via an RT signal upon an asynchronous IO completion. The POSIX specification states that when an AIO request completes, a signal can be delivered to the application as notification. This patch adds a struct sigevent *aio_sigeventp to the iocb. The relevant fields (pid, signal number and value) are stored in the kiocb for use when the request completes. That sigevent structure is filled by the application as part of the AIO request preparation. Upon request completion, the kernel notifies the application using those sigevent parameters. If SIGEV_NONE has been specified, then the old behaviour is retained and the application must rely on polling the completion queue using io_getevents(). Well, from what I see applications must rely on polling the completion queue using io_getevents() in any case, isn't that the only way how to free the kernel resources associated with the AIO request, even if it uses SIGEV_SIGNAL or thread notification? aio_error/aio_return/aio_suspend will still need to io_getevents, the only important difference with this patch is that a) the polling doesn't need to be asynchronous (i.e. have a special thread which just loops doing io_getevents) b) it doesn't need to care about notification itself. Jakub - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm 4/5][AIO] - AIO completion signal notification
On Wed, 29 Nov 2006 10:51:50 +, Christoph Hellwig [EMAIL PROTECTED] wrote:
I'm a little bit unhappy about the usage of the notify flag. The usage
seems correct but very confusing:
Well, I followed the logic from posix-timers.c, but it may be a poor
choice ;-)
For a start, the SIGEV_* flags are quite confusing (for me at least).
SIGEV_SIGNAL is defined as 0, SIGEV_NONE as 1 and SIGEV_THREAD_ID as 4. I
would rather have seen SIGEV_NONE defined as 0 to avoid all this.
I also wish I knew why those SIGEV_* constants were defined that way.
In io_submit_one we set ki_notify.notify to SIGEV_NONE and possibly
call aio_setup_sigevent:
+ /* handle setting up the sigevent for POSIX AIO signals */
+ req-ki_notify.notify = SIGEV_NONE;
+
+ if (iocb-aio_sigeventp) {
+ ret = aio_setup_sigevent(req-ki_notify,
+(struct sigevent __user *)(unsigned
long)
+iocb-aio_sigeventp);
+ if (ret)
+ goto out_put_req;
+ }
+
aio_setup_sigevent then checks the user passed even for which notify type
we have, and returns if it's none or otherwise sets notify-notify to it.
+ if (event.sigev_notify == SIGEV_NONE)
+ return 0;
+
+ notify-notify = event.sigev_notify;
Later aio_setup_sigevent gets a reference to the target task_structure
if notify-notify is (SIGEV_SIGNAL|SIGEV_THREAD_ID) but _always_ stores
the target pointer.
Yep, as SIGEV_SIGNAL is 0, this in fact checks that notify is SIGEV_THREAD_ID.
It could have been written as:
if (notify-notify == SIGEV_THREAD_ID)
And the target pointer is always store because at this point notify is either
SIGEV_SIGNAL or SIGEV_THREAD_ID.
+ if (notify-notify == (SIGEV_SIGNAL|SIGEV_THREAD_ID)) {
+ /*
+* This reference will be dropped in really_put_req() when
+* we're done with the request.
+*/
+ get_task_struct(target);
+ }
+
+ notify-target = target;
Once we're done with the iocb aio_complete aclls aio_send_signal if
notify.notify is not SIGEV_NONE.
Again, if it's not SIGEV_NONE, then it's either SIGEV_SIGNAL or
SIGEV_THREAD_ID.
+ if (iocb-ki_notify.notify != SIGEV_NONE) {
+ ret = aio_send_signal(iocb-ki_notify);
+
+ /* If signal generation failed, release the sigqueue */
+ if (ret)
+ sigqueue_free(iocb-ki_notify.sigq);
+ }
+
Which then uses notify-target to send the signal:
+ if (notify-notify SIGEV_THREAD_ID)
+ ret = send_sigqueue(notify-signo, sigq, notify-target);
+ else
+ ret = send_group_sigqueue(notify-signo, sigq, notify-target);
And finally really_put_req puts the target if notify.notify contains
either SIGEV_SIGNAL or SIGEV_THREAD_ID.
+ /* Release task ref */
+ if (req-ki_notify.notify == (SIGEV_SIGNAL|SIGEV_THREAD_ID))
+ put_task_struct(req-ki_notify.target);
Could have been if (req-ki_notify.notify == SIGEV_THREAD_ID)
Do you see the confusing? I think all the notify.notify != SIGEV_NONE
in the above code should be replaces by the much more descriptive
notify.notify == (SIGEV_SIGNAL|SIGEV_THREAD_ID). In addition we should
only store the target pointer inside the (SIGEV_SIGNAL|SIGEV_THREAD_ID)
if block that gets a reference to it.
No, I don't think so, notify == (SIGEV_SIGNAL|SIGEV_THREAD_ID) really means
notify == SIGEV_THREAD_ID which leaves out notify == SIGEV_SIGNAL. Or
am I grossly mistaken?
Thanks,
Sébastien.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm 4/5][AIO] - AIO completion signal notification
On Wed, 29 Nov 2006 06:33:35 -0500, Jakub Jelinek [EMAIL PROTECTED] wrote: On Wed, Nov 29, 2006 at 11:33:01AM +0100, S?bastien Dugu? wrote: AIO completion signal notification The current 2.6 kernel does not support notification of user space via an RT signal upon an asynchronous IO completion. The POSIX specification states that when an AIO request completes, a signal can be delivered to the application as notification. This patch adds a struct sigevent *aio_sigeventp to the iocb. The relevant fields (pid, signal number and value) are stored in the kiocb for use when the request completes. That sigevent structure is filled by the application as part of the AIO request preparation. Upon request completion, the kernel notifies the application using those sigevent parameters. If SIGEV_NONE has been specified, then the old behaviour is retained and the application must rely on polling the completion queue using io_getevents(). Well, from what I see applications must rely on polling the completion queue using io_getevents() in any case, isn't that the only way how to free the kernel resources associated with the AIO request, even if it uses SIGEV_SIGNAL or thread notification? Well, applications do not need to do any polling on the queue anymore. io_getevents() needs to be called only once when the signal has been received, either from a signal handler or from a thread blocking on the signal. aio_error/aio_return/aio_suspend will still need to io_getevents, Right, but only once. the only important difference with this patch is that a) the polling doesn't need to be asynchronous (i.e. have a special thread which just loops doing io_getevents) Yes, no more polling loop and I think this makes a big difference. b) it doesn't need to care about notification itself. Uh! what do you mean here? Sébastien. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm 4/5][AIO] - AIO completion signal notification
On Wed, Nov 29, 2006 at 02:08:01PM +0100, S?bastien Dugu? wrote:
On Wed, 29 Nov 2006 10:51:50 +, Christoph Hellwig [EMAIL PROTECTED]
wrote:
I'm a little bit unhappy about the usage of the notify flag. The usage
seems correct but very confusing:
Well, I followed the logic from posix-timers.c, but it may be a poor
choice ;-)
For a start, the SIGEV_* flags are quite confusing (for me at least).
SIGEV_SIGNAL is defined as 0, SIGEV_NONE as 1 and SIGEV_THREAD_ID as 4. I
would rather have seen SIGEV_NONE defined as 0 to avoid all this.
I also wish I knew why those SIGEV_* constants were defined that way.
Ah, I missed that. It explains some of the more wierd bits. I suspect
we should then use != SIGEV_NONE for the any kind of signal notification
bit and == SIGEV_THREAD_ID for the case where we want to deliver to
a particular thread.
But this means we only get a thread reference for SIGEV_THREAD_ID
here:
+ if (notify-notify == (SIGEV_SIGNAL|SIGEV_THREAD_ID)) {
+ /*
+ * This reference will be dropped in really_put_req() when
+ * we're done with the request.
+ */
+ get_task_struct(target);
+ }
But even use it for SIGEV_SIGNAL without SIGEV_THREAD_ID here:
+ if (notify-notify SIGEV_THREAD_ID)
+ ret = send_sigqueue(notify-signo, sigq, notify-target);
+ else
+ ret = send_group_sigqueue(notify-signo, sigq, notify-target);
Or do I miss something?
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm 4/5][AIO] - AIO completion signal notification
On Wed, 29 Nov 2006 13:50:12 +, Christoph Hellwig [EMAIL PROTECTED] wrote:
On Wed, Nov 29, 2006 at 02:08:01PM +0100, S?bastien Dugu? wrote:
On Wed, 29 Nov 2006 10:51:50 +, Christoph Hellwig [EMAIL PROTECTED]
wrote:
I'm a little bit unhappy about the usage of the notify flag. The usage
seems correct but very confusing:
Well, I followed the logic from posix-timers.c, but it may be a poor
choice ;-)
For a start, the SIGEV_* flags are quite confusing (for me at least).
SIGEV_SIGNAL is defined as 0, SIGEV_NONE as 1 and SIGEV_THREAD_ID as 4. I
would rather have seen SIGEV_NONE defined as 0 to avoid all this.
I also wish I knew why those SIGEV_* constants were defined that way.
Ah, I missed that. It explains some of the more wierd bits. I suspect
we should then use != SIGEV_NONE for the any kind of signal notification
bit and == SIGEV_THREAD_ID for the case where we want to deliver to
a particular thread.
Right, that would make things much cleaner. Will try for it.
But this means we only get a thread reference for SIGEV_THREAD_ID
here:
+ if (notify-notify == (SIGEV_SIGNAL|SIGEV_THREAD_ID)) {
+ /*
+* This reference will be dropped in really_put_req()
when
+* we're done with the request.
+*/
+ get_task_struct(target);
+ }
It's the way it is in posix-timers and I'm not sure I understand why. We take
a ref on the specific task if notify is SIGEV_THREAD_ID but not for
SIGEV_SIGNAL.
I'm wondering what I'm missing here, shouldn't we also take a ref on the task
group leader in the SIGEV_SIGNAL case in posix-timers?
But even use it for SIGEV_SIGNAL without SIGEV_THREAD_ID here:
+ if (notify-notify SIGEV_THREAD_ID)
+ ret = send_sigqueue(notify-signo, sigq,
notify-target);
+ else
+ ret = send_group_sigqueue(notify-signo, sigq,
notify-target);
Or do I miss something?
I missing something too here ;-)
If someone cared to explain why there is no ref taken on the task for the
SIGEV_SIGNAL case, it would be much appreciated. Is this a bug in posix-timers?
Thanks,
Sébastien.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
