Message ID | 1379837479-8419-2-git-send-email-pingfank@linux.vnet.ibm.com |
---|---|
State | New |
Headers | show |
On 2013-09-22 10:11, Liu Ping Fan wrote: > This lets the read-side access run outside the BQL. In fact, not only BQL. Didn't the original commit provide a changlog about the content of this patch? Otherwise, briefly describe use cases and maybe the typical invocation pattern. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> From says you, signed-off only Paolo - this is inconsistent. Jan > --- > 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..3ff118a > --- /dev/null > +++ b/include/qemu/seqlock.h > @@ -0,0 +1,72 @@ > +/* > + * Seqlock implementation for QEMU > + * > + * Copyright Red Hat, Inc. 2013 > + * > + * Author: > + * Paolo Bonzini <pbonzini@redhat.com> > + * > + * 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 <qemu/atomic.h> > +#include <qemu/thread.h> > + > +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_retry(const QemuSeqLock *sl, unsigned start) > +{ > + /* Read other fields before reading final sequence. */ > + smp_rmb(); > + return unlikely(sl->sequence != start); > +} > + > +#endif >
On Mon, Sep 23, 2013 at 2:21 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: > On 2013-09-22 10:11, Liu Ping Fan wrote: >> This lets the read-side access run outside the BQL. > > In fact, not only BQL. Didn't the original commit provide a changlog > about the content of this patch? Otherwise, briefly describe use cases > and maybe the typical invocation pattern. > Original commit provide no changelog (right? Paolo, if I do miss the latest one in your tree). What about the commit log like: Seqlock implementation for QEMU. Usage idiom reader: seqlock_read_begin() do{ }while(seqlock_read_try()) writer: seqlock_write_lock() ... seqlock_write_unlock() initialization: seqlock_init(QemuSeqLock *sl, QemuMutex *mutex) where mutex could be NULL if the caller has provided extra lock protection for seqlock_write_lock. >> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > From says you, signed-off only Paolo - this is inconsistent. > Oh, a mistake, the author is Paolo! Thanks and regards, Pingfan > Jan > >> --- >> 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..3ff118a >> --- /dev/null >> +++ b/include/qemu/seqlock.h >> @@ -0,0 +1,72 @@ >> +/* >> + * Seqlock implementation for QEMU >> + * >> + * Copyright Red Hat, Inc. 2013 >> + * >> + * Author: >> + * Paolo Bonzini <pbonzini@redhat.com> >> + * >> + * 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 <qemu/atomic.h> >> +#include <qemu/thread.h> >> + >> +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_retry(const QemuSeqLock *sl, unsigned start) >> +{ >> + /* Read other fields before reading final sequence. */ >> + smp_rmb(); >> + return unlikely(sl->sequence != start); >> +} >> + >> +#endif >> > > -- > Siemens AG, Corporate Technology, CT RTC ITP SES-DE > Corporate Competence Center Embedded Linux
Il 24/09/2013 07:33, liu ping fan ha scritto: > On Mon, Sep 23, 2013 at 2:21 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >> On 2013-09-22 10:11, Liu Ping Fan wrote: >>> This lets the read-side access run outside the BQL. >> >> In fact, not only BQL. Didn't the original commit provide a changlog >> about the content of this patch? Otherwise, briefly describe use cases >> and maybe the typical invocation pattern. >> > Original commit provide no changelog (right? Paolo, if I do miss the > latest one in your tree). Indeed I had never written one. > What about the commit log like: > > Seqlock implementation for QEMU. Usage idiom > reader: > seqlock_read_begin() > do{ > }while(seqlock_read_try()) > > writer: > seqlock_write_lock() > ... > seqlock_write_unlock() > > initialization: > seqlock_init(QemuSeqLock *sl, QemuMutex *mutex) > where mutex could be NULL if the caller has provided extra lock > protection for seqlock_write_lock. replace "has provided" with "will provide". Otherwise looks good. Paolo
diff --git a/include/qemu/seqlock.h b/include/qemu/seqlock.h new file mode 100644 index 0000000..3ff118a --- /dev/null +++ b/include/qemu/seqlock.h @@ -0,0 +1,72 @@ +/* + * Seqlock implementation for QEMU + * + * Copyright Red Hat, Inc. 2013 + * + * Author: + * Paolo Bonzini <pbonzini@redhat.com> + * + * 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 <qemu/atomic.h> +#include <qemu/thread.h> + +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_retry(const QemuSeqLock *sl, unsigned start) +{ + /* Read other fields before reading final sequence. */ + smp_rmb(); + return unlikely(sl->sequence != start); +} + +#endif
This lets the read-side access run outside the BQL. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- include/qemu/seqlock.h | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100644 include/qemu/seqlock.h