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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox