From: Jason Baron <jbaron@akamai•com>
To: "Mintz, Yuval" <Yuval.Mintz@cavium•com>,
"davem@davemloft•net" <davem@davemloft•net>
Cc: "netdev@vger•kernel.org" <netdev@vger•kernel.org>,
"Ariel.Elior@qlogic•com" <Ariel.Elior@qlogic•com>
Subject: Re: [PATCH net-next 2/2] bnx2x: allocate mac filtering pending list in PAGE_SIZE increments
Date: Mon, 19 Sep 2016 14:33:38 -0400 [thread overview]
Message-ID: <57E02F82.9060903@akamai.com> (raw)
In-Reply-To: <BL2PR07MB2306CEA6AD75D9D4EB77FB1F8DF50@BL2PR07MB2306.namprd07.prod.outlook.com>
On 09/18/2016 06:25 AM, Mintz, Yuval wrote:
>> Currently, we can have high order page allocations that specify
>> GFP_ATOMIC when configuring multicast MAC address filters.
>>
>> For example, we have seen order 2 page allocation failures with
>> ~500 multicast addresses configured.
>>
>> Convert the allocation for the pending list to be done in PAGE_SIZE
>> increments.
>>
>> Signed-off-by: Jason Baron <jbaron@akamai•com>
>
> While I appreciate the effort, I wonder whether it's worth it:
>
> - The hardware [even in its newer generation] provides an approximate
> based classification [I.e., hashed] with 256 bins.
> When configuring 500 multicast addresses, one can argue the
> difference between multicast-promisc mode and actual configuration
> is insignificant.
With 256 bins, I think it takes close to: 256*lg(256) or 2,048
multicast addresses to expect to have all bins have at least one hash,
assuming a uniform distribution of the hashes.
> Perhaps the easier-to-maintain alternative would simply be to
> determine the maximal number of multicast addresses that can be
> configured using a single PAGE, and if in need of more than that
> simply move into multicast-promisc.
>
sizeof(struct bnx2x_mcast_list_elem) = 24. So there are 170 per
page on x86. So if we want to fit 2,048 elements, we need 12 pages.
> - While GFP_ATOMIC is required in this flow due to the fact it's being
> called from sleepless context, I do believe this is mostly a remnant -
> it's possible that by slightly changing the locking scheme we can have
> the configuration done from sleepless context and simply switch to
> GFP_KERNEL instead.
>
Ok if its GFP_KERNEL, I think its still undesirable to do large page
order allocations (unless of course its converted to a single page, but
I'm not sure this makes sense as mentioned).
> Regarding the patch itself, only comment I have:
>> + elem_group = (struct bnx2x_mcast_elem_group *)
>> + elem_group->mcast_group_link.next;
> Let's use list_next_entry() instead.
>
>
Yes, agreed.
I think it would be easy to add a check to bnx2x_set_rx_mode_inner() to
enforce some maximum number of elements (perhaps 2,048 based on the
above math) for the !CHIP_IS_E1() case on top of what I already posted.
Thanks,
-Jason
next prev parent reply other threads:[~2016-09-19 18:33 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-16 21:30 [PATCH net-next 0/2] bnx2x: page allocation failure Jason Baron
2016-09-16 21:30 ` [PATCH net-next 1/2] bnx2x: allocate mac filtering 'mcast_list' in PAGE_SIZE increments Jason Baron
2016-09-16 21:30 ` [PATCH net-next 2/2] bnx2x: allocate mac filtering pending list " Jason Baron
2016-09-18 10:25 ` Mintz, Yuval
2016-09-19 18:33 ` Jason Baron [this message]
2016-09-20 7:41 ` Mintz, Yuval
2016-09-20 14:52 ` Jason Baron
2016-09-20 15:00 ` Mintz, Yuval
2016-09-20 15:19 ` Jason Baron
2016-09-20 11:30 ` David Laight
2016-09-20 18:46 ` Jason Baron
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=57E02F82.9060903@akamai.com \
--to=jbaron@akamai$(echo .)com \
--cc=Ariel.Elior@qlogic$(echo .)com \
--cc=Yuval.Mintz@cavium$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=netdev@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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