From owner-freebsd-threads@FreeBSD.ORG Mon Aug 16 08:43:03 2010 Return-Path: Delivered-To: freebsd-threads@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 0031C106566C for ; Mon, 16 Aug 2010 08:43:02 +0000 (UTC) (envelope-from davidxu@freebsd.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id C3E748FC14; Mon, 16 Aug 2010 08:43:02 +0000 (UTC) Received: from [127.0.0.1] (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.4/8.14.4) with ESMTP id o7G8h1ha019142; Mon, 16 Aug 2010 08:43:01 GMT (envelope-from davidxu@freebsd.org) Message-ID: <4C696A96.7020709@freebsd.org> Date: Mon, 16 Aug 2010 16:43:02 +0000 From: David Xu User-Agent: Thunderbird 2.0.0.24 (X11/20100630) MIME-Version: 1.0 To: Kostik Belousov References: <4C63D42D.8040606@freebsd.org> <20100812083006.GR2396@deviant.kiev.zoral.com.ua> <4C642E9B.8000300@freebsd.org> <20100812093353.GS2396@deviant.kiev.zoral.com.ua> <4C650D0F.9060905@freebsd.org> <4C650F27.1000305@freebsd.org> <20100813141402.GW2396@deviant.kiev.zoral.com.ua> <4C65E0FE.2030803@freebsd.org> <20100814144715.GB2396@deviant.kiev.zoral.com.ua> <4C6926D0.2020909@freebsd.org> <20100816082022.GO2396@deviant.kiev.zoral.com.ua> In-Reply-To: <20100816082022.GO2396@deviant.kiev.zoral.com.ua> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: freebsd-threads@freebsd.org Subject: Re: PTHREAD_CANCEL_DEFERRED X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 16 Aug 2010 08:43:03 -0000 Kostik Belousov wrote: > On Mon, Aug 16, 2010 at 11:53:52AM +0000, David Xu wrote: >> Kostik Belousov wrote: >> >>> Missed this, thank you for pointing it out. Updated patch is at >>> http://people.freebsd.org/~kib//misc/cancel_defer.2.patch >> I found SIGCANCEL is masked by >> thr_cancel_deferred(THR_CANCEL_DEFERRED_ENABLE), issignal() does not >> return the masked signal, so how a cancellation point syscall can be >> interrupted by SIGCANCEL ? I think if a thread being canceled calls >> msleep(PCATCH), it should find the signal and return EINTR. >> > Yes, for EINTR and ERESTART case, the thread should be canceled. > Please look at the check_cancel() helper that is called at the syscall > entry and before return. If the check_cancel() decided that the syscall > is cancellation point and the thread in the deferred cancel mode, and > EINTR or ERESTART is supplied as error code, then SIGCANCEL is removed > from the thread signal mask. It is restored in the mask by ast(). I saw your patch has following lines, on syscall entry, if the check_cancel finds that the syscall is cancellation point and SIGCANCEL exists, it returns non-zero value, then the real syscall body at line 319 of subr_trap.c is not executed and prematurely returns. This is not what I want,as you said the syscall's implementation should always be executed, and if the thread will be blocked, then it should be interrupted by existing SIGCANCEL via issignal() which is called by sleepqueue routines. I think the current patch also has potential performance problem, the check_cancel uses PROC_LOCK, which might be a performance hit. @@ -300,6 +332,9 @@ syscallenter(struct thread *td, struct syscall_args *sa) if (error != 0) goto retval; } + error = check_cancel(td, sa->callp, EINTR); + if (error != 0) + goto retval; error = syscall_thread_enter(td, sa->callp); if (error != 0) goto retval;