linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alex Elder <elder@linaro.org>
To: Leon Romanovsky <leon@kernel.org>
Cc: davem@davemloft.net, kuba@kernel.org, bjorn.andersson@linaro.org,
	evgreen@chromium.org, cpratapa@codeaurora.org,
	subashab@codeaurora.org, elder@kernel.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next 0/4] net: ipa: kill IPA_VALIDATION
Date: Tue, 27 Jul 2021 08:40:42 -0500	[thread overview]
Message-ID: <07765bd2-eade-ee52-fa18-56f2e573461a@linaro.org> (raw)
In-Reply-To: <YQACaxKhxDFZSCF3@unreal>

On 7/27/21 7:56 AM, Leon Romanovsky wrote:
>> In any case I take your point.  I will now add to my task list
>> a review of these spots.  I'd like to be sure an error message
>> *is*  reported at an appropriate level up the chain of callers so
>> I can always identify the culprit in the a WARN_ON() fires (even
>> though it should never
>>   happen).  And in each case I'll evaluate
>> whether returning is better than not.
> You can, but users don't :). So if it is valid but error flow, that
> needs user awareness, simply print something to the dmesg with *_err()
> prints.

For some reason you seem to care about users.

I guess the WARN stack trace tells me where it comes from.
This would be an invalid error flow, and should never happen.

I'll still plan to review each of these again.

> BTW, I'm trying to untangle some of the flows in net/core/devlink.c
> and such if(WARN()) pattern is even harmful, because it is very hard to
> understand when that error is rare/non-exist/real.

That's what assert() is for, but we've already had that
discussion :)

					-Alex

  reply	other threads:[~2021-07-27 13:40 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-26 17:40 [PATCH net-next 0/4] net: ipa: kill IPA_VALIDATION Alex Elder
2021-07-26 17:40 ` [PATCH net-next 1/4] net: ipa: fix ipa_cmd_table_valid() Alex Elder
2021-07-26 17:40 ` [PATCH net-next 2/4] net: ipa: always validate filter and route tables Alex Elder
2021-07-26 17:40 ` [PATCH net-next 3/4] net: ipa: kill the remaining conditional validation code Alex Elder
2021-07-26 17:40 ` [PATCH net-next 4/4] net: ipa: use WARN_ON() rather than assertions Alex Elder
2021-07-26 21:52 ` [PATCH net-next 0/4] net: ipa: kill IPA_VALIDATION patchwork-bot+netdevbpf
2021-07-27 11:16 ` Leon Romanovsky
2021-07-27 12:34   ` Alex Elder
2021-07-27 12:56     ` Leon Romanovsky
2021-07-27 13:40       ` Alex Elder [this message]
2021-07-27 14:03         ` Leon Romanovsky

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=07765bd2-eade-ee52-fa18-56f2e573461a@linaro.org \
    --to=elder@linaro.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=cpratapa@codeaurora.org \
    --cc=davem@davemloft.net \
    --cc=elder@kernel.org \
    --cc=evgreen@chromium.org \
    --cc=kuba@kernel.org \
    --cc=leon@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=subashab@codeaurora.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).