[Top] [All Lists]

Re: [PATCH v2 6/8] xfsdump: process thread exit status

To: Bill Kendall <wkendall@xxxxxxx>
Subject: Re: [PATCH v2 6/8] xfsdump: process thread exit status
From: Alex Elder <aelder@xxxxxxx>
Date: Mon, 7 Nov 2011 20:03:22 -0600
Cc: <xfs@xxxxxxxxxxx>
In-reply-to: <1320699511-12281-7-git-send-email-wkendall@xxxxxxx>
References: <1320699511-12281-1-git-send-email-wkendall@xxxxxxx> <1320699511-12281-7-git-send-email-wkendall@xxxxxxx>
Reply-to: <aelder@xxxxxxx>
On Mon, 2011-11-07 at 14:58 -0600, Bill Kendall wrote:
> When IRIX sprocs were in use, the main thread was notified of a thread
> exit just as if a child process exited -- it received SIGCHLD. The main
> thread would grab the pid and exit status, then call cldmgr_died() to
> inform it that the child was gone so the slot in the child array could
> be freed up for reuse.
> This patch implements a similar mechanism for pthreads. The "c_busy"
> field in struct cld has been replaced with a "c_state" field that
> indicates whether the array slot is free (C_AVAIL), in use (C_ALIVE), or
> is waiting to be joined (C_EXITED). Additionally a "c_exit_code" field
> has been added to store the thread's exit value. Normally this is set
> when the thread entry function returns, but it is initialized to
> EXIT_INTERRUPT in case the thread is cancelled or calls pthread_exit()
> rather than returning (neither of which happens in the code today).
> When the child thread starts, it registers a pthread cleanup handler
> which takes care of marking the child as C_EXITED and notifies the main
> thread that a child is gone. Doing this in a cleanup handler ensures
> that it's done regardless of how the thread exits. The main thread's
> loop is based around sigsuspsend(), so the notification is done by
> sending SIGUSR1. The main thread will then call cldmgr_join() to join
> all exited threads and obtain their exit status.
> Additional changes:
> * cldmgr_findbypid() has been removed, it's no longer referenced.
> * stream_dead() no longer grabs the lock(), because it's called
>   only from cldmgr_join() which already holds the lock().
> Signed-off-by: Bill Kendall <wkendall@xxxxxxx>
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>

Looks OK to me.  Again, I've looked over this
pretty closely, but have not been as thorough
as I often am.

Reviewed-by: Alex Elder <aelder@xxxxxxx>

<Prev in Thread] Current Thread [Next in Thread>