Re: [PATCH] nbd driver for 2.5+: fix for module removal & new block

[Lou Langholtz]

[e-mail] 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!