diff mbox

[dpdk-dev] vhost: make the page logging atomic

Message ID 1501578081-191381-1-git-send-email-tiwei.bie@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Yuanhan Liu
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Tiwei Bie Aug. 1, 2017, 9:01 a.m. UTC 
  Each dirty page logging operation should be atomic. But it's not
atomic in current implementation. So it's possible that some dirty
pages can't be logged successfully when different threads try to
log different pages into the same byte of the log buffer concurrently.
This patch fixes this issue.

Fixes: b171fad1ffa5 ("vhost: log used vring changes")
Cc: stable@dpdk.org

Reported-by: Xiao Wang <xiao.w.wang@intel.com>
Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
---
 lib/librte_vhost/vhost.h | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Comments

Maxime Coquelin Aug. 1, 2017, 12:05 p.m. UTC | #1 
On 08/01/2017 11:01 AM, Tiwei Bie wrote:
> Each dirty page logging operation should be atomic. But it's not
> atomic in current implementation. So it's possible that some dirty
> pages can't be logged successfully when different threads try to
> log different pages into the same byte of the log buffer concurrently.
> This patch fixes this issue.
> 
> Fixes: b171fad1ffa5 ("vhost: log used vring changes")
> Cc:stable@dpdk.org
> 
> Reported-by: Xiao Wang<xiao.w.wang@intel.com>
> Signed-off-by: Tiwei Bie<tiwei.bie@intel.com>
> ---
>   lib/librte_vhost/vhost.h | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime
Thomas Monjalon Aug. 3, 2017, 8:10 p.m. UTC | #2 
01/08/2017 14:05, Maxime Coquelin:
> On 08/01/2017 11:01 AM, Tiwei Bie wrote:
> > Each dirty page logging operation should be atomic. But it's not
> > atomic in current implementation. So it's possible that some dirty
> > pages can't be logged successfully when different threads try to
> > log different pages into the same byte of the log buffer concurrently.
> > This patch fixes this issue.
> > 
> > Fixes: b171fad1ffa5 ("vhost: log used vring changes")
> > Cc:stable@dpdk.org
> > 
> > Reported-by: Xiao Wang<xiao.w.wang@intel.com>
> > Signed-off-by: Tiwei Bie<tiwei.bie@intel.com>
> 
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Applied, thanks
diff mbox

Patch

diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
index 0f294f3..6fe72ae 100644
--- a/lib/librte_vhost/vhost.h
+++ b/lib/librte_vhost/vhost.h
@@ -201,10 +201,19 @@  struct virtio_net {
 
 #define VHOST_LOG_PAGE	4096
 
+/*
+ * Atomically set a bit in memory.
+ */
+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)
 {
-	log_base[page / 8] |= 1 << (page % 8);
+	vhost_set_bit(page % 8, &log_base[page / 8]);
 }
 
 static __rte_always_inline void