All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sukadev Bhattiprolu <sukadev@linux.ibm.com>
To: Lijun Pan <lijunp213@gmail.com>
Cc: Networking <netdev@vger.kernel.org>,
	Dany Madden <drt@linux.ibm.com>,
	Rick Lindsley <ricklind@linux.ibm.com>,
	Brian King <brking@linux.ibm.com>,
	Cristobal Forno <cforno12@linux.ibm.com>
Subject: Re: [PATCH net 1/7] Revert "ibmvnic: simplify reset_long_term_buff function"
Date: Thu, 24 Jun 2021 09:07:25 -0700	[thread overview]
Message-ID: <YNStvWD3gd3E5fgv@us.ibm.com> (raw)
In-Reply-To: <CAOhMmr7RbBNF8BbRbf3hEdD0cj9OjihuuKmXJogSz=8ewtGWog@mail.gmail.com>

Lijun Pan [lijunp213@gmail.com] wrote:
> On Wed, Jun 23, 2021 at 11:16 PM Sukadev Bhattiprolu
> <sukadev@linux.ibm.com> wrote:
> >
> > This reverts commit 1c7d45e7b2c29080bf6c8cd0e213cc3cbb62a054.
> >
> > We tried to optimize the number of hcalls we send and skipped sending
> > the REQUEST_MAP calls for some maps. However during resets, we need to
> > resend all the maps to the VIOS since the VIOS does not remember the
> > old values. In fact we may have failed over to a new VIOS which will
> > not have any of the mappings.
> >
> > When we send packets with map ids the VIOS does not know about, it
> > triggers a FATAL reset. While the client does recover from the FATAL
> > error reset, we are seeing a large number of such resets. Handling
> > FATAL resets is lot more unnecessary work than issuing a few more
> > hcalls so revert the commit and resend the maps to the VIOS.
> >
> 
> This was not an issue when the original optimization code was committed.
> VIOS changes over time and it is proprietary code, so people don't really know
> what it changes every time.

All the more reason to be careful about ripping out code.

>If you believe the verbose hcall is really necessary,
> you'd better document that in the function/source code.

It was necessary and present until you removed it. I am reverting it
after lot of debugging and with sufficient explanation. Feel free to
submit a new patch.

>This patch may be reverted again
> some time later when the verbose calling isn't needed.

Hopefully not without sufficient testing.

Sukadev

Sukadev

  reply	other threads:[~2021-06-24 16:07 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-24  4:13 [PATCH net 0/7] ibmvnic: Assorted bug fixes Sukadev Bhattiprolu
2021-06-24  4:13 ` [PATCH net 1/7] Revert "ibmvnic: simplify reset_long_term_buff function" Sukadev Bhattiprolu
2021-06-24  6:07   ` Lijun Pan
2021-06-24 16:07     ` Sukadev Bhattiprolu [this message]
2021-06-24  4:13 ` [PATCH net 2/7] Revert "ibmvnic: remove duplicate napi_schedule call in open function" Sukadev Bhattiprolu
2021-06-24  6:20   ` Lijun Pan
2021-06-24  6:42     ` Rick Lindsley
2021-06-24  7:07     ` Rick Lindsley
2021-06-24  7:02   ` Johaan Smith
2021-06-24  7:28     ` Rick Lindsley
2021-06-24 17:05       ` Lijun Pan
2021-06-24 18:18         ` Dany Madden
2021-06-24 23:33         ` Rick Lindsley
2021-06-24 16:53     ` Lijun Pan
2021-06-24  4:13 ` [PATCH net 3/7] ibmvnic: clean pending indirect buffs during reset Sukadev Bhattiprolu
2021-06-24  4:13 ` [PATCH net 4/7] ibmvnic: account for bufs already saved in indir_buf Sukadev Bhattiprolu
2021-06-24  4:13 ` [PATCH net 5/7] ibmvnic: set ltb->buff to NULL after freeing Sukadev Bhattiprolu
2021-06-24  4:13 ` [PATCH net 6/7] ibmvnic: free tx_pool if tso_pool alloc fails Sukadev Bhattiprolu
2021-06-24  4:13 ` [PATCH net 7/7] ibmvnic: parenthesize a check Sukadev Bhattiprolu
2021-06-24  5:50   ` Lijun Pan
2021-06-24 18:30 ` [PATCH net 0/7] ibmvnic: Assorted bug fixes patchwork-bot+netdevbpf

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=YNStvWD3gd3E5fgv@us.ibm.com \
    --to=sukadev@linux.ibm.com \
    --cc=brking@linux.ibm.com \
    --cc=cforno12@linux.ibm.com \
    --cc=drt@linux.ibm.com \
    --cc=lijunp213@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=ricklind@linux.ibm.com \
    /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.