Skip to content

Commit

Permalink
migration/multifd: Rewrite multifd_queue_page()
Browse files Browse the repository at this point in the history
The current multifd_queue_page() is not easy to read and follow.  It is not
good with a few reasons:

  - No helper at all to show what exactly does a condition mean; in short,
  readability is low.

  - Rely on pages->ramblock being cleared to detect an empty queue.  It's
  slightly an overload of the ramblock pointer, per Fabiano [1], which I
  also agree.

  - Contains a self recursion, even if not necessary..

Rewrite this function.  We add some comments to make it even clearer on
what it does.

[1] https://lore.kernel.org/r/87wmrpjzew.fsf@suse.de

Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20240202102857.110210-19-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
  • Loading branch information
xzpeter committed Feb 5, 2024
1 parent 3b40964 commit f88f86c
Showing 1 changed file with 37 additions and 19 deletions.
56 changes: 37 additions & 19 deletions migration/multifd.c
Original file line number Diff line number Diff line change
Expand Up @@ -506,35 +506,53 @@ static bool multifd_send_pages(void)
return true;
}

static inline bool multifd_queue_empty(MultiFDPages_t *pages)
{
return pages->num == 0;
}

static inline bool multifd_queue_full(MultiFDPages_t *pages)
{
return pages->num == pages->allocated;
}

static inline void multifd_enqueue(MultiFDPages_t *pages, ram_addr_t offset)
{
pages->offset[pages->num++] = offset;
}

/* Returns true if enqueue successful, false otherwise */
bool multifd_queue_page(RAMBlock *block, ram_addr_t offset)
{
MultiFDPages_t *pages = multifd_send_state->pages;
bool changed = false;
MultiFDPages_t *pages;

retry:
pages = multifd_send_state->pages;

if (!pages->block) {
/* If the queue is empty, we can already enqueue now */
if (multifd_queue_empty(pages)) {
pages->block = block;
multifd_enqueue(pages, offset);
return true;
}

if (pages->block == block) {
pages->offset[pages->num] = offset;
pages->num++;

if (pages->num < pages->allocated) {
return true;
/*
* Not empty, meanwhile we need a flush. It can because of either:
*
* (1) The page is not on the same ramblock of previous ones, or,
* (2) The queue is full.
*
* After flush, always retry.
*/
if (pages->block != block || multifd_queue_full(pages)) {
if (!multifd_send_pages()) {
return false;
}
} else {
changed = true;
}

if (!multifd_send_pages()) {
return false;
}

if (changed) {
return multifd_queue_page(block, offset);
goto retry;
}

/* Not empty, and we still have space, do it! */
multifd_enqueue(pages, offset);
return true;
}

Expand Down

0 comments on commit f88f86c

Please sign in to comment.