[Devel] [PATCH vz9 v2 00/65] ploop optimistations and scalling
Alexander Atanasov
alexander.atanasov at virtuozzo.com
Thu Feb 13 09:12:16 MSK 2025
On 13.02.25 7:40, Pavel Tikhomirov wrote:
>
>
> On 2/12/25 17:46, Alexander Atanasov wrote:
>> On 12.02.25 11:32, Konstantin Khorenko wrote:
>>> Ploop processes requsts in a different threads in parallel
>>> where possible which results in significant improvement in
>>> performance and makes further optimistations possible.
>>>
>>> v1:
>>> - addressed feedback, i've left a few requests to merge changes
>>> into bigger patches out, as to keep changes in smaller chunks
>>> - patches merged and separated changes - generic cleanup
>>> - fix endio for md page writeback
>>> - background async allocation of space
>>> - move enospc pios dispatch from timer to thread
>>> - fixed allocations in atomic context
>>> - fixed locking wrt userspace/interrupt context
>>> - reworked discard cleanup
>>> - removed workqueue, only use kthread code
>>> - make metadata writeback works in parallel
>>> - end fsync pios in parallel
>>> - fixed current flags manipulation
>>>
>>> v2:
>>> - commit "dm-ploop: introduce pio.llist" appeared
>>> It introduces the the pio.llist along with pio.list, gathered in an
>>> union, so during the processes of switching between list users and
>>> llist users, we could use appropriate fields without strict
>>> pointers
>>> casts.
>>
>>
>> No, we should not add things that later will be removed - instead union,
>> please change type and remove casts.
>
>
> Adding things what will be removed in the same series is generally not a
> good practice, correct.
>
> But, in this exact case this addition/removal is justified as it helps
> with the transition and makes other patches correct (no tricks with gcc
> type checking now) and more human readable. Readability is the key.
No, it is not. It is not readable and it is very easy to make a mistake
- also think of list_empty.
Explicit casts are better and that is why they are used.
Conversion will be finished when the type is changed to llist as planned
and it is the right thing to do, using union is not a soltion it just
hides the unfinished conversion.
--
Regards,
Alexander Atanasov
More information about the Devel
mailing list