drivers/syslog: fix deadlock by reverting part of the changes from b88a8cf39f

Signed-off-by: Petro Karashchenko <petro.karashchenko@gmail.com>
This commit is contained in:
Petro Karashchenko 2022-06-12 21:14:10 +02:00 committed by Xiang Xiao
parent 02682af759
commit e42e3aa642

View File

@ -38,7 +38,7 @@
#include <nuttx/arch.h>
#include <nuttx/kmalloc.h>
#include <nuttx/fs/fs.h>
#include <nuttx/mutex.h>
#include <nuttx/semaphore.h>
#include <nuttx/syslog/syslog.h>
#include <nuttx/compiler.h>
@ -54,8 +54,10 @@
*/
#define SYSLOG_OFLAGS (O_WRONLY | O_CREAT | O_APPEND)
#define syslog_dev_takesem(s) nxrmutex_lock(&(s)->sl_lock)
#define syslog_dev_givesem(s) nxrmutex_unlock(&(s)->sl_lock)
/* An invalid thread ID */
#define NO_HOLDER (INVALID_PROCESS_ID)
/****************************************************************************
* Private Types
@ -81,7 +83,8 @@ 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) */
rmutex_t sl_lock; /* Enforces mutually exclusive access */
sem_t sl_sem; /* Enforces mutually exclusive access */
pid_t sl_holder; /* PID of the thread that holds the semaphore */
struct file sl_file; /* The syslog file structure */
FAR char *sl_devpath; /* Full path to the character device */
};
@ -120,6 +123,63 @@ static const uint8_t g_syscrlf[2] =
* Private Functions
****************************************************************************/
/****************************************************************************
* Name: syslog_dev_takesem
****************************************************************************/
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
* 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)
{
/* Return an error (instead of deadlocking) */
return -EWOULDBLOCK;
}
/* Either the semaphore 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;
}
/****************************************************************************
* Name: syslog_dev_givesem
****************************************************************************/
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);
#endif
/* Relinquish the semaphore */
syslog_dev->sl_holder = NO_HOLDER;
nxsem_post(&syslog_dev->sl_sem);
}
/****************************************************************************
* Name: syslog_dev_open
*
@ -212,8 +272,9 @@ static int syslog_dev_open(FAR struct syslog_dev_s *syslog_dev,
/* The SYSLOG device is open and ready for writing. */
nxrmutex_init(&syslog_dev->sl_lock);
syslog_dev->sl_state = SYSLOG_OPENED;
nxsem_init(&syslog_dev->sl_sem, 0, 1);
syslog_dev->sl_holder = NO_HOLDER;
syslog_dev->sl_state = SYSLOG_OPENED;
return OK;
}
@ -242,7 +303,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_lock. Other threads should wait.
* that currently holds the sl_sem semaphore. Other threads should wait.
* that is why that case is handled in syslog_semtake().
*
* Input Parameters:
@ -291,7 +352,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);
nxrmutex_destroy(&syslog_dev->sl_lock);
nxsem_destroy(&syslog_dev->sl_sem);
syslog_dev->sl_state = SYSLOG_REOPEN;
}
@ -734,7 +795,7 @@ void syslog_dev_uninitialize(FAR struct syslog_channel_s *channel)
syslog_dev->sl_state == SYSLOG_FAILURE)
{
file_close(&syslog_dev->sl_file);
nxrmutex_destroy(&syslog_dev->sl_lock);
nxsem_destroy(&syslog_dev->sl_sem);
}
/* Set the device in UNINITIALIZED state. */