Re: [PATCH 2/3] xen-blkfront: rm BUG_ON(info->feature_persistent) in blkif_free

2015-07-21 Thread Bob Liu


On 07/22/2015 12:43 PM, Bob Liu wrote:
> 
> On 07/21/2015 05:25 PM, Roger Pau Monné wrote:
>> El 21/07/15 a les 5.30, Bob Liu ha escrit:
>>> This BUG_ON() in blkif_free() is incorrect, because indirect page can be 
>>> added
>>> to list info->indirect_pages in blkif_completion() no matter 
>>> feature_persistent
>>> is true or false.
>>>
>>> Signed-off-by: Bob Liu 
>>
>> Acked-by: Roger Pau Monné 
>>
>> This was probably an oversight from when blkif_completion was changed to
>> check for gnttab_query_foreign_access. It should be backported to stable
>> trees.
>>
> 
> Sorry, this patch is buggy and I haven't figure out why.
> 
> general protection fault:  [#1] SMP 
> Modules linked in:
> CPU: 0 PID: 39 Comm: xenwatch Tainted: GW   
> 4.1.0-rc3-3-g718cf80-dirty #67
> Hardware name: Xen HVM domU, BIOS 4.5.0-rc 11/23/2014
> task: 880283f4eca0 ti: 880283fb4000 task.ti: 880283fb4000
> RIP: 0010:[]  [] blkif_free+0x162/0x5a9
> RSP: 0018:880283fb7c48  EFLAGS: 00010087
> RAX: dead00200200 RBX: 88014140 RCX: 
> RDX: dead00100100 RSI: dead00100100 RDI: 88028f418bb8
> RBP: 880283fb7ca8 R08: dead00200200 R09: 0001
> R10: 0001 R11:  R12: 8801414481c8
> R13: dead001000e0 R14: 8801414481b8 R15: ea00
> FS:  () GS:88028f40() knlGS:
> CS:  0010 DS:  ES:  CR0: 80050033
> CR2: 01582e08 CR3: 00013345b000 CR4: 001406f0
> Stack:
>  880023aa8420 0286 880283fb7cb7 880023aa8420
>  8800363fe240 81862c50 880283fb7ce8 880023aa8440
>  8187 880023aa8400 88014140 88014148
> Call Trace:
>  [] blkfront_remove+0x4c/0xff
>  [] xenbus_dev_remove+0x76/0xb0
>  [] __device_release_driver+0x84/0xf8
>  [] device_release_driver+0x1e/0x2b
>  [] bus_remove_device+0x12c/0x141
>  [] device_del+0x161/0x1e5
>  [] ? xenbus_thread+0x239/0x239
>  [] device_unregister+0x43/0x4f
>  [] xenbus_dev_changed+0x82/0x17f
>  [] ? xenbus_otherend_changed+0xf0/0xff
>  [] frontend_changed+0x43/0x48
>  [] xenwatch_thread+0xf9/0x127
>  [] ? add_wait_queue+0x44/0x44
>  [] kthread+0xcd/0xd5
>  [] ? alloc_pid+0xe8/0x492
>  [] ? kthread_freezable_should_stop+0x48/0x48
>  [] ret_from_fork+0x42/0x70
>  [] ? kthread_freezable_should_stop+0x48/0x48
> Code: 04 00 4c 8b 28 48 8d 78 e0 49 83 ed 20 eb 3d 48 8b 47 28 48 8b 57 20 48 
> be 00 01 10 00 00 00 ad de 49 b8 00 02 20 00 00 00 ad de <48> 89 42 08 48 89 
> 10 48 89 77 20 4c 89 47 28 31 f6 e8 26 7d cf 
> RIP  [] blkif_free+0x162/0x5a9
>  RSP 
> ---[ end trace 5321d7f1ef8414d0 ]---
> 

The right fix should be:

--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -1124,8 +1124,10 @@ static void blkif_completion(struct blk_shadow *s, 
struct blkfront_info *info,
 * Add the used indirect page back to the list 
of
 * available pages for indirect grefs.
 */
-   indirect_page = 
pfn_to_page(s->indirect_grants[i]->pfn);
-   list_add(&indirect_page->lru, 
&info->indirect_pages);
+   if (!info->feature_persistent) {
+   indirect_page = 
pfn_to_page(s->indirect_grants[i]->pfn);
+   list_add(&indirect_page->lru, 
&info->indirect_pages);
+   }
s->indirect_grants[i]->gref = GRANT_INVALID_REF;
list_add_tail(&s->indirect_grants[i]->node, 
&info->grants);
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/3] xen-blkfront: rm BUG_ON(info->feature_persistent) in blkif_free

2015-07-21 Thread Bob Liu

On 07/21/2015 05:25 PM, Roger Pau Monné wrote:
> El 21/07/15 a les 5.30, Bob Liu ha escrit:
>> This BUG_ON() in blkif_free() is incorrect, because indirect page can be 
>> added
>> to list info->indirect_pages in blkif_completion() no matter 
>> feature_persistent
>> is true or false.
>>
>> Signed-off-by: Bob Liu 
> 
> Acked-by: Roger Pau Monné 
> 
> This was probably an oversight from when blkif_completion was changed to
> check for gnttab_query_foreign_access. It should be backported to stable
> trees.
> 

Sorry, this patch is buggy and I haven't figure out why.

general protection fault:  [#1] SMP 
Modules linked in:
CPU: 0 PID: 39 Comm: xenwatch Tainted: GW   
4.1.0-rc3-3-g718cf80-dirty #67
Hardware name: Xen HVM domU, BIOS 4.5.0-rc 11/23/2014
task: 880283f4eca0 ti: 880283fb4000 task.ti: 880283fb4000
RIP: 0010:[]  [] blkif_free+0x162/0x5a9
RSP: 0018:880283fb7c48  EFLAGS: 00010087
RAX: dead00200200 RBX: 88014140 RCX: 
RDX: dead00100100 RSI: dead00100100 RDI: 88028f418bb8
RBP: 880283fb7ca8 R08: dead00200200 R09: 0001
R10: 0001 R11:  R12: 8801414481c8
R13: dead001000e0 R14: 8801414481b8 R15: ea00
FS:  () GS:88028f40() knlGS:
CS:  0010 DS:  ES:  CR0: 80050033
CR2: 01582e08 CR3: 00013345b000 CR4: 001406f0
Stack:
 880023aa8420 0286 880283fb7cb7 880023aa8420
 8800363fe240 81862c50 880283fb7ce8 880023aa8440
 8187 880023aa8400 88014140 88014148
Call Trace:
 [] blkfront_remove+0x4c/0xff
 [] xenbus_dev_remove+0x76/0xb0
 [] __device_release_driver+0x84/0xf8
 [] device_release_driver+0x1e/0x2b
 [] bus_remove_device+0x12c/0x141
 [] device_del+0x161/0x1e5
 [] ? xenbus_thread+0x239/0x239
 [] device_unregister+0x43/0x4f
 [] xenbus_dev_changed+0x82/0x17f
 [] ? xenbus_otherend_changed+0xf0/0xff
 [] frontend_changed+0x43/0x48
 [] xenwatch_thread+0xf9/0x127
 [] ? add_wait_queue+0x44/0x44
 [] kthread+0xcd/0xd5
 [] ? alloc_pid+0xe8/0x492
 [] ? kthread_freezable_should_stop+0x48/0x48
 [] ret_from_fork+0x42/0x70
 [] ? kthread_freezable_should_stop+0x48/0x48
Code: 04 00 4c 8b 28 48 8d 78 e0 49 83 ed 20 eb 3d 48 8b 47 28 48 8b 57 20 48 
be 00 01 10 00 00 00 ad de 49 b8 00 02 20 00 00 00 ad de <48> 89 42 08 48 89 10 
48 89 77 20 4c 89 47 28 31 f6 e8 26 7d cf 
RIP  [] blkif_free+0x162/0x5a9
 RSP 
---[ end trace 5321d7f1ef8414d0 ]---
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/3] xen-blkfront: rm BUG_ON(info->feature_persistent) in blkif_free

2015-07-21 Thread Roger Pau Monné
El 21/07/15 a les 5.30, Bob Liu ha escrit:
> This BUG_ON() in blkif_free() is incorrect, because indirect page can be added
> to list info->indirect_pages in blkif_completion() no matter 
> feature_persistent
> is true or false.
> 
> Signed-off-by: Bob Liu 

Acked-by: Roger Pau Monné 

This was probably an oversight from when blkif_completion was changed to
check for gnttab_query_foreign_access. It should be backported to stable
trees.

Roger.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 2/3] xen-blkfront: rm BUG_ON(info->feature_persistent) in blkif_free

2015-07-20 Thread Bob Liu
This BUG_ON() in blkif_free() is incorrect, because indirect page can be added
to list info->indirect_pages in blkif_completion() no matter feature_persistent
is true or false.

Signed-off-by: Bob Liu 
---
 drivers/block/xen-blkfront.c |1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index e266d17..c98fcd0 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -986,7 +986,6 @@ static void blkif_free(struct blkfront_info *info, int 
suspend)
if (!list_empty(&info->indirect_pages)) {
struct page *indirect_page, *n;
 
-   BUG_ON(info->feature_persistent);
list_for_each_entry_safe(indirect_page, n, 
&info->indirect_pages, lru) {
list_del(&indirect_page->lru);
__free_page(indirect_page);
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/