linux-kernel - Re: [PATCH drm-next 1/2] vmalloc: Add atomic

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <de5b9722-905c-49e6-87dc-3fcdeb07fb09@redhat.com>
Date: Mon, 10 Mar 2025 11:23:45 +0100
From: Jocelyn Falempe <jfalempe@...hat.com>
To: Ryosuke Yasuoka <ryasuoka@...hat.com>
Cc: Matthew Wilcox <willy@...radead.org>, maarten.lankhorst@...ux.intel.com,
 mripard@...nel.org, tzimmermann@...e.de, airlied@...il.com, simona@...ll.ch,
 kraxel@...hat.com, gurchetansingh@...omium.org, olvaffe@...il.com,
 akpm@...ux-foundation.org, urezki@...il.com, hch@...radead.org,
 dmitry.osipenko@...labora.com, dri-devel@...ts.freedesktop.org,
 linux-kernel@...r.kernel.org, virtualization@...ts.linux.dev,
 linux-mm@...ck.org
Subject: Re: [PATCH drm-next 1/2] vmalloc: Add atomic_vmap

On 09/03/2025 09:07, Ryosuke Yasuoka wrote:
> On Fri, Mar 7, 2025 at 4:55 PM Jocelyn Falempe <jfalempe@...hat.com> wrote:
>>
>> On 06/03/2025 16:52, Simona Vetter wrote:
>>> On Thu, Mar 06, 2025 at 02:24:51PM +0100, Jocelyn Falempe wrote:
>>>> On 06/03/2025 05:52, Matthew Wilcox wrote:
>>>>> On Thu, Mar 06, 2025 at 12:25:53AM +0900, Ryosuke Yasuoka wrote:
>>>>>> Some drivers can use vmap in drm_panic, however, vmap is sleepable and
>>>>>> takes locks. Since drm_panic will vmap in panic handler, atomic_vmap
>>>>>> requests pages with GFP_ATOMIC and maps KVA without locks and sleep.
>>>>>
>>>>> In addition to the implicit GFP_KERNEL allocations Vlad mentioned, how
>>>>> is this supposed to work?
>>>>>
>>>>>> +  vn = addr_to_node(va->va_start);
>>>>>> +
>>>>>> +  insert_vmap_area(va, &vn->busy.root, &vn->busy.head);
>>>>>
>>>>> If someone else is holding the vn->busy.lock because they're modifying the
>>>>> busy tree, you'll corrupt the tree.  You can't just say "I can't take a
>>>>> lock here, so I won't bother".  You need to figure out how to do something
>>>>> safe without taking the lock.  For example, you could preallocate the
>>>>> page tables and reserve a vmap area when the driver loads that would
>>>>> then be usable for the panic situation.  I don't know that we have APIs
>>>>> to let you do that today, but it's something that could be added.
>>>>>
>>>> Regarding the lock, it should be possible to use the trylock() variant, and
>>>> fail if the lock is already taken. (In the panic handler, only 1 CPU remain
>>>> active, so it's unlikely the lock would be released anyway).
>>>>
>>>> If we need to pre-allocate the page table and reserve the vmap area, maybe
>>>> it would be easier to just always vmap() the primary framebuffer, so it can
>>>> be used in the panic handler?
>>>
>>> Yeah I really don't like the idea of creating some really brittle one-off
>>> core mm code just so we don't have to vmap a buffer unconditionally. I
>>> think even better would be if drm_panic can cope with non-linear buffers,
>>> it's entirely fine if the drawing function absolutely crawls and sets each
>>> individual byte ...
>>
>> It already supports some non-linear buffer, like Nvidia block-linear:
>> https://elixir.bootlin.com/linux/v6.13.5/source/drivers/gpu/drm/nouveau/dispnv50/wndw.c#L606
>>
>> And I've also sent some patches to support Intel's 4-tile and Y-tile format:
>> https://patchwork.freedesktop.org/patch/637200/?series=141936&rev=5
>> https://patchwork.freedesktop.org/patch/637202/?series=141936&rev=5
>>
>> Hopefully Color Compression can be disabled on intel's GPU, otherwise
>> that would be a bit harder to implement than tiling.
>>
>>>
>>> The only thing you're allowed to do in panic is try_lock on a raw spinlock
>>> (plus some really scare lockless tricks), imposing that on core mm sounds
>>> like a non-starter to me.
>>>
>>> Cheers, Sima
>>
> 
> Thank you all for your comments.
> I understand adding atomic_vmap is not possible as vmalloc is not compatible
> with GFP_ATOMIC. I'll re-implement this by pre-allocating the page table and
> reserve the vmap area while the kernel is alive. It'll might be
> allocated in driver
> codes so maybe I don't need to add any features in core mm code.

Maybe another way to do that, would be to atomically kmap only one page 
at a time. And when drawing the panic screen, make sure that for each 
pixel the right page is mapped.
Would kmap_local_page() fit for this purpose?

Best regards,

-- 

Jocelyn


> 
> Best regards,
> Ryosuke
>