Re: [PATCH] Fix the bug of multiple cores print

[Andrew Pinski <pinskia at gmail dot com>]

On Mon, Jan 11, 2016 at 6:17 PM, Zaibo Xu <xuzaibo@huawei.com> wrote:
> 1.  _VOID _DEFUN(__sinit, (s), struct _reent *s) in ./newlib/libc/stdio/findfp.c.
> 2. struct _reent in ./newlib/libc/include/sys/reent.h
> Reasons: We have fount this bug in debuging the function of printf on
> Sandybridge of Intel as multiple cores print to the cluster communication
> port at the same time. After the mending above, this bug is fixed.
>
> Signed-off-by: Zaibo Xu <xuzaibo@huawei.com>
> ---
>  newlib/libc/include/sys/reent.h | 4 ++--
>  newlib/libc/stdio/findfp.c      | 3 +++
>  2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/newlib/libc/include/sys/reent.h b/newlib/libc/include/sys/reent.h
> index 5481ca2..7b882fa 100644
> --- a/newlib/libc/include/sys/reent.h
> +++ b/newlib/libc/include/sys/reent.h
> @@ -382,7 +382,7 @@ struct _reent
>
>    char *_emergency;
>
> -  int __sdidinit;              /* 1 means stdio has been init'd */
> +  volatile int __sdidinit;             /* 1 means stdio has been init'd */
>
>    int _current_category;       /* unused */
>    _CONST char *_current_locale;        /* unused */
> @@ -580,7 +580,7 @@ struct _reent
>    int _current_category;       /* used by setlocale */
>    _CONST char *_current_locale;
>
> -  int __sdidinit;              /* 1 means stdio has been init'd */
> +  volatile int __sdidinit;             /* 1 means stdio has been init'd */
>
>    void _EXFNPTR(__cleanup, (struct _reent *));
>
> diff --git a/newlib/libc/stdio/findfp.c b/newlib/libc/stdio/findfp.c
> index 975a855..460809a 100644
> --- a/newlib/libc/stdio/findfp.c
> +++ b/newlib/libc/stdio/findfp.c
> @@ -210,6 +210,7 @@ _DEFUN(__sinit, (s),
>  {
>    __sinit_lock_acquire ();
>
> +  asm volatile("mfence;":::"memory");


__sinit_lock_acquire  should be enough for the lock here I think.  If
it is not then the bug is there instead of here.

>    if (s->__sdidinit)
>      {
>        __sinit_lock_release ();
> @@ -254,8 +255,10 @@ _DEFUN(__sinit, (s),
>       when the underlying fd 2 is write-only.  */
>    std (s->_stderr, __SRW | __SNBF, 2, s);
>
> +  asm volatile("mfence;":::"memory");
>    s->__sdidinit = 1;
>
> +  asm volatile("mfence;":::"memory");
>    __sinit_lock_release ();

__sinit_lock_release should be enough here and if it is not then the
bug is there instead.

Thanks,
Andrew

>  }
>
> --
> 2.5.1.windows.1
>