From: Yunsheng Lin <[email protected]>
Date: Wed, 29 Nov 2023 11:17:50 +0800
> On 2023/11/27 22:32, Alexander Lobakin wrote:
>>
>> Chris, any thoughts on a global flag for skipping DMA syncs ladder?
>
> It seems there was one already in the past:
>
> https://lore.kernel.org/netdev/[email protected]/T/
It addresses a different problem, meaningless indirect calls, while this
one addresses meaningless direct calls :>
When the above gets merged, we could combine these two into one global,
but Eric wasn't active with his patch and I remember there were some
problems, so I wouldn't count on that it will arrive soon.
>
>>
>>>
>>>
>
>>>> +static inline bool page_pool_set_dma_addr(const struct page_pool *pool,
>>>> + struct page *page,
>>>> + dma_addr_t addr)
>>>> {
>>>> + unsigned long val = addr;
>>>> +
>>>> + if (unlikely(!addr)) {
>>>> + page->dma_addr = 0;
>>>> + return true;
>>>> + }
>>>
>>> The above seems unrelated change?
>>
>> Related. We use page_put_set_dma_addr() to clear ::dma_addr as well
>> (grep for it in page_pool.c). In this case, we don't want
>> dma_need_sync() to be called as we explicitly pass zero. This check
>> zeroes the field and exits as quickly as possible.
>
> The question seems to be about if we need to ensure the LSB of
> page->dma_addr is not set when page_pool releases a page back to page
> allocator?
But why do we need to call dma_need_sync(0) when freeing a page wasting
CPU cycles on relatively hot path?
>
>> In case with the call mentioned above, zero is a compile-time constant
>> there, so that this little branch will be inlined with the rest dropped.
Thanks,
Olek
On 2023/11/29 21:17, Alexander Lobakin wrote:
> From: Yunsheng Lin <[email protected]>
> Date: Wed, 29 Nov 2023 11:17:50 +0800
>
>> On 2023/11/27 22:32, Alexander Lobakin wrote:
>>>
>>> Chris, any thoughts on a global flag for skipping DMA syncs ladder?
>>
>> It seems there was one already in the past:
>>
>> https://lore.kernel.org/netdev/[email protected]/T/
>
> It addresses a different problem, meaningless indirect calls, while this
> one addresses meaningless direct calls :>
> When the above gets merged, we could combine these two into one global,
> but Eric wasn't active with his patch and I remember there were some
> problems, so I wouldn't count on that it will arrive soon.
I went through the above patch, It seems to me that there was no
fundamental problem that stopping us from implementing it in the dma
layer basing on Eric' patch if Eric is no longer interested in working
on a newer version?
It is just that if we allow every subsystem and driver using dma API
doing their own trick of skipping dma sync, then there is less willing
to implement it in the dma layer.
From: Yunsheng Lin <[email protected]>
Date: Thu, 30 Nov 2023 16:46:11 +0800
> On 2023/11/29 21:17, Alexander Lobakin wrote:
>> From: Yunsheng Lin <[email protected]>
>> Date: Wed, 29 Nov 2023 11:17:50 +0800
>>
>>> On 2023/11/27 22:32, Alexander Lobakin wrote:
>>>>
>>>> Chris, any thoughts on a global flag for skipping DMA syncs ladder?
>>>
>>> It seems there was one already in the past:
>>>
>>> https://lore.kernel.org/netdev/[email protected]/T/
>>
>> It addresses a different problem, meaningless indirect calls, while this
>> one addresses meaningless direct calls :>
>> When the above gets merged, we could combine these two into one global,
>> but Eric wasn't active with his patch and I remember there were some
>> problems, so I wouldn't count on that it will arrive soon.
>
> I went through the above patch, It seems to me that there was no
> fundamental problem that stopping us from implementing it in the dma
> layer basing on Eric' patch if Eric is no longer interested in working
> on a newer version?
I'm somewhat interested in continuing working on Eric's patch, but not
now. Have some urgent projects to work on, I could take this in January
I guess...
This PP-specific shortcut was done earlier and gives good boosts. It
would be trivial to remove it together with the XSk shortcut once a
generic both indirect and direct call DMA shortcut lands.
Does this sounds good / justified enough? Or you and other
reviewers/maintainers would prefer to wait for the generic one without
taking this patch?
>
> It is just that if we allow every subsystem and driver using dma API
> doing their own trick of skipping dma sync, then there is less willing
> to implement it in the dma layer.
Thanks,
Olek
On 2023/11/30 19:58, Alexander Lobakin wrote:
> From: Yunsheng Lin <[email protected]>
> Date: Thu, 30 Nov 2023 16:46:11 +0800
>
>> On 2023/11/29 21:17, Alexander Lobakin wrote:
>>> From: Yunsheng Lin <[email protected]>
>>> Date: Wed, 29 Nov 2023 11:17:50 +0800
>>>
>>>> On 2023/11/27 22:32, Alexander Lobakin wrote:
>>>>>
>>>>> Chris, any thoughts on a global flag for skipping DMA syncs ladder?
>>>>
>>>> It seems there was one already in the past:
>>>>
>>>> https://lore.kernel.org/netdev/[email protected]/T/
>>>
>>> It addresses a different problem, meaningless indirect calls, while this
>>> one addresses meaningless direct calls :>
>>> When the above gets merged, we could combine these two into one global,
>>> but Eric wasn't active with his patch and I remember there were some
>>> problems, so I wouldn't count on that it will arrive soon.
>>
>> I went through the above patch, It seems to me that there was no
>> fundamental problem that stopping us from implementing it in the dma
>> layer basing on Eric' patch if Eric is no longer interested in working
>> on a newer version?
>
> I'm somewhat interested in continuing working on Eric's patch, but not
> now. Have some urgent projects to work on, I could take this in January
> I guess...
> This PP-specific shortcut was done earlier and gives good boosts. It
> would be trivial to remove it together with the XSk shortcut once a
> generic both indirect and direct call DMA shortcut lands.
> Does this sounds good / justified enough? Or you and other
> reviewers/maintainers would prefer to wait for the generic one without
> taking this patch?
>
I would prefer we could wait for the generic one as there is only about one
month between now and january:)
From: Yunsheng Lin <[email protected]>
Date: Thu, 30 Nov 2023 20:20:44 +0800
> On 2023/11/30 19:58, Alexander Lobakin wrote:
>> From: Yunsheng Lin <[email protected]>
>> Date: Thu, 30 Nov 2023 16:46:11 +0800
>>
>>> On 2023/11/29 21:17, Alexander Lobakin wrote:
>>>> From: Yunsheng Lin <[email protected]>
>>>> Date: Wed, 29 Nov 2023 11:17:50 +0800
>>>>
>>>>> On 2023/11/27 22:32, Alexander Lobakin wrote:
>>>>>>
>>>>>> Chris, any thoughts on a global flag for skipping DMA syncs ladder?
>>>>>
>>>>> It seems there was one already in the past:
>>>>>
>>>>> https://lore.kernel.org/netdev/[email protected]/T/
>>>>
>>>> It addresses a different problem, meaningless indirect calls, while this
>>>> one addresses meaningless direct calls :>
>>>> When the above gets merged, we could combine these two into one global,
>>>> but Eric wasn't active with his patch and I remember there were some
>>>> problems, so I wouldn't count on that it will arrive soon.
>>>
>>> I went through the above patch, It seems to me that there was no
>>> fundamental problem that stopping us from implementing it in the dma
>>> layer basing on Eric' patch if Eric is no longer interested in working
>>> on a newer version?
>>
>> I'm somewhat interested in continuing working on Eric's patch, but not
>> now. Have some urgent projects to work on, I could take this in January
>> I guess...
>> This PP-specific shortcut was done earlier and gives good boosts. It
>> would be trivial to remove it together with the XSk shortcut once a
>> generic both indirect and direct call DMA shortcut lands.
>> Does this sounds good / justified enough? Or you and other
>> reviewers/maintainers would prefer to wait for the generic one without
>> taking this patch?
>>
>
> I would prefer we could wait for the generic one as there is only about one
> month between now and january:)
Ok, let's do it this way. I'll try to revive Eric's idea soon and get it
taken if he doesn't mind :>
Thanks,
Olek
On Thu, Nov 30, 2023 at 08:20:44PM +0800, Yunsheng Lin wrote:
> I would prefer we could wait for the generic one as there is only about one
> month between now and january:)
Same here. Please fix this properly for everyone instead of adding
a pile of hac