[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20090902125540.9fc9d582.akpm@linux-foundation.org>
Date: Wed, 2 Sep 2009 12:55:40 -0700
From: Andrew Morton <akpm@...ux-foundation.org>
To: Ed Cashin <ecashin@...aid.com>
Cc: bonbons@...ux-vserver.org, ecashin@...aid.com, apw@...onical.com,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 0/1] aoe: ensure we initialise the request_queue
correctly
On Tue, 1 Sep 2009 15:15:20 -0400
Ed Cashin <ecashin@...aid.com> wrote:
> The patch looks fine to me.
I translated that into Acked-by:.
> I don't think it should go in my aoe tree for linux-next, since the
> patch addresses a regression.
>
> Based on the series file in mmotm, I don't think this patch is in mm
> at the moment. Andrew Morton, do you think this patch should go
> through your mm tree?
I have it now. Please check that the below is the right patch and that
my cobbled-together changelog makes sense (if not, please send
replacement changelog and/or patch).
From: Andy Whitcroft <apw@...onical.com>
When using the aoe driver we see an oops triggered by use of an
incorrectly initialised request_queue object:
[ 2645.959090] kobject '<NULL>' (ffff880059ca22c0): tried to add
an uninitialized object, something is seriously wrong.
[ 2645.959104] Pid: 6, comm: events/0 Not tainted 2.6.31-5-generic #24-Ubuntu
[ 2645.959107] Call Trace:
[ 2645.959139] [<ffffffff8126ca2f>] kobject_add+0x5f/0x70
[ 2645.959151] [<ffffffff8125b4ab>] blk_register_queue+0x8b/0xf0
[ 2645.959155] [<ffffffff8126043f>] add_disk+0x8f/0x160
[ 2645.959161] [<ffffffffa01673c4>] aoeblk_gdalloc+0x164/0x1c0 [aoe]
It seems this driver mearly embeds a request_queue object in its main
control structure and does not attempt to initialise it. Pull this out
and use blk_init_queue/blk_cleanup_queue to handle initialisation and
teardown of this object.
Bruno bisected this regression down to
cd43e26f071524647e660706b784ebcbefbd2e44
block: Expose stacked device queues in sysfs
"This seems to generate /sys/block/$device/queue and its contents for
everyone who is using queues, not just for those queues that have a
non-NULL queue->request_fn."
Addresses http://bugs.launchpad.net/bugs/410198
Addresses http://bugzilla.kernel.org/show_bug.cgi?id=13942
Signed-off-by: Andy Whitcroft <apw@...onical.com>
Acked-by: Ed Cashin <ecashin@...aid.com>
Cc: "Rafael J. Wysocki" <rjw@...k.pl>
Cc: Bruno Premont <bonbons@...ux-vserver.org>
Cc: Martin K. Petersen <martin.petersen@...cle.com>
Cc: Jens Axboe <jens.axboe@...cle.com>
Signed-off-by: Andrew Morton <akpm@...ux-foundation.org>
---
drivers/block/aoe/aoe.h | 2 +-
drivers/block/aoe/aoeblk.c | 6 +++---
drivers/block/aoe/aoedev.c | 11 ++++++++++-
3 files changed, 14 insertions(+), 5 deletions(-)
diff -puN drivers/block/aoe/aoe.h~aoe-ensure-we-initialise-the-request_queue-correctly drivers/block/aoe/aoe.h
--- a/drivers/block/aoe/aoe.h~aoe-ensure-we-initialise-the-request_queue-correctly
+++ a/drivers/block/aoe/aoe.h
@@ -155,7 +155,7 @@ struct aoedev {
u16 fw_ver; /* version of blade's firmware */
struct work_struct work;/* disk create work struct */
struct gendisk *gd;
- struct request_queue blkq;
+ struct request_queue *blkq;
struct hd_geometry geo;
sector_t ssize;
struct timer_list timer;
diff -puN drivers/block/aoe/aoeblk.c~aoe-ensure-we-initialise-the-request_queue-correctly drivers/block/aoe/aoeblk.c
--- a/drivers/block/aoe/aoeblk.c~aoe-ensure-we-initialise-the-request_queue-correctly
+++ a/drivers/block/aoe/aoeblk.c
@@ -264,8 +264,8 @@ aoeblk_gdalloc(void *vp)
goto err_disk;
}
- blk_queue_make_request(&d->blkq, aoeblk_make_request);
- if (bdi_init(&d->blkq.backing_dev_info))
+ blk_queue_make_request(d->blkq, aoeblk_make_request);
+ if (bdi_init(&d->blkq->backing_dev_info))
goto err_mempool;
spin_lock_irqsave(&d->lock, flags);
gd->major = AOE_MAJOR;
@@ -276,7 +276,7 @@ aoeblk_gdalloc(void *vp)
snprintf(gd->disk_name, sizeof gd->disk_name, "etherd/e%ld.%d",
d->aoemajor, d->aoeminor);
- gd->queue = &d->blkq;
+ gd->queue = d->blkq;
d->gd = gd;
d->flags &= ~DEVFL_GDALLOC;
d->flags |= DEVFL_UP;
diff -puN drivers/block/aoe/aoedev.c~aoe-ensure-we-initialise-the-request_queue-correctly drivers/block/aoe/aoedev.c
--- a/drivers/block/aoe/aoedev.c~aoe-ensure-we-initialise-the-request_queue-correctly
+++ a/drivers/block/aoe/aoedev.c
@@ -113,6 +113,7 @@ aoedev_freedev(struct aoedev *d)
if (d->bufpool)
mempool_destroy(d->bufpool);
skbpoolfree(d);
+ blk_cleanup_queue(d->blkq);
kfree(d);
}
@@ -202,6 +203,7 @@ aoedev_by_sysminor_m(ulong sysminor)
{
struct aoedev *d;
ulong flags;
+ struct request_queue *rq;
spin_lock_irqsave(&devlist_lock, flags);
@@ -210,9 +212,13 @@ aoedev_by_sysminor_m(ulong sysminor)
break;
if (d)
goto out;
+ rq = blk_init_queue(NULL, NULL);
+ if (!rq)
+ goto out;
d = kcalloc(1, sizeof *d, GFP_ATOMIC);
if (!d)
- goto out;
+ goto out_rq;
+ d->blkq = rq;
INIT_WORK(&d->work, aoecmd_sleepwork);
spin_lock_init(&d->lock);
skb_queue_head_init(&d->sendq);
@@ -234,6 +240,9 @@ aoedev_by_sysminor_m(ulong sysminor)
out:
spin_unlock_irqrestore(&devlist_lock, flags);
return d;
+ out_rq:
+ blk_cleanup_queue(rq);
+ goto out;
}
static void
_
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists