* [Tarantool-patches] [PATCH] quota: add is_enabled field
@ 2020-12-11 15:37 Ilya Kosarev
2020-12-14 23:41 ` Vladislav Shpilevoy
0 siblings, 1 reply; 4+ messages in thread
From: Ilya Kosarev @ 2020-12-11 15:37 UTC (permalink / raw)
To: v.shpilevoy, alyapunov; +Cc: tarantool-patches
By default the quota is enabled. If it is set to false, quota_use will
allow to overuse the total available memory limit.
In case of disabled quota smalloc() allocates (and frees) large slabs
using malloc so that the quota will be able to shrink back after those
slabs are freed. Test introduced.
Part of tarantool/tarantool#3807
---
Branch: https://github.com/tarantool/small/tree/i.kosarev/gh-3807-safe-alloc-on-truncation
Issue: https://github.com/tarantool/tarantool/issues/3807
small/quota.h | 21 ++++++++++++++++++++-
small/slab_cache.c | 6 ++++++
small/slab_cache.h | 3 +++
small/small.c | 10 ++++++----
test/small_alloc.c | 32 ++++++++++++++++++++++++++++++++
5 files changed, 67 insertions(+), 5 deletions(-)
diff --git a/small/quota.h b/small/quota.h
index 3d3b4f0..c71ee5d 100644
--- a/small/quota.h
+++ b/small/quota.h
@@ -57,6 +57,12 @@ struct quota {
* QUOTA_UNIT_SIZE.
*/
uint64_t value;
+ /**
+ * By default the quota is enabled. If it is set to
+ * false, quota_use will allow to overuse the total
+ * available memory limit.
+ */
+ bool is_enabled;
};
/**
@@ -68,6 +74,7 @@ quota_init(struct quota *quota, size_t total)
uint64_t new_total = (total + (QUOTA_UNIT_SIZE - 1)) /
QUOTA_UNIT_SIZE;
quota->value = new_total << 32;
+ quota->is_enabled = true;
}
/**
@@ -122,6 +129,18 @@ quota_set(struct quota *quota, size_t new_total)
return new_total_in_units * QUOTA_UNIT_SIZE;
}
+static inline void
+quota_enable(struct quota *quota)
+{
+ quota->is_enabled = true;
+}
+
+static inline void
+quota_disable(struct quota *quota)
+{
+ quota->is_enabled = false;
+}
+
/**
* Use up a quota
* @retval > 0 aligned value on success
@@ -143,7 +162,7 @@ quota_use(struct quota *quota, size_t size)
uint32_t new_used_in_units = used_in_units + size_in_units;
assert(new_used_in_units > used_in_units);
- if (new_used_in_units > total_in_units)
+ if (new_used_in_units > total_in_units && quota->is_enabled)
return -1;
uint64_t new_value =
diff --git a/small/slab_cache.c b/small/slab_cache.c
index 2ba56c5..b3b7004 100644
--- a/small/slab_cache.c
+++ b/small/slab_cache.c
@@ -462,3 +462,9 @@ slab_cache_check(struct slab_cache *cache)
return;
abort();
}
+
+bool
+slab_cache_quota_enabled(struct slab_cache *cache)
+{
+ return cache->arena->quota->is_enabled;
+}
diff --git a/small/slab_cache.h b/small/slab_cache.h
index d81c1c5..bbfdc74 100644
--- a/small/slab_cache.h
+++ b/small/slab_cache.h
@@ -271,6 +271,9 @@ slab_from_data(void *data)
void
slab_cache_check(struct slab_cache *cache);
+bool
+slab_cache_quota_enabled(struct slab_cache *cache);
+
/**
* Find the nearest power of 2 size capable of containing
* a chunk of the given size. Adjust for cache->order0_size
diff --git a/small/small.c b/small/small.c
index 48085fb..154a0e9 100644
--- a/small/small.c
+++ b/small/small.c
@@ -228,7 +228,7 @@ smalloc(struct small_alloc *alloc, size_t size)
struct mempool *pool;
int idx = (size - 1) >> STEP_SIZE_LB;
idx = (idx > (int) alloc->step_pool0_step_count) ? idx - alloc->step_pool0_step_count : 0;
- if (idx < STEP_POOL_MAX) {
+ if (idx < STEP_POOL_MAX && slab_cache_quota_enabled(alloc->cache)) {
/* Allocate in a stepped pool. */
pool = &alloc->step_pools[idx];
assert(size <= pool->objsize &&
@@ -238,7 +238,8 @@ smalloc(struct small_alloc *alloc, size_t size)
pattern.pool.objsize = size;
struct factor_pool *upper_bound =
factor_tree_nsearch(&alloc->factor_pools, &pattern);
- if (upper_bound == NULL) {
+ if (upper_bound == NULL ||
+ !slab_cache_quota_enabled(alloc->cache)) {
/* Object is too large, fallback to slab_cache */
struct slab *slab = slab_get_large(alloc->cache, size);
if (slab == NULL)
@@ -276,7 +277,7 @@ mempool_find(struct small_alloc *alloc, size_t size)
struct mempool *pool;
int idx = (size - 1) >> STEP_SIZE_LB;
idx = (idx > (int) alloc->step_pool0_step_count) ? idx - alloc->step_pool0_step_count : 0;
- if (idx < STEP_POOL_MAX) {
+ if (idx < STEP_POOL_MAX && slab_cache_quota_enabled(alloc->cache)) {
/* Allocated in a stepped pool. */
pool = &alloc->step_pools[idx];
assert((size + STEP_SIZE > pool->objsize) || (idx == 0));
@@ -286,7 +287,8 @@ mempool_find(struct small_alloc *alloc, size_t size)
pattern.pool.objsize = size;
struct factor_pool *upper_bound =
factor_tree_nsearch(&alloc->factor_pools, &pattern);
- if (upper_bound == NULL)
+ if (upper_bound == NULL ||
+ !slab_cache_quota_enabled(alloc->cache))
return NULL; /* Allocated by slab_cache. */
assert(size >= upper_bound->objsize_min);
pool = &upper_bound->pool;
diff --git a/test/small_alloc.c b/test/small_alloc.c
index 421442a..1563cc9 100644
--- a/test/small_alloc.c
+++ b/test/small_alloc.c
@@ -128,6 +128,37 @@ small_alloc_large(void)
footer();
}
+static void
+small_quota_release(void)
+{
+ header();
+
+ size_t total, used;
+ size_t alloc_size = 1000;
+ quota_get_total_and_used("a, &total, &used);
+ size_t amount = (total - used) / alloc_size;
+
+ int **a = calloc(amount, sizeof(int *));
+ int i = 0;
+ while (quota_use("a, 1) >= 0) {
+ quota_disable("a);
+ a[i++] = smalloc(&alloc, alloc_size);
+ quota_enable("a);
+ }
+
+ quota_get_total_and_used("a, &total, &used);
+ fail_unless((int)total - (int)used < 0);
+
+ quota_disable("a);
+ smfree(&alloc, a[0], alloc_size);
+ quota_enable("a);
+ free(a);
+
+ quota_get_total_and_used("a, &total, &used);
+ fail_unless((int)total - (int)used > 0);
+ footer();
+}
+
int main()
{
seed = time(0);
@@ -142,6 +173,7 @@ int main()
small_alloc_basic();
small_alloc_large();
+ small_quota_release();
slab_cache_destroy(&cache);
}
--
2.17.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Tarantool-patches] [PATCH] quota: add is_enabled field
2020-12-11 15:37 [Tarantool-patches] [PATCH] quota: add is_enabled field Ilya Kosarev
@ 2020-12-14 23:41 ` Vladislav Shpilevoy
0 siblings, 0 replies; 4+ messages in thread
From: Vladislav Shpilevoy @ 2020-12-14 23:41 UTC (permalink / raw)
To: Ilya Kosarev, alyapunov; +Cc: tarantool-patches
Thanks for the patch!
See 5 comments below.
On 11.12.2020 16:37, Ilya Kosarev wrote:
> By default the quota is enabled. If it is set to false, quota_use will
> allow to overuse the total available memory limit.
> In case of disabled quota smalloc() allocates (and frees) large slabs
> using malloc so that the quota will be able to shrink back after those
> slabs are freed. Test introduced.
>
> Part of tarantool/tarantool#3807
> ---
> Branch: https://github.com/tarantool/small/tree/i.kosarev/gh-3807-safe-alloc-on-truncation
> Issue: https://github.com/tarantool/tarantool/issues/3807
>
> small/quota.h | 21 ++++++++++++++++++++-
> small/slab_cache.c | 6 ++++++
> small/slab_cache.h | 3 +++
> small/small.c | 10 ++++++----
> test/small_alloc.c | 32 ++++++++++++++++++++++++++++++++
> 5 files changed, 67 insertions(+), 5 deletions(-)
>
> diff --git a/small/quota.h b/small/quota.h
> index 3d3b4f0..c71ee5d 100644
> --- a/small/quota.h
> +++ b/small/quota.h
> @@ -57,6 +57,12 @@ struct quota {
> * QUOTA_UNIT_SIZE.
> */
> uint64_t value;
> + /**
> + * By default the quota is enabled. If it is set to
> + * false, quota_use will allow to overuse the total
> + * available memory limit.
> + */
> + bool is_enabled;
1. Quota is accessed from many threads. This is why the 'value' is
updated using atomic cmpxchg operations. Here it seems you update
the quota is_enabled from tx thread, but the flag is used by
all threads.
Perhaps we could make the flag atomic as well. But I don't know
how often is the quota accessed, and if we can add more atomic
operations on each use().
I suggest you to ask Alexander. Since he is with us now, he should
be able to tell if we are going to the right direction.
> };
>
> /**
> diff --git a/small/slab_cache.c b/small/slab_cache.c
> index 2ba56c5..b3b7004 100644
> --- a/small/slab_cache.c
> +++ b/small/slab_cache.c
> @@ -462,3 +462,9 @@ slab_cache_check(struct slab_cache *cache)
> return;
> abort();
> }
> +
> +bool
> +slab_cache_quota_enabled(struct slab_cache *cache)
2. For checks we always include 'is' as a part of the name.
Also 'cache' can be declared 'const' here.
> +{
> + return cache->arena->quota->is_enabled;
> +}
> diff --git a/small/small.c b/small/small.c
> index 48085fb..154a0e9 100644
> --- a/small/small.c
> +++ b/small/small.c
> @@ -228,7 +228,7 @@ smalloc(struct small_alloc *alloc, size_t size)
> struct mempool *pool;
> int idx = (size - 1) >> STEP_SIZE_LB;
> idx = (idx > (int) alloc->step_pool0_step_count) ? idx - alloc->step_pool0_step_count : 0;
> - if (idx < STEP_POOL_MAX) {
> + if (idx < STEP_POOL_MAX && slab_cache_quota_enabled(alloc->cache)) {
3. These checks are super expensive - you dereference 3! pointers
for each such check. Why do you even needed? Why isn't it enough
to patch the quota?
> /* Allocate in a stepped pool. */
> pool = &alloc->step_pools[idx];
> assert(size <= pool->objsize &&
> diff --git a/test/small_alloc.c b/test/small_alloc.c
> index 421442a..1563cc9 100644
> --- a/test/small_alloc.c
> +++ b/test/small_alloc.c
> @@ -128,6 +128,37 @@ small_alloc_large(void)
> footer();
> }
>
> +static void
> +small_quota_release(void)
> +{
> + header();
> +
> + size_t total, used;
> + size_t alloc_size = 1000;
> + quota_get_total_and_used("a, &total, &used);
> + size_t amount = (total - used) / alloc_size;
> +
> + int **a = calloc(amount, sizeof(int *));
> + int i = 0;
> + while (quota_use("a, 1) >= 0) {
> + quota_disable("a);
> + a[i++] = smalloc(&alloc, alloc_size);
> + quota_enable("a);
> + }
> +
> + quota_get_total_and_used("a, &total, &used);
> + fail_unless((int)total - (int)used < 0);
4. Why so complex? Why can't you simply write 'total < used'?
The same below.
> +
> + quota_disable("a);
> + smfree(&alloc, a[0], alloc_size);
5. So you did 'amount' allocations, but freed only the
first one. Why?
> + quota_enable("a);
> + free(a);
> +
> + quota_get_total_and_used("a, &total, &used);
> + fail_unless((int)total - (int)used > 0);
> + footer();
> +}
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Tarantool-patches] [PATCH] quota: add is_enabled field
2020-02-14 19:31 Ilya Kosarev
@ 2020-02-14 22:48 ` Vladislav Shpilevoy
0 siblings, 0 replies; 4+ messages in thread
From: Vladislav Shpilevoy @ 2020-02-14 22:48 UTC (permalink / raw)
To: Ilya Kosarev, tarantool-patches
Hi!
On 14/02/2020 20:31, Ilya Kosarev wrote:
> By default the quota is enabled. If it is set to false, quota_use will
> allow to overuse the total available memory limit.
>
> Part of tarantool/tarantool#3807
> ---
> Branch: https://github.com/tarantool/small/tree/i.kosarev/gh-3807-safe-alloc-on-truncation
> Issue: https://github.com/tarantool/tarantool/issues/3807
>
> small/quota.h | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
Please, add tests.
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Tarantool-patches] [PATCH] quota: add is_enabled field
@ 2020-02-14 19:31 Ilya Kosarev
2020-02-14 22:48 ` Vladislav Shpilevoy
0 siblings, 1 reply; 4+ messages in thread
From: Ilya Kosarev @ 2020-02-14 19:31 UTC (permalink / raw)
To: tarantool-patches; +Cc: v.shpilevoy
By default the quota is enabled. If it is set to false, quota_use will
allow to overuse the total available memory limit.
Part of tarantool/tarantool#3807
---
Branch: https://github.com/tarantool/small/tree/i.kosarev/gh-3807-safe-alloc-on-truncation
Issue: https://github.com/tarantool/tarantool/issues/3807
small/quota.h | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/small/quota.h b/small/quota.h
index 3d3b4f0..fbb039e 100644
--- a/small/quota.h
+++ b/small/quota.h
@@ -57,6 +57,12 @@ struct quota {
* QUOTA_UNIT_SIZE.
*/
uint64_t value;
+ /**
+ * By default the quota is enabled. If it is set to
+ * false, quota_use will allow to overuse the total
+ * available memory limit.
+ */
+ bool is_enabled;
};
/**
@@ -68,6 +74,7 @@ quota_init(struct quota *quota, size_t total)
uint64_t new_total = (total + (QUOTA_UNIT_SIZE - 1)) /
QUOTA_UNIT_SIZE;
quota->value = new_total << 32;
+ quota->is_enabled = true;
}
/**
@@ -122,6 +129,12 @@ quota_set(struct quota *quota, size_t new_total)
return new_total_in_units * QUOTA_UNIT_SIZE;
}
+static inline void
+quota_on_off(struct quota *quota, bool is_enabled)
+{
+ quota->is_enabled = is_enabled;
+}
+
/**
* Use up a quota
* @retval > 0 aligned value on success
@@ -143,7 +156,7 @@ quota_use(struct quota *quota, size_t size)
uint32_t new_used_in_units = used_in_units + size_in_units;
assert(new_used_in_units > used_in_units);
- if (new_used_in_units > total_in_units)
+ if (new_used_in_units > total_in_units && quota->is_enabled)
return -1;
uint64_t new_value =
--
2.17.1
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-12-14 23:41 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-11 15:37 [Tarantool-patches] [PATCH] quota: add is_enabled field Ilya Kosarev
2020-12-14 23:41 ` Vladislav Shpilevoy
-- strict thread matches above, loose matches on Subject: below --
2020-02-14 19:31 Ilya Kosarev
2020-02-14 22:48 ` Vladislav Shpilevoy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox