Hi, On Thu, Oct 05, 2023 at 11:04:20AM +0200, Thomas Zimmermann wrote: > DRM's format-conversion helpers require temporary memory. Pass the > buffer from the caller and keep it allocated over several calls. Allow > the caller to preallocate the buffer memory. I'm sorry... but why? Why do you need to keep it allocated over several calls and preallocate the buffer? It's not clear to me at all. > The motivation for this patchset is the recent work on a DRM panic > handler. [1] The panic handler requires format conversion to display an > error to the screen. But allocating memory during kernel panics is > fragile. We agree that we shouldn't allocate memory during the panic. I still have concerns about how the panic handler will handle the driver currently set up for a plane that isn't using an RGB format, or a buffer not accessible by the kernel or CPU. You can't expect to get away with just a copy to the current active buffer. If that's the assumption that underlines that patch series, then I don't know why we need it at all, because that assumption is wrong to begin with, and way too restrictive. > The changes in this patchset enable the DRM panic handler to > preallocate buffer storage before the panic occurs. > > As an additonal benefit, drivers can now keep the temporary storage > across multiple updates. Avoiding memory allocation slightly reduces > the CPU overhead of the format helpers. I'm sorry to go over that again, but you can't write a performance improvement mechanism without some kind of benchmark. kmalloc has built-in caching, why do we absolutely need our own cache on top of it? If you never measured it, for all we know, we simply don't need it and kmalloc is good enough. Maxime
Attachment:
signature.asc
Description: PGP signature
