Re: about the patch: persistent grant maps for xen blk drivers

Re: about the patch: persistent grant maps for xen blk drivers

[Konrad Rzeszutek Wilk]

On Mon, Oct 15, 2012 at 01:01:51PM +0800, liuxiaolei1124 wrote:
> Dear konrad:
>    i have seen you put the patch "persisten grant maps for xen blk drivers" into you kernel, then dom0 crash .(https://lkml.org/lkml/2012/9/21/388 )my dom0 kernel is 2.6.32.36-0.5, and i put this patch in my kernel, there is a bug too, and the stack is much like yours. And i found  a strange phenomenon. when i add a printk log  such as "printk ("enter func") " in blkif_completion or other function in xen-blkfront.c, guest run well. But after i remove this printk log, guest crash when i start.


Hey. Roger posted a follow up patch that has this fixed. You should
look at that.

Also CC-ing xen-devel here.

>     and the crash stack is :
>     blkif_int -> blkif_completion
>     guest page fault in
>  
> + if (bret->operation == BLKIF_OP_READ)
> + rq_for_each_segment(bvec, s->request, iter) {
> + shared_data = kmap_atomic
> +                                             (pfn_to_page(s->grants_used[i++]->frame));  // page fault
> + bvec_data = bvec_kmap_irq(bvec, &flags);
> + memcpy(bvec_data, shared_data + bvec->bv_offset,
> +        bvec->bv_len);
> + bvec_kunmap_irq(bvec_data, &flags);
> + kunmap_atomic(shared_data);
> + }
> 
> in kernel 2.6.32.36-0.5, my patch is :
>  
> + if (bret->operation == BLKIF_OP_READ)
> + rq_for_each_segment(bvec, s->request, iter) {
> + shared_data = kmap_atomic
> + (pfn_to_page(s->grants_used[i++]->frame), KM_USER0);
> + bvec_data = bvec_kmap_irq(bvec, &flags);
> + memcpy(bvec_data, shared_data + bvec->bv_offset,
> +        bvec->bv_len);
> + bvec_kunmap_irq(bvec_data, &flags);
> + kunmap_atomic(shared_data, KM_USER0);
> + }
> 
>    I don't know what's wrong? mybe function kmap_atomic in my patch is incorrect. I look forward toyour reply, thank you.
>  
>   Best wishes.
>   eric.liu
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 

Re: about the patch: persistent grant maps for xen blk drivers

[liuxiaolei1124]

[-- Attachment #1.1: Type: text/plain, Size: 880 bytes --]










At 2012-10-15 21:19:30,"Konrad Rzeszutek Wilk" <konrad.wilk@oracle.com> wrote:
>On Mon, Oct 15, 2012 at 01:01:51PM +0800, liuxiaolei1124 wrote:
>> Dear konrad:
>>    i have seen you put the patch "persisten grant maps for xen blk drivers" into you kernel, then dom0 crash .(https://lkml.org/lkml/2012/9/21/388 )my dom0 kernel is 2.6.32.36-0.5, and i put this patch in my kernel, there is a bug too, and the stack is much like yours. And i found  a strange phenomenon. when i add a printk log  such as "printk ("enter func") " in blkif_completion or other function in xen-blkfront.c, guest run well. But after i remove this printk log, guest crash when i start.
>
>
>Hey. Roger posted a follow up patch that has this fixed. You should
>look at that.
sorry, but i can't find the patch Roger posted, could you show me the link of the patch? thank you.
Best wishes.
eric.liu

[-- Attachment #1.2: Type: text/html, Size: 1799 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: about the patch: persistent grant maps for xen blk drivers

[Konrad Rzeszutek Wilk]

On Tue, Oct 16, 2012 at 12:35:23PM +0800, liuxiaolei1124 wrote:
> 
> 
> 
> 
> 
> 
> 
> 
> 
> At 2012-10-15 21:19:30,"Konrad Rzeszutek Wilk" <konrad.wilk@oracle.com> wrote:
> >On Mon, Oct 15, 2012 at 01:01:51PM +0800, liuxiaolei1124 wrote:
> >> Dear konrad:
> >>    i have seen you put the patch "persisten grant maps for xen blk drivers" into you kernel, then dom0 crash .(https://lkml.org/lkml/2012/9/21/388 )my dom0 kernel is 2.6.32.36-0.5, and i put this patch in my kernel, there is a bug too, and the stack is much like yours. And i found  a strange phenomenon. when i add a printk log  such as "printk ("enter func") " in blkif_completion or other function in xen-blkfront.c, guest run well. But after i remove this printk log, guest crash when i start.
> >
> >
> >Hey. Roger posted a follow up patch that has this fixed. You should
> >look at that.
> sorry, but i can't find the patch Roger posted, could you show me the link of the patch? thank you.

Roger, could you repost them please?

> Best wishes.
> eric.liu

Re: about the patch: Persistent grant maps for xen blk drivers

[Roger Pau Monné]

On 24/10/12 14:40, liuxiaolei1124 wrote:
> Dear Roger:
>  
>      i have put the patch "Persistent grant maps for xen blk drivers"
> (https://lkml.org/lkml/2012/10/18/191) into my Dom0 which is 2.6.32.36.
> And when i start a vm, sometimes the blkback go to the BUG_ON.

I'm working on top of the next release (3.7.0-rc1), so I cannot assure
that this patch will work correctly on top of 2.6.32.36. I would
recommend you to switch to
git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen.git and apply
my patch on top of the linux-next branch to perform the testing.

> + for (i = 0, j = 0; i < nseg; i++) {
> + if (!persistent_gnts[i] || !persistent_gnts[i]->handle) {
> + /* This is a newly mapped grant */
> + BUG_ON(j >= segs_to_map);       // BUG_ON here
> + if (unlikely(map[j].status != 0)) {
> + pr_debug(DRV_PFX "invalid buffer -- could not remap it\n");
> + map[j].handle = BLKBACK_INVALID_HANDLE;
> + ret |= 1;
> + if (persistent_gnts[i]) {
> + rb_erase(&persistent_gnts[i]->node,
> + &blkif->persistent_gnts);
> + blkif->persistent_gnt_c--;
> + kfree(persistent_gnts[i]);
> + persistent_gnts[i] = NULL;
> + }
> + }
> + }
>  
> i don't understand when j  >=  segs_to_unmap ?

It should never happen, if j >= segs_to_unmap and we hit the bug it
means we are trying to process more segments than the ones we have mapped.

> And i make a test,  i don't put the patch which in xen-blkfront.c into
> kernel, no matter i forced set pers_grants = 1 or set pers_grants = 0
> ,it's OK when i start a vm.

Do you have the dmesg from the DomU? Does it even start? Could you also
post the serial output of the crash (and the lines before the crash)?

Thanks for the testing, Roger.