All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <gregkh@suse.de>
To: Stefan Richter <stefanr@s5r6.in-berlin.de>
Cc: Kay Sievers <kay.sievers@vrfy.org>,
	linux-kernel@vger.kernel.org, Jay Fenlason <fenlason@redhat.com>
Subject: Re: post 2.6.28 regression: device_initialize() now sleeps, and may fail without recovery strategy
Date: Fri, 9 Jan 2009 12:56:33 -0800	[thread overview]
Message-ID: <20090109205633.GB19904@suse.de> (raw)
In-Reply-To: <496798FE.8030900@s5r6.in-berlin.de>

On Fri, Jan 09, 2009 at 07:35:42PM +0100, Stefan Richter wrote:
> >From commit 2831fe6f9cc4e16c103504ee09a47a084297c0f3, "driver core:
> create a private portion of struct device":
> 
>  void device_initialize(struct device *dev)
>  {
> +	dev->p = kzalloc(sizeof(*dev->p), GFP_KERNEL);
> +	if (!dev->p) {
> +		WARN_ON(1);
> +		return;
> +	}
> +	dev->p->device = dev;
>  	dev->kobj.kset = devices_kset;
>  	kobject_init(&dev->kobj, &device_ktype);
> 
> 
> First of all, this prevents initialization of struct device in atomic
> contexts, such as drivers/firewire/fw-device.c::fw_node_event.

Ick, sorry, I didn't think that any callers ever did this.

> This is a bug in current mainline.
> 
> We can fix the bug by changing firewire-core, but
>   a) it'd be more than a one-liner,
>   b) who knows which other subsystems are affected.

I agree.

I originally looked at changing this to be at device_add time, but I
think there are some code paths that do device_initialize and then do
some operations on the device before calling device_add.  But I could be
wrong, let me do some testing first before forcing you to make that big
change to the firewire core.

> Next, the above code is bogus.  In 2.6.28, device_initialize() could
> never fail and was thus safe to use as a void-valued function.
> 
> How does driver core handle dev->p == NULL in subsequent usages of dev now?

It dies a flaming horrible death, pretty much like the whole rest of the
system if allocating such a small ammount of memory is causing failures
:)

Give me a few hours to test here, your change might not be necessary...

thanks,

greg k-h

  parent reply	other threads:[~2009-01-09 20:59 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-01-09 18:35 post 2.6.28 regression: device_initialize() now sleeps, and may fail without recovery strategy Stefan Richter
2009-01-09 19:49 ` [PATCH post 2.6.28] firewire: core: fix sleep in atomic context due to driver core change Stefan Richter
2009-01-09 21:17   ` Alan Cox
2009-01-09 21:54     ` Greg KH
2009-01-09 22:28       ` [git pull] FireWire fix Stefan Richter
2009-01-09 20:56 ` Greg KH [this message]
2009-01-09 21:13   ` post 2.6.28 regression: device_initialize() now sleeps, and may fail without recovery strategy Stefan Richter
2009-01-09 21:20     ` Stefan Richter
2009-01-09 21:34       ` Greg KH
2009-01-09 21:30     ` Greg KH
2009-01-09 21:40       ` Stefan Richter
2009-01-09 21:24   ` Alan Cox

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=20090109205633.GB19904@suse.de \
    --to=gregkh@suse.de \
    --cc=fenlason@redhat.com \
    --cc=kay.sievers@vrfy.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stefanr@s5r6.in-berlin.de \
    /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.