This is an automated email from the git hooks/post-receive script.

teigland pushed a commit to branch master in repository sanlock.

commit 66ed5ea259da54939ae8c02533a4238c6e3e053d Author: David Teigland teigland@redhat.com AuthorDate: Mon Aug 19 16:22:39 2024 -0500

sanlock: reuse io buffer with hugepages

Avoid repeating malloc/madvise/free every time an io buffer is needed to read the full paxos lease area. --- src/paxos_lease.c | 168 ++++++++++++++++++++++++++++++++++++------------- src/sanlock_internal.h | 6 +- src/task.c | 3 + 3 files changed, 133 insertions(+), 44 deletions(-)

diff --git a/src/paxos_lease.c b/src/paxos_lease.c index e62a93f..8f7d816 100644 --- a/src/paxos_lease.c +++ b/src/paxos_lease.c @@ -505,7 +505,8 @@ static int run_ballot(struct task *task, struct token *token, uint32_t flags, int read_len; int iobuf_len; int read_mem_align; - int is_huge = 0; + int use_huge = 0; + int use_task_hugebuf = 0; int phase2 = 0; int d, q, rv = 0; int q_max = -1; @@ -517,34 +518,59 @@ static int run_ballot(struct task *task, struct token *token, uint32_t flags, iobuf_len = token->align_size; /* can increase (from 1MB) for hugepage */ read_mem_align = getpagesize(); /* increases to 2MB for hugepage */

- /* TODO: for hugepages, the task should keep a huge iobuf around to reuse - rather than alloc+madvise+free each time; after a while of heavy use, - madvise may be less likely to successfully turn the iobuf into a hugepage. */ - - if (com.use_hugepages && ((iobuf_len == ONE_MB_IN_BYTES) || !(iobuf_len % TWO_MB_IN_BYTES))) { + /* Determine if hugepages/hubebuf is applicable. */ + if (com.use_hugepages && (num_disks == 1) && + ((iobuf_len == ONE_MB_IN_BYTES) || !(iobuf_len % TWO_MB_IN_BYTES))) { read_mem_align = TWO_MB_IN_BYTES; if (iobuf_len < TWO_MB_IN_BYTES) iobuf_len = TWO_MB_IN_BYTES; - is_huge = 1; + use_huge = 1; + } + + /* Reuse a previously allocated buffer. */ + if (use_huge && task->paxos_hugebuf && (task->paxos_hugebuf_size >= iobuf_len)) { + use_task_hugebuf = 1; + iobuf_array[0] = task->paxos_hugebuf; + task->paxos_hugebuf_used++; + goto mem_done; }

+ /* Allocate a new buffer. */ for (d = 0; d < num_disks; d++) { p_iobuf[d] = &iobuf_array[d];

rv = posix_memalign((void *)p_iobuf[d], read_mem_align, iobuf_len); if (rv) return rv; + } + + /* Make the new buffer use hugepages. */ + if (use_huge) { + char *bp; + iobuf = iobuf_array[0]; + madvise(iobuf, iobuf_len, MADV_HUGEPAGE); + /* allocate the pages */ + for (bp = iobuf; bp < iobuf+iobuf_len; bp += TWO_MB_IN_BYTES) + memset(bp, 0, 1);

- if (is_huge) { - char *bp; - iobuf = iobuf_array[d]; - madvise(iobuf, iobuf_len, MADV_HUGEPAGE); - /* allocate the pages */ - for (bp = iobuf; bp < iobuf+iobuf_len; bp += TWO_MB_IN_BYTES) - memset(bp, 0, 1); + /* + * Save the new buffer to reuse if the task doesn't + * have one (Note: the task may already have paxos_hugebuf + * with a smaller size than we need here, in which case we + * ignore that, allocate a new one, and free it at the end.) + * TODO: add config min_hugebuf_size to avoid that case? + */ + if (!task->paxos_hugebuf) { + task->paxos_hugebuf = iobuf; + task->paxos_hugebuf_size = iobuf_len; + task->paxos_hugebuf_created++; + use_task_hugebuf = 1; + log_taskd(task, "created hugebuf %p %u", iobuf, task->paxos_hugebuf_created); } }

+ mem_done: + /* * phase 1 * @@ -570,7 +596,7 @@ static int run_ballot(struct task *task, struct token *token, uint32_t flags, num_writes = 0;

for (d = 0; d < num_disks; d++) { - /* acquire io: write 1 */ + /* acquire io: write 1 (sector) */ rv = write_dblock(task, token, &token->disks[d], token->host_id, &dblock); if (rv < 0) continue; @@ -599,10 +625,17 @@ static int run_ballot(struct task *task, struct token *token, uint32_t flags,

memset(iobuf, 0, read_len);

- /* acquire io: read 2 */ + /* acquire io: read 2 (lease area) */ rv = read_iobuf(disk->fd, disk->offset, iobuf, read_len, task, token->io_timeout, NULL); - if (rv == SANLK_AIO_TIMEOUT) + if (rv == SANLK_AIO_TIMEOUT) { iobuf_array[d] = NULL; + if (use_task_hugebuf) { + /* hugebuf will be freed when aio completes */ + task->paxos_hugebuf = NULL; + task->paxos_hugebuf_size = 0; + use_task_hugebuf = 0; + } + } if (rv < 0) continue; num_reads++; @@ -788,7 +821,7 @@ static int run_ballot(struct task *task, struct token *token, uint32_t flags, num_writes = 0;

for (d = 0; d < num_disks; d++) { - /* acquire io: write 2 */ + /* acquire io: write 2 (sector) */ rv = write_dblock(task, token, &token->disks[d], token->host_id, &dblock); if (rv < 0) continue; @@ -817,10 +850,17 @@ static int run_ballot(struct task *task, struct token *token, uint32_t flags,

memset(iobuf, 0, read_len);

- /* acquire io: read 3 */ + /* acquire io: read 3 (lease area) */ rv = read_iobuf(disk->fd, disk->offset, iobuf, read_len, task, token->io_timeout, NULL); - if (rv == SANLK_AIO_TIMEOUT) + if (rv == SANLK_AIO_TIMEOUT) { iobuf_array[d] = NULL; + if (use_task_hugebuf) { + /* hugebuf will be freed when aio completes */ + task->paxos_hugebuf = NULL; + task->paxos_hugebuf_size = 0; + use_task_hugebuf = 0; + } + } if (rv < 0) continue; num_reads++; @@ -939,13 +979,19 @@ static int run_ballot(struct task *task, struct token *token, uint32_t flags, memcpy(dblock_out, &dblock, sizeof(struct paxos_dblock)); error = SANLK_OK; out: - for (d = 0; d < num_disks; d++) { - /* don't free iobufs that have timed out */ - if (!iobuf_array[d]) - continue; - - /* TODO: the task should keep huge iobuf and reuse it. */ - free(iobuf_array[d]); + if (use_task_hugebuf) { + if (rv != SANLK_AIO_TIMEOUT) { + /* Keep buffer to reuse. */ + memset(task->paxos_hugebuf, 0, task->paxos_hugebuf_size); + } + } else { + for (d = 0; d < num_disks; d++) { + /* don't free iobufs that have timed out */ + if (!iobuf_array[d]) + continue; + free(iobuf_array[d]); + iobuf_array[d] = NULL; + } }

if (phase2 && (error < 0) && @@ -1415,7 +1461,8 @@ static int _lease_read_one(struct task *task, int read_len; int iobuf_len; int read_mem_align; - int is_huge = 0; + int use_huge = 0; + int use_task_hugebuf = 0; int q, tmp_q = -1, rv;

/* FIXME: what case is this? */ @@ -1428,27 +1475,48 @@ static int _lease_read_one(struct task *task, iobuf_len = token->align_size; /* can increase (from 1MB) for hugepage */ read_mem_align = getpagesize(); /* increases to 2MB for hugepage */

+ /* Determine if hugepages/hubebuf is applicable. */ if (com.use_hugepages && ((iobuf_len == ONE_MB_IN_BYTES) || !(iobuf_len % TWO_MB_IN_BYTES))) { read_mem_align = TWO_MB_IN_BYTES; if (iobuf_len < TWO_MB_IN_BYTES) iobuf_len = TWO_MB_IN_BYTES; - is_huge = 1; + use_huge = 1; }

- p_iobuf = &iobuf; + /* Reuse a previously allocated buffer. */ + if (use_huge && task->paxos_hugebuf && (task->paxos_hugebuf_size >= iobuf_len)) { + use_task_hugebuf = 1; + iobuf = task->paxos_hugebuf; + task->paxos_hugebuf_used++; + goto mem_done; + }

+ /* Allocate a new buffer. */ + p_iobuf = &iobuf; rv = posix_memalign((void *)p_iobuf, read_mem_align, iobuf_len); if (rv) return rv;

- if (is_huge) { + /* Make the new buffer use hugepages. */ + if (use_huge) { char *bp; madvise(iobuf, iobuf_len, MADV_HUGEPAGE); /* allocate the pages */ for (bp = iobuf; bp < iobuf+iobuf_len; bp += TWO_MB_IN_BYTES) memset(bp, 0, 1); + + /* Save the new buffer to reuse if the task doesn't have one. */ + if (!task->paxos_hugebuf) { + task->paxos_hugebuf = iobuf; + task->paxos_hugebuf_size = iobuf_len; + task->paxos_hugebuf_created++; + use_task_hugebuf = 1; + log_taskd(task, "created hugebuf %p %u", iobuf, task->paxos_hugebuf_created); + } }

+ mem_done: + rv = read_iobuf(disk->fd, disk->offset, iobuf, read_len, task, token->io_timeout, NULL); if (rv < 0) goto out; @@ -1520,8 +1588,20 @@ static int _lease_read_one(struct task *task, bk_debug);

out: - if (rv != SANLK_AIO_TIMEOUT) - free(iobuf); + if (use_task_hugebuf) { + if (rv != SANLK_AIO_TIMEOUT) { + /* Keep buffer to reuse. */ + memset(task->paxos_hugebuf, 0, task->paxos_hugebuf_size); + } else { + /* hugebuf will be freed when aio completes */ + task->paxos_hugebuf = NULL; + task->paxos_hugebuf_size = 0; + } + } else { + if (rv != SANLK_AIO_TIMEOUT) + free(iobuf); + /* else iobuf will be freed when aio completes */ + } return rv; }

@@ -1717,19 +1797,19 @@ static int write_new_leader(struct task *task, */

/* - * i/o required to acquire a free lease - * (1 disk in token, 512 byte sectors, default num_hosts of 2000) + * acquire io: disk ios needed to acquire a free and uncontended lease * * paxos_lease_acquire() - * paxos_lease_read() 1 read 1 MB (entire lease area) - * run_ballot() - * write_dblock() 1 write 512 bytes (1 dblock sector) - * read_iobuf() 1 read 1 MB (round up num_hosts + 2 sectors) - * write_dblock() 1 write 512 bytes (1 dblock sector) - * read_iobuf() 1 read 1 MB (round up num_hosts + 2 sectors) - * write_new_leader() 1 write 512 bytes (1 leader sector) + * R1 paxos_lease_read() 1 read 1 MB (lease area size) + * run_ballot() + * W1 write_dblock() 1 write 512 bytes (1 dblock sector) + * R2 read_iobuf() 1 read 1 MB (lease area size) + * W2 write_dblock() 1 write 512 bytes (1 dblock sector) + * R3 read_iobuf() 1 read 1 MB (lease area size) + * W3 write_new_leader() 1 write 512 bytes (1 leader sector) * * 6 i/os = 3 1MB reads, 3 512 byte writes + * (assuming 512 sector size and 1MB area size) */

int paxos_lease_acquire(struct task *task, @@ -1778,7 +1858,7 @@ int paxos_lease_acquire(struct task *task, memset(&tmp_leader, 0, sizeof(tmp_leader)); copy_cur_leader = 0;

- /* acquire io: read 1 */ + /* acquire io: read 1 (lease area) */ error = paxos_lease_read(task, token, flags, &cur_leader, &max_mbal, "paxos_acquire", 1); if (error < 0) goto out; @@ -2216,6 +2296,7 @@ int paxos_lease_acquire(struct task *task, next_lver = larger_lver + 1; }

+ /* acquire io: write 1 (sector), read 2 (lease area), write 2 (sector), read 3 (lease area) */ error = run_ballot(task, token, flags, cur_leader.num_hosts, next_lver, our_mbal, &dblock, &other_max_mbal, &larger_lver);

if ((error == SANLK_DBLOCK_MBAL) || (error == SANLK_DBLOCK_LVER)) { @@ -2296,6 +2377,7 @@ int paxos_lease_acquire(struct task *task,

new_leader.checksum = 0; /* set after leader_record_out */

+ /* acquire io: write 3 (sector) */ error = write_new_leader(task, token, &new_leader, "paxos_acquire"); if (error < 0) { /* See comment in run_ballot about this flag. */ diff --git a/src/sanlock_internal.h b/src/sanlock_internal.h index 7725eb6..22b7698 100644 --- a/src/sanlock_internal.h +++ b/src/sanlock_internal.h @@ -276,10 +276,14 @@ struct task {

unsigned int io_count; /* stats */ unsigned int to_count; /* stats */ + unsigned int paxos_hugebuf_used; /* stats */ + unsigned int paxos_hugebuf_created; /* stats */

int use_aio; int cb_size; - char *iobuf; + int paxos_hugebuf_size; + char *paxos_hugebuf; /* paxos_lease_acquire reads of lease area */ + char *iobuf; /* delta_lease_renew reads of lease area */ io_context_t aio_ctx; struct aicb *read_iobuf_timeout_aicb; struct aicb *callbacks; diff --git a/src/task.c b/src/task.c index ad2e761..8b73097 100644 --- a/src/task.c +++ b/src/task.c @@ -145,6 +145,9 @@ void close_task_aio(struct task *task) if (task->iobuf) free(task->iobuf);

+ if (task->paxos_hugebuf) + free(task->paxos_hugebuf); + skip_aio: if (task->callbacks) free(task->callbacks);