[Beignet] [PATCH 2/5] Add a lock in the place of printf output

[Zhigang Gong]

This patch LGTM, pushed, thanks.

On Wed, Jun 18, 2014 at 02:42:15PM +0800, junyan.he at inbox.com wrote:
> From: Junyan He <junyan.he at linux.intel.com>
> 
> If multi-thread run the kernel simultaneously, the output
> may interlace with each other. Add a lock to avoid this.
> 
> Signed-off-by: Junyan He <junyan.he at linux.intel.com>
> ---
>  backend/src/ir/printf.cpp |  3 +++
>  backend/src/ir/printf.hpp | 44 ++++++++++++++++++++++++++++++++++----------
>  2 files changed, 37 insertions(+), 10 deletions(-)
> 
> diff --git a/backend/src/ir/printf.cpp b/backend/src/ir/printf.cpp
> index 3729155..0a943ac 100644
> --- a/backend/src/ir/printf.cpp
> +++ b/backend/src/ir/printf.cpp
> @@ -29,6 +29,8 @@ namespace gbe
>  {
>    namespace ir
>    {
> +    pthread_mutex_t PrintfSet::lock = PTHREAD_MUTEX_INITIALIZER;
> +
>      uint32_t PrintfSet::append(PrintfFmt* fmt, Unit& unit)
>      {
>        fmts.push_back(*fmt);
> @@ -74,6 +76,7 @@ namespace gbe
>      void PrintfSet::outputPrintf(void* index_addr, void* buf_addr, size_t global_wk_sz0,
>                                   size_t global_wk_sz1, size_t global_wk_sz2)
>      {
> +      LockOutput lock;
>        size_t i, j, k;
>        std::string pf_str;
>        vector<int>* contents = NULL;
> diff --git a/backend/src/ir/printf.hpp b/backend/src/ir/printf.hpp
> index 5763a65..b49ad0d 100644
> --- a/backend/src/ir/printf.hpp
> +++ b/backend/src/ir/printf.hpp
> @@ -89,12 +89,14 @@ namespace gbe
>          void *ptr;
>        };
>  
> -      PrintfSlot(void) {
> +      PrintfSlot(void)
> +      {
>          type = PRINTF_SLOT_TYPE_NONE;
>          ptr = NULL;
>        }
>  
> -      PrintfSlot(const char * s) {
> +      PrintfSlot(const char * s)
> +      {
>          type = PRINTF_SLOT_TYPE_STRING;
>          int len = strlen(s);
>          str = (char*)malloc((len + 1) * sizeof(char));
> @@ -102,13 +104,15 @@ namespace gbe
>          str[len] = 0;
>        }
>  
> -      PrintfSlot(PrintfState * st) {
> +      PrintfSlot(PrintfState * st)
> +      {
>          type = PRINTF_SLOT_TYPE_STATE;
>          state = (PrintfState *)malloc(sizeof(PrintfState));
>          memcpy(state, st, sizeof(PrintfState));
>        }
>  
> -      PrintfSlot(const PrintfSlot & other) {
> +      PrintfSlot(const PrintfSlot & other)
> +      {
>          if (other.type == PRINTF_SLOT_TYPE_STRING) {
>            int len = strlen(other.str);
>            str = (char*)malloc((len + 1) * sizeof(char));
> @@ -125,7 +129,8 @@ namespace gbe
>          }
>        }
>  
> -      PrintfSlot(PrintfSlot && other) {
> +      PrintfSlot(PrintfSlot && other)
> +      {
>          void *p = other.ptr;
>          type = other.type;
>          other.ptr = ptr;
> @@ -133,7 +138,8 @@ namespace gbe
>  
>        }
>  
> -      ~PrintfSlot(void) {
> +      ~PrintfSlot(void)
> +      {
>          if (ptr)
>            free(ptr);
>        }
> @@ -144,7 +150,8 @@ namespace gbe
>      class PrintfSet //: public Serializable
>      {
>      public:
> -      PrintfSet(const PrintfSet& other) {
> +      PrintfSet(const PrintfSet& other)
> +      {
>          for (auto &f : other.fmts) {
>            fmts.push_back(f);
>          }
> @@ -158,18 +165,33 @@ namespace gbe
>  
>        PrintfSet(void) = default;
>  
> +      struct LockOutput {
> +        LockOutput(void)
> +        {
> +          pthread_mutex_lock(&lock);
> +        }
> +
> +        ~LockOutput(void)
> +        {
> +          pthread_mutex_unlock(&lock);
> +        }
> +      };
> +
>        typedef vector<PrintfSlot> PrintfFmt;
>        uint32_t append(PrintfFmt* fmt, Unit &unit);
>  
> -      uint32_t getPrintfNum(void) const {
> +      uint32_t getPrintfNum(void) const
> +      {
>          return fmts.size();
>        }
>  
> -      uint32_t getPrintfSizeOfSize(void) const {
> +      uint32_t getPrintfSizeOfSize(void) const
> +      {
>          return sizeOfSize;
>        }
>  
> -      uint32_t getPrintfBufferElementSize(uint32_t i) {
> +      uint32_t getPrintfBufferElementSize(uint32_t i)
> +      {
>          PrintfSlot* slot = slots[i];
>          switch (slot->state->conversion_specifier) {
>            case PRINTF_CONVERSION_I:
> @@ -189,6 +211,8 @@ namespace gbe
>        vector<PrintfFmt> fmts;
>        vector<PrintfSlot*> slots;
>        uint32_t sizeOfSize; // Total sizeof size.
> +      friend struct LockOutput;
> +      static pthread_mutex_t lock;
>        GBE_CLASS(PrintfSet);
>      };
>    } /* namespace ir */
> -- 
> 1.8.3.2
> 
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet