From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jens Axboe Subject: Re: 2.6.27-rc5-mm1: rmmod ide-cd_mod: tried to init an initialized object, something is seriously wrong. Date: Mon, 8 Sep 2008 11:27:17 +0200 Message-ID: <20080908092717.GH20055@kernel.dk> References: <200809051528.54213.rjw@sisk.pl> <200809051544.35443.m.kozlowski@tuxland.pl> <200809051825.04829.m.kozlowski@tuxland.pl> <20080906213525V.fujita.tomonori@lab.ntt.co.jp> <20080908084312.GF20055@kernel.dk> Mime-Version: 1.0 Return-path: Content-Disposition: inline In-Reply-To: <20080908084312.GF20055-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org> Sender: kernel-testers-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-ID: Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: FUJITA Tomonori Cc: m.kozlowski-NWF1p15JEu3VItvQsEIGlw@public.gmane.org, rjw-KKrjLPT3xs0@public.gmane.org, akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org, bzolnier-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, kernel-testers-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-ide-u79uwXL29TY76Z2rM5mHXA@public.gmane.org On Mon, Sep 08 2008, Jens Axboe wrote: > On Sat, Sep 06 2008, FUJITA Tomonori wrote: > > On Fri, 5 Sep 2008 18:25:04 +0200 > > Mariusz Kozlowski wrote: > > > > > Hello, > > > > > > > > > Again 100% reproducible rmmod ide-cd_mod problem. Kernel is tainted because > > > > > > of earlier sysfs acpi problems similar (probably identical) to those reported > > > > > > by Li Zefan here http://marc.info/?l=linux-kernel&m=121921059026064&w=2 > > > > > > > > > > > > Steps to reproduce: unload ide-cd_mod > > > > > > > > > > > > kobject (dd9e4a7c): tried to init an initialized object, something is seriously wrong. > > > > > > Pid: 4734, comm: modprobe Tainted: G W 2.6.27-rc5-mm1 #1 > > > > > > [] kobject_init+0xc4/0xc9 > > > > > > [] ? _spin_unlock+0x27/0x3f > > > > > > [] ? sysfs_find_dirent+0x21/0x2b > > > > > > [] ? __sysfs_add_one+0x46/0x6d > > > > > > [] ? sysfs_add_one+0xf/0x44 > > > > > > [] ? sysfs_addrm_start+0x4d/0x90 > > > > > > [] ? sysfs_do_create_link+0x9a/0x14c > > > > > > [] kobject_init_and_add+0x14/0x30 > > > > > > [] ? sysfs_create_link+0x12/0x19 > > > > > > [] blk_register_filter+0x3b/0x46 > > > > > > [] ide_cd_probe+0x253/0x5a8 [ide_cd_mod] > > > > > > [] ? sysfs_addrm_start+0x17/0x90 > > > > > > [] ? sysfs_do_create_link+0x9a/0x14c > > > > > > [] ? sysfs_addrm_start+0x65/0x90 > > > > > > [] generic_ide_probe+0x1f/0x21 > > > > > > [] driver_probe_device+0x77/0x15b > > > > > > [] ? _spin_unlock_irqrestore+0x39/0x60 > > > > > > [] __driver_attach+0x60/0x62 > > > > > > [] bus_for_each_dev+0x44/0x62 > > > > > > [] ? generic_ide_remove+0x0/0x1e > > > > > > [] driver_attach+0x19/0x1b > > > > > > [] ? __driver_attach+0x0/0x62 > > > > > > [] bus_add_driver+0x1ab/0x213 > > > > > > [] ? generic_ide_remove+0x0/0x1e > > > > > > [] driver_register+0x4f/0x118 > > > > > > [] ? ide_cdrom_init+0x0/0xf [ide_cd_mod] > > > > > > [] ide_cdrom_init+0xd/0xf [ide_cd_mod] > > > > > > [] do_one_initcall+0x24/0x12f > > > > > > [] ? mutex_unlock+0x8/0xa > > > > > > [] sys_init_module+0xa5/0x1c1 > > > > > > [] ? sys_read+0x3d/0x64 > > > > > > [] sysenter_do_call+0x12/0x35 > > > > > > [] ? __set_special_pids+0x43/0x71 > > > > > > > > > > > > First time I modprobe/rmmod ide-cd_mod the system works but quickly gets unstable. > > > > > > Second modprobe/rmmod is 100% fatal. Memory gets corruped seriously I guess. > > > > > > pcspeaker beeps all the time, kernel throws dumps on the screen until > > > > > > its really dead, sadly blinking 'leds of panic' ;) > > > > > > > > > > Can you please verify if that happens with the current mainline? > > > > > > > > Oops. How come I didn't find it earlier? hmm... > > > > > > It's relatively new, that's why :) And this is the culprit: > > > > > > abf5439370491dd6fbb4fe1a7939680d2a9bc9d4 is first bad commit > > > commit abf5439370491dd6fbb4fe1a7939680d2a9bc9d4 > > > Author: FUJITA Tomonori > > > Date: Sat Aug 16 14:10:05 2008 +0900 > > > > > > block: move cmdfilter from gendisk to request_queue > > > > > > cmd_filter works only for the block layer SG_IO with SCSI block > > > devices. It breaks scsi/sg.c, bsg, and the block layer SG_IO with SCSI > > > character devices (such as st). We hit a kernel crash with them. > > > > > > The problem is that cmd_filter code accesses to gendisk (having struct > > > blk_scsi_cmd_filter) via inode->i_bdev->bd_disk. It works for only > > > SCSI block device files. With character device files, inode->i_bdev > > > leads you to struct cdev. inode->i_bdev->bd_disk->blk_scsi_cmd_filter > > > isn't safe. > > > > > > SCSI ULDs don't expose gendisk; they keep it private. bsg needs to be > > > independent on any protocols. We shouldn't change ULDs to expose their > > > gendisk. > > > > > > This patch moves struct blk_scsi_cmd_filter from gendisk to > > > request_queue, a common object, which eveyone can access to. > > > > > > The user interface doesn't change; users can change the filters via > > > /sys/block/. gendisk has a pointer to request_queue so the cmd_filter > > > code accesses to struct blk_scsi_cmd_filter. > > > > > > Signed-off-by: FUJITA Tomonori > > > Signed-off-by: Jens Axboe > > > > > > > This is current mainline: > > > > > > > > kobject (ddb049fc): tried to init an initialized object, something is seriously wrong. > > > > Pid: 4650, comm: modprobe Not tainted 2.6.27-rc5-00132-gb380b0d #8 > > > > [] kobject_init+0x6a/0x6c > > > > [] kobject_init_and_add+0x14/0x30 > > > > [] ? kobject_get+0x12/0x17 > > > > [] blk_register_filter+0x4b/0x5a > > > > [] ide_cd_probe+0x289/0x5ae [ide_cd_mod] > > > > [] ? sysfs_addrm_start+0x65/0x90 > > > > [] ? sysfs_do_create_link+0x9a/0x11c > > > > [] generic_ide_probe+0x1f/0x21 > > > > [] driver_probe_device+0x77/0x15b > > > > [] ? _spin_unlock_irqrestore+0x39/0x60 > > > > [] __driver_attach+0x60/0x62 > > > > [] bus_for_each_dev+0x44/0x62 > > > > [] ? generic_ide_remove+0x0/0x1e > > > > [] driver_attach+0x19/0x1b > > > > [] ? __driver_attach+0x0/0x62 > > > > [] bus_add_driver+0x1ae/0x216 > > > > [] ? generic_ide_remove+0x0/0x1e > > > > [] driver_register+0x4f/0x118 > > > > [] ide_cdrom_init+0xd/0xf [ide_cd_mod] > > > > [] do_one_initcall+0x2a/0x14c > > > > [] ? native_sched_clock+0x58/0xa1 > > > > [] ? ide_cdrom_init+0x0/0xf [ide_cd_mod] > > > > [] ? trace_hardirqs_on+0xb/0xd > > > > [] ? trace_hardirqs_on_caller+0xac/0x134 > > > > [] sys_init_module+0x7e/0x19f > > > > [] ? trace_hardirqs_on_caller+0xac/0x134 > > > > [] ? trace_hardirqs_on_thunk+0xc/0x10 > > > > [] sysenter_do_call+0x12/0x35 > > > > [] ? put_fs_struct+0x5/0x2e > > > > ide-cd uses multiple gendisks share one request_queue? > > > > Here's a patch for mainline. > > Hmm, I don't think that it does. There's a queue per drive in the old > IDE driver, so there should be a 1:1 relation between queues and gendisk > there. I think the problem here is due to the usage of kobject_init_and_add(). When we hit the add the second time, the ->state_initalised in the kojb is still 1. The below should fix it. The ->state_initalised stuff is a disaster imho, it should be shot and killed. diff --git a/block/blk-core.c b/block/blk-core.c index 6cb3c6d..820132b 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -495,6 +495,7 @@ struct request_queue *blk_alloc_queue_node(gfp_t gfp_mask, int node_id) INIT_LIST_HEAD(&q->timeout_list); kobject_init(&q->kobj, &blk_queue_ktype); + kobject_init(&q->cmd_filter.kobj, &rcf_ktype); mutex_init(&q->sysfs_lock); spin_lock_init(&q->__queue_lock); diff --git a/block/blk.h b/block/blk.h index eb13740..47d6b22 100644 --- a/block/blk.h +++ b/block/blk.h @@ -9,6 +9,7 @@ extern struct kmem_cache *blk_requestq_cachep; extern struct kobj_type blk_queue_ktype; +extern struct kobj_type rcf_ktype; void init_request_from_bio(struct request *req, struct bio *bio); void blk_rq_bio_prep(struct request_queue *q, struct request *rq, diff --git a/block/cmd-filter.c b/block/cmd-filter.c index da7f7a4..9556e85 100644 --- a/block/cmd-filter.c +++ b/block/cmd-filter.c @@ -201,7 +201,7 @@ static struct sysfs_ops rcf_sysfs_ops = { .store = rcf_attr_store, }; -static struct kobj_type rcf_ktype = { +struct kobj_type rcf_ktype = { .sysfs_ops = &rcf_sysfs_ops, .default_attrs = default_attrs, }; @@ -211,8 +211,7 @@ int blk_register_filter(struct gendisk *disk) int ret; struct blk_cmd_filter *filter = &disk->queue->cmd_filter; - ret = kobject_init_and_add(&filter->kobj, &rcf_ktype, - &disk_to_dev(disk)->kobj, + ret = kobject_add(&filter->kobj, &disk_to_dev(disk)->kobj, "%s", "cmd_filter"); if (ret < 0) return ret; -- Jens Axboe