On Thu, Mar 17, 2022 at 01:07:12AM +0100, Mark Kettenis wrote:
> > Date: Thu, 17 Mar 2022 01:01:46 +0100 (CET)
> > From: Mark Kettenis <mark.kette...@xs4all.nl>
> >
> > > Date: Thu, 17 Mar 2022 00:47:15 +0100
> > > From: Alexander Bluhm <alexander.bl...@gmx.net>
> > >
> > > Hi,
> > >
> > > My previous atempt to add a mutex to in_pcb.h was reverted as it
> > > broke userland build.
> > >
> > > Is the correct fix to include sys/mutex.h in every .c file that
> > > includes netinet/in_pcb.h ? I made a release with it.
> > > Or should I include sys/mutex.h in netinet/in_pcb.h ?
> >
> > Neither?
> >
> > It makes no sense to export the kernel mutex stuff to userland. Is
> > there a way to avoid doing that by adding a bit for #ifdef _KERNEL?
> ^^^^^^^^^
> a bit more
My diff adds struct mutex to struct inpcbtable. My later plan is
to add a mutex also to struct inpcb.
tcpbench uses libkvm to extract information from struct inpcbtable.
netstat does that for struct inpcb. Also post mortem analysis from
a kernel core dump is possible.
I don't understand why userland must not know the size of struct
mutex when tools where written to analyze these structs.
Is there something special about struct mutex that should not shown
to userland?
Do you like this? Different structs for kernel and userland.
I think this is confusing when used with libkvm.
struct inpcbtable {
TAILQ_HEAD(inpthead, inpcb) inpt_queue; /* [t] inet PCB queue */
struct inpcbhead *inpt_hashtbl; /* [t] local and foreign hash */
struct inpcbhead *inpt_lhashtbl; /* [t] local port hash */
SIPHASH_KEY inpt_key, inpt_lkey; /* [t] secrets for hashes */
u_long inpt_mask, inpt_lmask; /* [t] hash masks */
int inpt_count, inpt_size; /* [t] queue count, hash size */
#ifdef _KERNEL
struct mutex inpt_mtx; /* protect queue and hash */
#endif
};
And we have code like this in lib/libkvm/kvm_file2.c
#define _KERNEL
#include <sys/file.h>
#include <sys/mount.h>
#undef _KERNEL
Or can we include a minimal non kernel version of sys/mutex.h
everywhere? (not tested yet)
bluhm
Index: netinet/in_pcb.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/in_pcb.h,v
retrieving revision 1.125
diff -u -p -r1.125 in_pcb.h
--- netinet/in_pcb.h 14 Mar 2022 22:38:43 -0000 1.125
+++ netinet/in_pcb.h 17 Mar 2022 00:44:54 -0000
@@ -65,6 +65,7 @@
#define _NETINET_IN_PCB_H_
#include <sys/queue.h>
+#include <sys/mutex.h>
#include <sys/refcnt.h>
#include <netinet/ip6.h>
#include <netinet6/ip6_var.h>
Index: sys/mutex.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/sys/mutex.h,v
retrieving revision 1.18
diff -u -p -r1.18 mutex.h
--- sys/mutex.h 23 Apr 2019 13:35:12 -0000 1.18
+++ sys/mutex.h 17 Mar 2022 00:44:23 -0000
@@ -48,6 +48,8 @@ struct mutex {
#endif
};
+#ifdef _KERNEL
+
/*
* To prevent lock ordering problems with the kernel lock, we need to
* make sure we block all interrupts that can grab the kernel lock.
@@ -148,7 +150,7 @@ void _mtx_init_flags(struct mutex *, int
#endif /* WITNESS */
-#if defined(_KERNEL) && defined(DDB)
+#ifdef DDB
struct db_mutex {
struct cpu_info *mtx_owner;
@@ -160,6 +162,8 @@ struct db_mutex {
void db_mtx_enter(struct db_mutex *);
void db_mtx_leave(struct db_mutex *);
-#endif /* _KERNEL && DDB */
+#endif /* DDB */
+
+#endif /* _KERNEL */
#endif
