PTHREAD_CANCEL_DEFERRED

[David Xu]

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;