On Thu, Dec 17, 2020 at 12:00:12PM +0100, mwilck@xxxxxxxx wrote:
> From: Martin Wilck <mwilck@xxxxxxxx>
>
> logq_lock protects internal data structures of log.c, and should
> be handled there. This patch doesn't change functionality, except
> improving cancel-safety somewhat.
>
Reviewed-by: Benjamin Marzinski <bmarzins@xxxxxxxxxx>
> Signed-off-by: Martin Wilck <mwilck@xxxxxxxx>
> ---
> libmultipath/log.c | 34 ++++++++++++++++++++++++++++++++--
> libmultipath/log_pthread.c | 9 ---------
> 2 files changed, 32 insertions(+), 11 deletions(-)
>
> diff --git a/libmultipath/log.c b/libmultipath/log.c
> index debd36d..7f33787 100644
> --- a/libmultipath/log.c
> +++ b/libmultipath/log.c
> @@ -9,13 +9,16 @@
> #include <string.h>
> #include <syslog.h>
> #include <time.h>
> +#include <pthread.h>
>
> #include "memory.h"
> #include "log.h"
> +#include "util.h"
>
> #define ALIGN(len, s) (((len)+(s)-1)/(s)*(s))
>
> struct logarea* la;
> +static pthread_mutex_t logq_lock = PTHREAD_MUTEX_INITIALIZER;
>
> #if LOGDBG
> static void dump_logarea (void)
> @@ -101,12 +104,17 @@ void log_close (void)
>
> void log_reset (char *program_name)
> {
> + pthread_mutex_lock(&logq_lock);
> + pthread_cleanup_push(cleanup_mutex, &logq_lock);
> +
> closelog();
> tzset();
> openlog(program_name, 0, LOG_DAEMON);
> +
> + pthread_cleanup_pop(1);
> }
>
> -int log_enqueue (int prio, const char * fmt, va_list ap)
> +static int _log_enqueue(int prio, const char * fmt, va_list ap)
> {
> int len, fwd;
> char buff[MAX_MSG_SIZE];
> @@ -165,7 +173,18 @@ int log_enqueue (int prio, const char * fmt, va_list ap)
> return 0;
> }
>
> -int log_dequeue (void * buff)
> +int log_enqueue(int prio, const char *fmt, va_list ap)
> +{
> + int ret;
> +
> + pthread_mutex_lock(&logq_lock);
> + pthread_cleanup_push(cleanup_mutex, &logq_lock);
> + ret = _log_enqueue(prio, fmt, ap);
> + pthread_cleanup_pop(1);
> + return ret;
> +}
> +
> +static int _log_dequeue(void *buff)
> {
> struct logmsg * src = (struct logmsg *)la->head;
> struct logmsg * dst = (struct logmsg *)buff;
> @@ -194,6 +213,17 @@ int log_dequeue (void * buff)
> return 0;
> }
>
> +int log_dequeue(void *buff)
> +{
> + int ret;
> +
> + pthread_mutex_lock(&logq_lock);
> + pthread_cleanup_push(cleanup_mutex, &logq_lock);
> + ret = _log_dequeue(buff);
> + pthread_cleanup_pop(1);
> + return ret;
> +}
> +
> /*
> * this one can block under memory pressure
> */
> diff --git a/libmultipath/log_pthread.c b/libmultipath/log_pthread.c
> index 0d48c52..6599210 100644
> --- a/libmultipath/log_pthread.c
> +++ b/libmultipath/log_pthread.c
> @@ -18,7 +18,6 @@
> static pthread_t log_thr;
>
> /* logev_lock must not be taken with logq_lock held */
> -static pthread_mutex_t logq_lock = PTHREAD_MUTEX_INITIALIZER;
> static pthread_mutex_t logev_lock = PTHREAD_MUTEX_INITIALIZER;
> static pthread_cond_t logev_cond = PTHREAD_COND_INITIALIZER;
>
> @@ -41,10 +40,7 @@ void log_safe (int prio, const char * fmt, va_list ap)
> running = logq_running;
>
> if (running) {
> - pthread_mutex_lock(&logq_lock);
> - pthread_cleanup_push(cleanup_mutex, &logq_lock);
> log_enqueue(prio, fmt, ap);
> - pthread_cleanup_pop(1);
>
> log_messages_pending = 1;
> pthread_cond_signal(&logev_cond);
> @@ -60,9 +56,7 @@ static void flush_logqueue (void)
> int empty;
>
> do {
> - pthread_mutex_lock(&logq_lock);
> empty = log_dequeue(la->buff);
> - pthread_mutex_unlock(&logq_lock);
> if (!empty)
> log_syslog(la->buff);
> } while (empty == 0);
> @@ -138,10 +132,7 @@ void log_thread_start (pthread_attr_t *attr)
> void log_thread_reset (void)
> {
> logdbg(stderr,"resetting log\n");
> -
> - pthread_mutex_lock(&logq_lock);
> log_reset("multipathd");
> - pthread_mutex_unlock(&logq_lock);
> }
>
> void log_thread_stop (void)
> --
> 2.29.0
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
