svn commit: r284535 - head/sys/kern

[Konstantin Belousov]

On Thu, Jun 18, 2015 at 07:59:43AM -0700, Sean Bruno wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA512
> 
> On 06/17/15 20:07, Konstantin Belousov wrote:
> > On Thu, Jun 18, 2015 at 02:04:21AM +0000, Sean Bruno wrote:
> >> Author: sbruno Date: Thu Jun 18 02:04:20 2015 New Revision:
> >> 284535 URL: https://svnweb.freebsd.org/changeset/base/284535
> >> 
> >> Log: This change replaces the mutex with a sx lock for the
> >> interpreter list to avoid the problem of holding a non-sleep lock
> >> during a page fault as reported by witness. It also uses atomics
> >> where possible to avoid having to acquire the exclusive lock. In
> >> addition, it consistently uses memset()/memcpy() instead of
> >> bzero()/bcopy().
> >> 
> >> Differential Revision:	https://reviews.freebsd.org/D1971 
> >> Submitted by:	sson Reviewed by:	jhb
> > What are the page faults during image activator run ? Or, if the
> > page faults are not during image activation, then where ?
> > 
> 
> The original witness panic was one we discussed a while ago on current.
> https://lists.freebsd.org/pipermail/freebsd-current/2015-February/054698
> .html
> 
> I wanted to resolve that witness issue before I tried to reproduce any
> other failure cases.
> 
> Kernel page fault with the following non-sleepable locks held:
> exclusive sleep mutex imgact_binmisc (imgact_binmisc) r = 0
> (0xffffffff82012418) locked @
> /usr/src/sys/modules/imgact_binmisc/../../kern/imgact_binmisc.c:596
> KDB: stack backtrace:
> db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame
> 0xfffffe046a236280
> witness_warn() at witness_warn+0x4ae/frame 0xfffffe046a236350
> trap_pfault() at trap_pfault+0x59/frame 0xfffffe046a2363f0
> trap() at trap+0x45e/frame 0xfffffe046a236600
> calltrap() at calltrap+0x8/frame 0xfffffe046a236600
> - - --- trap 0xc, rip = 0xffffffff80d21279, rsp = 0xfffffe046a2366c0, rbp
> = 0xfffffe046a2366d0 ---
> bcopy() at bcopy+0x39/frame 0xfffffe046a2366d0
> imgact_binmisc_exec() at imgact_binmisc_exec+0x23d/frame
> 0xfffffe046a236720
> kern_execve() at kern_execve+0x4c6/frame 0xfffffe046a236a80
> sys_execve() at sys_execve+0x37/frame 0xfffffe046a236ae0
> amd64_syscall() at amd64_syscall+0x27f/frame 0xfffffe046a236bf0
> Xfast_syscall() at Xfast_syscall+0xfb/frame 0xfffffe046a236bf0
> - - --- syscall (59, FreeBSD ELF64, sys_execve), rip = 0x4297ba, rsp =
> 0x7fffffffdaf8, rbp = 0x7fffffffdb00 ---
> 
What is the source line for imgact_binmisc_exec+0x23d ?
I see only one direct bcopy() call in the imgact_binmisc_exec(),
which is accessing the exec_map swappable memory, indeed.  There might
be compiler-generated bcopy() calls, and in this case the faulting access
probably indicates other bug.

BTW, why imgact_binmisc_exec() is not static ?

> 
> >> 
> >> @@ -404,12 +404,12 @@ imgact_binmisc_get_all_entries(struct sy 
> >> imgact_binmisc_entry_t *ibe; int error = 0, count;
> >> 
> >> -	mtx_lock(&interp_list_mtx); +	sx_slock(&interp_list_sx); count
> >> = interp_list_entry_count; /* Don't block in malloc() while
> >> holding lock. */ xbe = malloc(sizeof(*xbe) * count, M_BINMISC,
> >> M_NOWAIT|M_ZERO);
> > This is definitely no longer true statement. Even the original use
> > of M_NOWAIT there is not warranted.
> > 
> 
> Dead comment?  I should remove it then as it is
> invalid/inaccurate/never was true?
> 
> I should remove M_NOWAIT as well?

Yes and yes.  Also, M_NOWAIT does not return NULL.