Date: Fri, 19 Jun 2020 08:08:43 -0700 From: John Baldwin <jhb@FreeBSD.org> To: cem@freebsd.org Cc: Eric van Gyzen <vangyzen@freebsd.org>, src-committers <src-committers@freebsd.org>, svn-src-all <svn-src-all@freebsd.org>, svn-src-head <svn-src-head@freebsd.org> Subject: Re: svn commit: r362126 - head/sys/vm Message-ID: <68fedb9b-04a1-5d52-34e5-ff32800c7eeb@FreeBSD.org> In-Reply-To: <CAG6CVpWr4hg-uZaS2EO6Qd2JsnEGpOV8SkWQE=zeNsBp3ahCZQ@mail.gmail.com> References: <202006122153.05CLr8JN091722@repo.freebsd.org> <CAG6CVpXR6BH6kPKGURH9aHFs829XCB1CRvjm3eciEufDdBXmMA@mail.gmail.com> <cdbd93bd-ddea-87fc-c361-90364f82ea6f@FreeBSD.org> <CAG6CVpX7oVK00SCscDRo6yLg_HsddF8jhu50Vzp3veTWqyc61Q@mail.gmail.com> <fe9ef24d-d902-789a-eb94-3f6f43f717c3@FreeBSD.org> <CAG6CVpWr4hg-uZaS2EO6Qd2JsnEGpOV8SkWQE=zeNsBp3ahCZQ@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On 6/18/20 10:53 AM, Conrad Meyer wrote: > On Thu, Jun 18, 2020 at 10:19 AM John Baldwin <jhb@freebsd.org> wrote: >> >> On 6/17/20 5:48 PM, Conrad Meyer wrote: >>> db_printf checks the pager, via db_putc. >> >> It doesn't break out of the loops for you though (e.g. via setjmp or the >> like). Commands still have to check db_pager_quit directly if they wish >> to abort early to honor a user entering 'q' at the pager prompt. > > It does for Ctrl-C, but not 'q', true. It could easily do the same > for 'q' as Ctrl-C: db_error(NULL) => kdb_reenter_silent(). That's only safe if commands are always idempotent. Some commands can use try locks (though they really shouldn't). Having printf() auto-convert is potentially worse for this as it affects doing a 'call' of an arbitrary function (which is likely not written to be safe for DDB). Though that's already a bit fraught with dragons since any nested panic already does the longjmp. We skip locking (though only somewhat, you can "unlock" a lock held by the interrupted code in your thread by calling a function). It may be that having 'q' longjmp is ok and wouldn't really be more fragile than what is already there. That is still orthogonal to your patch (which is printf -> db_printf in effect) (and so 2 separate things to consider). -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?68fedb9b-04a1-5d52-34e5-ff32800c7eeb>