Date: Fri, 3 Oct 2014 22:56:15 +0300 From: Konstantin Belousov <kostikbel@gmail.com> To: Larry Baird <lab@gta.com> Cc: "freebsd-hackers@freebsd.org" <freebsd-hackers@freebsd.org>, Ryan Stone <rysto32@gmail.com>, Dimitry Andric <dim@FreeBSD.org>, Bryan Drewery <bdrewery@FreeBSD.org> Subject: Re: Kernel/Compiler bug Message-ID: <20141003195615.GI26076@kib.kiev.ua> In-Reply-To: <20141003164613.GA78971@gta.com> References: <CAFMmRNxAYcr8eEY0SJsX3zkRadjT29-mfsGcSTmG_Yx-Hidi6w@mail.gmail.com> <20141001134044.GA57022@gta.com> <FBB9E4C3-55B9-4917-9953-F8BC9AE43619@FreeBSD.org> <542C8C75.30007@FreeBSD.org> <20141002075537.GU26076@kib.kiev.ua> <20141002140232.GA52387@gta.com> <20141002143345.GY26076@kib.kiev.ua> <20141003015456.GA27080@gta.com> <20141003073517.GC26076@kib.kiev.ua> <20141003164613.GA78971@gta.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Oct 03, 2014 at 12:46:13PM -0400, Larry Baird wrote:
> On Fri, Oct 03, 2014 at 10:35:17AM +0300, Konstantin Belousov wrote:
> > I have several notes. Mostly, it comes from my desire to make the patch
> > committable.
> I went ahead of made the changes over lunch. Hopefully the patch below
> addresses all of of the issues. Are you able to shepherd this into head or
> should I open a PR?
>
> Index: kern_intr.c
> ===================================================================
> --- kern_intr.c (revision 44897)
> +++ kern_intr.c (working copy)
> @@ -1386,6 +1386,14 @@
> }
> }
>
> +#ifdef DEBUG_KERNEL_THREAD_STACK
> +static int max_kern_thread_stack_used;
> +
> +SYSCTL_INT(_kern, OID_AUTO, max_kern_thread_stack_used, CTLFLAG_RD,
> + &max_kern_thread_stack_used, 0,
> + "Maxiumum stack depth used by a kernel thread");
> +#endif /* DEBUG_KERNEL_THREAD_STACK */
> +
> /*
> * Main interrupt handling body.
> *
> @@ -1407,6 +1415,40 @@
>
> td = curthread;
>
> +#ifdef DEBUG_KERNEL_THREAD_STACK
> + /*
> + * Track maximum stack used by a kernel thread.
> + *
> + * Testing for kernel thread isn't strictly needed. It optimizes the
> + * execution, since interrupts from usermode will have only the trap
> + * frame on the stack.
> + */
> + char *bottom_of_stack;
> + char *current;
> + int used;
> +
> + if (!TRAPF_USERMODE(frame)) {
> + bottom_of_stack = (char *)(td->td_kstack + td->td_kstack_pages *
> + PAGE_SIZE - 1);
> + current = (char *)&ih;
> +
> + /*
> + * Try to detect if interrupt is using kernel thread stack.
> + * Hardware could use a dedicated stack for interrupt handling.
> + */
> + if (bottom_of_stack > current &&
> + current > (char *)(td->td_kstack - PAGE_SIZE)) {
Why do you substract PAGE_SIZE in the right size of the second condition ?
> + used = bottom_of_stack - current;
> +
> + while (atomic_load_acq_int(&max_kern_thread_stack_used)
> + < used) {
> + atomic_store_rel_int(&max_kern_thread_stack_used,
> + used);
> + }
> + }
> + }
> +#endif /* DEBUG_KERNEL_THREAD_STACK */
> +
> /* An interrupt with no event or handlers is a stray interrupt. */
> if (ie == NULL || TAILQ_EMPTY(&ie->ie_handlers))
> return (EINVAL);
>
I rewrote the patch to my liking. Please test.
You should add
options KSTACK_USAGE_PROF
to your kernel config to get this activated.
I dislike the location of the intr_prof_stack_use() declaration, but
I was unable to find any other reasonable place for it.
commit 356881144088e850e725da2bb6f28dd52c4334b9
Author: Konstantin Belousov <kib@freebsd.org>
Date: Fri Oct 3 22:52:48 2014 +0300
KSTACK_USAGE_PROF
diff --git a/sys/conf/NOTES b/sys/conf/NOTES
index 5baa306..5cc146e 100644
--- a/sys/conf/NOTES
+++ b/sys/conf/NOTES
@@ -2958,6 +2958,7 @@ options SC_RENDER_DEBUG # syscons rendering debugging
options VFS_BIO_DEBUG # VFS buffer I/O debugging
options KSTACK_MAX_PAGES=32 # Maximum pages to give the kernel stack
+options KSTACK_USAGE_PROF
# Adaptec Array Controller driver options
options AAC_DEBUG # Debugging levels:
diff --git a/sys/conf/options b/sys/conf/options
index 42113c3..8337521 100644
--- a/sys/conf/options
+++ b/sys/conf/options
@@ -136,6 +136,7 @@ KDTRACE_FRAME opt_kdtrace.h
KN_HASHSIZE opt_kqueue.h
KSTACK_MAX_PAGES
KSTACK_PAGES
+KSTACK_USAGE_PROF
KTRACE
KTRACE_REQUEST_POOL opt_ktrace.h
LIBICONV
diff --git a/sys/kern/kern_intr.c b/sys/kern/kern_intr.c
index 6e9a4e8..d6de611 100644
--- a/sys/kern/kern_intr.c
+++ b/sys/kern/kern_intr.c
@@ -28,6 +28,7 @@
__FBSDID("$FreeBSD$");
#include "opt_ddb.h"
+#include "opt_kstack_usage_prof.h"
#include <sys/param.h>
#include <sys/bus.h>
@@ -1396,6 +1397,10 @@ intr_event_handle(struct intr_event *ie, struct trapframe *frame)
td = curthread;
+#ifdef KSTACK_USAGE_PROF
+ intr_prof_stack_use(td, frame);
+#endif
+
/* An interrupt with no event or handlers is a stray interrupt. */
if (ie == NULL || TAILQ_EMPTY(&ie->ie_handlers))
return (EINVAL);
diff --git a/sys/sys/systm.h b/sys/sys/systm.h
index 0f2732c..c484b7b 100644
--- a/sys/sys/systm.h
+++ b/sys/sys/systm.h
@@ -443,4 +443,6 @@ bitcount16(uint32_t x)
return (x);
}
+void intr_prof_stack_use(struct thread *td, struct trapframe *frame);
+
#endif /* !_SYS_SYSTM_H_ */
diff --git a/sys/vm/vm_glue.c b/sys/vm/vm_glue.c
index 61c003b..c9ee890 100644
--- a/sys/vm/vm_glue.c
+++ b/sys/vm/vm_glue.c
@@ -62,6 +62,7 @@ __FBSDID("$FreeBSD$");
#include "opt_vm.h"
#include "opt_kstack_pages.h"
#include "opt_kstack_max_pages.h"
+#include "opt_kstack_usage_prof.h"
#include <sys/param.h>
#include <sys/systm.h>
@@ -98,6 +99,8 @@ __FBSDID("$FreeBSD$");
#include <vm/vm_pager.h>
#include <vm/swap_pager.h>
+#include <machine/cpu.h>
+
#ifndef NO_SWAPPING
static int swapout(struct proc *);
static void swapclear(struct proc *);
@@ -486,6 +489,52 @@ kstack_cache_init(void *nulll)
SYSINIT(vm_kstacks, SI_SUB_KTHREAD_INIT, SI_ORDER_ANY, kstack_cache_init, NULL);
+#ifdef KSTACK_USAGE_PROF
+/*
+ * Track maximum stack used by a thread in kernel.
+ */
+static int max_kstack_used;
+
+SYSCTL_INT(_debug, OID_AUTO, max_kstack_used, CTLFLAG_RD,
+ &max_kstack_used, 0,
+ "Maxiumum stack depth used by a thread in kernel");
+
+void
+intr_prof_stack_use(struct thread *td, struct trapframe *frame)
+{
+ vm_offset_t stack_top;
+ vm_offset_t current;
+ int used, prev_used;
+
+ /*
+ * Testing for interrupted kernel mode isn't strictly
+ * needed. It optimizes the execution, since interrupts from
+ * usermode will have only the trap frame on the stack.
+ */
+ if (TRAPF_USERMODE(frame))
+ return;
+
+ stack_top = td->td_kstack + td->td_kstack_pages * PAGE_SIZE;
+ current = (vm_offset_t)(uintptr_t)&stack_top;
+
+ /*
+ * Try to detect if interrupt is using kernel thread stack.
+ * Hardware could use a dedicated stack for interrupt handling.
+ */
+ if (stack_top <= current || current < td->td_kstack)
+ return;
+
+ used = stack_top - current;
+ for (;;) {
+ prev_used = max_kstack_used;
+ if (prev_used >= used)
+ break;
+ if (atomic_cmpset_int(&max_kstack_used, prev_used, used))
+ break;
+ }
+}
+#endif /* KSTACK_USAGE_PROF */
+
#ifndef NO_SWAPPING
/*
* Allow a thread's kernel stack to be paged out.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20141003195615.GI26076>