All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Johannes Berg <johannes@sipsolutions.net>
Cc: linux-kernel@vger.kernel.org,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	Emmanuel Grumbach <emmanuel.grumbach@intel.com>,
	Luciano Coelho <luca@coelho.fi>, Kalle Valo <kvalo@adurom.com>,
	dri-devel@lists.freedesktop.org, linux-wireless@vger.kernel.org
Subject: Re: [RFC v2] device coredump: add new device coredump class
Date: Fri, 12 Sep 2014 09:40:45 -0700	[thread overview]
Message-ID: <20140912164045.GA27663@kroah.com> (raw)
In-Reply-To: <1410165487.7983.8.camel@jlt4.sipsolutions.net>

On Mon, Sep 08, 2014 at 10:38:07AM +0200, Johannes Berg wrote:
> On Fri, 2014-09-05 at 15:13 -0700, Greg Kroah-Hartman wrote:
> 
> > > +	/*
> > > +	 * this seems racy, but I don't see a notifier or such on
> > > +	 * a struct device to know when it goes away?
> > > +	 */
> > > +	if (devcd->failing_dev->kobj.sd)
> > > +		sysfs_delete_link(&devcd->failing_dev->kobj, &dev->kobj,
> > > +				  "dev_coredump");
> > 
> > What is this link?  It should "just go away" if this:
> > 
> > > +	put_device(devcd->failing_dev);
> > 
> > was the last put_device() call on the failing_dev, right?  So you
> > shouldn't need to make this call to sysfs_delete_link().
> 
> I looked at this again, and it's the other way around. This is the link
> that Daniel requested, from the original device to the one that's being
> freed. For whatever reason though, symlinks don't automatically go away
> when freed:
> 
> (with a test patch that makes "mac80211-hwsim" crash whenever we have
> radar)
> 
> # echo 1 > /sys/kernel/debug/ieee80211/phy0/hwsim/dfs_simulate_radar
> # ls /sys/class/devcoredump/
> devcd1
> # ls /sys/class/devcoredump/devcd1/
> data            failing_device/ power/          subsystem/      uevent
> # ls -l /sys/class/mac80211_hwsim/hwsim0/dev_coredump
> lrwxrwxrwx 1 root root 0 Sep  8 08:34 /sys/class/mac80211_hwsim/hwsim0/dev_coredump -> ../../devcoredump/devcd1
> # echo > /sys/class/devcoredump/devcd1/data
> # ls /sys/class/devcoredump/
> # ls -l /sys/class/mac80211_hwsim/hwsim0/dev_coredump
> lrwxrwxrwx 1 root root 0 Sep  8 08:34 /sys/class/mac80211_hwsim/hwsim0/dev_coredump -> ../../devcoredump/devcd1
> 
> (but the link is now dead)
> 
> Maybe I'm creating the links in the wrong way so they don't
> automatically get removed when the struct device is released?

No, you are correct, sorry for the noise, I thought the symlink was the
other way around for some reason.

greg k-h

WARNING: multiple messages have this Message-ID (diff)
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Johannes Berg <johannes@sipsolutions.net>
Cc: Emmanuel Grumbach <emmanuel.grumbach@intel.com>,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	Kalle Valo <kvalo@adurom.com>,
	linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
	dri-devel@lists.freedesktop.org, Luciano Coelho <luca@coelho.fi>
Subject: Re: [RFC v2] device coredump: add new device coredump class
Date: Fri, 12 Sep 2014 09:40:45 -0700	[thread overview]
Message-ID: <20140912164045.GA27663@kroah.com> (raw)
In-Reply-To: <1410165487.7983.8.camel@jlt4.sipsolutions.net>

On Mon, Sep 08, 2014 at 10:38:07AM +0200, Johannes Berg wrote:
> On Fri, 2014-09-05 at 15:13 -0700, Greg Kroah-Hartman wrote:
> 
> > > +	/*
> > > +	 * this seems racy, but I don't see a notifier or such on
> > > +	 * a struct device to know when it goes away?
> > > +	 */
> > > +	if (devcd->failing_dev->kobj.sd)
> > > +		sysfs_delete_link(&devcd->failing_dev->kobj, &dev->kobj,
> > > +				  "dev_coredump");
> > 
> > What is this link?  It should "just go away" if this:
> > 
> > > +	put_device(devcd->failing_dev);
> > 
> > was the last put_device() call on the failing_dev, right?  So you
> > shouldn't need to make this call to sysfs_delete_link().
> 
> I looked at this again, and it's the other way around. This is the link
> that Daniel requested, from the original device to the one that's being
> freed. For whatever reason though, symlinks don't automatically go away
> when freed:
> 
> (with a test patch that makes "mac80211-hwsim" crash whenever we have
> radar)
> 
> # echo 1 > /sys/kernel/debug/ieee80211/phy0/hwsim/dfs_simulate_radar
> # ls /sys/class/devcoredump/
> devcd1
> # ls /sys/class/devcoredump/devcd1/
> data            failing_device/ power/          subsystem/      uevent
> # ls -l /sys/class/mac80211_hwsim/hwsim0/dev_coredump
> lrwxrwxrwx 1 root root 0 Sep  8 08:34 /sys/class/mac80211_hwsim/hwsim0/dev_coredump -> ../../devcoredump/devcd1
> # echo > /sys/class/devcoredump/devcd1/data
> # ls /sys/class/devcoredump/
> # ls -l /sys/class/mac80211_hwsim/hwsim0/dev_coredump
> lrwxrwxrwx 1 root root 0 Sep  8 08:34 /sys/class/mac80211_hwsim/hwsim0/dev_coredump -> ../../devcoredump/devcd1
> 
> (but the link is now dead)
> 
> Maybe I'm creating the links in the wrong way so they don't
> automatically get removed when the struct device is released?

No, you are correct, sorry for the noise, I thought the symlink was the
other way around for some reason.

greg k-h

  reply	other threads:[~2014-09-12 16:40 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-05  8:50 [RFC v2] device coredump: add new device coredump class Johannes Berg
2014-09-05  9:40 ` Jonas Gorski
2014-09-05  9:56   ` Johannes Berg
2014-09-05  9:56     ` Johannes Berg
2014-09-05 22:13 ` Greg Kroah-Hartman
2014-09-05 22:13   ` Greg Kroah-Hartman
2014-09-07  9:37   ` Johannes Berg
2014-09-07  9:37     ` Johannes Berg
2014-09-08  8:38   ` Johannes Berg
2014-09-08  8:38     ` Johannes Berg
2014-09-12 16:40     ` Greg Kroah-Hartman [this message]
2014-09-12 16:40       ` Greg Kroah-Hartman
2014-09-07  7:42 ` Arik Nemtsov
2014-09-07  7:42   ` Arik Nemtsov
2014-09-08  8:36 ` Arend van Spriel
2014-09-08  8:36   ` Arend van Spriel

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=20140912164045.GA27663@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=daniel.vetter@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=emmanuel.grumbach@intel.com \
    --cc=johannes@sipsolutions.net \
    --cc=kvalo@adurom.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=luca@coelho.fi \
    /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.