| Message ID | 20210522220904.7012-1-jamrial@gmail.com |
|---|---|
| State | Accepted |
| Commit | be96f4b616878c6245acd626e26cdd65a491b68d |
| Headers | show |
| Series | [FFmpeg-devel,1/3] avutil/mem: make max_alloc_size an atomic type | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
James Almer: > This is in preparation for the following commit. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavutil/mem.c | 17 +++++++++++------ > 1 file changed, 11 insertions(+), 6 deletions(-) > > diff --git a/libavutil/mem.c b/libavutil/mem.c > index fa227f5e12..c12c24aa90 100644 > --- a/libavutil/mem.c > +++ b/libavutil/mem.c > @@ -31,6 +31,7 @@ > #include <limits.h> > #include <stdint.h> > #include <stdlib.h> > +#include <stdatomic.h> > #include <string.h> > #if HAVE_MALLOC_H > #include <malloc.h> > @@ -68,17 +69,17 @@ void free(void *ptr); > * dynamic libraries and remove -Wl,-Bsymbolic from the linker flags. > * Note that this will cost performance. */ > > -static size_t max_alloc_size= INT_MAX; > +static atomic_size_t max_alloc_size = ATOMIC_VAR_INIT(INT_MAX); > > void av_max_alloc(size_t max){ > - max_alloc_size = max; > + atomic_store_explicit(&max_alloc_size, max, memory_order_relaxed); > } > > void *av_malloc(size_t size) > { > void *ptr = NULL; > > - if (size > max_alloc_size) > + if (size > atomic_load_explicit(&max_alloc_size, memory_order_relaxed)) > return NULL; > > #if HAVE_POSIX_MEMALIGN > @@ -134,7 +135,7 @@ void *av_malloc(size_t size) > void *av_realloc(void *ptr, size_t size) > { > void *ret; > - if (size > max_alloc_size) > + if (size > atomic_load_explicit(&max_alloc_size, memory_order_relaxed)) > return NULL; > > #if HAVE_ALIGNED_MALLOC > @@ -483,15 +484,19 @@ void av_memcpy_backptr(uint8_t *dst, int back, int cnt) > > void *av_fast_realloc(void *ptr, unsigned int *size, size_t min_size) > { > + size_t max_size; > + > if (min_size <= *size) > return ptr; > > - if (min_size > max_alloc_size) { > + max_size = atomic_load_explicit(&max_alloc_size, memory_order_relaxed); > + > + if (min_size > max_size) { > *size = 0; > return NULL; > } > > - min_size = FFMIN(max_alloc_size, FFMAX(min_size + min_size / 16 + 32, min_size)); > + min_size = FFMIN(max_size, FFMAX(min_size + min_size / 16 + 32, min_size)); > > ptr = av_realloc(ptr, min_size); > /* we could set this to the unmodified min_size but this is safer > LGTM apart from the commit message. - Andreas
On 5/23/2021 11:14 AM, Andreas Rheinhardt wrote: > James Almer: >> This is in preparation for the following commit. >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavutil/mem.c | 17 +++++++++++------ >> 1 file changed, 11 insertions(+), 6 deletions(-) >> >> diff --git a/libavutil/mem.c b/libavutil/mem.c >> index fa227f5e12..c12c24aa90 100644 >> --- a/libavutil/mem.c >> +++ b/libavutil/mem.c >> @@ -31,6 +31,7 @@ >> #include <limits.h> >> #include <stdint.h> >> #include <stdlib.h> >> +#include <stdatomic.h> >> #include <string.h> >> #if HAVE_MALLOC_H >> #include <malloc.h> >> @@ -68,17 +69,17 @@ void free(void *ptr); >> * dynamic libraries and remove -Wl,-Bsymbolic from the linker flags. >> * Note that this will cost performance. */ >> >> -static size_t max_alloc_size= INT_MAX; >> +static atomic_size_t max_alloc_size = ATOMIC_VAR_INIT(INT_MAX); >> >> void av_max_alloc(size_t max){ >> - max_alloc_size = max; >> + atomic_store_explicit(&max_alloc_size, max, memory_order_relaxed); >> } >> >> void *av_malloc(size_t size) >> { >> void *ptr = NULL; >> >> - if (size > max_alloc_size) >> + if (size > atomic_load_explicit(&max_alloc_size, memory_order_relaxed)) >> return NULL; >> >> #if HAVE_POSIX_MEMALIGN >> @@ -134,7 +135,7 @@ void *av_malloc(size_t size) >> void *av_realloc(void *ptr, size_t size) >> { >> void *ret; >> - if (size > max_alloc_size) >> + if (size > atomic_load_explicit(&max_alloc_size, memory_order_relaxed)) >> return NULL; >> >> #if HAVE_ALIGNED_MALLOC >> @@ -483,15 +484,19 @@ void av_memcpy_backptr(uint8_t *dst, int back, int cnt) >> >> void *av_fast_realloc(void *ptr, unsigned int *size, size_t min_size) >> { >> + size_t max_size; >> + >> if (min_size <= *size) >> return ptr; >> >> - if (min_size > max_alloc_size) { >> + max_size = atomic_load_explicit(&max_alloc_size, memory_order_relaxed); >> + >> + if (min_size > max_size) { >> *size = 0; >> return NULL; >> } >> >> - min_size = FFMIN(max_alloc_size, FFMAX(min_size + min_size / 16 + 32, min_size)); >> + min_size = FFMIN(max_size, FFMAX(min_size + min_size / 16 + 32, min_size)); >> >> ptr = av_realloc(ptr, min_size); >> /* we could set this to the unmodified min_size but this is safer >> > LGTM apart from the commit message. Fixed commit message and applied, thanks.
Quoting James Almer (2021-05-23 00:09:02) > This is in preparation for the following commit. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavutil/mem.c | 17 +++++++++++------ > 1 file changed, 11 insertions(+), 6 deletions(-) > > diff --git a/libavutil/mem.c b/libavutil/mem.c > index fa227f5e12..c12c24aa90 100644 > --- a/libavutil/mem.c > +++ b/libavutil/mem.c > @@ -31,6 +31,7 @@ > #include <limits.h> > #include <stdint.h> > #include <stdlib.h> > +#include <stdatomic.h> > #include <string.h> > #if HAVE_MALLOC_H > #include <malloc.h> > @@ -68,17 +69,17 @@ void free(void *ptr); > * dynamic libraries and remove -Wl,-Bsymbolic from the linker flags. > * Note that this will cost performance. */ > > -static size_t max_alloc_size= INT_MAX; > +static atomic_size_t max_alloc_size = ATOMIC_VAR_INIT(INT_MAX); > > void av_max_alloc(size_t max){ > - max_alloc_size = max; > + atomic_store_explicit(&max_alloc_size, max, memory_order_relaxed); Any specific reason for using a non-default memory order? AFAIK it is recommended by the spec authors to use default (sequentially consistent) operations unless there is a very good reason to do something else.
On 5/31/2021 5:09 AM, Anton Khirnov wrote: > Quoting James Almer (2021-05-23 00:09:02) >> This is in preparation for the following commit. >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavutil/mem.c | 17 +++++++++++------ >> 1 file changed, 11 insertions(+), 6 deletions(-) >> >> diff --git a/libavutil/mem.c b/libavutil/mem.c >> index fa227f5e12..c12c24aa90 100644 >> --- a/libavutil/mem.c >> +++ b/libavutil/mem.c >> @@ -31,6 +31,7 @@ >> #include <limits.h> >> #include <stdint.h> >> #include <stdlib.h> >> +#include <stdatomic.h> >> #include <string.h> >> #if HAVE_MALLOC_H >> #include <malloc.h> >> @@ -68,17 +69,17 @@ void free(void *ptr); >> * dynamic libraries and remove -Wl,-Bsymbolic from the linker flags. >> * Note that this will cost performance. */ >> >> -static size_t max_alloc_size= INT_MAX; >> +static atomic_size_t max_alloc_size = ATOMIC_VAR_INIT(INT_MAX); >> >> void av_max_alloc(size_t max){ >> - max_alloc_size = max; >> + atomic_store_explicit(&max_alloc_size, max, memory_order_relaxed); > > Any specific reason for using a non-default memory order? > AFAIK it is recommended by the spec authors to use default (sequentially > consistent) operations unless there is a very good reason to do > something else. I copied the existing behavior of av_force_cpu_flags() and av_get_cpu_flags(), which seems to be enough for a similar atomic variable as max_alloc_size.
diff --git a/libavutil/mem.c b/libavutil/mem.c index fa227f5e12..c12c24aa90 100644 --- a/libavutil/mem.c +++ b/libavutil/mem.c @@ -31,6 +31,7 @@ #include <limits.h> #include <stdint.h> #include <stdlib.h> +#include <stdatomic.h> #include <string.h> #if HAVE_MALLOC_H #include <malloc.h> @@ -68,17 +69,17 @@ void free(void *ptr); * dynamic libraries and remove -Wl,-Bsymbolic from the linker flags. * Note that this will cost performance. */ -static size_t max_alloc_size= INT_MAX; +static atomic_size_t max_alloc_size = ATOMIC_VAR_INIT(INT_MAX); void av_max_alloc(size_t max){ - max_alloc_size = max; + atomic_store_explicit(&max_alloc_size, max, memory_order_relaxed); } void *av_malloc(size_t size) { void *ptr = NULL; - if (size > max_alloc_size) + if (size > atomic_load_explicit(&max_alloc_size, memory_order_relaxed)) return NULL; #if HAVE_POSIX_MEMALIGN @@ -134,7 +135,7 @@ void *av_malloc(size_t size) void *av_realloc(void *ptr, size_t size) { void *ret; - if (size > max_alloc_size) + if (size > atomic_load_explicit(&max_alloc_size, memory_order_relaxed)) return NULL; #if HAVE_ALIGNED_MALLOC @@ -483,15 +484,19 @@ void av_memcpy_backptr(uint8_t *dst, int back, int cnt) void *av_fast_realloc(void *ptr, unsigned int *size, size_t min_size) { + size_t max_size; + if (min_size <= *size) return ptr; - if (min_size > max_alloc_size) { + max_size = atomic_load_explicit(&max_alloc_size, memory_order_relaxed); + + if (min_size > max_size) { *size = 0; return NULL; } - min_size = FFMIN(max_alloc_size, FFMAX(min_size + min_size / 16 + 32, min_size)); + min_size = FFMIN(max_size, FFMAX(min_size + min_size / 16 + 32, min_size)); ptr = av_realloc(ptr, min_size); /* we could set this to the unmodified min_size but this is safer
This is in preparation for the following commit. Signed-off-by: James Almer <jamrial@gmail.com> --- libavutil/mem.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)