All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: Siddharth Gupta <sidgup@codeaurora.org>
Cc: bjorn.andersson@linaro.org, ohad@wizery.com,
	linux-remoteproc@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-msm@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, psodagud@codeaurora.org,
	stable@vger.kernel.org
Subject: Re: [PATCH v3 1/4] remoteproc: core: Move cdev add before device add
Date: Wed, 16 Jun 2021 07:58:48 +0200	[thread overview]
Message-ID: <YMmTGD6hAKbpGWMp@kroah.com> (raw)
In-Reply-To: <0a196786-f624-d9bb-8ef9-55c04ed57497@codeaurora.org>

On Tue, Jun 15, 2021 at 12:03:26PM -0700, Siddharth Gupta wrote:
> 
> On 6/14/2021 9:56 PM, Greg KH wrote:
> > On Mon, Jun 14, 2021 at 07:21:08PM -0700, Siddharth Gupta wrote:
> > > When cdev_add is called after device_add has been called there is no
> > > way for the userspace to know about the addition of a cdev as cdev_add
> > > itself doesn't trigger a uevent notification, or for the kernel to
> > > know about the change to devt. This results in two problems:
> > >   - mknod is never called for the cdev and hence no cdev appears on
> > >     devtmpfs.
> > >   - sysfs links to the new cdev are not established.
> > > 
> > > The cdev needs to be added and devt assigned before device_add() is
> > > called in order for the relevant sysfs and devtmpfs entries to be
> > > created and the uevent to be properly populated.
> > So this means no one ever ran this code on a system that used devtmpfs?
> > 
> > How was it ever tested?
> My testing was done with toybox + Android's ueventd ramdisk.
> As I mentioned in the discussion, the race became evident
> recently. I will make sure to test all such changes without
> systemd/ueventd in the future.

It isn't an issue of systemd/ueventd, those do not control /dev on a
normal system, that is what devtmpfs is for.

And devtmpfs nodes are only created if you create a struct device
somewhere with a proper major/minor, which you were not doing here, so
you must have had a static /dev on your test systems, right?

thanks,

greg k-h

WARNING: multiple messages have this Message-ID (diff)
From: Greg KH <gregkh@linuxfoundation.org>
To: Siddharth Gupta <sidgup@codeaurora.org>
Cc: bjorn.andersson@linaro.org, ohad@wizery.com,
	linux-remoteproc@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-msm@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, psodagud@codeaurora.org,
	stable@vger.kernel.org
Subject: Re: [PATCH v3 1/4] remoteproc: core: Move cdev add before device add
Date: Wed, 16 Jun 2021 07:58:48 +0200	[thread overview]
Message-ID: <YMmTGD6hAKbpGWMp@kroah.com> (raw)
In-Reply-To: <0a196786-f624-d9bb-8ef9-55c04ed57497@codeaurora.org>

On Tue, Jun 15, 2021 at 12:03:26PM -0700, Siddharth Gupta wrote:
> 
> On 6/14/2021 9:56 PM, Greg KH wrote:
> > On Mon, Jun 14, 2021 at 07:21:08PM -0700, Siddharth Gupta wrote:
> > > When cdev_add is called after device_add has been called there is no
> > > way for the userspace to know about the addition of a cdev as cdev_add
> > > itself doesn't trigger a uevent notification, or for the kernel to
> > > know about the change to devt. This results in two problems:
> > >   - mknod is never called for the cdev and hence no cdev appears on
> > >     devtmpfs.
> > >   - sysfs links to the new cdev are not established.
> > > 
> > > The cdev needs to be added and devt assigned before device_add() is
> > > called in order for the relevant sysfs and devtmpfs entries to be
> > > created and the uevent to be properly populated.
> > So this means no one ever ran this code on a system that used devtmpfs?
> > 
> > How was it ever tested?
> My testing was done with toybox + Android's ueventd ramdisk.
> As I mentioned in the discussion, the race became evident
> recently. I will make sure to test all such changes without
> systemd/ueventd in the future.

It isn't an issue of systemd/ueventd, those do not control /dev on a
normal system, that is what devtmpfs is for.

And devtmpfs nodes are only created if you create a struct device
somewhere with a proper major/minor, which you were not doing here, so
you must have had a static /dev on your test systems, right?

thanks,

greg k-h

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2021-06-16  5:58 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-15  2:21 [PATCH v3 0/4] remoteproc: core: Fixes for rproc cdev and add Siddharth Gupta
2021-06-15  2:21 ` [PATCH v3 1/4] remoteproc: core: Move cdev add before device add Siddharth Gupta
2021-06-15  4:54   ` Greg KH
2021-06-15  4:54     ` Greg KH
2021-06-15  4:56   ` Greg KH
2021-06-15  4:56     ` Greg KH
2021-06-15 19:03     ` Siddharth Gupta
2021-06-16  5:58       ` Greg KH [this message]
2021-06-16  5:58         ` Greg KH
2021-06-16 18:47         ` Siddharth Gupta
2021-06-23  7:27           ` Greg KH
2021-06-23  7:27             ` Greg KH
2021-06-23 18:56             ` Siddharth Gupta
2021-06-15  2:21 ` [PATCH v3 2/4] remoteproc: core: Move validate " Siddharth Gupta
2021-06-15  4:54   ` Greg KH
2021-06-15  4:54     ` Greg KH
2021-06-15  2:21 ` [PATCH v3 3/4] remoteproc: core: Fix cdev remove and rproc del Siddharth Gupta
2021-06-15  4:54   ` Greg KH
2021-06-15  4:54     ` Greg KH
2021-06-15  2:21 ` [PATCH v3 4/4] remoteproc: core: Cleanup device in case of failure Siddharth Gupta
2021-06-15  4:54   ` Greg KH
2021-06-15  4:54     ` Greg KH

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=YMmTGD6hAKbpGWMp@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-remoteproc@vger.kernel.org \
    --cc=ohad@wizery.com \
    --cc=psodagud@codeaurora.org \
    --cc=sidgup@codeaurora.org \
    --cc=stable@vger.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 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.