[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <514860B1.2070509@citrix.com>
Date: Tue, 19 Mar 2013 13:57:21 +0100
From: Roger Pau Monné <roger.pau@...rix.com>
To: Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
CC: "xen-devel@...ts.xen.org" <xen-devel@...ts.xen.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 3/5] xen-blkfront: switch from llist to list
On 19/03/13 13:51, Konrad Rzeszutek Wilk wrote:
> On Mon, Mar 18, 2013 at 05:49:34PM +0100, Roger Pau Monne wrote:
>> Replace the use of llist with list.
>>
>> llist_for_each_entry_safe can trigger a bug in GCC 4.1, so it's best
>> to remove it and use a doubly linked list, which is used extensively
>> in the kernel already.
>>
>> Specifically this bug can be triggered by hot-unplugging a disk,
>> either by doing xm block-detach or by save/restore cycle.
>>
>> BUG: unable to handle kernel paging request at fffffffffffffff0
>> IP: [<ffffffffa0047223>] blkif_free+0x63/0x130 [xen_blkfront]
>> The crash call trace is:
>> ...
>> bad_area_nosemaphore+0x13/0x20
>> do_page_fault+0x25e/0x4b0
>> page_fault+0x25/0x30
>> ? blkif_free+0x63/0x130 [xen_blkfront]
>> blkfront_resume+0x46/0xa0 [xen_blkfront]
>> xenbus_dev_resume+0x6c/0x140
>> pm_op+0x192/0x1b0
>> device_resume+0x82/0x1e0
>> dpm_resume+0xc9/0x1a0
>> dpm_resume_end+0x15/0x30
>> do_suspend+0x117/0x1e0
>>
>> When drilling down to the assembler code, on newer GCC it does
>> .L29:
>> cmpq $-16, %r12 #, persistent_gnt check
>> je .L30 #, out of the loop
>> .L25:
>> ... code in the loop
>> testq %r13, %r13 # n
>> je .L29 #, back to the top of the loop
>> cmpq $-16, %r12 #, persistent_gnt check
>> movq 16(%r12), %r13 # <variable>.node.next, n
>> jne .L25 #, back to the top of the loop
>> .L30:
>>
>> While on GCC 4.1, it is:
>> L78:
>> ... code in the loop
>> testq %r13, %r13 # n
>> je .L78 #, back to the top of the loop
>> movq 16(%rbx), %r13 # <variable>.node.next, n
>> jmp .L78 #, back to the top of the loop
>>
>> Which basically means that the exit loop condition instead of
>> being:
>>
>> &(pos)->member != NULL;
>>
>> is:
>> ;
>>
>> which makes the loop unbound.
>>
>> Since we always manipulate the list while holding the io_lock, there's
>> no need for additional locking (llist used previously is safe to use
>> concurrently without additional locking).
>>
>> Should be backported to 3.8 stable.
>
> I get
> konrad@...nom:~/linux$ patch -p1 < /tmp/kk
> patching file drivers/block/xen-blkfront.c
> Hunk #1 FAILED at 44.
> Hunk #2 FAILED at 68.
> Hunk #3 FAILED at 105.
> Hunk #4 FAILED at 371.
> patch: **** malformed patch at line 172: ork)
>
>
> and idea why? Could you resent the patch as an attachment please?
Strange, I've used git send-email. I've attached it, but you can also
get them from:
http://xenbits.xen.org/gitweb/?p=people/royger/linux.git;a=shortlog;h=refs/heads/blk-for-3.9
View attachment "0003-xen-blkfront-switch-from-llist-to-list.patch" of type "text/plain" (6070 bytes)
Powered by blists - more mailing lists