On 3/26/21 7:30 PM, Ben Pfaff wrote:
> The thread-local data allocators can't increment coverage counters
> because this can cause reentrancy.  Until now, this code has used
> explicit calls to malloc().  This code replaces them by calls to the
> new functions.  This will make it easier in an upcoming patch to update
> all the code that can run out of memory.

Commit message should be adjusted if we're no going to have patch #3.
Beside that and a couple of minor comments inline it looks OK to me.
Have no strong opinion if we need this patch or not.

Anyway,
Acked-by: Ilya Maximets <[email protected]>

> 
> Signed-off-by: Ben Pfaff <[email protected]>
> ---
>  lib/ovs-thread.h | 12 ++----------
>  lib/util.c       | 41 +++++++++++++++++++++++++++++++++--------
>  lib/util.h       | 10 +++++++++-
>  3 files changed, 44 insertions(+), 19 deletions(-)
> 
> diff --git a/lib/ovs-thread.h b/lib/ovs-thread.h
> index 1050fc29af7c..7552a4e4b215 100644
> --- a/lib/ovs-thread.h
> +++ b/lib/ovs-thread.h
> @@ -294,11 +294,7 @@ void xpthread_join(pthread_t, void **);
>          value = NAME##_get_unsafe();                                    \
>          if (!value) {                                                   \
>              static const NAME##_type initial_value = __VA_ARGS__;       \
> -                                                                        \

I'd like to keep the empty line here for readability.

> -            value = malloc(sizeof *value);                              \
> -            if (value == NULL) {                                        \
> -                out_of_memory();                                        \
> -            }                                                           \
> +            value = xmalloc__(sizeof *value);                           \
>              *value = initial_value;                                     \
>              xpthread_setspecific(NAME##_key, value);                    \
>          }                                                               \
> @@ -334,11 +330,7 @@ void xpthread_join(pthread_t, void **);
>          value = NAME##_get_unsafe();                                    \
>          if (!value) {                                                   \
>              static const NAME##_type initial_value = __VA_ARGS__;       \
> -                                                                        \

Same here.

> -            value = malloc(sizeof *value);                              \
> -            if (value == NULL) {                                        \
> -                out_of_memory();                                        \
> -            }                                                           \
> +            value = xmalloc__(sizeof *value);                           \
>              *value = initial_value;                                     \
>              xpthread_setspecific(NAME##_key, value);                    \
>          }                                                               \
> diff --git a/lib/util.c b/lib/util.c
> index 25635b27ff00..1195c7982118 100644
> --- a/lib/util.c
> +++ b/lib/util.c
> @@ -116,10 +116,9 @@ out_of_memory(void)
>  }
>  
>  void *
> -xcalloc(size_t count, size_t size)
> +xcalloc__(size_t count, size_t size)
>  {
>      void *p = count && size ? calloc(count, size) : malloc(1);
> -    COVERAGE_INC(util_xalloc);
>      if (p == NULL) {
>          out_of_memory();
>      }
> @@ -127,16 +126,15 @@ xcalloc(size_t count, size_t size)
>  }
>  
>  void *
> -xzalloc(size_t size)
> +xzalloc__(size_t size)
>  {
> -    return xcalloc(1, size);
> +    return xcalloc__(1, size);
>  }
>  
>  void *
> -xmalloc(size_t size)
> +xmalloc__(size_t size)
>  {
>      void *p = malloc(size ? size : 1);
> -    COVERAGE_INC(util_xalloc);
>      if (p == NULL) {
>          out_of_memory();
>      }
> @@ -144,16 +142,43 @@ xmalloc(size_t size)
>  }
>  
>  void *
> -xrealloc(void *p, size_t size)
> +xrealloc__(void *p, size_t size)
>  {
>      p = realloc(p, size ? size : 1);
> -    COVERAGE_INC(util_xalloc);
>      if (p == NULL) {
>          out_of_memory();
>      }
>      return p;
>  }
>  
> +void *
> +xcalloc(size_t count, size_t size)
> +{
> +    COVERAGE_INC(util_xalloc);
> +    return xcalloc__(count, size);
> +}
> +
> +void *
> +xzalloc(size_t size)
> +{
> +    COVERAGE_INC(util_xalloc);
> +    return xzalloc__(size);
> +}
> +
> +void *
> +xmalloc(size_t size)
> +{
> +    COVERAGE_INC(util_xalloc);
> +    return xmalloc__(size);
> +}
> +
> +void *
> +xrealloc(void *p, size_t size)
> +{
> +    COVERAGE_INC(util_xalloc);
> +    return xrealloc__(p, size);
> +}
> +
>  void *
>  xmemdup(const void *p_, size_t size)
>  {
> diff --git a/lib/util.h b/lib/util.h
> index 067dcad15786..9c2b14fae304 100644
> --- a/lib/util.h
> +++ b/lib/util.h
> @@ -145,8 +145,9 @@ void ovs_print_version(uint8_t min_ofp, uint8_t max_ofp);
>  
>  void set_memory_locked(void);
>  bool memory_locked(void);
> -

And here.

>  OVS_NO_RETURN void out_of_memory(void);
> +
> +/* Allocation wrappers that abort if memory is exhausted. */
>  void *xmalloc(size_t) MALLOC_LIKE;
>  void *xcalloc(size_t, size_t) MALLOC_LIKE;
>  void *xzalloc(size_t) MALLOC_LIKE;
> @@ -160,6 +161,13 @@ char *xasprintf(const char *format, ...) 
> OVS_PRINTF_FORMAT(1, 2) MALLOC_LIKE;
>  char *xvasprintf(const char *format, va_list) OVS_PRINTF_FORMAT(1, 0) 
> MALLOC_LIKE;
>  void *x2nrealloc(void *p, size_t *n, size_t s);
>  
> +/* Allocation wrappers for specialized situations where coverage counters
> + * cannot be used. */
> +void *xmalloc__(size_t) MALLOC_LIKE;
> +void *xcalloc__(size_t, size_t) MALLOC_LIKE;
> +void *xzalloc__(size_t) MALLOC_LIKE;
> +void *xrealloc__(void *, size_t);
> +
>  void *xmalloc_cacheline(size_t) MALLOC_LIKE;
>  void *xzalloc_cacheline(size_t) MALLOC_LIKE;
>  void free_cacheline(void *);
> 

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to