All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Xu <peterx@redhat.com>
To: hongmainquan <hongmianquan@bytedance.com>
Cc: qemu-devel@nongnu.org, farosas@suse.de, mark.kanda@oracle.com,
	bchaney@akamai.com
Subject: Re: [RFC V2 0/2] migration: optimize cpr fd lookup using GHashTable
Date: Thu, 26 Mar 2026 11:40:03 -0400	[thread overview]
Message-ID: <acVTU42xvTqzx2wA@x1.local> (raw)
In-Reply-To: <6df681fb-0db1-48a1-bac0-a05229fb2c7d@bytedance.com>

Hi, hongmainquan,

On Thu, Mar 19, 2026 at 11:03:48PM +0800, hongmainquan wrote:
> Thanks for the reminder. I apologize if I missed any specific points 
> from your v1 review. Could you please point out which specific comments 
> I left unanswered? I will address them immediately.
> Regarding your main questions in v1 (performance data and why migrate 
> hash), I have updated them in the cover letter of the v2 series, but 
> I'll summarize the answers here directly for your convenience:

Thanks, please still always reply directly so we can discuss before you
send new versions, and sorry for the late reply.

> 
> 1. Why the lookup is slow, and the performance improvement data: 
> Currently, `find_fd` uses a linear search (O(N)) on a QLIST.
> The latency increases with a rising number of fds, and the supporting 
> test data is provided in the cover letter.

Yes, the number is persuasive.

> 
> 2. Why migrate a hash instead of migrating a list and reconstructing it:
> My primary goal was to accelerate all `find_fd` lookups. To achieve 
> this, the underlying data source needs to be replaced with a hash table. 
> My initial thought was to simply change the fds structure entirely to a 
> GHashTable and migrate it directly. This approach seemed much more 
> straightforward to implement than maintaining a conversion between QLIST 
> and hash, though, as you rightly pointed out, it does inevitably involve 
> changing the CPR ABI.
> Secondly, I was concerned about the timing of converting a QLIST to a 
> GHashTable. The conversion must happen when the fds list is in its final 
> state with no further modifications. However, during normal VM 
> operation, there are ongoing operations that save or modify fds. If the 
> GHashTable were merely an auxiliary structure, we need to keep the QLIST 
> and the GHashTable synchronized.
> That said, your suggestion makes a lot of sense. If we treat the 
> GHashTable as the primary structure and only convert it to a QLIST in 
> the pre_save hook right before migration—then reconstruct the hash table 
> from the QLIST on the destination, it would completely work. This 
> preserves the original QLIST-based migration ABI.
> In short, I believe both approaches are technically feasible, but I 
> agree that my current implementation has the downside of breaking the 
> CPR ABI. I look forward to discussing this further with you.

Yes exactly, you can stick with the hash for most of the lifespan of QEMU.

It makes sense to be able to migrate a ghash, but cpr may not be the best
first user due to ABI break.  You can also wait to see if the Reviewers
have anything to say.

If you want to be on the safe side, you can still evaluate the pre_save /
post_load approach dynamically constructing the qlist, then dynamically
rebuild the qdict, to measure the perf.  I'd expect it'll be similarly
good without ABI break.

Thanks,

> 
> Thanks.
> 
> 在 2026/3/19 21:51, Peter Xu 写道:
> > On Thu, Mar 19, 2026 at 07:38:28PM +0800, hongmianquan wrote:
> >> Currently, the CPR subsystem in QEMU uses a QLIST to store fds.
> >> In scenarios where a large number of
> >> fds are involved (such as a VM with many vfio-pci devices), looking up an fd
> >> via `cpr_find_fd` becomes a performance bottleneck due to the O(N) linear search.
> >> This patch series optimizes the cpr fd storage by replacing the QLIST
> >> with a GHashTable. The time complexity for `cpr_find_fd` is reduced
> >> from O(N) to O(1).
> > 
> > Some unanswered comments here for v1:
> > 
> > https://lore.kernel.org/all/aacuTpoMucQ1wrUN@x1.local/#t
> >
> 

-- 
Peter Xu



  reply	other threads:[~2026-03-26 15:40 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-19 11:38 [RFC V2 0/2] migration: optimize cpr fd lookup using GHashTable hongmianquan
2026-03-19 11:38 ` [RFC v2 1/2] migration: Support ghash migration hongmianquan
2026-03-19 11:38 ` [RFC v2 2/2] cpr: use hashtable for cpr fds hongmianquan
2026-03-19 13:51 ` [RFC V2 0/2] migration: optimize cpr fd lookup using GHashTable Peter Xu
2026-03-19 15:03   ` hongmainquan
2026-03-26 15:40     ` Peter Xu [this message]
2026-03-27  3:01       ` hongmainquan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=acVTU42xvTqzx2wA@x1.local \
    --to=peterx@redhat.com \
    --cc=bchaney@akamai.com \
    --cc=farosas@suse.de \
    --cc=hongmianquan@bytedance.com \
    --cc=mark.kanda@oracle.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.