Skip to contentSkip to conversation

Provide owner name via rt_mutex_inquire

PATCH Applied Jan Kiszka 3 participants 7 messages Jul 10, 2008 – Jul 31, 2008

lore
Share & tools
lore

Jan Kiszka

author
#0 lore raw

PATCH 2/2 Provide owner name via rt_mutex_inquire

This can be helpful for debugging the (futile) release attempts of mutexes by tasks that do not own them.

Returning the RT_TASK reference may appear more consistent on first sight, but it cannot be guaranteed that the owner is actually a native task. Therefore this patch uses the symbolic name.

Signed-off-by Jan Kiszka <[email protected]>
---
 include/native/mutex.h    |    3 +++
 ksrc/skins/native/mutex.c |    5 +++++
 2 files changed, 8 insertions(+)
include/native/mutex.h +3 -0
Index: b/include/native/mutex.h
===================================================================
@@ -38,6 +38,9 @@ typedef struct rt_mutex_info {
 
 	char name[XNOBJECT_NAME_LEN]; /**< Symbolic name. */
 
+	char owner[XNOBJECT_NAME_LEN]; /**< Symbolic name of the current owner,
+					    empty if unlocked. */
+
 } RT_MUTEX_INFO;
 
 typedef struct rt_mutex_placeholder {
ksrc/skins/native/mutex.c +5 -0
Index: b/ksrc/skins/native/mutex.c
===================================================================
@@ -597,6 +597,11 @@ int rt_mutex_inquire(RT_MUTEX *mutex, RT
 	strcpy(info->name, mutex->name);
 	info->lockcnt = mutex->lockcnt;
 	info->nwaiters = xnsynch_nsleepers(&mutex->synch_base);
+	if (mutex->lockcnt)
+		strcpy(info->owner,
+		       xnthread_name(xnsynch_owner(&mutex->synch_base)));
+	else
+		info->owner[0] = 0;
 
       unlock_and_exit:
#1 lore raw
Quoted 32 lines · Jan Kiszka

This can be helpful for debugging the (futile) release attempts of mutexes by tasks that do not own them.

Returning the RT_TASK reference may appear more consistent on first sight, but it cannot be guaranteed that the owner is actually a native task. Therefore this patch uses the symbolic name.

Signed-off-by Jan Kiszka <[email protected]>
---
 include/native/mutex.h    |    3 +++
 ksrc/skins/native/mutex.c |    5 +++++
 2 files changed, 8 insertions(+)

Index: b/include/native/mutex.h
===================================================================

include/native/mutex.h +3 -0
@@ -38,6 +38,9 @@ typedef struct rt_mutex_info {
 
 	char name[XNOBJECT_NAME_LEN]; /**< Symbolic name. */
 
+	char owner[XNOBJECT_NAME_LEN]; /**< Symbolic name of the current owner,
+					    empty if unlocked. */
+
 } RT_MUTEX_INFO;
 
 typedef struct rt_mutex_placeholder {

Index: b/ksrc/skins/native/mutex.c
===================================================================

ksrc/skins/native/mutex.c int rt_mutex_inquire(RT_MUTEX *mutex, RT · line 602

Quoted 6 lines
ksrc/skins/native/mutex.c +3 -0
@@ -597,6 +597,11 @@ int rt_mutex_inquire(RT_MUTEX *mutex, RT
 	strcpy(info->name, mutex->name);
 	info->lockcnt = mutex->lockcnt;
 	info->nwaiters = xnsynch_nsleepers(&mutex->synch_base);
+	if (mutex->lockcnt)
+		strcpy(info->owner,
+		       xnthread_name(xnsynch_owner(&mutex->synch_base)));

xnthread_name is not necessarily null terminated, so I would suggest to use:

	snprintf(info->owner, sizeof(info->owner), "%s",
		 xnthread_name(xnsynch_owner(&mutex->synch_base)));

(and not strncpy, strncpy is crippled)

Another possibility would be to use snprintf instead of strncpy in xnobject_copy_name, and use xnobject_copy_name here.

Signature · 1 line
                                                 Gilles.
author
#2 lore raw
Quoted 28 lines · Gilles Chanteperdrix
Quoted 27 lines · Jan Kiszka

This can be helpful for debugging the (futile) release attempts of mutexes by tasks that do not own them.

Returning the RT_TASK reference may appear more consistent on first sight, but it cannot be guaranteed that the owner is actually a native task. Therefore this patch uses the symbolic name.

Signed-off-by Jan Kiszka <[email protected]>
---
 include/native/mutex.h    |    3 +++
 ksrc/skins/native/mutex.c |    5 +++++
 2 files changed, 8 insertions(+)

Index: b/include/native/mutex.h
===================================================================

include/native/mutex.h +3 -0
@@ -38,6 +38,9 @@ typedef struct rt_mutex_info {
 
     char name[XNOBJECT_NAME_LEN]; /**< Symbolic name. */
 
+    char owner[XNOBJECT_NAME_LEN]; /**< Symbolic name of the current
 owner,
+                        empty if unlocked. */
+
 } RT_MUTEX_INFO;
Quoted 14 lines

ksrc/skins/native/mutex.c int rt_mutex_inquire(RT_MUTEX *mutex, RT · line 602

Quoted 12 lines

typedef struct rt_mutex_placeholder {
Index: b/ksrc/skins/native/mutex.c
===================================================================

ksrc/skins/native/mutex.c +3 -0
@@ -597,6 +597,11 @@ int rt_mutex_inquire(RT_MUTEX *mutex, RT
     strcpy(info->name, mutex->name);
     info->lockcnt = mutex->lockcnt;
     info->nwaiters = xnsynch_nsleepers(&mutex->synch_base);
+    if (mutex->lockcnt)
+        strcpy(info->owner,
+               xnthread_name(xnsynch_owner(&mutex->synch_base)));

xnthread_name is not necessarily null terminated, so I would suggest to

In fact, that is lethal for other users as well (trace points and proc dumps come to my mind) and should be fixed ASAP.

Jan

Signature · 2 lines
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux
#3 lore raw
Quoted 43 lines · Jan Kiszka
Quoted 28 lines · Gilles Chanteperdrix
Quoted 27 lines · Jan Kiszka

This can be helpful for debugging the (futile) release attempts of mutexes by tasks that do not own them.

Returning the RT_TASK reference may appear more consistent on first sight, but it cannot be guaranteed that the owner is actually a native task. Therefore this patch uses the symbolic name.

Signed-off-by Jan Kiszka <[email protected]>
---
 include/native/mutex.h    |    3 +++
 ksrc/skins/native/mutex.c |    5 +++++
 2 files changed, 8 insertions(+)

Index: b/include/native/mutex.h
===================================================================

include/native/mutex.h +3 -0
@@ -38,6 +38,9 @@ typedef struct rt_mutex_info {
 
     char name[XNOBJECT_NAME_LEN]; /**< Symbolic name. */
 
+    char owner[XNOBJECT_NAME_LEN]; /**< Symbolic name of the current
 owner,
+                        empty if unlocked. */
+
 } RT_MUTEX_INFO;
Quoted 13 lines

ksrc/skins/native/mutex.c int rt_mutex_inquire(RT_MUTEX *mutex, RT · line 602

Quoted 12 lines

typedef struct rt_mutex_placeholder {
Index: b/ksrc/skins/native/mutex.c
===================================================================

ksrc/skins/native/mutex.c +3 -0
@@ -597,6 +597,11 @@ int rt_mutex_inquire(RT_MUTEX *mutex, RT
     strcpy(info->name, mutex->name);
     info->lockcnt = mutex->lockcnt;
     info->nwaiters = xnsynch_nsleepers(&mutex->synch_base);
+    if (mutex->lockcnt)
+        strcpy(info->owner,
+               xnthread_name(xnsynch_owner(&mutex->synch_base)));

xnthread_name is not necessarily null terminated, so I would suggest to

Quoted 2 lines

In fact, that is lethal for other users as well (trace points and proc dumps come to my mind) and should be fixed ASAP.

I propose the following patch then. It should solve the issue at the source.

Signature · 1 line
                                                 Gilles.
nucleus/types.h +1 -1
Index: include/nucleus/types.h
===================================================================
@@ -97,7 +97,7 @@ typedef atomic_flags_t xnflags_t;
 static inline void xnobject_copy_name(char *dst, const char *src)
 {
     if (src)
-        strncpy(dst, src, XNOBJECT_NAME_LEN);
+	snprintf(dst, XNOBJECT_NAME_LEN, "%s", src);
     else
         *dst = '\0';
 }
#4 lore raw
In reply to Jan Kiszka
Quoted 43 lines · Jan Kiszka
Quoted 28 lines · Gilles Chanteperdrix
Quoted 27 lines · Jan Kiszka

This can be helpful for debugging the (futile) release attempts of mutexes by tasks that do not own them.

Returning the RT_TASK reference may appear more consistent on first sight, but it cannot be guaranteed that the owner is actually a native task. Therefore this patch uses the symbolic name.

Signed-off-by Jan Kiszka <[email protected]>
---
 include/native/mutex.h    |    3 +++
 ksrc/skins/native/mutex.c |    5 +++++
 2 files changed, 8 insertions(+)

Index: b/include/native/mutex.h
===================================================================

include/native/mutex.h +3 -0
@@ -38,6 +38,9 @@ typedef struct rt_mutex_info {
 
     char name[XNOBJECT_NAME_LEN]; /**< Symbolic name. */
 
+    char owner[XNOBJECT_NAME_LEN]; /**< Symbolic name of the current
 owner,
+                        empty if unlocked. */
+
 } RT_MUTEX_INFO;
Quoted 13 lines

ksrc/skins/native/mutex.c int rt_mutex_inquire(RT_MUTEX *mutex, RT · line 602

Quoted 12 lines

typedef struct rt_mutex_placeholder {
Index: b/ksrc/skins/native/mutex.c
===================================================================

ksrc/skins/native/mutex.c +3 -0
@@ -597,6 +597,11 @@ int rt_mutex_inquire(RT_MUTEX *mutex, RT
     strcpy(info->name, mutex->name);
     info->lockcnt = mutex->lockcnt;
     info->nwaiters = xnsynch_nsleepers(&mutex->synch_base);
+    if (mutex->lockcnt)
+        strcpy(info->owner,
+               xnthread_name(xnsynch_owner(&mutex->synch_base)));

xnthread_name is not necessarily null terminated, so I would suggest to

Quoted 3 lines

In fact, that is lethal for other users as well (trace points and proc dumps come to my mind) and should be fixed ASAP.

xnthread_name() shall be considered as null terminated, the payload area is actually XNOBJECT_NAME_LEN - 1. Users who do not assume this are the ones that need fixing (like xnobject_copy_name).

Signature · 1 line
Philippe.
#5 lore raw
In reply to Gilles Chanteperdrix
Quoted 1 line · Gilles Chanteperdrix

Another possibility would be to use snprintf instead of strncpy in xnobject_copy_name, and use xnobject_copy_name here.

Yet another possibility would be to use strlcpy, available in kernel-space. But it is funny, glibc people does not seem to like strlcpy for reasons I do not quite understand...

http://sources.redhat.com/ml/libc-alpha/2000-08/msg00053.html

Signature · 1 line
                                                 Gilles.
Applied
#6 lore raw
In reply to Jan Kiszka
Quoted 9 lines · Jan Kiszka

This can be helpful for debugging the (futile) release attempts of mutexes by tasks that do not own them.

Returning the RT_TASK reference may appear more consistent on first sight, but it cannot be guaranteed that the owner is actually a native task. Therefore this patch uses the symbolic name.

Signed-off-by Jan Kiszka <[email protected]>

Applied to the trunk/, thanks.

PS: no 2.4.x backport due to the ABI breakage this involves.

---

Quoted 40 lines
 include/native/mutex.h    |    3 +++
 ksrc/skins/native/mutex.c |    5 +++++
 2 files changed, 8 insertions(+)

Index: b/include/native/mutex.h
===================================================================

include/native/mutex.h +3 -0
@@ -38,6 +38,9 @@ typedef struct rt_mutex_info {
 
 	char name[XNOBJECT_NAME_LEN]; /**< Symbolic name. */
 
+	char owner[XNOBJECT_NAME_LEN]; /**< Symbolic name of the current owner,
+					    empty if unlocked. */
+
 } RT_MUTEX_INFO;
 
 typedef struct rt_mutex_placeholder {

Index: b/ksrc/skins/native/mutex.c
===================================================================

ksrc/skins/native/mutex.c +5 -0
@@ -597,6 +597,11 @@ int rt_mutex_inquire(RT_MUTEX *mutex, RT
 	strcpy(info->name, mutex->name);
 	info->lockcnt = mutex->lockcnt;
 	info->nwaiters = xnsynch_nsleepers(&mutex->synch_base);
+	if (mutex->lockcnt)
+		strcpy(info->owner,
+		       xnthread_name(xnsynch_owner(&mutex->synch_base)));
+	else
+		info->owner[0] = 0;
 
       unlock_and_exit:

_______________________________________________
Xenomai-core mailing list
[email protected]
https://mail.gna.org/listinfo/xenomai-core

Signature · 1 line
Philippe.