From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55858) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V9Abo-0002F6-HJ for qemu-devel@nongnu.org; Tue, 13 Aug 2013 05:09:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V9Abi-00009l-M5 for qemu-devel@nongnu.org; Tue, 13 Aug 2013 05:09:44 -0400 Received: from thoth.sbs.de ([192.35.17.2]:27968) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V9Abi-00009S-Bg for qemu-devel@nongnu.org; Tue, 13 Aug 2013 05:09:38 -0400 Message-ID: <5209F7C7.7000907@siemens.com> Date: Tue, 13 Aug 2013 11:09:27 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <1376372630-1983-1-git-send-email-pingfank@linux.vnet.ibm.com> <1376372630-1983-2-git-send-email-pingfank@linux.vnet.ibm.com> <5209EDB0.1060500@siemens.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 1/4] seqlock: introduce read-write seqlock List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: liu ping fan Cc: Kevin Wolf , Stefan Hajnoczi , qemu-devel@nongnu.org, Alex Bligh , Paolo Bonzini , MORITA Kazutaka On 2013-08-13 10:39, liu ping fan wrote: > On Tue, Aug 13, 2013 at 4:26 PM, Jan Kiszka wrote: >> On 2013-08-13 07:43, Liu Ping Fan wrote: >>> From: Paolo Bonzini >>> >>> This lets the read-side access run outside the BQL. >>> >>> Signed-off-by: Paolo Bonzini >>> --- >>> include/qemu/seqlock.h | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 72 insertions(+) >>> create mode 100644 include/qemu/seqlock.h >>> >>> diff --git a/include/qemu/seqlock.h b/include/qemu/seqlock.h >>> new file mode 100644 >>> index 0000000..8f1c89f >>> --- /dev/null >>> +++ b/include/qemu/seqlock.h >>> @@ -0,0 +1,72 @@ >>> +/* >>> + * Seqlock implementation for QEMU >>> + * >>> + * Copyright Red Hat, Inc. 2013 >>> + * >>> + * Author: >>> + * Paolo Bonzini >>> + * >>> + * This work is licensed under the terms of the GNU GPL, version 2 or later. >>> + * See the COPYING file in the top-level directory. >>> + * >>> + */ >>> +#ifndef QEMU_SEQLOCK_H >>> +#define QEMU_SEQLOCK_H 1 >>> + >>> +#include >>> +#include >>> + >>> +typedef struct QemuSeqLock QemuSeqLock; >>> + >>> +struct QemuSeqLock { >>> + QemuMutex *mutex; >>> + unsigned sequence; >>> +}; >>> + >>> +static inline void seqlock_init(QemuSeqLock *sl, QemuMutex *mutex) >>> +{ >>> + sl->mutex = mutex; >>> + sl->sequence = 0; >>> +} >>> + >>> +/* Lock out other writers and update the count. */ >>> +static inline void seqlock_write_lock(QemuSeqLock *sl) >>> +{ >>> + if (sl->mutex) { >>> + qemu_mutex_lock(sl->mutex); >>> + } >>> + ++sl->sequence; >>> + >>> + /* Write sequence before updating other fields. */ >>> + smp_wmb(); >>> +} >>> + >>> +static inline void seqlock_write_unlock(QemuSeqLock *sl) >>> +{ >>> + /* Write other fields before finalizing sequence. */ >>> + smp_wmb(); >>> + >>> + ++sl->sequence; >>> + if (sl->mutex) { >>> + qemu_mutex_unlock(sl->mutex); >>> + } >>> +} >>> + >>> +static inline unsigned seqlock_read_begin(QemuSeqLock *sl) >>> +{ >>> + /* Always fail if a write is in progress. */ >>> + unsigned ret = sl->sequence & ~1; >>> + >>> + /* Read sequence before reading other fields. */ >>> + smp_rmb(); >>> + return ret; >>> +} >>> + >>> +static int seqlock_read_check(const QemuSeqLock *sl, unsigned start) >>> +{ >>> + /* Read other fields before reading final sequence. */ >>> + smp_rmb(); >>> + return unlikely(sl->sequence != start); >>> +} >>> + >>> +#endif >>> >> >> As the usage pattern is >> >> seqlock_read_begin() >> do >> ... >> while (seqlock_read_check()) >> >> I would suggest to call the latter seqlock_read_retry(), just like the >> kernel does. >> > I think it contains the meaning of check-success-or-retry. So may > check be nicer? "Check" alone has no obvious semantic for me, therefore "retry". Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux