Skip to content

drivers/syslog: reuse rmutex_t for the recursive check #6435

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pkarashchenko merged 1 commit into from
Jun 16, 2022
Merged

drivers/syslog: reuse rmutex_t for the recursive check #6435

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 12 additions & 40 deletions drivers/syslog/syslog_device.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
#include <nuttx/arch.h>
#include <nuttx/kmalloc.h>
#include <nuttx/fs/fs.h>
#include <nuttx/semaphore.h>
#include <nuttx/mutex.h>
#include <nuttx/syslog/syslog.h>
#include <nuttx/compiler.h>

Expand All @@ -55,10 +55,6 @@

#define SYSLOG_OFLAGS (O_WRONLY | O_CREAT | O_APPEND)

/* An invalid thread ID */

#define NO_HOLDER (INVALID_PROCESS_ID)

/****************************************************************************
* Private Types
****************************************************************************/
Expand All @@ -83,8 +79,7 @@ struct syslog_dev_s
uint8_t sl_state; /* See enum syslog_dev_state */
uint8_t sl_oflags; /* Saved open mode (for re-open) */
uint16_t sl_mode; /* Saved open flags (for re-open) */
sem_t sl_sem; /* Enforces mutually exclusive access */
pid_t sl_holder; /* PID of the thread that holds the semaphore */
rmutex_t sl_lock; /* Enforces mutually exclusive access */
Comment thread
pkarashchenko marked this conversation as resolved.
struct file sl_file; /* The syslog file structure */
FAR char *sl_devpath; /* Full path to the character device */
};
Expand Down Expand Up @@ -129,38 +124,24 @@ static const uint8_t g_syscrlf[2] =

static inline int syslog_dev_takesem(FAR struct syslog_dev_s *syslog_dev)
{
pid_t me = getpid();
int ret;

/* Does this thread already hold the semaphore? That could happen if
/* Does this thread already hold the lock? That could happen if
* we were called recursively, i.e., if the logic kicked off by
* file_write() where to generate more debug output. Return an
* error in that case.
*/

if (syslog_dev->sl_holder == me)
if (nxrmutex_is_hold(&syslog_dev->sl_lock))
{
/* Return an error (instead of deadlocking) */

return -EWOULDBLOCK;
}

/* Either the semaphore is available or is currently held by another
/* Either the lock is available or is currently held by another
* thread. Wait for it to become available.
*/

ret = nxsem_wait(&syslog_dev->sl_sem);
if (ret < 0)
{
return ret;
}

/* We hold the semaphore. We can safely mark ourself as the holder
* of the semaphore.
*/

syslog_dev->sl_holder = me;
return OK;
return nxrmutex_lock(&syslog_dev->sl_lock);
}

/****************************************************************************
Expand All @@ -169,15 +150,7 @@ static inline int syslog_dev_takesem(FAR struct syslog_dev_s *syslog_dev)

static inline void syslog_dev_givesem(FAR struct syslog_dev_s *syslog_dev)
{
#ifdef CONFIG_DEBUG_ASSERTIONS
pid_t me = getpid();
DEBUGASSERT(syslog_dev->sl_holder == me);
Comment thread
xiaoxiang781216 marked this conversation as resolved.
#endif

/* Relinquish the semaphore */

syslog_dev->sl_holder = NO_HOLDER;
nxsem_post(&syslog_dev->sl_sem);
nxrmutex_unlock(&syslog_dev->sl_lock);
}

/****************************************************************************
Expand Down Expand Up @@ -272,9 +245,8 @@ static int syslog_dev_open(FAR struct syslog_dev_s *syslog_dev,

/* The SYSLOG device is open and ready for writing. */

nxsem_init(&syslog_dev->sl_sem, 0, 1);
syslog_dev->sl_holder = NO_HOLDER;
syslog_dev->sl_state = SYSLOG_OPENED;
nxrmutex_init(&syslog_dev->sl_lock);
syslog_dev->sl_state = SYSLOG_OPENED;
return OK;
}

Expand Down Expand Up @@ -303,7 +275,7 @@ static int syslog_dev_open(FAR struct syslog_dev_s *syslog_dev,
* close the device, and set it for later re-opening.
*
* NOTE: That the third case is different. It applies only to the thread
* that currently holds the sl_sem semaphore. Other threads should wait.
* that currently holds the sl_lock. Other threads should wait.
* that is why that case is handled in syslog_semtake().
*
* Input Parameters:
Expand Down Expand Up @@ -352,7 +324,7 @@ static int syslog_dev_outputready(FAR struct syslog_dev_s *syslog_dev)
if (syslog_dev->sl_state == SYSLOG_FAILURE)
{
file_close(&syslog_dev->sl_file);
nxsem_destroy(&syslog_dev->sl_sem);
nxrmutex_destroy(&syslog_dev->sl_lock);

syslog_dev->sl_state = SYSLOG_REOPEN;
}
Expand Down Expand Up @@ -795,7 +767,7 @@ void syslog_dev_uninitialize(FAR struct syslog_channel_s *channel)
syslog_dev->sl_state == SYSLOG_FAILURE)
{
file_close(&syslog_dev->sl_file);
nxsem_destroy(&syslog_dev->sl_sem);
nxrmutex_destroy(&syslog_dev->sl_lock);
}

/* Set the device in UNINITIALIZED state. */
Expand Down
45 changes: 31 additions & 14 deletions include/nuttx/mutex.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,25 @@ static inline bool nxrmutex_is_locked(FAR rmutex_t *rmutex)
return rmutex->count > 0;
}

/****************************************************************************
* Name: nxrmutex_is_hold
*
* Description:
* This function check whether the caller hold the recursive mutex
* referenced by 'rmutex'.
*
* Parameters:
* rmutex - Recursive mutex descriptor.
*
* Return Value:
*
****************************************************************************/

static inline bool nxrmutex_is_hold(FAR rmutex_t *rmutex)
{
return rmutex->holder == gettid();
Comment thread
xiaoxiang781216 marked this conversation as resolved.
}

/****************************************************************************
* Name: nxrmutex_unlock
*
Expand All @@ -446,22 +465,20 @@ static inline bool nxrmutex_is_locked(FAR rmutex_t *rmutex)
static inline int nxrmutex_unlock(FAR rmutex_t *rmutex)
{
pid_t tid = gettid();
int ret = -EPERM;
int ret = OK;

if (rmutex->holder == tid)
DEBUGASSERT(rmutex->holder == tid);
DEBUGASSERT(rmutex->count > 0);

if (rmutex->count == 1)
{
DEBUGASSERT(rmutex->count > 0);
if (rmutex->count == 1)
{
rmutex->count = 0;
rmutex->holder = NXRMUTEX_NO_HOLDER;
ret = nxmutex_unlock(&rmutex->mutex);
}
else
{
rmutex->count--;
ret = OK;
}
rmutex->count = 0;
rmutex->holder = NXRMUTEX_NO_HOLDER;
ret = nxmutex_unlock(&rmutex->mutex);
}
else
{
rmutex->count--;
}

return ret;
Expand Down