Re: [PATCH RFC v2 2/2] pidfs: use maple tree

[Marek Szyprowski <m.szyprowski@samsung.com>]

On 13.12.2024 14:07, Christian Brauner wrote:
> On Fri, Dec 13, 2024 at 11:35:48AM +0100, Marek Szyprowski wrote:
>> On 09.12.2024 14:46, Christian Brauner wrote:
>>> So far we've been using an idr to track pidfs inodes. For some time now
>>> each struct pid has a unique 64bit value that is used as the inode
>>> number on 64 bit. That unique inode couldn't be used for looking up a
>>> specific struct pid though.
>>>
>>> Now that we support file handles we need this ability while avoiding to
>>> leak actual pid identifiers into userspace which can be problematic in
>>> containers.
>>>
>>> So far I had used an idr-based mechanism where the idr is used to
>>> generate a 32 bit number and each time it wraps we increment an upper
>>> bit value and generate a unique 64 bit value. The lower 32 bits are used
>>> to lookup the pid.
>>>
>>> I've been looking at the maple tree because it now has
>>> mas_alloc_cyclic(). Since it uses unsigned long it would simplify the
>>> 64bit implementation and its dense node mode supposedly also helps to
>>> mitigate fragmentation.
>>>
>>> Signed-off-by: Christian Brauner <brauner@kernel.org>
>> This patch landed in today's linux-next as commit a2c8e88a30f7 ("pidfs:
>> use maple tree"). In my tests I found that it triggers the following
>> lockdep warning, what probably means that something has not been
>> properly initialized:
> Ah, no, I think the issue that it didn't use irq{save,restore} spin lock
> variants in that codepath as this is free_pid() which needs it.
>
> I pushed a fix. Please yell if this issue persists.

I've applied this patch:

https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git/commit/?h=vfs-6.14.pidfs&id=34a0a75fd0887b599d68088b1dd40b3e48cfdc42

onto next-20241213 and it fixed my issue. Thanks!

Feel free to add:

Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland