nuttx/nuttx-update
1
2
Fork 1
Code Issues 1 Pull requests 1 Projects Releases Packages Wiki Activity Actions

Squashed commit of the following:

Browse source 
libs/libc/pthread/pthread_spinlock.c:  Resolve several TODO issues by accessing up_testset() via the boardctl() interface rather than attempting to call it directly.

    configs/boardctl.c, include/sys/boardctl.h:  Add access to architecture-specific up_testset() via boardctl().

    arch/Kconfig's, sched/Kconfig, and include/nuttx/spinlock.h:  Spinlocks are not available unless the architecture supports the up_testset() operation.
This commit is contained in:
Gregory Nutt 2019-03-04 14:22:50 -06:00
parent 8326a4fa9c
commit 5fe6981c9a
12 changed files with 132 additions and 76 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

38
TODO
View file

@ -14,7 +14,7 @@ nuttx/:
(1) Memory Management (mm/)
(0) Power Management (drivers/pm)
(5) Signals (sched/signal, arch/)
(3) pthreads (sched/pthread, libs/libc/pthread)
(2) pthreads (sched/pthread, libs/libc/pthread)
(0) Message Queues (sched/mqueue)
(10) Kernel/Protected Build
(3) C++ Support
@ -896,42 +896,6 @@ o pthreads (sched/pthreads libs/libc/pthread)
Priority: Low. This change would improve real-time performance of the
OS but is not otherwise required.
Title: PTHREAD SPINLOCK ISSUES
Description: Basic support for pthread spinlocks was added on 2019-02-28,
However, there are some issues related to the implementation
as it stands.
1. The Use is not fully verified. How are these interfaces
used? How should they be tested?
2. These interfaces depend on architecture specific support
support fraom each architecture that support is not be
fully available to the pthread library in all cases.
There should a setting, say CONFIG_ARCH_HAVE_TESTSET=y
that indicates if the architect provides up_testset().
3. It is also restricted to CONFIG_BUILD_FLAT because the
critical test and set function (up_testset()) as
prototyped in include/nuttx/spinlock() does not permit an
inline function or macro to be used in user-mode
application space.
Update: A better alternative would be to move the CPU-
depending up_testset() implementation from the arch/
directories and into libs/libc/machine.
That assumes, of course, that the spinlock operation can
be perform with only user level priveleges. That may not
always be the case.
For these reasons, the selection is marked EXPERIMENTAL.
Status: Open
Priority: Low. These are new interfaces and not yet in wide use. The
use case for these interfaces is not well defined, but I
think it would be restricted to multi-core CPU implmentations
which are not widespread in the embedded world.
o Message Queues (sched/mqueue)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

6
arch/Kconfig
View file

@ -66,6 +66,7 @@ config ARCH_SIM
select ARCH_HAVE_TLS
select ARCH_HAVE_TICKLESS
select ARCH_HAVE_POWEROFF
select ARCH_HAVE_TESTSET
select SERIAL_CONSOLE
---help---
Linux/Cywgin user-mode simulation.
@ -79,6 +80,7 @@ config ARCH_XTENSA
bool "Xtensa"
select ARCH_HAVE_STACKCHECK
select ARCH_HAVE_CUSTOMOPT
select ARCH_HAVE_TESTSET
---help---
Cadence® Tensilica® Xtensa® actictures.
@ -221,6 +223,10 @@ config ARCH_HAVE_RESET
bool
default n
config ARCH_HAVE_TESTSET
bool
default n
config ARCH_HAVE_FETCHADD
bool
default n

12
arch/arm/Kconfig
View file

@ -495,6 +495,7 @@ config ARCH_CORTEXM3
select ARCH_HAVE_LAZYFPU
select ARCH_HAVE_HIPRI_INTERRUPT
select ARCH_HAVE_RESET
select ARCH_HAVE_TESTSET
select ARCH_HAVE_HARDFAULT_DEBUG
select ARCH_HAVE_MEMFAULT_DEBUG
@ -511,6 +512,7 @@ config ARCH_CORTEXM4
select ARCH_HAVE_LAZYFPU
select ARCH_HAVE_HIPRI_INTERRUPT
select ARCH_HAVE_RESET
select ARCH_HAVE_TESTSET
select ARCH_HAVE_HARDFAULT_DEBUG
select ARCH_HAVE_MEMFAULT_DEBUG
@ -524,6 +526,7 @@ config ARCH_CORTEXM7
select ARCH_HAVE_LAZYFPU
select ARCH_HAVE_HIPRI_INTERRUPT
select ARCH_HAVE_RESET
select ARCH_HAVE_TESTSET
select ARCH_HAVE_COHERENT_DCACHE if ELF || MODULE
select ARCH_HAVE_HARDFAULT_DEBUG
select ARCH_HAVE_MEMFAULT_DEBUG
@ -533,6 +536,7 @@ config ARCH_CORTEXA5
default n
select ARCH_HAVE_MMU
select ARCH_USE_MMU
select ARCH_HAVE_TESTSET
select ARCH_HAVE_COHERENT_DCACHE if ELF || MODULE
config ARCH_CORTEXA8
@ -540,6 +544,7 @@ config ARCH_CORTEXA8
default n
select ARCH_HAVE_MMU
select ARCH_USE_MMU
select ARCH_HAVE_TESTSET
select ARCH_HAVE_COHERENT_DCACHE if ELF || MODULE
config ARCH_CORTEXA9
@ -547,12 +552,14 @@ config ARCH_CORTEXA9
default n
select ARCH_HAVE_MMU
select ARCH_USE_MMU
select ARCH_HAVE_TESTSET
select ARCH_HAVE_COHERENT_DCACHE if ELF || MODULE
config ARCH_CORTEXR4
bool
default n
select ARCH_HAVE_MPU
select ARCH_HAVE_TESTSET
select ARCH_HAVE_COHERENT_DCACHE if ELF || MODULE
config ARCH_CORTEXR4F
@ -560,12 +567,14 @@ config ARCH_CORTEXR4F
default n
select ARCH_HAVE_MPU
select ARCH_HAVE_FPU
select ARCH_HAVE_TESTSET
select ARCH_HAVE_COHERENT_DCACHE if ELF || MODULE
config ARCH_CORTEXR5
bool
default n
select ARCH_HAVE_MPU
select ARCH_HAVE_TESTSET
select ARCH_HAVE_COHERENT_DCACHE if ELF || MODULE
config ARCH_CORTEXR5F
@ -573,12 +582,14 @@ config ARCH_CORTEXR5F
default n
select ARCH_HAVE_MPU
select ARCH_HAVE_FPU
select ARCH_HAVE_TESTSET
select ARCH_HAVE_COHERENT_DCACHE if ELF || MODULE
config ARCH_CORTEXR7
bool
default n
select ARCH_HAVE_MPU
select ARCH_HAVE_TESTSET
select ARCH_HAVE_COHERENT_DCACHE if ELF || MODULE
config ARCH_CORTEXR7F
@ -586,6 +597,7 @@ config ARCH_CORTEXR7F
default n
select ARCH_HAVE_MPU
select ARCH_HAVE_FPU
select ARCH_HAVE_TESTSET
select ARCH_HAVE_COHERENT_DCACHE if ELF || MODULE
config ARCH_FAMILY

7
configs/Kconfig
View file

@ -2596,6 +2596,13 @@ config BOARDCTL_USBDEVCTRL
---help---
Enables support for the BOARDIOC_USBDEV_CONTROL boardctl() command.
config BOARDCTL_TESTSET
bool "Architecture-specific test/set operation"
default n
---help---
Enables support for the BOARDIOC_SPINLOCK boardctl() command.
Architecture specific logic must provide up_testset() interface.
config BOARDCTL_IOCTL
bool "Board-specific boardctl() commands"
default n

32
configs/boardctl.c
View file

@ -1,7 +1,7 @@
/****************************************************************************
* configs/boardctl.c
*
* Copyright (C) 2015-2017, 2018 Gregory Nutt. All rights reserved.
* Copyright (C) 2015-2017, 2018-2019 Gregory Nutt. All rights reserved.
* Author: Gregory Nutt <gnutt@nuttx.org>
*
* Redistribution and use in source and binary forms, with or without
@ -60,6 +60,10 @@
# include <nuttx/usb/composite.h>
#endif
#ifdef CONFIG_BOARDCTL_TESTSET
# include <nuttx/spinlock.h>
#endif
#ifdef CONFIG_LIB_BOARDCTL
/****************************************************************************
@ -420,6 +424,32 @@ int boardctl(unsigned int cmd, uintptr_t arg)
break;
#endif
#ifdef CONFIG_BOARDCTL_TESTSET
/* CMD: BOARDIOC_TESTSET
* DESCRIPTION: Access architecture-specific up_testset() operation
* ARG: A pointer to a write-able spinlock object. On success
* the preceding spinlock state is returned: 0=unlocked,
* 1=locked.
* CONFIGURATION: CONFIG_BOARDCTL_TESTSET
* DEPENDENCIES: Architecture-specific logic provides up_testset()
*/
case BOARDIOC_TESTSET:
{
volatile FAR spinlock_t *lock = (volatile FAR spinlock_t *)arg;
if (lock == NULL)
{
ret = -EINVAL;
}
else
{
ret = up_testset(lock) == SP_LOCKED ? 1 : 0;
}
}
break;
#endif
default:
{
#ifdef CONFIG_BOARDCTL_IOCTL

2
include/nuttx/arch.h
View file

@ -1,7 +1,7 @@
/****************************************************************************
* include/nuttx/arch.h
*
* Copyright (C) 2007-2018 Gregory Nutt. All rights reserved.
* Copyright (C) 2007-2019 Gregory Nutt. All rights reserved.
* Author: Gregory Nutt <gnutt@nuttx.org>
*
* Redistribution and use in source and binary forms, with or without

2
include/nuttx/spinlock.h
View file

@ -1,7 +1,7 @@
/****************************************************************************
* include/nuttx/spinlock.h
*
* Copyright (C) 2016 Gregory Nutt. All rights reserved.
* Copyright (C) 2016, 2019 Gregory Nutt. All rights reserved.
* Author: Gregory Nutt <gnutt@nuttx.org>
*
* Redistribution and use in source and binary forms, with or without

15
include/sys/boardctl.h
View file

@ -1,7 +1,7 @@
/****************************************************************************
* include/sys/boardctl.h
*
* Copyright (C) 2015-2018 Gregory Nutt. All rights reserved.
* Copyright (C) 2015-2019 Gregory Nutt. All rights reserved.
* Author: Gregory Nutt <gnutt@nuttx.org>
*
* Redistribution and use in source and binary forms, with or without
@ -110,10 +110,18 @@
* DEPENDENCIES: Board logic must provide board_<usbdev>_initialize()
*
* CMD: BOARDIOC_NX_START
* DESCRIPTION: Start the NX servier
* DESCRIPTION: Start the NX server
* ARG: None
* CONFIGURATION: CONFIG_NX
* DEPENDENCIES: Base graphics logic provides nxmu_start()
*
* CMD: BOARDIOC_TESTSET
* DESCRIPTION: Access architecture-specific up_testset() operation
* ARG: A pointer to a write-able spinlock object. On success
* the preceding spinlock state is returned: 0=unlocked,
* 1=locked.
* CONFIGURATION: CONFIG_BOARDCTL_TESTSET
* DEPENDENCIES: Architecture-specific logic provides up_testset()
*/
#define BOARDIOC_INIT _BOARDIOC(0x0001)
@ -125,6 +133,7 @@
#define BOARDIOC_OS_SYMTAB _BOARDIOC(0x0007)
#define BOARDIOC_USBDEV_CONTROL _BOARDIOC(0x0008)
#define BOARDIOC_NX_START _BOARDIOC(0x0009)
#define BOARDIOC_TESTSET _BOARDIOC(0x000a)
/* If CONFIG_BOARDCTL_IOCTL=y, then board-specific commands will be support.
* In this case, all commands not recognized by boardctl() will be forwarded
@ -133,7 +142,7 @@
* User defined board commands may begin with this value:
*/
#define BOARDIOC_USER _BOARDIOC(0x0009)
#define BOARDIOC_USER _BOARDIOC(0x000b)
/****************************************************************************
* Public Type Definitions

17
libs/libc/pthread/Kconfig
View file

@ -4,23 +4,14 @@
#
menu "pthread support"
depends on !CONFIG_DISABLE_PTHREAD
depends on !DISABLE_PTHREAD
config PTHREAD_SPINLOCKS
bool "pthread spinlock support"
default n
depends on SPINLOCK && BUILD_FLAT && EXPERIMENTAL
depends on SPINLOCK && LIB_BOARDCTL
select BOARDCTL_TESTSET
---help---
Enable EXPERIMENTAL support for pthread spinlocks.
This is marked EXPERIMENTAL for two reasons (1) the use case is not
fully verified, and (2) it depends on architecture specific
support provided by each architecture that may not be fully
available to the pthread library.
It also currently depends on CONFIG_BUILD_FLAT because the
critical test and set function (up_testset()) as prototyped in
include/nuttx/spinlock() does not permit an inline function or
macro to be used in user-mode application space.
Enable support for pthread spinlocks.
endmenu # pthread support

72
libs/libc/pthread/pthread_spinlock.c
View file

@ -40,20 +40,19 @@
#include <nuttx/config.h>
#include <sys/types.h>
#include <sys/boardctl.h>
#include <pthread.h>
#include <sched.h>
/* The architecture specific spinlock.h header file must provide the
* following:
*
* SP_LOCKED - A definition of the locked state value (usually 1)
* SP_UNLOCKED - A definition of the unlocked state value (usually 0)
* spinlock_t - The type of a spinlock memory object (usually uint8_t).
* SP_DSB(), - Memory barriers
* SP_DMB
* SP_LOCKED - A definition of the locked state value (usually 1)
* SP_UNLOCKED - A definition of the unlocked state value (usually 0)
* spinlock_t - The type of a spinlock memory object (usually uint8_t).
*/
#include <nuttx/spinlock.h>
#include <arch/spinlock.h>
#ifdef CONFIG_PTHREAD_SPINLOCKS
@ -184,6 +183,7 @@ int pthread_spin_destroy(pthread_spinlock_t *lock)
int pthread_spin_lock(pthread_spinlock_t *lock)
{
pthread_t me = pthread_self();
int ret;
DEBUGASSERT(lock != NULL);
if (lock == NULL)
@ -195,14 +195,32 @@ int pthread_spin_lock(pthread_spinlock_t *lock)
return EDEADLOCK;
}
while (up_testset(&lock->sp_lock) == SP_LOCKED)
/* Loop until we successfully take the spinlock (i.e., until the previous
* state of the spinlock was SP_UNLOCKED). NOTE that the test/set operaion
* is performed via boardctl() to avoid a variety of issues.
*/
do
{
SP_DSB();
ret = boardctl(BOARDIOC_TESTSET, (uintptr_t)&lock->sp_lock);
}
while (ret == 1);
/* Check for success (previous state was SP_UNLOCKED) */
if (ret == 0)
{
lock->sp_holder = me;
}
else
{
/* An error of some kind is the only other possibility */
DEBUGASSERT(ret < 0);
ret = -ret;
}
lock->sp_holder = me;
SP_DMB();
return OK;
return ret;
}
/****************************************************************************
@ -229,26 +247,40 @@ int pthread_spin_lock(pthread_spinlock_t *lock)
int pthread_spin_trylock(pthread_spinlock_t *lock)
{
pthread_t me = pthread_self();
int ret;
DEBUGASSERT(lock != NULL);
if (lock == NULL)
{
return EINVAL;
ret = EINVAL;
}
else if (lock->sp_holder == me)
{
return OK;
}
else if (up_testset(&lock->sp_lock) == SP_LOCKED)
{
lock->sp_holder = me;
SP_DMB();
return OK;
ret = OK;
}
else
{
return EBUSY;
/* Perform the test/set operation via boardctl() */
ret = boardctl(BOARDIOC_TESTSET, (uintptr_t)&lock->sp_lock);
switch (ret)
{
case 0: /* Previously unlocked. We hold the spinlock */
lock->sp_holder = me;
break;
case 1: /* Previously locked. We did not get the spinlock */
ret = EBUSY;
break;
default:
DEBUGASSERT(ret < 0);
ret = -ret;
break;
}
}
return ret;
}
/****************************************************************************

1
sched/Kconfig
View file

@ -257,6 +257,7 @@ menu "Tasks and Scheduling"
config SPINLOCK
bool "Support Spinlocks"
default n
depends on ARCH_HAVE_TESTSET
---help---
Enables suppport for spinlocks. Spinlocks are current used only for
SMP suppport.

4
sched/semaphore/spinlock.c
View file

@ -458,6 +458,7 @@ void spin_unlockr(FAR struct spinlock_s *lock)
*
****************************************************************************/
#ifdef CONFIG_SMP
void spin_setbit(FAR volatile cpu_set_t *set, unsigned int cpu,
FAR volatile spinlock_t *setlock,
FAR volatile spinlock_t *orlock)
@ -499,6 +500,7 @@ void spin_setbit(FAR volatile cpu_set_t *set, unsigned int cpu,
spin_unlock(setlock);
up_irq_restore(flags);
}
#endif
/****************************************************************************
* Name: spin_clrbit
@ -517,6 +519,7 @@ void spin_setbit(FAR volatile cpu_set_t *set, unsigned int cpu,
*
****************************************************************************/
#ifdef CONFIG_SMP
void spin_clrbit(FAR volatile cpu_set_t *set, unsigned int cpu,
FAR volatile spinlock_t *setlock,
FAR volatile spinlock_t *orlock)
@ -560,5 +563,6 @@ void spin_clrbit(FAR volatile cpu_set_t *set, unsigned int cpu,
spin_unlock(setlock);
up_irq_restore(flags);
}
#endif
#endif /* CONFIG_SPINLOCK */