Re: [UNIKRAFT PATCH, v2, 08/15] lib/ukring: Use Unikraft to enable ring buffer debugging

[Costin Lupu <costin.lupu@xxxxxxxxx>]

Hi Alexander,

I thought about this and I think it would be better to keep
DEBUG_BUFRING. Btw, this is used to debug the buffer ring and it will be
extremely rare enabled when debugging. Besides, it has nothing to do
with the debug printing, there is no debug printing here.

So we can simply drop this patch altogether.

Cheers,
Costin

On 7/21/20 6:39 PM, Alexander Jung wrote:
> This commit uses the flags set from ukdebug to determine whether
> to output critical messages from the ring buffer's debug
> sequences.
> 
> Signed-off-by: Alexander Jung <alexander.jung@xxxxxxxxx>
> ---
>  lib/ukring/include/uk/ring.h | 18 +++++++++---------
>  lib/ukring/ring.c            |  2 +-
>  2 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/lib/ukring/include/uk/ring.h b/lib/ukring/include/uk/ring.h
> index 1cc8982..01adaf3 100644
> --- a/lib/ukring/include/uk/ring.h
> +++ b/lib/ukring/include/uk/ring.h
> @@ -50,7 +50,7 @@ struct uk_ring {
>    volatile uint32_t  cons_tail;
>    int                cons_size;
>    int                cons_mask;
> -#ifdef DEBUG_BUFRING
> +#ifdef CONFIG_LIBUKDEBUG
>    struct uk_mutex   *lock;
>  #endif
>    void              *ring[0] __aligned(CACHE_LINE_SIZE);
> @@ -66,7 +66,7 @@ uk_ring_enqueue(struct uk_ring *r, void *buf)
>  {
>    uint32_t prod_head, prod_next, cons_tail;
>  
> -#ifdef DEBUG_BUFRING
> +#ifdef CONFIG_LIBUKDEBUG_PRINTK_CRIT
>    int i;
>  
>    /*
> @@ -99,7 +99,7 @@ uk_ring_enqueue(struct uk_ring *r, void *buf)
>      }
>    } while (!ukarch_compare_exchange_sync(&r->prod_head, prod_head, 
> prod_next));
>  
> -#ifdef DEBUG_BUFRING
> +#ifdef CONFIG_LIBUKDEBUG_PRINTK_CRIT
>    if (r->ring[prod_head] != NULL)
>      uk_pr_crit("dangling value in enqueue\n");
>  #endif
> @@ -147,7 +147,7 @@ uk_ring_dequeue(struct uk_ring *r)
>  
>    buf = r->ring[cons_head];
>  
> -#ifdef DEBUG_BUFRING
> +#ifdef CONFIG_LIBUKDEBUG
>    r->ring[cons_head] = NULL;
>  #endif
>  
> @@ -237,7 +237,7 @@ uk_ring_dequeue_single(struct uk_ring *r)
>    r->cons_head = cons_next;
>    buf = r->ring[cons_head];
>  
> -#ifdef DEBUG_BUFRING
> +#ifdef CONFIG_LIBUKDEBUG_PRINTK_CRIT
>    r->ring[cons_head] = NULL;
>  
>    if (!uk_mutex_is_locked(r->lock))
> @@ -272,7 +272,7 @@ uk_ring_advance_single(struct uk_ring *r)
>  
>    r->cons_head = cons_next;
>  
> -#ifdef DEBUG_BUFRING
> +#ifdef CONFIG_LIBUKDEBUG
>    r->ring[cons_head] = NULL;
>  #endif
>  
> @@ -311,7 +311,7 @@ uk_ring_putback_single(struct uk_ring *r, void *new)
>  static __inline void *
>  uk_ring_peek(struct uk_ring *r)
>  {
> -#ifdef DEBUG_BUFRING
> +#ifdef CONFIG_LIBUKDEBUG_PRINTK_CRIT
>    if ((r->lock != NULL) && !uk_mutex_is_locked(r->lock))
>      uk_pr_crit("lock not held on single consumer dequeue\n");
>  #endif
> @@ -331,7 +331,7 @@ uk_ring_peek(struct uk_ring *r)
>  static __inline void *
>  uk_ring_peek_clear_single(struct uk_ring *r)
>  {
> -#ifdef DEBUG_BUFRING
> +#ifdef CONFIG_LIBUKDEBUG_PRINTK_CRIT
>    void *ret;
>  
>    if (!uk_mutex_is_locked(r->lock))
> @@ -356,7 +356,7 @@ uk_ring_peek_clear_single(struct uk_ring *r)
>    /* atomic_thread_fence_acq(); */
>  #endif
>  
> -#ifdef DEBUG_BUFRING
> +#ifdef CONFIG_LIBUKDEBUG
>    /*
>     * Single consumer, i.e. cons_head will not move while we are
>     * running, so atomic_swap_ptr() is not necessary here.
> diff --git a/lib/ukring/ring.c b/lib/ukring/ring.c
> index 1431cb9..db5600f 100644
> --- a/lib/ukring/ring.c
> +++ b/lib/ukring/ring.c
> @@ -56,7 +56,7 @@ uk_ring_alloc(struct uk_alloc *a, int count, int flags, 
> struct uk_mutex *lock)
>      return NULL;
>    }
>  
> -#ifdef DEBUG_BUFRING
> +#ifdef CONFIG_LIBUKDEBUG
>    r->lock = lock;
>  #endif
>  
>