I am sorry for delay in answering, you will see the reason for it below.
On Fri, Jan 15, 2016 at 01:53:14PM +0200, Boris Astardzhiev wrote:
> kb>Big issue with the implementation is the interposing stuff, why do you
> kb> need it at all ? Is it to correctly handle cancellation, to not fall
> kb> into sleepable syscall when previous loop step was cancelled ?
> Yes. I initially thought it was better to use the interposing table.
>
> kb> If yes, you _can_ use pthread_testcancel(3) etc in libc. Libc provides
> kb> stubs for them with trivial implementation, which is reset to the real
> kb> one if libthr is loaded. Then you can simplify your patch
> significantly,
> kb> avoiding the need for interposing and writing the loops both in libc and
> kb> libthr.
> Got it. See patch. I think I removed the interposing stuff as
> suggested. I didn't know about the stubs. But how for instance
> pthread_testcancel() will cope with sleeping recvmmsg for example? I'm
> not sure about the cancellation stuff here. Probably my approach is
> not correct? I looked through lib/ for an example and only stumbled on
> lib/libc/gen/sem.c where pthread_testcancel() is used but again I'm
> not sure if I'm tackling it correctly in the calls.
pthread_testcancel() does not need to do anything WRT sleeps in recvmsg.
It adds the cancellation point, which you already have by the call
to recvmsg().
> kb> BTW, do you have tests for the cancellation of the new functions ?
> Unfortunately no. Ideas and guidelines how to stress test the calls
> regarding functionality
> and especially cancellation?
Write a test which would do controlled sendmmsg or recvmmsg in one
thread, e.g. over the unix domain socket, and another thread doing
cancellation. You should check both async and deferred modes.
>
> kb> Again, the patch lacks man page updates.
> I'll try to write some soon.
So I thought how to implement the desired behaviour of the recvmmsg and
recvmmsg loops WRT cancellation using sendmsg(2) and POSIX pthread API
and realized that it is impossible. In other words, if you want to write
a loop with several calls to say recvmsg and not cancel the loop if
anything was already read by previous recvmsg calls, you cannot.
I also discussed this with jilles@ who is our POSIX expert, and who
confirmed my conclusion.
After thinking some more, I believe I managed to construct a possible
way to implement this, in libc, with some libthr extensions. Basically,
the idea is to have function pthread_cancel_is_pending_np(), which
would return the state of pending cancel. For some time I thought that
this cannot work, because cancellation could happen between call to
the cancel_is_pending() and next recvmmsg(). But, libc has a privilege
of having access to the syscalls without libthr interposing, just
call __sys_recvmmsg(), which would give EINTR on the cancel attempt.
This is an implementation detail, but we can rely on it in implementation.
In other words, the structure of the code would be like this
for (i = 0; i < vlen; i++) {
if (pthread_cancel_is_pending_np())
goto out;
error = __sys_recvmsg(...);
if (error != 0)
goto out;
...
}
out:
if (any data received)
return (0);
pthread_testcancel(); /* no data received, cancel us if requested */
handle errors
...
Patch to implement pthread_cancel_is_pending_np() is below.
We have three viable strategies, after all
1. Ignore cancellation at all for now, since cancellation is not very
popular among apps. This means that your latest, libc only patch
should be finalized.
2. Implement cancellation with libthr interposing, i.e. finalize your
libc + libthr patch.
3. Implement cancellation in libc as outlined above.
It is your choice of the approach.
diff --git a/include/pthread.h b/include/pthread.h
index 8b59223..c6858bb 100644
--- a/include/pthread.h
+++ b/include/pthread.h
@@ -298,6 +298,7 @@ void pthread_testcancel(void);
int pthread_getprio(pthread_t);
int pthread_setprio(pthread_t, int);
void pthread_yield(void);
+int pthread_cancel_is_pending_np(void);
#endif
int pthread_mutexattr_getprioceiling(pthread_mutexattr_t *,
diff --git a/lib/libc/gen/Symbol.map b/lib/libc/gen/Symbol.map
index ee4d619..5cfbd6f 100644
--- a/lib/libc/gen/Symbol.map
+++ b/lib/libc/gen/Symbol.map
@@ -411,6 +411,7 @@ FBSD_1.3 {
FBSD_1.4 {
scandir_b;
+ pthread_cancel_is_pending_np;
};
FBSDprivate_1.0 {
@@ -439,6 +440,7 @@ FBSDprivate_1.0 {
_pthread_cancel;
_pthread_cancel_enter;
_pthread_cancel_leave;
+ _pthread_cancel_is_pending_np;
_pthread_cleanup_pop;
_pthread_cleanup_push;
_pthread_cond_broadcast;
diff --git a/lib/libc/gen/_pthread_stubs.c b/lib/libc/gen/_pthread_stubs.c
index bd35bd2..b9b9c14 100644
--- a/lib/libc/gen/_pthread_stubs.c
+++ b/lib/libc/gen/_pthread_stubs.c
@@ -125,6 +125,7 @@ pthread_func_entry_t __thr_jtable[PJT_MAX] = {
{PJT_DUAL_ENTRY(stub_zero)}, /* PJT_CLEANUP_PUSH_IMP */
{PJT_DUAL_ENTRY(stub_zero)}, /* PJT_CANCEL_ENTER */
{PJT_DUAL_ENTRY(stub_zero)}, /* PJT_CANCEL_LEAVE */
+ {PJT_DUAL_ENTRY(stub_zero)}, /* PJT_CANCEL_IS_PENDING_NP */
};
/*
@@ -275,6 +276,7 @@ STUB_FUNC1(__pthread_cleanup_pop_imp, PJT_CLEANUP_POP_IMP,
int, int)
STUB_FUNC2(__pthread_cleanup_push_imp, PJT_CLEANUP_PUSH_IMP, void, void*, void
*);
STUB_FUNC1(_pthread_cancel_enter, PJT_CANCEL_ENTER, int, int)
STUB_FUNC1(_pthread_cancel_leave, PJT_CANCEL_LEAVE, int, int)
+STUB_FUNC(pthread_cancel_is_pending_np, PJT_CANCEL_IS_PENDING_NP, int)
static int
stub_zero(void)
diff --git a/lib/libc/include/libc_private.h b/lib/libc/include/libc_private.h
index 5caf9a3..4f9f90d 100644
--- a/lib/libc/include/libc_private.h
+++ b/lib/libc/include/libc_private.h
@@ -168,6 +168,7 @@ typedef enum {
PJT_CLEANUP_PUSH_IMP,
PJT_CANCEL_ENTER,
PJT_CANCEL_LEAVE,
+ PJT_CANCEL_IS_PENDING_NP,
PJT_MAX
} pjt_index_t;
diff --git a/lib/libc/include/namespace.h b/lib/libc/include/namespace.h
index 739d7b1..42967a2 100644
--- a/lib/libc/include/namespace.h
+++ b/lib/libc/include/namespace.h
@@ -115,6 +115,7 @@
#define pthread_barrierattr_init
_pthread_barrierattr_init
#define pthread_barrierattr_setpshared
_pthread_barrierattr_setpshared
#define pthread_cancel _pthread_cancel
+#define pthread_cancel_is_pending_np
_pthread_cancel_is_pending_np
#define pthread_cond_broadcast _pthread_cond_broadcast
#define pthread_cond_destroy _pthread_cond_destroy
#define pthread_cond_init _pthread_cond_init
diff --git a/lib/libthr/pthread.map b/lib/libthr/pthread.map
index 0903989..a7c4573 100644
--- a/lib/libthr/pthread.map
+++ b/lib/libthr/pthread.map
@@ -172,6 +172,7 @@ FBSDprivate_1.0 {
_pthread_cancel;
_pthread_cancel_enter;
_pthread_cancel_leave;
+ _pthread_cancel_is_pending_np;
_pthread_cleanup_pop;
_pthread_cleanup_push;
_pthread_cond_broadcast;
@@ -317,3 +318,8 @@ FBSD_1.1 {
FBSD_1.2 {
pthread_getthreadid_np;
};
+
+FBSD_1.4 {
+ pthread_cancel_is_pending_np;
+ };
+
\ No newline at end of file
diff --git a/lib/libthr/thread/thr_cancel.c b/lib/libthr/thread/thr_cancel.c
index beae707..9592a82 100644
--- a/lib/libthr/thread/thr_cancel.c
+++ b/lib/libthr/thread/thr_cancel.c
@@ -37,6 +37,7 @@ __weak_reference(_pthread_cancel, pthread_cancel);
__weak_reference(_pthread_setcancelstate, pthread_setcancelstate);
__weak_reference(_pthread_setcanceltype, pthread_setcanceltype);
__weak_reference(_pthread_testcancel, pthread_testcancel);
+__weak_reference(_pthread_cancel_is_pending_np, pthread_cancel_is_pending_np);
static inline void
testcancel(struct pthread *curthread)
@@ -175,3 +176,10 @@ _pthread_cancel_leave(int maycancel)
{
_thr_cancel_leave(_get_curthread(), maycancel);
}
+
+int
+_pthread_cancel_is_pending_np(void)
+{
+
+ return (SHOULD_CANCEL(_get_curthread()));
+}
diff --git a/lib/libthr/thread/thr_init.c b/lib/libthr/thread/thr_init.c
index e0400e4..3d319a6 100644
--- a/lib/libthr/thread/thr_init.c
+++ b/lib/libthr/thread/thr_init.c
@@ -263,7 +263,8 @@ static pthread_func_t jmp_table[][2] = {
{DUAL_ENTRY(__pthread_cleanup_pop_imp)},/* PJT_CLEANUP_POP_IMP */
{DUAL_ENTRY(__pthread_cleanup_push_imp)},/* PJT_CLEANUP_PUSH_IMP */
{DUAL_ENTRY(_pthread_cancel_enter)}, /* PJT_CANCEL_ENTER */
- {DUAL_ENTRY(_pthread_cancel_leave)} /* PJT_CANCEL_LEAVE */
+ {DUAL_ENTRY(_pthread_cancel_leave)}, /* PJT_CANCEL_LEAVE */
+ {DUAL_ENTRY(_pthread_cancel_is_pending_np)}, /*
PJT_CANCEL_IS_PENDING_NP */
};
static int init_once = 0;
_______________________________________________
freebsd-net@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
