| Message ID | 20191023152620.13166-2-kwolf@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qcow2: Fix image corruption bug in 4.1 | expand |
On 10/23/19 6:26 PM, Kevin Wolf wrote: > Some functions require that the caller holds a certain CoMutex for them > to operate correctly. Add a function so that they can assert the lock is > really held. > > Cc: qemu-stable@nongnu.org > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > include/qemu/coroutine.h | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h > index 9801e7f5a4..a36bcfe5c8 100644 > --- a/include/qemu/coroutine.h > +++ b/include/qemu/coroutine.h > @@ -167,6 +167,13 @@ void coroutine_fn qemu_co_mutex_lock(CoMutex *mutex); > */ > void coroutine_fn qemu_co_mutex_unlock(CoMutex *mutex); > > +/** > + * Assert that the current coroutine holds @mutex. > + */ > +static inline coroutine_fn void qemu_co_mutex_assert_locked(CoMutex *mutex) > +{ > + assert(mutex->locked && mutex->holder == qemu_coroutine_self()); > +} > > /** > * CoQueues are a mechanism to queue coroutines in order to continue executing I think that we should use atomic_read(&mutex->locked) and require barriers working with holder. Den
Am 24.10.2019 um 11:59 hat Denis Lunev geschrieben: > On 10/23/19 6:26 PM, Kevin Wolf wrote: > > Some functions require that the caller holds a certain CoMutex for them > > to operate correctly. Add a function so that they can assert the lock is > > really held. > > > > Cc: qemu-stable@nongnu.org > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > > --- > > include/qemu/coroutine.h | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h > > index 9801e7f5a4..a36bcfe5c8 100644 > > --- a/include/qemu/coroutine.h > > +++ b/include/qemu/coroutine.h > > @@ -167,6 +167,13 @@ void coroutine_fn qemu_co_mutex_lock(CoMutex *mutex); > > */ > > void coroutine_fn qemu_co_mutex_unlock(CoMutex *mutex); > > > > +/** > > + * Assert that the current coroutine holds @mutex. > > + */ > > +static inline coroutine_fn void qemu_co_mutex_assert_locked(CoMutex *mutex) > > +{ > > + assert(mutex->locked && mutex->holder == qemu_coroutine_self()); > > +} > > > > /** > > * CoQueues are a mechanism to queue coroutines in order to continue executing > I think that we should use atomic_read(&mutex->locked) and require barriers > working with holder. Hm, this would only be necessary for the case that the assertion doesn't hold true. I'll do the atomic_read() because it's easy enough, but I don't think we need or want barriers here. If another thread modifies this concurrently, the condition is false either way. Kevin
On 10/24/19 1:54 PM, Kevin Wolf wrote: > Am 24.10.2019 um 11:59 hat Denis Lunev geschrieben: >> On 10/23/19 6:26 PM, Kevin Wolf wrote: >>> Some functions require that the caller holds a certain CoMutex for them >>> to operate correctly. Add a function so that they can assert the lock is >>> really held. >>> >>> Cc: qemu-stable@nongnu.org >>> Signed-off-by: Kevin Wolf <kwolf@redhat.com> >>> --- >>> include/qemu/coroutine.h | 7 +++++++ >>> 1 file changed, 7 insertions(+) >>> >>> diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h >>> index 9801e7f5a4..a36bcfe5c8 100644 >>> --- a/include/qemu/coroutine.h >>> +++ b/include/qemu/coroutine.h >>> @@ -167,6 +167,13 @@ void coroutine_fn qemu_co_mutex_lock(CoMutex *mutex); >>> */ >>> void coroutine_fn qemu_co_mutex_unlock(CoMutex *mutex); >>> >>> +/** >>> + * Assert that the current coroutine holds @mutex. >>> + */ >>> +static inline coroutine_fn void qemu_co_mutex_assert_locked(CoMutex *mutex) >>> +{ >>> + assert(mutex->locked && mutex->holder == qemu_coroutine_self()); >>> +} >>> >>> /** >>> * CoQueues are a mechanism to queue coroutines in order to continue executing >> I think that we should use atomic_read(&mutex->locked) and require barriers >> working with holder. > Hm, this would only be necessary for the case that the assertion doesn't > hold true. I'll do the atomic_read() because it's easy enough, but I > don't think we need or want barriers here. If another thread modifies > this concurrently, the condition is false either way. > > Kevin > It looks like you are correct. We will see either NULL or something other if we are in the process of lock taking. Does this worth a comment? :) Den
diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h index 9801e7f5a4..a36bcfe5c8 100644 --- a/include/qemu/coroutine.h +++ b/include/qemu/coroutine.h @@ -167,6 +167,13 @@ void coroutine_fn qemu_co_mutex_lock(CoMutex *mutex); */ void coroutine_fn qemu_co_mutex_unlock(CoMutex *mutex); +/** + * Assert that the current coroutine holds @mutex. + */ +static inline coroutine_fn void qemu_co_mutex_assert_locked(CoMutex *mutex) +{ + assert(mutex->locked && mutex->holder == qemu_coroutine_self()); +} /** * CoQueues are a mechanism to queue coroutines in order to continue executing
Some functions require that the caller holds a certain CoMutex for them to operate correctly. Add a function so that they can assert the lock is really held. Cc: qemu-stable@nongnu.org Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- include/qemu/coroutine.h | 7 +++++++ 1 file changed, 7 insertions(+)