Date: Tue, 24 Apr 2018 17:05:43 -0700 From: Conrad Meyer <cem@freebsd.org> To: Mateusz Guzik <mjguzik@gmail.com> Cc: src-committers <src-committers@freebsd.org>, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r332957 - head/sys/kern Message-ID: <CAG6CVpWFt%2B_ZyjzDPp30sYGHOvLbM5OAjsE7S3hnQPBx64ucbA@mail.gmail.com> In-Reply-To: <CAGudoHFQ5B5EH=Dmq=KR-jCKnmrNMH-JdmTjfbygexvY5OGSYw@mail.gmail.com> References: <201804241841.w3OIfE0U074131@repo.freebsd.org> <CAGudoHFQ5B5EH=Dmq=KR-jCKnmrNMH-JdmTjfbygexvY5OGSYw@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
I am fine with any optimization to this path you like, so long as locks are neutered in panic and don't cause secondary panics. On Tue, Apr 24, 2018 at 4:22 PM, Mateusz Guzik <mjguzik@gmail.com> wrote: > On Tue, Apr 24, 2018 at 8:41 PM, Conrad Meyer <cem@freebsd.org> wrote: >> >> Author: cem >> Date: Tue Apr 24 18:41:14 2018 >> New Revision: 332957 >> URL: https://svnweb.freebsd.org/changeset/base/332957 >> >> Log: >> lockmgr: Add missed neutering during panic >> >> r313683 introduced new lockmgr APIs that missed the panic-time neutering >> present in the rest of our locks. Correct that by adding the usual >> check. >> >> Additionally, move the __lockmgr_args neutering above the assertions at >> the >> top of the function. Drop the interlock unlock because we shouldn't >> have >> an unneutered interlock either. No point trying to unlock it. >> >> PR: 227749 >> Reported by: jtl >> Sponsored by: Dell EMC Isilon >> >> Modified: >> head/sys/kern/kern_lock.c >> >> Modified: head/sys/kern/kern_lock.c >> >> ============================================================================== >> --- head/sys/kern/kern_lock.c Tue Apr 24 18:19:30 2018 (r332956) >> +++ head/sys/kern/kern_lock.c Tue Apr 24 18:41:14 2018 (r332957) >> @@ -918,6 +918,9 @@ lockmgr_lock_fast_path(struct lock *lk, u_int flags, s >> u_int op; >> bool locked; >> >> + if (__predict_false(panicstr != NULL)) >> + return (0); >> + >> > > This only further pollutes the slowpath. I mean fastpath. All the fallback > funcs check for panic, so there is no need to do it in regular kernels at > this level. > > Instead, the firing assertion should also see if perhaps the kernel > already has panicked. > > Alternatively, the checks added here can be conditional on INVARIANTS. > > -- > Mateusz Guzik <mjguzik gmail.com>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAG6CVpWFt%2B_ZyjzDPp30sYGHOvLbM5OAjsE7S3hnQPBx64ucbA>