From 28a9fb05372696b8a07428acbbd4f583b039fbc4 Mon Sep 17 00:00:00 2001 From: Kevin Zeng Date: Wed, 5 Jun 2024 21:23:36 +0000 Subject: [PATCH] pw_multisink: Add option to inject a user defined lock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change-Id: I0a80b675fdd117973803e209f51d0a2d52018056 Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/213211 Pigweed-Auto-Submit: Kevin Zeng Commit-Queue: Auto-Submit Lint: Lint 🤖 Reviewed-by: Carlos Chinchilla Reviewed-by: Wyatt Hepler Presubmit-Verified: CQ Bot Account --- pw_multisink/BUILD.gn | 2 + pw_multisink/docs.rst | 16 +++--- pw_multisink/public/pw_multisink/config.h | 51 +++++++++++++++----- pw_multisink/public/pw_multisink/multisink.h | 17 +++++-- 4 files changed, 63 insertions(+), 23 deletions(-) diff --git a/pw_multisink/BUILD.gn b/pw_multisink/BUILD.gn index 960ecdacbc..5f3c8f4858 100644 --- a/pw_multisink/BUILD.gn +++ b/pw_multisink/BUILD.gn @@ -46,6 +46,8 @@ pw_source_set("pw_multisink") { "$dir_pw_sync:interrupt_spin_lock", "$dir_pw_sync:lock_annotations", "$dir_pw_sync:mutex", + "$dir_pw_sync:virtual_basic_lockable", + dir_pw_assert, dir_pw_bytes, dir_pw_containers, dir_pw_function, diff --git a/pw_multisink/docs.rst b/pw_multisink/docs.rst index 996a229619..6a25103039 100644 --- a/pw_multisink/docs.rst +++ b/pw_multisink/docs.rst @@ -14,16 +14,18 @@ of this module, see the :ref:`module documentation ` for more details. -.. c:macro:: PW_MULTISINK_CONFIG_LOCK_INTERRUPT_SAFE +.. c:macro:: PW_MULTISINK_CONFIG_LOCK_TYPE - Whether an interrupt-safe lock is used to guard multisink read/write - operations. + Set to configure the underlying lock used to guard multisink read/write + operations. By default, this is set to uses an interrupt spin-lock to guard + the multisink transactions. Should be set to one of the following options: - By default, this option is enabled and the multisink uses an interrupt - spin-lock to guard its transactions. If disabled, a mutex is used instead. + - PW_MULTISINK_INTERRUPT_SPIN_LOCK: Use an interrupt spin lock + - PW_MULTISINK_MUTEX: Use a mutex. Not safe to use multisink in interrupt + context + - PW_MULTISINK_VIRTUAL_LOCK: User provided locking implementation. Interrupt + support will be left to the user to manage. - Disabling this will alter the entry precondition of the multisink, - requiring that it not be called from an interrupt context. .. _module-pw_multisink-late_drain_attach: diff --git a/pw_multisink/public/pw_multisink/config.h b/pw_multisink/public/pw_multisink/config.h index 98de4a71ca..86bb764772 100644 --- a/pw_multisink/public/pw_multisink/config.h +++ b/pw_multisink/public/pw_multisink/config.h @@ -13,27 +13,54 @@ // the License. #pragma once -// PW_MULTISINK_LOCK_INTERRUPT_SAFE controls whether an interrupt-safe lock is -// used when reading and writing from the underlying ring-buffer. This is -// enabled by default, using an interrupt spin-lock instead of a mutex. -// Disabling this alters the entry precondition of the multisink, requiring that -// it not be invoked from an interrupt context. -#if !defined(PW_MULTISINK_CONFIG_LOCK_INTERRUPT_SAFE) -#define PW_MULTISINK_CONFIG_LOCK_INTERRUPT_SAFE 1 -#endif // !defined(PW_MULTISINK_CONFIG_LOCK_INTERRUPT_SAFE) +// Enum for different lock configurations. +// Use non-obvious numbers so users use the macro instead of an integer +#define PW_MULTISINK_INTERRUPT_SPIN_LOCK 100 +#define PW_MULTISINK_MUTEX 200 +#define PW_MULTISINK_VIRTUAL_LOCK 300 -#if PW_MULTISINK_CONFIG_LOCK_INTERRUPT_SAFE +// PW_MULTISINK_CONFIG_LOCK_TYPE controls which lock is used when reading and +// writing from the underlying ring-buffer. The interrupt spin lock is +// enabled by default if PW_MULTISINK_CONFIG_LOCK_TYPE is not set. Otherwise, +// one of the following locking implementations can be set using the enums +// above. PW_MULTISINK_VIRTUAL_LOCK can be used if the user wants to provide +// their own locking implementation. +#ifndef PW_MULTISINK_CONFIG_LOCK_TYPE +// Backwards compatibility +#ifdef PW_MULTISINK_LOCK_INTERRUPT_SAFE +#warning "Multisink PW_MULTISINK_INTERRUPT_SPIN_LOCK is deprecated!!!" +#if PW_MULTISINK_LOCK_INTERRUPT_SAFE +#define PW_MULTISINK_CONFIG_LOCK_TYPE PW_MULTISINK_INTERRUPT_SPIN_LOCK +#else +#define PW_MULTISINK_CONFIG_LOCK_TYPE PW_MULTISINK_MUTEX +#endif +#else +// Default to use interrupt spin lock. +#define PW_MULTISINK_CONFIG_LOCK_TYPE PW_MULTISINK_INTERRUPT_SPIN_LOCK +#endif +#endif // PW_MULTISINK_CONFIG_LOCK_TYPE + +static_assert(PW_MULTISINK_CONFIG_LOCK_TYPE == + PW_MULTISINK_INTERRUPT_SPIN_LOCK || + PW_MULTISINK_CONFIG_LOCK_TYPE == PW_MULTISINK_MUTEX || + PW_MULTISINK_CONFIG_LOCK_TYPE == PW_MULTISINK_VIRTUAL_LOCK); + +#if PW_MULTISINK_CONFIG_LOCK_TYPE == PW_MULTISINK_INTERRUPT_SPIN_LOCK #include "pw_sync/interrupt_spin_lock.h" -#else // !PW_MULTISINK_CONFIG_LOCK_INTERRUPT_SAFE +#elif PW_MULTISINK_CONFIG_LOCK_TYPE == PW_MULTISINK_VIRTUAL_LOCK +#include "pw_sync/virtual_basic_lockable.h" +#elif PW_MULTISINK_CONFIG_LOCK_TYPE == PW_MULTISINK_MUTEX #include "pw_sync/mutex.h" #endif // PW_MULTISINK_CONFIG_LOCK_INTERRUPT_SAFE namespace pw { namespace multisink { -#if PW_MULTISINK_CONFIG_LOCK_INTERRUPT_SAFE +#if PW_MULTISINK_CONFIG_LOCK_TYPE == PW_MULTISINK_INTERRUPT_SPIN_LOCK using LockType = pw::sync::InterruptSpinLock; -#else // !PW_MULTISINK_CONFIG_LOCK_INTERRUPT_SAFE +#elif PW_MULTISINK_CONFIG_LOCK_TYPE == PW_MULTISINK_VIRTUAL_LOCK +using LockType = pw::sync::VirtualBasicLockable&; +#elif PW_MULTISINK_CONFIG_LOCK_TYPE == PW_MULTISINK_MUTEX using LockType = pw::sync::Mutex; #endif // PW_MULTISINK_CONFIG_LOCK_INTERRUPT_SAFE diff --git a/pw_multisink/public/pw_multisink/multisink.h b/pw_multisink/public/pw_multisink/multisink.h index b464e54a7f..39d442bcaa 100644 --- a/pw_multisink/public/pw_multisink/multisink.h +++ b/pw_multisink/public/pw_multisink/multisink.h @@ -16,6 +16,7 @@ #include #include +#include "pw_assert/assert.h" #include "pw_bytes/span.h" #include "pw_function/function.h" #include "pw_multisink/config.h" @@ -310,10 +311,18 @@ class MultiSink { } // Constructs a multisink using a ring buffer backed by the provided buffer. + // If we're using a virtual lock, then the lock needs to be passed in. +#if PW_MULTISINK_CONFIG_LOCK_TYPE == PW_MULTISINK_VIRTUAL_LOCK + MultiSink(ByteSpan buffer, LockType lock) + : lock_(lock), +#else MultiSink(ByteSpan buffer) - : ring_buffer_(true), sequence_id_(0), total_ingress_drops_(0) { - ring_buffer_.SetBuffer(buffer) - .IgnoreError(); // TODO: b/242598609 - Handle Status properly + : +#endif + ring_buffer_(true), + sequence_id_(0), + total_ingress_drops_(0) { + PW_ASSERT(ring_buffer_.SetBuffer(buffer).ok()); AttachDrain(oldest_entry_drain_); } @@ -415,12 +424,12 @@ class MultiSink { // Notifies attached listeners of new entries or an updated drop count. void NotifyListeners() PW_EXCLUSIVE_LOCKS_REQUIRED(lock_); + LockType lock_; IntrusiveList listeners_ PW_GUARDED_BY(lock_); ring_buffer::PrefixedEntryRingBufferMulti ring_buffer_ PW_GUARDED_BY(lock_); Drain oldest_entry_drain_ PW_GUARDED_BY(lock_); uint32_t sequence_id_ PW_GUARDED_BY(lock_); uint32_t total_ingress_drops_ PW_GUARDED_BY(lock_); - LockType lock_; }; } // namespace multisink