virtio-fs.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Juan Quintela <quintela@redhat.com>
To: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Cc: qemu-devel@nongnu.org, "Jason Wang" <jasowang@redhat.com>,
	qemu-arm@nongnu.org, "Hanna Reitz" <hreitz@redhat.com>,
	"Halil Pasic" <pasic@linux.ibm.com>,
	"Eric Blake" <eblake@redhat.com>,
	"Michael Tokarev" <mjt@tls.msk.ru>, "Fam Zheng" <fam@euphon.net>,
	"David Hildenbrand" <david@redhat.com>,
	"Alex Williamson" <alex.williamson@redhat.com>,
	"John Snow" <jsnow@redhat.com>,
	"Stefan Hajnoczi" <stefanha@redhat.com>,
	"Ilya Leoshkevich" <iii@linux.ibm.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"Gonglei (Arei)" <arei.gonglei@huawei.com>,
	"Eric Farman" <farman@linux.ibm.com>,
	qemu-s390x@nongnu.org, "Kevin Wolf" <kwolf@redhat.com>,
	qemu-block@nongnu.org,
	"Pavel Dovgalyuk" <pavel.dovgaluk@ispras.ru>,
	"Laurent Vivier" <lvivier@redhat.com>,
	"Christian Borntraeger" <borntraeger@linux.ibm.com>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Thomas Huth" <thuth@redhat.com>,
	"Mathieu Poirier" <mathieu.poirier@linaro.org>,
	"Peter Maydell" <peter.maydell@linaro.org>,
	"Viresh Kumar" <viresh.kumar@linaro.org>,
	"Alex Bennée" <alex.bennee@linaro.org>,
	"Eduardo Habkost" <eduardo@habkost.net>,
	"Klaus Jensen" <its@irrelevant.dk>,
	"Raphael Norwitz" <raphael.norwitz@nutanix.com>,
	"Laurent Vivier" <laurent@vivier.eu>,
	"Xiaojuan Yang" <yangxiaojuan@loongson.cn>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	"Keith Busch" <kbusch@kernel.org>,
	"Thomas Huth" <huth@tuxfamily.org>,
	"Song Gao" <gaosong@loongson.cn>,
	"Richard Henderson" <richard.henderson@linaro.org>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	qemu-trivial@nongnu.org, virtio-fs@redhat.com
Subject: Re: [Virtio-fs] [PATCH v3] migration: Remove res_compatible parameter
Date: Thu, 02 Feb 2023 11:12:45 +0100	[thread overview]
Message-ID: <87v8kktlya.fsf@secure.mitica> (raw)
In-Reply-To: <f37db85a-b0aa-b34a-7d5a-b1ab650f1683@yandex-team.ru> (Vladimir Sementsov-Ogievskiy's message of "Tue, 31 Jan 2023 11:33:22 +0300")

Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> wrote:
> On 1/30/23 11:06, Juan Quintela wrote:
>> It was only used for RAM, and in that case, it means that this amount
>> of data was sent for memory.  Just delete the field in all callers.
>
> Could you describe, why it's safe to change the behavior for RAM?

I will start for the change in migration.c

-    uint64_t pend_pre, pend_compat, pend_post;
+    uint64_t pend_pre, pend_post;
     bool in_postcopy = s->state == MIGRATION_STATUS_POSTCOPY_ACTIVE;
 
-    qemu_savevm_state_pending_estimate(&pend_pre, &pend_compat, &pend_post);
-    uint64_t pending_size = pend_pre + pend_compat + pend_post;

Here we add the three to pending_size, so it don't matter.

+    qemu_savevm_state_pending_estimate(&pend_pre, &pend_post);
+    uint64_t pending_size = pend_pre + pend_post;
 
-    trace_migrate_pending_estimate(pending_size,
-                                   pend_pre, pend_compat, pend_post);
+    trace_migrate_pending_estimate(pending_size, pend_pre, pend_post);

Trace, we don't care again.

-    if (pend_pre + pend_compat <= s->threshold_size) {

We do the test on pend_pre + pend_compat.
So, the only place where we are using pend_compat alone is on trace
messages.

I guess we can agree that changing trace messages don't matter.


-        qemu_savevm_state_pending_exact(&pend_pre, &pend_compat, &pend_post);
-        pending_size = pend_pre + pend_compat + pend_post;
-        trace_migrate_pending_exact(pending_size,
-                                    pend_pre, pend_compat, pend_post);
+    if (pend_pre <= s->threshold_size) {
+        qemu_savevm_state_pending_exact(&pend_pre, &pend_post);
+        pending_size = pend_pre + pend_post;
+        trace_migrate_pending_exact(pending_size, pend_pre, pend_post);


Ok, Now go to ram.c changes:

The only change that we do is that if we are on postcopy, we add the

@@ -3448,9 +3444,9 @@ static void ram_state_pending_exact(void *opaque,
 
     if (migrate_postcopy_ram()) {
         /* We can do postcopy, and all the data is postcopiable */
-        *res_compatible += remaining_size;
+        *res_postcopy += remaining_size;
     } else {
-        *res_precopy_only += remaining_size;
+        *res_precopy += remaining_size;
     }
 }

The only use of res_compatible was when we were on postocpy, nothing
else sets it.

But when we are on postcopy, we can send that pages trough postcopy, so
we add them there and if we are on precopy, res_compatible was already zero.

So I think that is ok, i.e. no behaviour change.

What do you think?

> Also, I think it would be a lot better to split the change for RAM
> (s/res_compatible/res_postcopy) in one patch, and then drop the
> totally unused res_compatible file in the second patch

Ok, will do for next version.

Later, Juan.


      reply	other threads:[~2023-02-02 10:12 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-30  8:06 [Virtio-fs] [PATCH v3] migration: Remove res_compatible parameter Juan Quintela
2023-01-31  8:33 ` Vladimir Sementsov-Ogievskiy
2023-02-02 10:12   ` Juan Quintela [this message]

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=87v8kktlya.fsf@secure.mitica \
    --to=quintela@redhat.com \
    --cc=alex.bennee@linaro.org \
    --cc=alex.williamson@redhat.com \
    --cc=arei.gonglei@huawei.com \
    --cc=borntraeger@linux.ibm.com \
    --cc=david@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=eblake@redhat.com \
    --cc=eduardo@habkost.net \
    --cc=fam@euphon.net \
    --cc=farman@linux.ibm.com \
    --cc=gaosong@loongson.cn \
    --cc=hreitz@redhat.com \
    --cc=huth@tuxfamily.org \
    --cc=iii@linux.ibm.com \
    --cc=its@irrelevant.dk \
    --cc=jasowang@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=kbusch@kernel.org \
    --cc=kraxel@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=laurent@vivier.eu \
    --cc=lvivier@redhat.com \
    --cc=mathieu.poirier@linaro.org \
    --cc=mjt@tls.msk.ru \
    --cc=mst@redhat.com \
    --cc=pasic@linux.ibm.com \
    --cc=pavel.dovgaluk@ispras.ru \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-s390x@nongnu.org \
    --cc=qemu-trivial@nongnu.org \
    --cc=raphael.norwitz@nutanix.com \
    --cc=richard.henderson@linaro.org \
    --cc=stefanha@redhat.com \
    --cc=thuth@redhat.com \
    --cc=viresh.kumar@linaro.org \
    --cc=virtio-fs@redhat.com \
    --cc=vsementsov@yandex-team.ru \
    --cc=yangxiaojuan@loongson.cn \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).