Date: Wed, 6 Jun 2018 17:08:15 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Bruce Evans <brde@optusnet.com.au> Cc: Mahmoud Al-Qudsi <mqudsi@neosmart.net>, bugs@freebsd.org Subject: Re: [Bug 228755] libvgl under syscons causes system reboot (via SDL 1.2) Message-ID: <20180606165445.S1790@besplex.bde.org> In-Reply-To: <20180606031534.I5150@besplex.bde.org> References: <bug-228755-227@https.bugs.freebsd.org/bugzilla/> <20180605175705.Q2604@besplex.bde.org> <20180605213242.K3458@besplex.bde.org> <CACcTrKfNhhWEjXsnV--uyNd3xYD2T5a9x4tE7qrKT=ztaau83w@mail.gmail.com> <20180606031534.I5150@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, 6 Jun 2018, Bruce Evans wrote: > On Tue, 5 Jun 2018, Mahmoud Al-Qudsi wrote: > >> On Tue, Jun 5, 2018 at 7:32 AM, Bruce Evans <brde@optusnet.com.au> wrote: >>> ... >>> 1920x1080x8. I use this fix: >>> ... >>> - if (offset > adp->va_window_size - PAGE_SIZE) >>> + if (offset > trunc_page(adp->va_window_size)) >> ... >> Since you seem to know what you're doing - is the comparison with >> `trunc_page(adp->va_window_size)` here the way to go or the alternative >> approach with `offset >= adp->va_info.vi_buffer_size` range checking >> instead? > > I think the tests are equivalent under the assumptions that the caller has > rounded down 'offset' to a page boundary and that the hardware frame buffer > size is a multiple of the page size. The trunc_page() test is clearer but > still cyptic. The range being checked is from offset to offset + PAGE_SIZE > -1 > under the simplifying assumpting that offset has been rounded. It is the > end of this range, not the start, that should be compared with the rounded-uo > buffer size. But after rounding, '<' means at least 1 page smaller, not just > at least 1 byte smaller, so we can take the shortcut of replacing > offset + PAGE_SIZE - 1 by that less about PAGE_SIZE, but it is still not > obvious that this doesn't have an off-by-1-byte error. It is my fix that has an off-by-1 error. If fails to reduce the buffer size to the offset of the last page in the buffer in the usual case where the buffer size is a multiple of the page size. trunc_page(buffer_size - 1) does that. Checking the start offsets (reduced to page boundaries) is less clear than checking the end offsets, but is more robust because in theory an offset may be for the last page so rounding up to a page boundary overflows to 0. vm code has to worry about this more than most code, so it usually rounds down to page boundaries. I see a bug in the size calculation for kernel-level mappings of the frame buffer for direct-mapped modes (that is, mainly modes with 15, 16, 24 or 32 planes of colors). vgl doesn't support these modes (that is one reason it is slow. Which modes do you use, and which ones panic? Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20180606165445.S1790>