All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Mintz, Yuval" <Yuval.Mintz@cavium.com>
To: Jason Baron <jbaron@akamai.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: Sun, 18 Sep 2016 10:25:05 +0000	[thread overview]
Message-ID: <BL2PR07MB2306CEA6AD75D9D4EB77FB1F8DF50@BL2PR07MB2306.namprd07.prod.outlook.com> (raw)
In-Reply-To: <d46b4fa36c32affff20fcc92148e33ee6176f899.1474060371.git.jbaron@akamai.com>

> 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.
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.

 - 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.

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.

  reply	other threads:[~2016-09-18 10:25 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 [this message]
2016-09-19 18:33     ` Jason Baron
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=BL2PR07MB2306CEA6AD75D9D4EB77FB1F8DF50@BL2PR07MB2306.namprd07.prod.outlook.com \
    --to=yuval.mintz@cavium.com \
    --cc=Ariel.Elior@qlogic.com \
    --cc=davem@davemloft.net \
    --cc=jbaron@akamai.com \
    --cc=netdev@vger.kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.