lsregion: introduce lsregion_to_iovec method

[sergepetrenko]

lsregion is a good candidate for an output buffer, maybe even better than obuf:

Obuf may be freed / flushed only completely, meaning you need two obufs to be able to simultaneously flush one of them and write to the other.

At the same time, lsregion may be gradually freed up to the last flushed position without blocking the incoming writes.

In order to use lsregion as an output buffer, introduce lsregion_to_iovec method, which will simply enumerate all the allocated slabs and their sizes and save their contents to the provided iovec array.

Since the last flushed slab may still be used by further allocations (which may happen while the flushed data is being written), introduce a lsregion_svp helper struct, which tracks the last flushed position.

Last flushed slab is determined by flush_id - max_id used for allocations in this slab at the moment of flush.

Compared to obuf (which stores data in a set of <= 32 progressively growing iovecs), it's theoretically possible to overflow the IOV_MAX (1024) by using lsregion, and thus require multiple writev calls to flush it, but given that lsregion is usually created with runtime slab arenam, which allocates 4 megabyte slabs to store the data, it's highly unlikely. It would require allocating 4 gigabytes of data per one flush.

Needed for https://github.com/tarantool/tarantool/issues/10161

[sergepetrenko]

@Gerold103 BTW I noticed there's a separate lsregion_asan implementation. Do I need to provide lsregion_to_iovec for it as well?

[locker]

BTW I noticed there's a separate lsregion_asan implementation. Do I need to provide lsregion_to_iovec for it as well?

Yep, it seems like you do - we use different implementations of small allocators for ASAN-enabled builds to detect leaks and invalid accesses.

Cc: @nshy who implemented the feature.

[sergepetrenko]

Yep, it seems like you do - we use different implementations of small allocators for ASAN-enabled builds to detect leaks and invalid accesses.

Cc: @nshy who implemented the feature.

Ok, thanks!

[sergepetrenko]

I've added the asan implementation and rearranged the test a bit:

asan implementation

```diff diff --git a/include/small/lsregion.h b/include/small/lsregion.h index 12b9370..c300a54 100644 --- a/include/small/lsregion.h +++ b/include/small/lsregion.h @@ -31,6 +31,31 @@ * SUCH DAMAGE. */ #include "small_config.h" +#include + +#define LSLAB_NOT_USED_ID -1 + +/** + * A helper struct remembering a specific position in the lsregion. Is stable to + * lslab reuse. + */ +struct lsregion_svp { + /** + * The max slab id that was seen during a flush. Using it we can + * find the slab we flushed previously and check if it has any extra + * data to flush. + */ + int64_t slab_id; + /** A position in the last seen slab. */ + size_t pos; +}; + +static inline void +lsregion_svp_create(struct lsregion_svp *svp) +{ + svp->slab_id = LSLAB_NOT_USED_ID; + svp->pos = 0; +} #ifdef ENABLE_ASAN # include "lsregion_asan.h" @@ -38,7 +63,6 @@ #ifndef ENABLE_ASAN -#include #include #include #include @@ -51,8 +75,6 @@ extern "C" { #endif /* defined(__cplusplus) */ -#define LSLAB_NOT_USED_ID -1 - /** * Wrapper for a slab that tracks a size of used memory and * maximal identifier of memory that was allocated in the slab. @@ -175,28 +197,6 @@ lslab_end(struct lslab *slab) return (char *)slab + slab->slab_size; } -/** - * A helper struct remembering a specific position in the lsregion. Is stable to - * lslab reuse. - */ -struct lsregion_svp { - /** - * The max slab id that was seen during a flush. Using it we can - * find the slab we flushed previously and check if it has any extra - * data to flush. - */ - int64_t slab_id; - /** A position in the lslab containing an allocation with \a id. */ - size_t pos; -}; - -static inline void -lsregion_svp_create(struct lsregion_svp *svp) -{ - svp->slab_id = LSLAB_NOT_USED_ID; - svp->pos = 0; -} - /** * Initialize log structured allocator. * @param lsregion Allocator object. @@ -364,6 +364,7 @@ lsregion_gc(struct lsregion *lsregion, int64_t min_id) * Returns how many iovecs were actually used. * @param[in,out] svp A savepoint pointing at the end of flushed data. * @return max allocation id that was flushed. + * LSLAB_NOT_USED_ID if nothing was flushed. */ int64_t lsregion_to_iovec(const struct lsregion *lsregion, struct iovec *iov, diff --git a/include/small/lsregion_asan.h b/include/small/lsregion_asan.h index 7976551..9b8f0c9 100644 --- a/include/small/lsregion_asan.h +++ b/include/small/lsregion_asan.h @@ -30,6 +30,7 @@ */ #include #include +#include #include "rlist.h" #include "slab_arena.h" @@ -87,6 +88,10 @@ lsregion_alloc(struct lsregion *lsregion, size_t size, int64_t id) void lsregion_gc(struct lsregion *lsregion, int64_t min_id); +int64_t +lsregion_to_iovec(const struct lsregion *lsregion, struct iovec *iov, + int *iovcnt, struct lsregion_svp *svp); + static inline void lsregion_destroy(struct lsregion *lsregion) { diff --git a/small/lsregion.c b/small/lsregion.c index 4c8b594..4b3446c 100644 --- a/small/lsregion.c +++ b/small/lsregion.c @@ -98,7 +98,7 @@ lsregion_to_iovec(const struct lsregion *lsregion, struct iovec *iov, { struct lslab *lslab; int64_t prev_id = svp->slab_id; - int64_t max_alloc_id = 0; + int64_t max_alloc_id = LSLAB_NOT_USED_ID; size_t prev_pos = svp->pos; int max_iovcnt = *iovcnt; int cnt = 0; diff --git a/small/lsregion_asan.c b/small/lsregion_asan.c index b3f29c8..78cfa76 100644 --- a/small/lsregion_asan.c +++ b/small/lsregion_asan.c @@ -59,3 +59,36 @@ lsregion_gc(struct lsregion *lsregion, int64_t min_id) small_asan_free(alloc); } } + +SMALL_NO_SANITIZE_ADDRESS int64_t +lsregion_to_iovec(const struct lsregion *lsregion, struct iovec *iov, + int *iovcnt, struct lsregion_svp *svp) +{ + struct lsregion_allocation *alloc; + int64_t prev_id = svp->slab_id; + int max_iovcnt = *iovcnt; + int64_t max_alloc_id = LSLAB_NOT_USED_ID; + int cnt = 0; + rlist_foreach_entry(alloc, &lsregion->allocations, link) { + /* An already seen allocation. */ + if (alloc->id <= prev_id) + continue; + if (cnt >= max_iovcnt) + break; + iov->iov_base = small_asan_payload_from_header(alloc); + iov->iov_len = alloc->size; + svp->slab_id = alloc->id; + /* + * This isn't correct but will do for tests which span one slab + * in a normal lsregion implementation. pos isn't used to + * determine flush position here anyway, so it's ok. + */ + svp->pos += alloc->size; + max_alloc_id = alloc->id; + + iov++; + cnt++; + } + *iovcnt = cnt; + return max_alloc_id; +} ```

test diff

```diff diff --git a/test/lsregion.c b/test/lsregion.c index ab51d27..9394998 100644 --- a/test/lsregion.c +++ b/test/lsregion.c @@ -532,10 +532,24 @@ test_aligned(void) check_plan(); } +static void +iov_to_data(const struct iovec *iov, int iovcnt, char **data, uint32_t count, + uint32_t size) +{ + uint32_t cnt = 0; + for (int i = 0; i < iovcnt; i++) { + fail_if(iov[i].iov_len % size != 0); + for (int j = 0; j * size < iov[i].iov_len; j++) { + data[cnt++] = (char *)iov[i].iov_base + j * size; + } + } + fail_if(cnt != count); +} + static void test_to_iovec(void) { - plan(18); + plan(13); header(); struct quota quota; @@ -549,72 +563,69 @@ test_to_iovec(void) int count = TEST_ARRAY_SIZE; size_t size = 10; char *data[count]; + struct iovec iov[count]; + memset(iov, 0, sizeof(iov)); + int iovcnt = 0; + struct lsregion_svp svp; + lsregion_svp_create(&svp); /* Test multiple allocations using the same slab. */ fail_if(count * size >= arena.slab_size); fill_data(data, count, size, id, &allocator); - struct iovec iov[1] = {0}; - int iovcnt = 0; - struct lsregion_svp svp; - lsregion_svp_create(&svp); /* Test zero iovcnt. */ - lsregion_to_iovec(&allocator, iov, &iovcnt, &svp); + int64_t rc_id = lsregion_to_iovec(&allocator, iov, &iovcnt, &svp); + is(rc_id, LSLAB_NOT_USED_ID); is(iovcnt, 0); is(iov->iov_base, NULL); is(iov->iov_len, 0); - iovcnt = 1; - int64_t rc_id = lsregion_to_iovec(&allocator, iov, &iovcnt, &svp); + iovcnt = lengthof(iov); + rc_id = lsregion_to_iovec(&allocator, iov, &iovcnt, &svp); /* Check basic return values. */ - is(iov->iov_len, count * size); - is(iovcnt, 1); id += count; is(rc_id, id - 1); - is(svp.pos, iov->iov_len); - /* Check iovec contents. */ - for (int i = 0; i < count; i++) { - data[i] = iov->iov_base + i * size; - } + is(svp.pos, count * size); + iov_to_data(iov, iovcnt, data, count, size); test_data(data, count, size); /* Check second flush of the same data */ struct lsregion_svp old_svp = svp; int64_t old_id = rc_id; rc_id = lsregion_to_iovec(&allocator, iov, &iovcnt, &svp); - is(rc_id, 0); + is(rc_id, LSLAB_NOT_USED_ID); is(old_svp.slab_id, svp.slab_id); is(old_svp.pos, svp.pos); is(iovcnt, 0); /* Check new alloc and flush without gc. */ - iovcnt = 1; + iovcnt = 10; fill_data(data, count, size, id, &allocator); rc_id = lsregion_to_iovec(&allocator, iov, &iovcnt, &svp); is(rc_id, old_id + count); is(svp.pos, old_svp.pos + count * size); - is(iov->iov_len, count * size); - is(iovcnt, 1); - for (int i = 0; i < count; i++) { - data[i] = iov->iov_base + i * size; - } + iov_to_data(iov, iovcnt, data, count, size); test_data(data, count, size); +#ifndef ENABLE_ASAN id += count; lsregion_gc(&allocator, id - 1); + fail_if(lsregion_used(&allocator) != 0); /* * Check allocation spanning multiple slabs. * Flush one slab at a time. + * + * The test doesn't make sense for asan build, since there one + * allocation always takes a separate "slab". */ size = arena.slab_size / 4; fill_data(data, count, size, id, &allocator); - int cnt = 0; + iovcnt = 1; lsregion_to_iovec(&allocator, iov, &iovcnt, &svp); + struct iovec *tmp_iov = iov; while (iovcnt != 0) { - for (int i = 0; i * size < iov->iov_len; i++) { - data[cnt++] = iov->iov_base + i * size; - } - lsregion_to_iovec(&allocator, iov, &iovcnt, &svp); + tmp_iov += iovcnt; + lsregion_to_iovec(&allocator, tmp_iov, &iovcnt, &svp); } - is(cnt, count); + iov_to_data(iov, tmp_iov - iov, data, count, size); test_data(data, count, size); id += count; @@ -624,18 +635,12 @@ test_to_iovec(void) * Check allocation spanning multiple slabs. * Flush them all at once. */ - struct iovec iov_long[4]; - iovcnt = lengthof(iov_long); + iovcnt = lengthof(iov); fill_data(data, count, size, id, &allocator); - lsregion_to_iovec(&allocator, iov_long, &iovcnt, &svp); - cnt = 0; - for (int i = 0; i < iovcnt; i++) { - for (int j = 0; j * size < iov_long[i].iov_len; j++) { - data[cnt++] = iov_long[i].iov_base + j * size; - } - } - is(cnt, count); + lsregion_to_iovec(&allocator, iov, &iovcnt, &svp); + iov_to_data(iov, iovcnt, data, count, size); test_data(data, count, size); +#endif lsregion_destroy(&allocator); slab_arena_destroy(&arena); ```