Netdev List
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@linaro.org>
To: Sai Krishna Gajula <saikrishnag@marvell.com>
Cc: "davem@davemloft.net" <davem@davemloft.net>,
	"edumazet@google.com" <edumazet@google.com>,
	"kuba@kernel.org" <kuba@kernel.org>,
	"pabeni@redhat.com" <pabeni@redhat.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Sunil Kovvuri Goutham <sgoutham@marvell.com>,
	"maciej.fijalkowski@intel.com" <maciej.fijalkowski@intel.com>,
	Naveen Mamindlapalli <naveenm@marvell.com>
Subject: Re: [net PATCH v2] octeontx2-af: Move validation of ptp pointer before its usage
Date: Fri, 23 Jun 2023 14:44:01 +0300	[thread overview]
Message-ID: <3894dd38-377b-4691-907b-ec3d47fddffd@kadam.mountain> (raw)
In-Reply-To: <BY3PR18MB470788B4096D586DEB9A3B22A023A@BY3PR18MB4707.namprd18.prod.outlook.com>

On Fri, Jun 23, 2023 at 11:28:19AM +0000, Sai Krishna Gajula wrote:
> > This probe function is super weird how it returns success on the failure path.
> > One concern, I had initially was that if anything returns -EPROBE_DEFER then
> > we cannot recover.  That's not possible in the current code, but it makes me
> > itch...  But here is a different crash.
> > 
> 
> In few circumstances, the PTP device is probed before the AF device in
> the driver. In such instance, -EPROBE_DEFER is used.
> -- EDEFER_PROBE is useful when probe order changes. Ex: AF driver probes before PTP.
> 

You're describing how -EPROBE_DEFER is *supposed* to work.  But that's
not what this driver does.

If the AF driver is probed before the PTP driver then ptp_probe() should
return -EPROBE_DEFER and that would allow the kernel to automatically
retry ptp_probe() later.  But instead of that, ptp_probe() returns
success.  So I guess the user would have to manually rmmod it and insmod
it again?  So, what I'm saying I don't understand why we can't do this
in the normal way.

The other thing I'm saying is that the weird return success on error
stuff hasn't been tested or we would have discovered the crash through
testing.

regards,
dan carpenter


  reply	other threads:[~2023-06-23 11:44 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-09 11:58 [net PATCH v2] octeontx2-af: Move validation of ptp pointer before its usage Sai Krishna
2023-06-09 13:18 ` Dan Carpenter
2023-06-23 11:28   ` Sai Krishna Gajula
2023-06-23 11:44     ` Dan Carpenter [this message]
2023-06-30  5:19       ` Sai Krishna Gajula
2023-06-30  5:45         ` Dan Carpenter
2023-06-30  6:46           ` Sunil Kovvuri

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=3894dd38-377b-4691-907b-ec3d47fddffd@kadam.mountain \
    --to=dan.carpenter@linaro.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maciej.fijalkowski@intel.com \
    --cc=naveenm@marvell.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=saikrishnag@marvell.com \
    --cc=sgoutham@marvell.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox