Attachment 7415 Details for Bug 8836 – Patch from master

[patch] Patch from master

aio_fork.patch (text/plain), 3.97 KB, created by Volker Lendecke on 2012-04-02 07:36:10 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Volker Lendecke

Created: 2012-04-02 07:36:10 UTC

Size: 3.97 KB

patch

obsolete

>From 736c5f26e57760325e9023fa3e5c2beb1d581483 Mon Sep 17 00:00:00 2001
>From: Volker Lendecke <vl@samba.org>
>Date: Sat, 31 Mar 2012 13:34:42 +0200
>Subject: [PATCH 1/2] s3-aio-fork: Fix aio_suspend event hierarchy
>
>We end up here multiple times. There's no real point putting the events into
>the child struct, at the end of this routine we need to free them anyway.
>---
> source3/modules/vfs_aio_fork.c |   18 ++++++++----------
> 1 files changed, 8 insertions(+), 10 deletions(-)
>
>diff --git a/source3/modules/vfs_aio_fork.c b/source3/modules/vfs_aio_fork.c
>index a66d39d..9a978dd 100644
>--- a/source3/modules/vfs_aio_fork.c
>+++ b/source3/modules/vfs_aio_fork.c
>@@ -827,6 +827,8 @@ static int aio_fork_suspend(struct vfs_handle_struct *handle,
> 		 */
> 
> 		for (child = children->children; child != NULL; child = child->next) {
>+			struct tevent_fd *event;
>+
> 			if (child->aiocb == NULL) {
> 				continue;
> 			}
>@@ -841,16 +843,12 @@ static int aio_fork_suspend(struct vfs_handle_struct *handle,
> 				continue;
> 			}
> 
>-			/* We're never using this event on the
>-			 * main event context again... */
>-			TALLOC_FREE(child->sock_event);
>-
>-			child->sock_event = event_add_fd(ev,
>-						child,
>-						child->sockfd,
>-						EVENT_FD_READ,
>-						handle_aio_completion,
>-						child);
>+			event = event_add_fd(ev,
>+					     frame,
>+					     child->sockfd,
>+					     EVENT_FD_READ,
>+					     handle_aio_completion,
>+					     child);
> 
> 			while (1) {
> 				if (tevent_loop_once(ev) == -1) {
>-- 
>1.7.3.4
>
>
>From 3374dbd99c819d763b4469366c3591638f2605d3 Mon Sep 17 00:00:00 2001
>From: Volker Lendecke <vl@samba.org>
>Date: Sat, 31 Mar 2012 13:37:20 +0200
>Subject: [PATCH 2/2] s3-aio-fork: Fix a segfault in vfs_aio_fork
>
>aio_suspend does not signal the main process with a signal, it just waits. The
>aio_fork module does not use the signal at all, it directly calls back into the
>main smbd by calling smbd_aio_complete_aio_ex. This is an abstraction
>violation, but the alternative would have been to use signals where they are
>not needed. However, in wait_for_aio_completion this bites us: With aio_fork we
>call handle_aio_completed twice on the same aio_ex struct: Once from the call
>to handle_aio_completion within the aio_fork module and once from the code in
>wait_for_aio_completion.
>
>This patch fixes it in a pretty bad way by introducing flag variables and more
>state. But the mid-term plan is to replace the posix aio calls from the vfs and
>do pread_send/recv and pwrite_send/recv at the vfs layer, so this will
>significantly change anyway.
>
>Thanks to Kirill Malkin <kirill.malkin@starboardstorage.com> for reporting this
>crash!
>---
> source3/modules/vfs_aio_fork.c |   16 +++++++++-------
> 1 files changed, 9 insertions(+), 7 deletions(-)
>
>diff --git a/source3/modules/vfs_aio_fork.c b/source3/modules/vfs_aio_fork.c
>index 9a978dd..187166c 100644
>--- a/source3/modules/vfs_aio_fork.c
>+++ b/source3/modules/vfs_aio_fork.c
>@@ -101,6 +101,8 @@ struct aio_child {
> 	bool dont_delete;	/* Marked as in use since last cleanup */
> 	bool cancelled;
> 	bool read_cmd;
>+	bool called_from_suspend;
>+	bool completion_done;
> };
> 
> struct aio_child_list {
>@@ -432,6 +434,10 @@ static void handle_aio_completion(struct event_context *event_ctx,
> 		       child->retval.size);
> 	}
> 
>+	if (child->called_from_suspend) {
>+		child->completion_done = true;
>+		return;
>+	}
> 	aio_ex = (struct aio_extra *)child->aiocb->aio_sigevent.sigev_value.sival_ptr;
> 	smbd_aio_complete_aio_ex(aio_ex);
> 	TALLOC_FREE(aio_ex);
>@@ -850,7 +856,9 @@ static int aio_fork_suspend(struct vfs_handle_struct *handle,
> 					     handle_aio_completion,
> 					     child);
> 
>-			while (1) {
>+			child->called_from_suspend = true;
>+
>+			while (!child->completion_done) {
> 				if (tevent_loop_once(ev) == -1) {
> 					goto out;
> 				}
>@@ -859,12 +867,6 @@ static int aio_fork_suspend(struct vfs_handle_struct *handle,
> 					errno = EAGAIN;
> 					goto out;
> 				}
>-
>-				/* We set child->aiocb to NULL in our hooked
>-				 * AIO_RETURN(). */
>-				if (child->aiocb == NULL) {
>-					break;
>-				}
> 			}
> 		}
> 	}
>-- 
>1.7.3.4
>

Flags:

jra: review+

Actions: View

Attachments on bug 8836: 7415