Date: Tue, 17 Aug 2010 09:29:52 +0000 From: David Xu <davidxu@freebsd.org> To: Kostik Belousov <kostikbel@gmail.com> Cc: freebsd-threads@freebsd.org Subject: Re: PTHREAD_CANCEL_DEFERRED Message-ID: <4C6A5690.6060000@freebsd.org> In-Reply-To: <20100816104303.GP2396@deviant.kiev.zoral.com.ua> References: <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> <4C696A96.7020709@freebsd.org> <20100816104303.GP2396@deviant.kiev.zoral.com.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
Kostik Belousov wrote: > On Mon, Aug 16, 2010 at 04:43:02PM +0000, David Xu wrote: >> 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 > No, I do not think what you describe is completely right behaviour. > As I understand SUSv4, the deferred cancel behaviour should be > as following: > - when we reach cancellation point with cancel already pending, > the syscall should not be executed at all, cancel happens and > cleanup handlers executed before thread exiting. > - when we reach cancellation point without cancel pending, and syscall > goes to sleep, and cancellation request arrives during the sleep, > again the syscall should be aborted, and cancellation happens. > so what's the problem with current libthr code ? it already did above in this way. using close() as an example, what does your modification gains ? is the kern_close() always executed for close() syscall? or if the cancellation is already pending and the syscall prematurely returns, will the file descriptor be leaked ? if it is leaked, then your modification is not better than current libthr code, it is even worse than current one, because your code involves kernel. > So I check for the cancellation at two points: one is the syscall > entry, to catch already pending cancellation request. Second is on > the syscall exit path, where cancellation is only checked in the > case the sleep was aborted. >> performance problem, the check_cancel uses PROC_LOCK, which might be a >> performance hit. > Might be. Lets first make sure that the semantic is right. > >> @@ -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; >>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4C6A5690.6060000>