Netdev List
 help / color / mirror / Atom feed
From: Antoine Tenart <atenart@kernel.org>
To: Jakub Kicinski <kuba@kernel.org>
Cc: davem@davemloft.net, pabeni@redhat.com, edumazet@google.com,
	netdev@vger.kernel.org, gregkh@linuxfoundation.org,
	mhocko@suse.com, stephen@networkplumber.org
Subject: Re: [RFC PATCH net-next 1/4] net-sysfs: remove rtnl_trylock from device attributes
Date: Thu, 16 Jan 2025 14:36:17 +0100	[thread overview]
Message-ID: <173703457791.6390.1011724914365700977@kwain> (raw)
In-Reply-To: <20250107090641.39d70828@kernel.org>

Hi Jakub,

Quoting Jakub Kicinski (2025-01-07 18:06:41)
> On Tue, 07 Jan 2025 17:30:03 +0100 Antoine Tenart wrote:
> > Quoting Jakub Kicinski (2025-01-02 23:36:47)
> > > My version, FWIW:
> > > https://github.com/kuba-moo/linux/commit/2724bb7275496a254b001fe06fe20ccc5addc9d2  
> > 
> > I might take a few of your changes in there, eg. I see you used an
> > interruptible lock. With this and the few minors comments this RFC got I
> > can prepare a new series.
> 
> Perfect.

While refreshing the series, especially after adding the dev_isalive()
check, I found out we actually do not need to drop the sysfs protection
and hold a reference to the net device during the whole rtnl locking
section. This is because after getting the rtnl lock and once we know
the net device dismantle hasn't started yet, we're sure dismantle won't
start (and the device won't be freed) until we give back the rtnl lock.

This makes the new helpers easier to use, does not require to expose
the kernfs node to users, making the code more contained; but the
locking order is not as perfect.

We would go from (version 1),

1. unlocking sysfs
2. locking rtnl
3. unlocking rtnl
4. locking sysfs

to (version 2),

1. unlocking sysfs
2. locking rtnl
3. locking sysfs
4. unlocking rtnl

This is actually fine because the "sysfs lock" isn't a lock but a
refcnt, with the only deadlock situation being when draining it.

Version 1: https://github.com/atenart/linux/commit/596c5d9895ccdb75057978abd6be1a42ee4b448e
Version 2: https://github.com/atenart/linux/commit/c6659bb26f564f1fd63d1c279616f57141e9f2bf

Thoughts? Apart from that question, either series is ready for
submission.

Thanks,
Antoine

  reply	other threads:[~2025-01-16 13:36 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-18 15:47 [RFC PATCH net-next 0/4] net-sysfs: remove rtnl_trylock/restart_syscall use Antoine Tenart
2023-10-18 15:47 ` [RFC PATCH net-next 1/4] net-sysfs: remove rtnl_trylock from device attributes Antoine Tenart
2023-10-18 16:49   ` Greg KH
2023-10-19  8:13     ` Antoine Tenart
2023-10-19 15:37       ` Greg KH
2023-10-18 18:13   ` Stephen Hemminger
2023-10-19  7:48     ` Antoine Tenart
2025-01-02 22:36   ` Jakub Kicinski
2025-01-03  8:41     ` Eric Dumazet
2025-01-07 16:30     ` Antoine Tenart
2025-01-07 17:06       ` Jakub Kicinski
2025-01-16 13:36         ` Antoine Tenart [this message]
2025-01-16 23:42           ` Jakub Kicinski
2025-01-16 23:43             ` Jakub Kicinski
2025-01-17  8:26               ` Antoine Tenart
2023-10-18 15:47 ` [RFC PATCH net-next 2/4] net-sysfs: move queue attribute groups outside the default groups Antoine Tenart
2023-10-18 15:47 ` [RFC PATCH net-next 3/4] net-sysfs: prevent uncleared queues from being re-added Antoine Tenart
2023-10-18 15:47 ` [RFC PATCH net-next 4/4] net-sysfs: remove rtnl_trylock from queue attributes Antoine Tenart
2023-10-18 15:57 ` [RFC PATCH net-next 0/4] net-sysfs: remove rtnl_trylock/restart_syscall use Stephen Hemminger
2023-10-18 16:34   ` Antoine Tenart
2023-10-18 18:15 ` Stephen Hemminger
2023-10-19  7:47   ` Antoine Tenart
2023-10-19 14:54     ` Stephen Hemminger

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=173703457791.6390.1011724914365700977@kwain \
    --to=atenart@kernel.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=kuba@kernel.org \
    --cc=mhocko@suse.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=stephen@networkplumber.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