在 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
