From: Sukadev Bhattiprolu <sukadev@linux•ibm.com>
To: Jakub Kicinski <kuba@kernel•org>
Cc: netdev@vger•kernel.org, Dany Madden <drt@linux•ibm.com>,
Lijun Pan <ljp@linux•ibm.com>
Subject: Re: [PATCH 2/7] ibmvnic: update reset function prototypes
Date: Mon, 11 Jan 2021 11:57:59 -0800 [thread overview]
Message-ID: <20210111195759.GA178503@us.ibm.com> (raw)
In-Reply-To: <20210111113249.1026433f@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Jakub Kicinski [kuba@kernel•org] wrote:
> On Sun, 10 Jan 2021 19:12:21 -0800 Sukadev Bhattiprolu wrote:
> > Jakub Kicinski [kuba@kernel•org] wrote:
> > > On Thu, 7 Jan 2021 23:12:31 -0800 Sukadev Bhattiprolu wrote:
> > > > The reset functions need just the 'reset reason' parameter and not
> > > > the ibmvnic_rwi list element. Update the functions so we can simplify
> > > > the handling of the ->rwi_list in a follow-on patch.
> > > >
> > > > Fixes: 2770a7984db5 ("ibmvnic: Introduce hard reset recovery")
> > > >
> > >
> > > No empty lines after Fixes tags, please. They should also not be
> > > wrapped.
> >
> > Ah ok, will fix.
> > >
> > > > Signed-off-by: Sukadev Bhattiprolu <sukadev@linux•ibm.com>
> > >
> > > Are these patches for net or net-next? It looks like they are fixing
> > > races, but at the same time they are rather large. Can you please
> > > produce minimal fixes, e.g. patch 3 should just fix the existing leaks
> > > rather than refactor the code to not do allocations. 130+ LoC is a lot
> > > for a fix.
> >
> > This is a set of bug fixes, but yes a bit large. Should I submit to
> > net-next instead?
>
> I'd rather you tried to address the problems with minimal patches, then
> you can refactor the code in net-next.
I had thought about that but fixing the leaks and then throwing away the
code did not seem very useful. The diff stat shows 78 insertions and 59
deletions. The bulk of the new code, about 70 lines including comments,
is just the fairly straightforward helper functions:
- get_pending_reset()
- add_pending_reset()
Fixing the leak in the existing code would not reduce the size of these
helpers. We are now using a simpler approach with no allocations, so no
leaks.
Sukadev
next prev parent reply other threads:[~2021-01-11 19:58 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-08 7:12 [PATCH 0/7] ibmvnic: Use more consistent locking Sukadev Bhattiprolu
2021-01-08 7:12 ` [PATCH 1/7] ibmvnic: restore state in change-param reset Sukadev Bhattiprolu
2021-01-08 7:12 ` [PATCH 2/7] ibmvnic: update reset function prototypes Sukadev Bhattiprolu
2021-01-10 3:37 ` Jakub Kicinski
2021-01-11 3:12 ` Sukadev Bhattiprolu
2021-01-11 19:32 ` Jakub Kicinski
2021-01-11 19:57 ` Sukadev Bhattiprolu [this message]
2021-01-08 7:12 ` [PATCH 3/7] ibmvnic: avoid allocating rwi entries Sukadev Bhattiprolu
2021-01-08 7:12 ` [PATCH 4/7] ibmvnic: switch order of checks in ibmvnic_reset Sukadev Bhattiprolu
2021-01-08 7:12 ` [PATCH 5/7] ibmvnic: use a lock to serialize remove/reset Sukadev Bhattiprolu
2021-01-10 3:41 ` Jakub Kicinski
2021-01-11 3:52 ` Sukadev Bhattiprolu
2021-01-11 19:43 ` Jakub Kicinski
2021-01-11 20:15 ` Sukadev Bhattiprolu
2021-01-08 7:12 ` [PATCH 6/7] ibmvnic: check adapter->state under state_lock Sukadev Bhattiprolu
2021-01-08 7:12 ` [PATCH 7/7] ibmvnic: add comments about adapter->state_lock Sukadev Bhattiprolu
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=20210111195759.GA178503@us.ibm.com \
--to=sukadev@linux$(echo .)ibm.com \
--cc=drt@linux$(echo .)ibm.com \
--cc=kuba@kernel$(echo .)org \
--cc=ljp@linux$(echo .)ibm.com \
--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