Re: [Qemu-devel] [PATCH v2 1/4] seqlock: introduce read-write seqlock

[Jan Kiszka <jan.kiszka@siemens.com>]

On 2013-08-13 10:39, liu ping fan wrote:
> On Tue, Aug 13, 2013 at 4:26 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> On 2013-08-13 07:43, Liu Ping Fan wrote:
>>> From: Paolo Bonzini <pbonzini@redhat.com>
>>>
>>> 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
>>>
>>> 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 <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_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