linux-kernel - Re: [PATCH v1 00/11] dm-pcache – persistent-memory cache for block devices

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <0953f766-48bc-416f-9089-7403e938569c@linux.dev>
Date: Tue, 1 Jul 2025 00:30:32 +0800
From: Dongsheng Yang <dongsheng.yang@...ux.dev>
To: Mikulas Patocka <mpatocka@...hat.com>
Cc: agk@...hat.com, snitzer@...nel.org, axboe@...nel.dk, hch@....de,
 dan.j.williams@...el.com, Jonathan.Cameron@...wei.com,
 linux-block@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-cxl@...r.kernel.org, nvdimm@...ts.linux.dev, dm-devel@...ts.linux.dev
Subject: Re: [PATCH v1 00/11] dm-pcache – persistent-memory cache for block devices


在 6/30/2025 11:45 PM, Mikulas Patocka 写道:
>
> On Mon, 30 Jun 2025, Dongsheng Yang wrote:
>
>> Hi Mikulas,
>>
>>      The reason why we don’t release the spinlock here is that if we do, the
>> subtree could change.
>>
>> For example, in the `fixup_overlap_contained()` function, we may need to split
>> a certain `cache_key`, and that requires allocating a new `cache_key`.
>>
>> If we drop the spinlock at this point and then re-acquire it after the
>> allocation, the subtree might already have been modified, and we cannot safely
>> continue with the split operation.
> Yes, I understand this.
>
>>      In this case, we would have to restart the entire subtree search and walk.
>> But the new walk might require more memory—or less,
>>
>> so it's very difficult to know in advance how much memory will be needed
>> before acquiring the spinlock.
>>
>>      So allocating memory inside a spinlock is actually a more direct and
>> feasible approach. `GFP_NOWAIT` fails too easily, maybe `GFP_ATOMIC` is more
>> appropriate.
> Even GFP_ATOMIC can fail. And it is not appropriate to return error and
> corrupt data when GFP_ATOMIC fails.
>
>> What do you think?
> If you need memory, you should drop the spinlock, allocate the memory
> (with mempool_alloc(GFP_NOIO)), attach the allocated memory to "struct
> pcache_request" and retry the request (call pcache_cache_handle_req
> again).
>
> When you retry the request, there are these possibilities:
> * you don't need the memory anymore - then, you just free it
> * you need the amount of memory that was allocated - you just proceed
>    while holding the spinlock
> * you need more memory than what you allocated - you drop the spinlock,
>    free the memory, allocate a larger memory block and retry again


Yes, that was exactly my initial idea when I first came up with this 
solution—it just seemed a bit convoluted. That’s why I wondered if using 
GFP_ATOMIC might be a more straightforward approach.

Okay, compared to simply returning an error to the upper-level user, I 
think implementing this mechanism is well worth it.

Dongsheng

>
> Mikulas