linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alan Stern <stern@rowland.harvard.edu>
To: Vincent MAILHOL <mailhol.vincent@wanadoo.fr>
Cc: Andrew Lunn <andrew@lunn.ch>,
	linux-can@vger.kernel.org, Marc Kleine-Budde <mkl@pengutronix.de>,
	linux-kernel@vger.kernel.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	netdev@vger.kernel.org, linux-usb@vger.kernel.org,
	Saeed Mahameed <saeed@kernel.org>, Jiri Pirko <jiri@nvidia.com>,
	Lukas Magel <lukas.magel@posteo.net>
Subject: Re: [PATCH v4 2/6] can: etas_es58x: add devlink support
Date: Sun, 27 Nov 2022 10:36:15 -0500	[thread overview]
Message-ID: <Y4OD70GD4KnoRk0k@rowland.harvard.edu> (raw)
In-Reply-To: <CAMZ6RqK6AQVsRufw5Jr5aKpPQcy+05jq3TjrKqbaqk7NVgK+_Q@mail.gmail.com>

On Sun, Nov 27, 2022 at 02:10:32PM +0900, Vincent MAILHOL wrote:
> > Should devlink_free() be after usb_set_inftdata()?
> 
> A look at
>   $ git grep -W "usb_set_intfdata(.*NULL)"
> 
> shows that the two patterns (freeing before or after
> usb_set_intfdata()) coexist.
> 
> You are raising an important question here. usb_set_intfdata() does
> not have documentation that freeing before it is risky. And the
> documentation of usb_driver::disconnect says that:
>   "@disconnect: Called when the interface is no longer accessible,
>    usually because its device has been (or is being) disconnected
>    or the driver module is being unloaded."
>   Ref: https://elixir.bootlin.com/linux/v6.1-rc6/source/include/linux/usb.h#L1130
> 
> So the interface no longer being accessible makes me assume that the
> order does not matter. If it indeed matters, then this is a foot gun
> and there is some clean-up work waiting for us on many drivers.
> 
> @Greg, any thoughts on whether or not the order of usb_set_intfdata()
> and resource freeing matters or not?

In fact, drivers don't have to call usb_set_intfdata(NULL) at all; the 
USB core does it for them after the ->disconnect() callback returns.

But if a driver does make the call, it should be careful to ensure that 
the call happens _after_ the driver is finished using the interface-data 
pointer.  For example, after all outstanding URBs have completed, if the
completion handlers will need to call usb_get_intfdata().

Remember, the interface-data pointer is owned by the driver.  Nothing 
else in the kernel uses it.  So the driver merely has to be careful not 
to clear the pointer while it is still using it.

Alan Stern

  reply	other threads:[~2022-11-27 15:36 UTC|newest]

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20221104073659.414147-1-mailhol.vincent@wanadoo.fr>
2022-11-04 17:16 ` [PATCH v2 0/3] can: etas_es58x: report firmware version Vincent Mailhol
2022-11-04 17:16   ` [PATCH v2 1/3] USB: core: export usb_cache_string() Vincent Mailhol
2022-11-05  8:22     ` Greg Kroah-Hartman
2022-11-04 17:16   ` [PATCH v2 2/3] can: etas_es58x: use usb_cache_string() to retrieve the product info string Vincent Mailhol
2022-11-05  8:21     ` Greg Kroah-Hartman
2022-11-04 17:16   ` [PATCH v2 3/3] can: etas_es58x: report the firmware version through ethtool Vincent Mailhol
2022-11-05  8:23     ` Greg Kroah-Hartman
2022-11-05  9:27       ` Vincent MAILHOL
2022-11-05 17:21         ` Vincent MAILHOL
2022-11-05 17:38           ` Greg Kroah-Hartman
2022-11-06  0:45             ` Alan Stern
2022-11-06  5:34               ` Vincent MAILHOL
2022-11-06 11:21               ` Greg Kroah-Hartman
2022-11-06 12:47                 ` Vincent MAILHOL
2022-11-06 14:22                   ` Greg Kroah-Hartman
2022-11-06 14:44                     ` Vincent MAILHOL
2022-11-06 16:02                       ` Greg Kroah-Hartman
2022-11-12 15:40                         ` Vincent MAILHOL
2022-11-06 15:18                   ` Alan Stern
2022-11-13  4:01 ` [PATCH v3 0/3] can: etas_es58x: report firmware, bootloader and hardware version Vincent Mailhol
2022-11-13  4:01   ` [PATCH v3 1/3] USB: core: export usb_cache_string() Vincent Mailhol
2022-11-22 16:00     ` Greg Kroah-Hartman
2022-11-13  4:01   ` [PATCH v3 2/3] can: etas_es58x: export firmware, bootloader and hardware versions in sysfs Vincent Mailhol
2022-11-15 22:47     ` Saeed Mahameed
2022-11-16  0:36       ` Vincent MAILHOL
2022-11-16 23:20         ` Saeed Mahameed
2022-11-13  4:01   ` [PATCH v3 3/3] can: etas_es58x: report firmware-version through ethtool Vincent Mailhol
2022-11-13 16:48   ` [PATCH v3 0/3] can: etas_es58x: report firmware, bootloader and hardware version Andrew Lunn
2022-11-14 16:49     ` Vincent MAILHOL
2022-11-14 17:08       ` Andrew Lunn
2022-11-26 16:22 ` [PATCH v4 0/6] " Vincent Mailhol
2022-11-26 16:22   ` [PATCH v4 1/6] USB: core: export usb_cache_string() Vincent Mailhol
2022-11-26 16:22   ` [PATCH v4 2/6] can: etas_es58x: add devlink support Vincent Mailhol
2022-11-26 16:51     ` Andrew Lunn
2022-11-27  5:10       ` Vincent MAILHOL
2022-11-27 15:36         ` Alan Stern [this message]
2022-11-28  1:34           ` Vincent MAILHOL
2022-11-28  5:32             ` Vincent MAILHOL
2022-11-28 15:50               ` Alan Stern
2022-11-28 23:30                 ` Vincent MAILHOL
2022-11-28 13:42             ` Andrew Lunn
2022-11-28 14:29               ` Vincent MAILHOL
2022-11-26 16:22   ` [PATCH v4 3/6] can: etas_es58x: export product information through devlink_ops::info_get() Vincent Mailhol
2022-11-26 17:16     ` Andrew Lunn
2022-11-27  3:42       ` Vincent MAILHOL
2022-11-27  4:31         ` Vincent MAILHOL
2022-11-27 15:07         ` Andrew Lunn
2022-11-28  1:21           ` Vincent MAILHOL
2022-11-28 13:43             ` Andrew Lunn
2022-11-28 13:47     ` Andrew Lunn
2022-11-28 14:43       ` Vincent MAILHOL
2022-11-28 22:27         ` Jakub Kicinski
2022-11-28 23:17           ` Vincent MAILHOL
2022-11-26 16:22   ` [PATCH v4 4/6] can: etas_es58x: remove es58x_get_product_info() Vincent Mailhol
2022-11-28 13:44     ` Andrew Lunn
2022-11-28 14:36       ` Vincent MAILHOL
2022-11-26 16:22   ` [PATCH v4 5/6] can: etas_es58x: report the firmware version through ethtool Vincent Mailhol
2022-11-28 13:44     ` Andrew Lunn
2022-11-28 22:29     ` Jakub Kicinski
2022-11-29 17:12       ` Vincent MAILHOL
2022-11-30  2:31         ` Jakub Kicinski
2022-11-26 16:22   ` [PATCH v4 6/6] Documentation: devlink: add devlink documentation for the etas_es58x driver Vincent Mailhol
2022-11-30 17:46 ` [PATCH v5 0/7] can: etas_es58x: report firmware, bootloader and hardware version Vincent Mailhol
2022-11-30 17:46   ` [PATCH v5 1/7] can: etas_es58x: add devlink support Vincent Mailhol
2022-11-30 17:46   ` [PATCH v5 2/7] can: etas_es58x: add devlink port support Vincent Mailhol
2022-11-30 17:46   ` [PATCH v5 3/7] USB: core: export usb_cache_string() Vincent Mailhol
2022-11-30 17:46   ` [PATCH v5 4/7] net: devlink: add DEVLINK_INFO_VERSION_GENERIC_FW_BOOTLOADER Vincent Mailhol
2022-11-30 17:46   ` [PATCH v5 5/7] can: etas_es58x: export product information through devlink_ops::info_get() Vincent Mailhol
2022-11-30 17:46   ` [PATCH v5 6/7] can: etas_es58x: remove es58x_get_product_info() Vincent Mailhol
2022-11-30 17:46   ` [PATCH v5 7/7] Documentation: devlink: add devlink documentation for the etas_es58x driver Vincent Mailhol
2022-12-02 12:27     ` Marc Kleine-Budde
2022-12-02 13:15       ` Vincent MAILHOL

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=Y4OD70GD4KnoRk0k@rowland.harvard.edu \
    --to=stern@rowland.harvard.edu \
    --cc=andrew@lunn.ch \
    --cc=gregkh@linuxfoundation.org \
    --cc=jiri@nvidia.com \
    --cc=linux-can@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=lukas.magel@posteo.net \
    --cc=mailhol.vincent@wanadoo.fr \
    --cc=mkl@pengutronix.de \
    --cc=netdev@vger.kernel.org \
    --cc=saeed@kernel.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).