On Thu, Oct 19, 2023 at 08:41:26PM +0200, Juan Quintela wrote:
> We can changing pending_job to a bool if you preffer.  I think that we
> have nailed all the off_by_one errors by now (famous last words).

Would it work to make pending_job a bool, even with SYNC?  It seems to me
multifd_send_sync_main() now can boost pending_job even if pending_job==1.

That's also the place where I really think confusing too; where it looks
like the migration thread can modify a pending job's flag as long as it is
fast enough before the send thread put that onto the wire.  Then it's
unpredictable whether the SYNC flag will be sent with current packet (where
due to pending_jobs==1 already, can contain valid pages), or will be only
set for the next one (where there will have 0 real page).

IMHO it'll be good to separate the sync task, then we can change
pending_jobs to bool.  Something like:

  bool pending_send_page;
  bool pending_send_sync;

Then multifd_send_thread() handles them separately, only attaching
p->flags=SYNC when pending_send_sync is requested.  It guarantees a SYNC
message will always be a separate packet, which will be crystal clear then.

-- 
Peter Xu


Reply via email to