All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Jiri Pirko <jiri@resnulli.us>
Cc: Ido Schimmel <idosch@nvidia.com>,
	netdev@vger.kernel.org, pabeni@redhat.com, davem@davemloft.net,
	edumazet@google.com, moshe@nvidia.com
Subject: Re: [patch net-next] devlink: remove reload failed checks in params get/set callbacks
Date: Wed, 12 Jul 2023 12:21:03 -0700	[thread overview]
Message-ID: <20230712122103.4263c112@kernel.org> (raw)
In-Reply-To: <ZK7EyBcE7sFVvYvh@nanopsycho>

On Wed, 12 Jul 2023 17:20:40 +0200 Jiri Pirko wrote:
> >> Back then, it was a possible fix. Alternative way to fix this was to
> >> make sure drivers register/unregister params in the code where it is
> >> ensured that the data accessed by params callbacks are available.
> >> But that was problematic as the list of params wes static durint  
> >
> >s/wes/was/
> >s/durint/during/  
> 
> Maintainers, I will send v2 with these typos fixed tomorrow, if these
> are not any other comments.

Feel free to toss in

pw-bot: changes-requested

so we don't have to update the status manually.

The commit message would benefit from a rewrite, TBH I don't understand
half of it, specially:

  Alternative way to fix this was to make sure drivers
  register/unregister params in the code where it is ensured that 
  the data accessed by params callbacks are available.

Can't parse.

  list of params [was] static [during] devlink instance being
  registered.

You mean that list of params can't change after the instance was
registered?

  register/unregister params alongside with the data it touches

Meaning params for a sub-object are registered when the sub-object 
is registered? An example could help clarify the meaning.

> >> devlink instance being registered.
> >> 
> >> Eventually this limitation was lifted and also the alternative fix
> >> (which also fixed another issue) was done for mlxsw by
> >> commit 74cbc3c03c82 ("mlxsw: spectrum_acl_tcam: Move devlink param to TCAM code").
> >> 
> >> The checks are no longer relevant, each driver should make sure to
> >> register/unregister params alongside with the data it touches. Remove
> >> the checks.

  reply	other threads:[~2023-07-12 19:21 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-12 11:37 [patch net-next] devlink: remove reload failed checks in params get/set callbacks Jiri Pirko
2023-07-12 13:47 ` Ido Schimmel
2023-07-12 15:20   ` Jiri Pirko
2023-07-12 19:21     ` Jakub Kicinski [this message]
2023-07-13  9:00       ` Jiri Pirko
2023-07-13 15:55         ` Jakub Kicinski

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=20230712122103.4263c112@kernel.org \
    --to=kuba@kernel.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=idosch@nvidia.com \
    --cc=jiri@resnulli.us \
    --cc=moshe@nvidia.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.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.