Skip to content

Commit

Permalink
aoe: allocate unused request_queue for sysfs
Browse files Browse the repository at this point in the history
Andy Whitcroft reported an oops in aoe 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]

The request queue of an aoe device is not used but can be allocated in
code that does not sleep.

Bruno bisected this regression down to

  cd43e26

  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

Note that embedding a queue inside another object has always been
an illegal construct, since the queues are reference counted and
must persist until the last reference is dropped. So aoe was
always buggy in this respect (Jens).

Signed-off-by: Ed Cashin <[email protected]>
Cc: Andy Whitcroft <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
Cc: Bruno Premont <[email protected]>
Cc: Martin K. Petersen <[email protected]>
Cc: Andrew Morton <[email protected]>
Signed-off-by: Jens Axboe <[email protected]>
  • Loading branch information
ecashin authored and Jens Axboe committed Sep 9, 2009
1 parent e6890f6 commit 7135a71
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 4 deletions.
2 changes: 1 addition & 1 deletion drivers/block/aoe/aoe.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
12 changes: 9 additions & 3 deletions drivers/block/aoe/aoeblk.c
Original file line number Diff line number Diff line change
Expand Up @@ -264,9 +264,12 @@ aoeblk_gdalloc(void *vp)
goto err_disk;
}

blk_queue_make_request(&d->blkq, aoeblk_make_request);
if (bdi_init(&d->blkq.backing_dev_info))
d->blkq = blk_alloc_queue(GFP_KERNEL);
if (!d->blkq)
goto err_mempool;
blk_queue_make_request(d->blkq, aoeblk_make_request);
if (bdi_init(&d->blkq->backing_dev_info))
goto err_blkq;
spin_lock_irqsave(&d->lock, flags);
gd->major = AOE_MAJOR;
gd->first_minor = d->sysminor * AOE_PARTITIONS;
Expand All @@ -276,7 +279,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;
Expand All @@ -287,6 +290,9 @@ aoeblk_gdalloc(void *vp)
aoedisk_add_sysfs(d);
return;

err_blkq:
blk_cleanup_queue(d->blkq);
d->blkq = NULL;
err_mempool:
mempool_destroy(d->bufpool);
err_disk:
Expand Down
1 change: 1 addition & 0 deletions drivers/block/aoe/aoedev.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down

0 comments on commit 7135a71

Please sign in to comment.