From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by dpdk.org (Postfix) with ESMTP id 3F06D2BF7; Tue, 27 Mar 2018 13:24:49 +0200 (CEST) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 8CD234040859; Tue, 27 Mar 2018 11:24:48 +0000 (UTC) Received: from [10.36.112.19] (ovpn-112-19.ams2.redhat.com [10.36.112.19]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 33A0E10B0098; Tue, 27 Mar 2018 11:24:43 +0000 (UTC) To: "Tan, Jianfeng" , "dev@dpdk.org" , "Bie, Tiwei" , "jfreimann@redhat.com" Cc: "stable@dpdk.org" References: <20180321154413.1120-1-maxime.coquelin@redhat.com> From: Maxime Coquelin Message-ID: <905c5e05-7d76-a45b-26c3-39dd5e6a56b1@redhat.com> Date: Tue, 27 Mar 2018 13:24:41 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.5]); Tue, 27 Mar 2018 11:24:48 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.5]); Tue, 27 Mar 2018 11:24:48 +0000 (UTC) for IP:'10.11.54.3' DOMAIN:'int-mx03.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'maxime.coquelin@redhat.com' RCPT:'' Subject: Re: [dpdk-dev] [PATCH] vhost: avoid concurrency when logging dirty pages X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 27 Mar 2018 11:24:49 -0000 On 03/27/2018 03:15 AM, Tan, Jianfeng wrote: > > >> -----Original Message----- >> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com] >> Sent: Wednesday, March 21, 2018 11:44 PM >> To: dev@dpdk.org; Tan, Jianfeng; Bie, Tiwei; jfreimann@redhat.com >> Cc: stable@dpdk.org; Maxime Coquelin >> Subject: [PATCH] vhost: avoid concurrency when logging dirty pages >> >> This patch aims at fixing a migration performance regression >> faced since atomic operation is used to log pages as dirty when >> doing live migration. >> >> Instead of setting a single bit by doing an atomic read-modify-write >> operation to log a page as dirty, this patch write 0xFF to the >> corresponding byte, and so logs 8 page as dirty. >> >> The advantage is that it avoids concurrent atomic operations by >> multiple PMD threads, the drawback is that some clean pages are >> marked as dirty and so are transferred twice. >> >> Fixes: 897f13a1f726 ("vhost: make page logging atomic") >> Cc: stable@dpdk.org >> >> Signed-off-by: Maxime Coquelin > > As you mentioned, the concern is if it affects the rate of convergence. Hope we could have some tests on that. We tested internally with live migration during PVP testing, at a rate of 3Mpps. In these conditions, we obtain same numbers as if we revert 897f13a1f726. > Reviewed-by: Jianfeng Tan Thanks, Maxime > Thanks! > >> --- >> lib/librte_vhost/vhost.h | 12 ++++-------- >> 1 file changed, 4 insertions(+), 8 deletions(-) >> >> diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h >> index d947bc9e3..aa2606f8a 100644 >> --- a/lib/librte_vhost/vhost.h >> +++ b/lib/librte_vhost/vhost.h >> @@ -247,18 +247,14 @@ struct virtio_net { >> #define VHOST_LOG_PAGE 4096 >> >> /* >> - * Atomically set a bit in memory. >> + * Mark all pages belonging to the same dirty log bitmap byte >> + * as dirty. The goal is to avoid concurrency between different >> + * threads doing atomic read-modify-writes on the same byte. >> */ >> -static __rte_always_inline void >> -vhost_set_bit(unsigned int nr, volatile uint8_t *addr) >> -{ >> - __sync_fetch_and_or_8(addr, (1U << nr)); >> -} >> - >> static __rte_always_inline void >> vhost_log_page(uint8_t *log_base, uint64_t page) >> { >> - vhost_set_bit(page % 8, &log_base[page / 8]); >> + log_base[page / 8] = 0xff; >> } >> >> static __rte_always_inline void >> -- >> 2.14.3 >