All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@osdl.org>
Cc: cova@ferrara.linux.it, linux-kernel@vger.kernel.org,
	linux-usb-devel@lists.sourceforge.net, zaitcev@redhat.com,
	linux-scsi@vger.kernel.org
Subject: Re: 2.6.10-rc2-mm2 usb storage still oopses
Date: Mon, 29 Nov 2004 16:32:31 -0800	[thread overview]
Message-ID: <20041129163231.33affbde.akpm@osdl.org> (raw)
In-Reply-To: <20041118155542.324f56c7@lembas.zaitcev.lan>

Pete Zaitcev <zaitcev@redhat.com> wrote:
>
> On Fri, 19 Nov 2004 00:42:40 +0100, Fabio Coatti <cova@ferrara.linux.it> wrote:
> 
> > Nov 18 20:33:05 kefk kernel: sdb: assuming drive cache: write through
> > Nov 18 20:33:05 kefk kernel:  sdb: sdb1
> > Nov 18 20:33:05 kefk kernel:  sdb: sdb1
> > Nov 18 20:33:05 kefk kernel: kobject_register failed for sdb1 (-17)
> 
> This looks as if SCSI falls victim of the general problem which ub addresses
> with the following fragment:

Guys, is this problem still present in Linus's tree?  If so, is a fix for
2.6.10 looking feasible?

Thanks.

> --- linux-2.6.10-rc1/drivers/block/ub.c	2004-10-28 09:46:38.000000000 -0700
> +++ linux-2.6.10-rc1-ub/drivers/block/ub.c	2004-11-06 23:59:20.000000000 -0800
> @@ -267,6 +263,7 @@ struct ub_dev {
>  	int changed;			/* Media was changed */
>  	int removable;
>  	int readonly;
> +	int first_open;			/* Kludge. See ub_bd_open. */
>  	char name[8];
>  	struct usb_device *dev;
>  	struct usb_interface *intf;
> @@ -1428,6 +1420,26 @@ static int ub_bd_open(struct inode *inod
>  	sc->openc++;
>  	spin_unlock_irqrestore(&ub_lock, flags);
>  
> +	/*
> +	 * This is a workaround for a specific problem in our block layer.
> +	 * In 2.6.9, register_disk duplicates the code from rescan_partitions.
> +	 * However, if we do add_disk with a device which persistently reports
> +	 * a changed media, add_disk calls register_disk, which does do_open,
> +	 * which will call rescan_paritions for changed media. After that,
> +	 * register_disk attempts to do it all again and causes double kobject
> +	 * registration and a eventually an oops on module removal.
> +	 *
> +	 * The bottom line is, Al Viro says that we should not allow
> +	 * bdev->bd_invalidated to be set when doing add_disk no matter what.
> +	 */
> +	if (sc->first_open) {
> +		if (sc->changed) {
> +			sc->first_open = 0;
> +			rc = -ENOMEDIUM;
> +			goto err_open;
> +		}
> +	}
> +
>  	if (sc->removable || sc->readonly)
>  		check_disk_change(inode->i_bdev);
>  
> @@ -1467,6 +1479,8 @@ static int ub_bd_release(struct inode *i
>  
>  	spin_lock_irqsave(&ub_lock, flags);
>  	--sc->openc;
> +	if (sc->openc == 0)
> +		sc->first_open = 0;
>  	if (sc->openc == 0 && atomic_read(&sc->poison))
>  		ub_cleanup(sc);
>  	spin_unlock_irqrestore(&ub_lock, flags);
> @@ -1919,6 +1932,8 @@ static int ub_probe(struct usb_interface
>  	}
>  
>  	sc->removable = 1;		/* XXX Query this from the device */
> +	sc->changed = 1;		/* ub_revalidate clears only */
> +	sc->first_open = 1;
>  
>  	ub_revalidate(sc);
>  	/* This is pretty much a long term P3 */
> 
> This feels kludgy, but my excuse is "James and Viro made me do it".
> I have an IRC log to prove it laying somewhere...
> 
> I'm adding the linux-scsi to cc: in case any comments are forthcoming.
> 
> -- Pete


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now. 
http://productguide.itmanagersjournal.com/
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

WARNING: multiple messages have this Message-ID (diff)
From: Andrew Morton <akpm@osdl.org>
To: Pete Zaitcev <zaitcev@redhat.com>
Cc: cova@ferrara.linux.it, linux-kernel@vger.kernel.org,
	linux-usb-devel@lists.sourceforge.net, zaitcev@redhat.com,
	linux-scsi@vger.kernel.org
Subject: Re: 2.6.10-rc2-mm2 usb storage still oopses
Date: Mon, 29 Nov 2004 16:32:31 -0800	[thread overview]
Message-ID: <20041129163231.33affbde.akpm@osdl.org> (raw)
In-Reply-To: <20041118155542.324f56c7@lembas.zaitcev.lan>

Pete Zaitcev <zaitcev@redhat.com> wrote:
>
> On Fri, 19 Nov 2004 00:42:40 +0100, Fabio Coatti <cova@ferrara.linux.it> wrote:
> 
> > Nov 18 20:33:05 kefk kernel: sdb: assuming drive cache: write through
> > Nov 18 20:33:05 kefk kernel:  sdb: sdb1
> > Nov 18 20:33:05 kefk kernel:  sdb: sdb1
> > Nov 18 20:33:05 kefk kernel: kobject_register failed for sdb1 (-17)
> 
> This looks as if SCSI falls victim of the general problem which ub addresses
> with the following fragment:

Guys, is this problem still present in Linus's tree?  If so, is a fix for
2.6.10 looking feasible?

Thanks.

> --- linux-2.6.10-rc1/drivers/block/ub.c	2004-10-28 09:46:38.000000000 -0700
> +++ linux-2.6.10-rc1-ub/drivers/block/ub.c	2004-11-06 23:59:20.000000000 -0800
> @@ -267,6 +263,7 @@ struct ub_dev {
>  	int changed;			/* Media was changed */
>  	int removable;
>  	int readonly;
> +	int first_open;			/* Kludge. See ub_bd_open. */
>  	char name[8];
>  	struct usb_device *dev;
>  	struct usb_interface *intf;
> @@ -1428,6 +1420,26 @@ static int ub_bd_open(struct inode *inod
>  	sc->openc++;
>  	spin_unlock_irqrestore(&ub_lock, flags);
>  
> +	/*
> +	 * This is a workaround for a specific problem in our block layer.
> +	 * In 2.6.9, register_disk duplicates the code from rescan_partitions.
> +	 * However, if we do add_disk with a device which persistently reports
> +	 * a changed media, add_disk calls register_disk, which does do_open,
> +	 * which will call rescan_paritions for changed media. After that,
> +	 * register_disk attempts to do it all again and causes double kobject
> +	 * registration and a eventually an oops on module removal.
> +	 *
> +	 * The bottom line is, Al Viro says that we should not allow
> +	 * bdev->bd_invalidated to be set when doing add_disk no matter what.
> +	 */
> +	if (sc->first_open) {
> +		if (sc->changed) {
> +			sc->first_open = 0;
> +			rc = -ENOMEDIUM;
> +			goto err_open;
> +		}
> +	}
> +
>  	if (sc->removable || sc->readonly)
>  		check_disk_change(inode->i_bdev);
>  
> @@ -1467,6 +1479,8 @@ static int ub_bd_release(struct inode *i
>  
>  	spin_lock_irqsave(&ub_lock, flags);
>  	--sc->openc;
> +	if (sc->openc == 0)
> +		sc->first_open = 0;
>  	if (sc->openc == 0 && atomic_read(&sc->poison))
>  		ub_cleanup(sc);
>  	spin_unlock_irqrestore(&ub_lock, flags);
> @@ -1919,6 +1932,8 @@ static int ub_probe(struct usb_interface
>  	}
>  
>  	sc->removable = 1;		/* XXX Query this from the device */
> +	sc->changed = 1;		/* ub_revalidate clears only */
> +	sc->first_open = 1;
>  
>  	ub_revalidate(sc);
>  	/* This is pretty much a long term P3 */
> 
> This feels kludgy, but my excuse is "James and Viro made me do it".
> I have an IRC log to prove it laying somewhere...
> 
> I'm adding the linux-scsi to cc: in case any comments are forthcoming.
> 
> -- Pete

  reply	other threads:[~2004-11-30  0:32 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-11-18 21:03 2.6.10-rc2-mm2 usb storage still oopses Fabio Coatti
2004-11-18 21:35 ` Andrew Morton
2004-11-18 21:58   ` Pete Zaitcev
2004-11-18 23:42     ` Fabio Coatti
2004-11-18 23:55       ` Pete Zaitcev
2004-11-18 23:55         ` Pete Zaitcev
2004-11-30  0:32         ` Andrew Morton [this message]
2004-11-30  0:32           ` Andrew Morton
2004-11-30  3:22           ` James Bottomley
2004-11-30  9:13             ` Fabio Coatti
2004-11-30  9:13               ` Fabio Coatti
2004-11-30 19:27             ` Fabio Coatti
2004-11-30 19:27               ` Fabio Coatti
2004-11-30 20:58               ` Alan Stern
2004-11-30 20:58                 ` [linux-usb-devel] " Alan Stern
2004-12-01  1:06                 ` Fabio Coatti
2004-12-01  1:06                   ` [linux-usb-devel] " Fabio Coatti
2004-11-19  2:41       ` Maneesh Soni
2004-11-19 16:47         ` 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=20041129163231.33affbde.akpm@osdl.org \
    --to=akpm@osdl.org \
    --cc=cova@ferrara.linux.it \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=linux-usb-devel@lists.sourceforge.net \
    --cc=zaitcev@redhat.com \
    /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.