Hello,
just for the record...
</snip>
> > in current tree the ether_input() is protected by NET_LOCK(), which is
> > grabbed
> > by caller as a writer. bluhm's diff changes NET_LOCK() readlock, so
> > ether_input() can run concurrently. Switching NET_LOCK() to r-lock has
> > implications on smr read section above. The ting is the call to
> > eb->eb_input()
> > can sleep now. This is something what needs to be avoided within smr
> > section.
>
> Is the new sleeping point introduced by the fact the PF_LOCK() is a
> rwlock? Did you consider using a mutex, at least for the time being,
> in order to not run in such issues?
below is a diff, which trades both pf(4) rw-locks for mutexes.
diff compiles, it still needs testing/experimenting.
regards
sashan
--------8<---------------8<---------------8<------------------8<--------
diff --git a/sys/net/pf_ioctl.c b/sys/net/pf_ioctl.c
index ae7bb008351..30c78da7d16 100644
--- a/sys/net/pf_ioctl.c
+++ b/sys/net/pf_ioctl.c
@@ -146,8 +146,8 @@ TAILQ_HEAD(pf_tags, pf_tagname) pf_tags =
TAILQ_HEAD_INITIALIZER(pf_tags),
* grab the lock as writer. Whenever packet creates state it grabs pf_lock
* first then it locks pf_state_lock as the writer.
*/
-struct rwlock pf_lock = RWLOCK_INITIALIZER("pf_lock");
-struct rwlock pf_state_lock = RWLOCK_INITIALIZER("pf_state_lock");
+struct mutex pf_lock = MUTEX_INITIALIZER(IPL_NET);
+struct mutex pf_state_lock = MUTEX_INITIALIZER(IPL_NET);
#if (PF_QNAME_SIZE != PF_TAG_NAME_SIZE)
#error PF_QNAME_SIZE must be equal to PF_TAG_NAME_SIZE
diff --git a/sys/net/pfvar_priv.h b/sys/net/pfvar_priv.h
index b1a122af409..c5ae392b468 100644
--- a/sys/net/pfvar_priv.h
+++ b/sys/net/pfvar_priv.h
@@ -39,7 +39,7 @@
#include <sys/rwlock.h>
-extern struct rwlock pf_lock;
+extern struct mutex pf_lock;
struct pf_pdesc {
struct {
@@ -104,52 +104,41 @@ extern struct timeout pf_purge_to;
struct pf_state *pf_state_ref(struct pf_state *);
void pf_state_unref(struct pf_state *);
-extern struct rwlock pf_lock;
-extern struct rwlock pf_state_lock;
+extern struct mutex pf_lock;
+extern struct mutex pf_state_lock;
#define PF_LOCK() do { \
NET_ASSERT_LOCKED(); \
- rw_enter_write(&pf_lock); \
+ mtx_enter(&pf_lock); \
} while (0)
#define PF_UNLOCK() do { \
PF_ASSERT_LOCKED(); \
- rw_exit_write(&pf_lock); \
+ mtx_leave(&pf_lock); \
} while (0)
-#define PF_ASSERT_LOCKED() do { \
- if (rw_status(&pf_lock) != RW_WRITE) \
- splassert_fail(RW_WRITE, \
- rw_status(&pf_lock),__func__);\
- } while (0)
+#define PF_ASSERT_LOCKED() (void)(0)
-#define PF_ASSERT_UNLOCKED() do { \
- if (rw_status(&pf_lock) == RW_WRITE) \
- splassert_fail(0, rw_status(&pf_lock), __func__);\
- } while (0)
+#define PF_ASSERT_UNLOCKED() (void)(0)
#define PF_STATE_ENTER_READ() do { \
- rw_enter_read(&pf_state_lock); \
+ mtx_enter(&pf_state_lock); \
} while (0)
#define PF_STATE_EXIT_READ() do { \
- rw_exit_read(&pf_state_lock); \
+ mtx_leave(&pf_state_lock); \
} while (0)
#define PF_STATE_ENTER_WRITE() do { \
- rw_enter_write(&pf_state_lock); \
+ mtx_enter(&pf_state_lock); \
} while (0)
#define PF_STATE_EXIT_WRITE() do { \
PF_ASSERT_STATE_LOCKED(); \
- rw_exit_write(&pf_state_lock); \
+ mtx_leave(&pf_state_lock); \
} while (0)
-#define PF_ASSERT_STATE_LOCKED() do { \
- if (rw_status(&pf_state_lock) != RW_WRITE)\
- splassert_fail(RW_WRITE, \
- rw_status(&pf_state_lock), __func__);\
- } while (0)
+#define PF_ASSERT_STATE_LOCKED() (void)(0)
extern void pf_purge_timeout(void *);
extern void pf_purge(void *);
