From: Lou Langholtz <ldl@aros.net>
To: viro@parcelfarce.linux.theplanet.co.uk
Cc: linux-kernel@vger.kernel.org, Andrew Morton <akpm@digeo.com>,
Pavel Machek <pavel@ucw.cz>,
Steven Whitehouse <steve@chygwyn.com>
Subject: Re: [PATCH] nbd driver for 2.5+: fix for module removal & new block device layer
Date: Sat, 21 Jun 2003 17:16:10 -0600 [thread overview]
Message-ID: <3EF4E73A.4070108@aros.net> (raw)
In-Reply-To: <20030621225500.GL6754@parcelfarce.linux.theplanet.co.uk>
viro@parcelfarce.linux.theplanet.co.uk wrote:
>On Sat, Jun 21, 2003 at 03:48:56PM -0600, Lou Langholtz wrote:
>
>
>>This patch prevents memory corruption from "rmmod nbd" with the existing
>>2.5.72 (and earlier) nbd driver. It does this by updating the nbd driver
>>to the new block layer requirement that every disk has its own
>>request_queue structure. This is the first of a series of patchlets
>>designed to break down the essential changes I proposed in my last
>>"enormous" patch. Note that another patchlet will make the whole
>>allocation of per nbd_device memory be dynamic (rather than staticly
>>tied to MAX_NBD). Please try out this patch and let me know how nbd is
>>working for you before versus after. With any luck, some of these
>>smaller patch breakdowns can actually see there way into new kernel
>>releases. Thanks.
>>
>>
> a) you don't have to have queue per device. It often does make
>sense (for nbd it's almost certainly a win), but it's not required.
>
Unfortunately the way the sysfs code is implemented for struct gendisk
it is actually the case that every gendisk has to have its own
request_queue or else dcache.c BUG's and memory gets corrupted when you
put and release this disks:
Jun 21 12:59:07 cyprus kernel: nbd: registered device at major 43
Jun 21 12:59:25 cyprus kernel: ------------[ cut here ]------------
Jun 21 12:59:25 cyprus kernel: kernel BUG at include/linux/dcache.h:273!
Jun 21 12:59:25 cyprus kernel: invalid operand: 0000 [#1]
Jun 21 12:59:25 cyprus kernel: CPU: 0
Jun 21 12:59:25 cyprus kernel: EIP: 0060:[<c017849d>] Tainted: GF
Jun 21 12:59:25 cyprus kernel: EFLAGS: 00210246
Jun 21 12:59:25 cyprus kernel: EIP is at sysfs_remove_dir+0x1d/0x13f
Jun 21 12:59:25 cyprus kernel: eax: 00000000 ebx: e0947ac4 ecx:
c026b6e0 edx: 00000000
Jun 21 12:59:25 cyprus kernel: esi: 00000078 edi: db10b900 ebp:
d1f2fee0 esp: d1f2fec8
Jun 21 12:59:25 cyprus kernel: ds: 007b es: 007b ss: 0068
Jun 21 12:59:25 cyprus kernel: Process rmmod (pid: 3794,
threadinfo=d1f2e000 task=cf2bb300)
Jun 21 12:59:25 cyprus kernel: Stack: d4a45d00 dfda21c0 dfda21e8
e0947ac4 00000078 d0c10344 d1f2fef8 c0205873
Jun 21 12:59:25 cyprus kernel: e0947ac4 c0450280 e0947ac4
e0947ac4 d1f2ff08 c02058b4 e0947ac4 d0c10280
Jun 21 12:59:25 cyprus kernel: d1f2ff18 c02673ee e0947ac4
d0c10280 d1f2ff2c c026b224 d0c10280 d0c10280
Jun 21 12:59:25 cyprus kernel: Call Trace:
Jun 21 12:59:25 cyprus kernel: [<e0947ac4>] nbd_queue+0x44/0x180 [nbd]
Jun 21 12:59:25 cyprus kernel: [<c0205873>] kobject_del+0x43/0x70
Jun 21 12:59:25 cyprus kernel: [<e0947ac4>] nbd_queue+0x44/0x180 [nbd]
Jun 21 12:59:25 cyprus last message repeated 2 times
Jun 21 12:59:25 cyprus kernel: [<c02058b4>] kobject_unregister+0x14/0x20
Jun 21 12:59:25 cyprus kernel: [<e0947ac4>] nbd_queue+0x44/0x180 [nbd]
Jun 21 12:59:25 cyprus kernel: [<c02673ee>] elv_unregister_queue+0x1e/0x40
Jun 21 12:59:25 cyprus kernel: [<e0947ac4>] nbd_queue+0x44/0x180 [nbd]
Jun 21 12:59:25 cyprus kernel: [<c026b224>] unlink_gendisk+0x14/0x40
Jun 21 12:59:25 cyprus kernel: [<c0177301>] del_gendisk+0x61/0xe0
Jun 21 12:59:25 cyprus kernel: [<e0945a5c>] nbd_dev+0x1dc/0x2200 [nbd]
Jun 21 12:59:25 cyprus kernel: [<e0944c5d>] +0x1d/0x60 [nbd]
Jun 21 12:59:25 cyprus kernel: [<e0947c00>] +0x0/0x140 [nbd]
Jun 21 12:59:25 cyprus kernel: [<c012ce9e>] sys_delete_module+0x12e/0x170
Jun 21 12:59:25 cyprus kernel: [<e0947c00>] +0x0/0x140 [nbd]
Jun 21 12:59:25 cyprus kernel: [<c013e5c7>] sys_munmap+0x57/0x80
Jun 21 12:59:25 cyprus kernel: [<c010ac4d>] sysenter_past_esp+0x52/0x71
Jun 21 12:59:25 cyprus kernel:
Jun 21 12:59:25 cyprus kernel: Code: 0f 0b 11 01 3b fa 3a c0 ff 07 85 ff
0f 84 08 01 00 00 8b 47
> b) you definitely don't have to use separate queue locks. The
>thing will work fine with spinlock being shared and I doubt that there
>will be any noticable extra contention.
>
Probably not noticeable no.
> c) please, make allocation of queue dynamic _and_ separate from
>any other allocated objects.
>
>
Will do!
next prev parent reply other threads:[~2003-06-21 23:02 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2003-06-21 21:48 [PATCH] nbd driver for 2.5+: fix for module removal & new block device layer Lou Langholtz
2003-06-21 22:55 ` viro
2003-06-21 23:16 ` Lou Langholtz [this message]
2003-06-22 10:03 ` Jens Axboe
[not found] ` <20030621151818.081139fc.akpm@digeo.com>
2003-06-21 23:09 ` [PATCH] nbd driver 2.5+: fix for incorrect struct bio usage Lou Langholtz
2003-06-22 8:50 ` Jens Axboe
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=3EF4E73A.4070108@aros.net \
--to=ldl@aros.net \
--cc=akpm@digeo.com \
--cc=linux-kernel@vger.kernel.org \
--cc=pavel@ucw.cz \
--cc=steve@chygwyn.com \
--cc=viro@parcelfarce.linux.theplanet.co.uk \
/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.