On 7/7/20 9:46 AM, Jakub Kicinski wrote:
On Tue, 7 Jul 2020 09:10:38 -0700 Shannon Nelson wrote:
On 7/6/20 10:33 AM, Jakub Kicinski wrote:
On Thu,  2 Jul 2020 16:39:17 -0700 Shannon Nelson wrote:
The queue reset pattern is used in a couple different places,
only slightly different from each other, and could cause
issues if one gets changed and the other didn't.  This puts
them together so that only one version is needed, yet each
can have slighty different effects by passing in a pointer
to a work function to do whatever configuration twiddling is
needed in the middle of the reset.

Fixes: 4d03e00a2140 ("ionic: Add initial ethtool support")
Signed-off-by: Shannon Nelson <snel...@pensando.io>
Is this fixing anything?
Yes, this fixes issues seen similar to what was fixed with b59eabd23ee5
("ionic: tame the watchdog timer on reconfig") where under loops of
changing parameters we could occasionally bump into the netdev watchdog.
User-visible bug should always be part of the commit message for a fix,
please amend.

Sure, I'll follow up later today.


I think the pattern of having a separate structure describing all the
parameters and passing that into reconfig is a better path forward,
because it's easier to take that forward in the correct direction of
allocating new resources before old ones are freed. IOW not doing a
full close/open.

E.g. nfp_net_set_ring_size().
This has been suggested before and looks great when you know you've got
the resources for dual allocations.  In our case this code is also used
inside our device where memory is tight: we are much more likely to have
allocation issues if we try to allocate everything without first
releasing what we already have.
Are you saying that inside the device the memory allocated for the
rings is close to the amount of max free memory? I find that hard to
believe.

I was a bit surprised as well, but there is a lot going on inside the device and it turns out there are one or two specific use cases where this is an issue.

sln

Reply via email to