From: Jacob Keller <jacob.e.keller@intel.com>
To: <intel-wired-lan@osuosl.org>
Subject: Re: [Intel-wired-lan] [net-queue v4 1/1] ice: Do not use WQ_MEM_RECLAIM flag for workqueue
Date: Thu, 26 Jan 2023 10:49:10 -0800 [thread overview]
Message-ID: <a03bd398-7355-98fc-3674-90d4cfecff2a@intel.com> (raw)
In-Reply-To: <Y9Kl+OFJRbDWYxoy@boxer>
On 1/26/2023 8:10 AM, Maciej Fijalkowski wrote:
> On Thu, Jan 19, 2023 at 01:16:08PM -0800, Tony Nguyen wrote:
>> From: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
>>
>> When both ice and the irdma driver are loaded, a warning
>> in check_flush_dependency is being triggered. This seems
>> to be because of the ice driver workqueue is allocated with
>> the WQ_MEM_RECLAIM flag, and the irdma one is not.
>>
>> Looking at the kernel documentation, it doesn't seem like
>> the ice driver needs to use WQ_MEM_RECLAIM. Remove it.
>
> Can we have a better reasoning rather than 'it doesn't seem like ice
> driver needs...' ?
>
> Also, why was reclaim flag added in the first place?
>
Yes. So this is caused by historical decision/accident. Way back in the
day we used to use create_singlethread_workqueue, which got converted to
alloc_workqueue by 6992a6c9c435 ("i40e: use alloc_workqueue instead of
create_singlethread_workqueue"). This conversion copied how
create_singlethread_workqueue set flags at the time:
> ee64e7f697ad Tejun Heo 2012-08-21 13:18 -0700 419│ #define create_singlethread_workqueue(name) \
> 23d11a58a9a6 Tejun Heo 2016-01-29 05:59 -0500 420│ alloc_ordered_workqueue("%s", __WQ_LEGACY | WQ_MEM_RECLAIM, name)
Since this was in i40e, when the development began on ice, it seems to
have simply copied what i40e did without regard for why the flag was set.
I suspect create_singlethread_workqueue set it because it was designed
to be a simple API that didn't expose flags so they added the reclaim
flag because the workqueue "might" need it.
My understanding of WQ_MEM_RECLAIM is that such work queues will still
execute when the system memory is low. Only work queues that must
execute in order to free resources ought to set it. (otherwise, if every
work queue sets it, it is basically the same as no work queue setting it...)
Hope this explanation helps.
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
prev parent reply other threads:[~2023-01-26 18:49 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-19 21:16 [Intel-wired-lan] [net-queue v4 1/1] ice: Do not use WQ_MEM_RECLAIM flag for workqueue Tony Nguyen
2023-01-25 8:03 ` Andrysiak, Jakub
2023-01-26 16:10 ` Maciej Fijalkowski
2023-01-26 18:16 ` Anirudh Venkataramanan
2023-01-26 18:55 ` Jacob Keller
2023-01-26 18:49 ` Jacob Keller [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a03bd398-7355-98fc-3674-90d4cfecff2a@intel.com \
--to=jacob.e.keller@intel.com \
--cc=intel-wired-lan@osuosl.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox