[PATCH v2 1/1] log: mute messages generated by log drivers

[PATCH v2 1/1] log: mute messages generated by log drivers

[Heinrich Schuchardt]

When a message is written by a log driver (e.g. via the network stack) this
may result in the generation of further messages. We cannot allow these
additional messages to be emitted as this might result in an infinite
recursion.

Up to now only the syslog driver was safeguarded. We should safeguard all
log drivers instead.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
This patch is based on

[PATCH v3 0/3] doc: global data pointer
https://lists.denx.de/pipermail/u-boot/2020-October/428475.html

v2:
	move processing_msg to global data pointer
	(Simon reported a problem with SPL log messages when useing a
	static variable)
---
 common/log.c                      | 12 +++++++++++-
 common/log_syslog.c               |  8 --------
 include/asm-generic/global_data.h |  8 ++++++++
 3 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/common/log.c b/common/log.c
index 734d26de4a..54f2fdeb36 100644
--- a/common/log.c
+++ b/common/log.c
@@ -192,11 +192,21 @@ static int log_dispatch(struct log_rec *rec)
 {
 	struct log_device *ldev;

+	/*
+	 * When a log driver writes messages (e.g. via the network stack) this
+	 * may result in further generated messages. We cannot process them here
+	 * as this might result in infinite recursion.
+	 */
+	if (gd->processing_msg)
+		return 0;
+
+	/* Emit message */
+	gd->processing_msg = 1;
 	list_for_each_entry(ldev, &gd->log_head, sibling_node) {
 		if (log_passes_filters(ldev, rec))
 			ldev->drv->emit(ldev, rec);
 	}
-
+	gd->processing_msg = 0;
 	return 0;
 }

diff --git a/common/log_syslog.c b/common/log_syslog.c
index 149ff5af31..2ae703fed7 100644
--- a/common/log_syslog.c
+++ b/common/log_syslog.c
@@ -35,16 +35,9 @@ static int log_syslog_emit(struct log_device *ldev, struct log_rec *rec)
 	char *log_msg;
 	int eth_hdr_size;
 	struct in_addr bcast_ip;
-	static int processing_msg;
 	unsigned int log_level;
 	char *log_hostname;

-	/* Fend off messages from the network stack while writing a message */
-	if (processing_msg)
-		return 0;
-
-	processing_msg = 1;
-
 	/* Setup packet buffers */
 	net_init();
 	/* Disable hardware and put it into the reset state */
@@ -108,7 +101,6 @@ static int log_syslog_emit(struct log_device *ldev, struct log_rec *rec)
 	net_send_packet((uchar *)msg, ptr - msg);

 out:
-	processing_msg = 0;
 	return ret;
 }

diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
index ebb740d34f..6bb7f93678 100644
--- a/include/asm-generic/global_data.h
+++ b/include/asm-generic/global_data.h
@@ -363,6 +363,14 @@ struct global_data {
 	 * &enum log_fmt defines the bits of the bit mask.
 	 */
 	int log_fmt;
+
+	/**
+	 * @processing_msg: a log message is being processed
+	 *
+	 * This flag is used to suppress the creation of additional messages
+	 * while another message is being processed.
+	 */
+	int processing_msg;
 #endif
 #if CONFIG_IS_ENABLED(BLOBLIST)
 	/**
--
2.28.0

[PATCH v2 1/1] log: mute messages generated by log drivers

[Heinrich Schuchardt]

On 05.10.20 12:21, Heinrich Schuchardt wrote:
> When a message is written by a log driver (e.g. via the network stack) this
> may result in the generation of further messages. We cannot allow these
> additional messages to be emitted as this might result in an infinite
> recursion.
>
> Up to now only the syslog driver was safeguarded. We should safeguard all
> log drivers instead.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> This patch is based on
>
> [PATCH v3 0/3] doc: global data pointer
> https://lists.denx.de/pipermail/u-boot/2020-October/428475.html
>
> v2:
> 	move processing_msg to global data pointer
> 	(Simon reported a problem with SPL log messages when useing a
> 	static variable)

Hello Simon,

this is what is needed based on origin/master.

In origin/next the first version of the patch was merged.

What should I use as basis?

Best regards

Heinrich
> ---
>  common/log.c                      | 12 +++++++++++-
>  common/log_syslog.c               |  8 --------
>  include/asm-generic/global_data.h |  8 ++++++++
>  3 files changed, 19 insertions(+), 9 deletions(-)
>
> diff --git a/common/log.c b/common/log.c
> index 734d26de4a..54f2fdeb36 100644
> --- a/common/log.c
> +++ b/common/log.c
> @@ -192,11 +192,21 @@ static int log_dispatch(struct log_rec *rec)
>  {
>  	struct log_device *ldev;
>
> +	/*
> +	 * When a log driver writes messages (e.g. via the network stack) this
> +	 * may result in further generated messages. We cannot process them here
> +	 * as this might result in infinite recursion.
> +	 */
> +	if (gd->processing_msg)
> +		return 0;
> +
> +	/* Emit message */
> +	gd->processing_msg = 1;
>  	list_for_each_entry(ldev, &gd->log_head, sibling_node) {
>  		if (log_passes_filters(ldev, rec))
>  			ldev->drv->emit(ldev, rec);
>  	}
> -
> +	gd->processing_msg = 0;
>  	return 0;
>  }
>
> diff --git a/common/log_syslog.c b/common/log_syslog.c
> index 149ff5af31..2ae703fed7 100644
> --- a/common/log_syslog.c
> +++ b/common/log_syslog.c
> @@ -35,16 +35,9 @@ static int log_syslog_emit(struct log_device *ldev, struct log_rec *rec)
>  	char *log_msg;
>  	int eth_hdr_size;
>  	struct in_addr bcast_ip;
> -	static int processing_msg;
>  	unsigned int log_level;
>  	char *log_hostname;
>
> -	/* Fend off messages from the network stack while writing a message */
> -	if (processing_msg)
> -		return 0;
> -
> -	processing_msg = 1;
> -
>  	/* Setup packet buffers */
>  	net_init();
>  	/* Disable hardware and put it into the reset state */
> @@ -108,7 +101,6 @@ static int log_syslog_emit(struct log_device *ldev, struct log_rec *rec)
>  	net_send_packet((uchar *)msg, ptr - msg);
>
>  out:
> -	processing_msg = 0;
>  	return ret;
>  }
>
> diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
> index ebb740d34f..6bb7f93678 100644
> --- a/include/asm-generic/global_data.h
> +++ b/include/asm-generic/global_data.h
> @@ -363,6 +363,14 @@ struct global_data {
>  	 * &enum log_fmt defines the bits of the bit mask.
>  	 */
>  	int log_fmt;
> +
> +	/**
> +	 * @processing_msg: a log message is being processed
> +	 *
> +	 * This flag is used to suppress the creation of additional messages
> +	 * while another message is being processed.
> +	 */
> +	int processing_msg;
>  #endif
>  #if CONFIG_IS_ENABLED(BLOBLIST)
>  	/**
> --
> 2.28.0
>

[PATCH v2 1/1] log: mute messages generated by log drivers

[Simon Glass]

Hi Heinrich,

On Mon, 5 Oct 2020 at 04:42, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 05.10.20 12:21, Heinrich Schuchardt wrote:
> > When a message is written by a log driver (e.g. via the network stack) this
> > may result in the generation of further messages. We cannot allow these
> > additional messages to be emitted as this might result in an infinite
> > recursion.
> >
> > Up to now only the syslog driver was safeguarded. We should safeguard all
> > log drivers instead.
> >
> > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > ---
> > This patch is based on
> >
> > [PATCH v3 0/3] doc: global data pointer
> > https://lists.denx.de/pipermail/u-boot/2020-October/428475.html
> >
> > v2:
> >       move processing_msg to global data pointer
> >       (Simon reported a problem with SPL log messages when useing a
> >       static variable)
>
> Hello Simon,
>
> this is what is needed based on origin/master.
>
> In origin/next the first version of the patch was merged.
>
> What should I use as basis?

I think you need to use -next since that is where we are now. Also,
could you add a log_ prefix to your global_data var and perhaps make
it a bool?

Regards,
Simon