On Fri, Jun 10, 2016 at 11:47:48AM -0700, Florian Fainelli wrote:
> On 06/10/2016 05:00 AM, Andrew Lunn wrote:
> >> @@ -148,6 +155,9 @@ struct bcm_sf2_priv {
> >> struct device_node *master_mii_dn;
> >> struct mii_bus *slave_mii_bus;
> >> struct mii_bus *master_mii_bus;
> >> +
> >> + /* Cache of programmed VLANs */
> >> + struct bcm_sf2_vlan vlans[VLAN_N_VID];
> >
> > Hi Florian
> >
> > This is a 16Kbyte array. So i assume the whole priv structure needs 5
> > pages. Have you had any trouble allocating this much memory,
> > particularly once it has been used for a while and fragmented?
>
> Well, since this is using the old binding, we can't unload the driver,
> it's built into the kernel, so initializes early enough we have got
> plenty of memory.
Don't you want to use the new binding at some point?
> > I just wondered if it might be better to use vmalloc() for the
> > vlans.
>
> That's a very good point, I can't really see a drawback to doing this,
> will submit a patch moving this to a dynamic allocation.
>
> Another possible approach would have been to allocate the vlan structure
> upong port_vlan_prepare() though that would typically result in more
> fragmentation over time once se start using more VLANs.
It is a trade off, code complexity vs saved memory. I don't think such
a table is too bad, assuming your not trying to run the whole system
in 32Mbyes of RAM.
Andrew
