Re: [Xen-devel] [PATCH] Re: Another blktap2-ish shutdown crash

[Daniel Stodden <daniel.stodden@xxxxxxxxxx>]

On Mon, 2010-06-07 at 09:43 -0400, Jan Beulich wrote:
> >>> On 07.06.10 at 13:12, Daniel Stodden <daniel.stodden@xxxxxxxxxx> wrote:
> > On Mon, 2010-06-07 at 03:29 -0400, Jan Beulich wrote:
> >> >>> On 03.06.10 at 03:50, Daniel Stodden <daniel.stodden@xxxxxxxxxx> wrote:
> >> 
> >> Why would you want blk_start_request() only after the blk_fs_request()
> >> check, but not after the blk_barrier_rq() one?
> > 
> > Huh? But cases did get the blk_start_request call (?!)
> 
> I have to admit that I don't understand your response at all.

".. both ..", sorrry :)

> Assuming that you think my original question was rubbish, this is
> the original (before your patch) code I look at
> 
>       while ((req = blk_peek_request(rq)) != NULL) {
>               if (!blk_fs_request(req)) {
>                       blk_end_request(req, -EIO, 0);
>                       continue;
>               }
> 
>               if (blk_barrier_rq(req)) {
>                       blk_end_request(req, -EIO, 0);
>                       continue;
>               }
> ...
>               blk_start_request(req);
> ...
> 
> Your patch inserts a call to blk_start_request() into the
> first if clause's body, and I was asking why the second
> one's wouldn't also need such a call.
> 
> Sorry if I'm being dense - I'll appreciate any enlightenment.

This is the relevant hunk from 

http://git.kernel.org/?p=linux/kernel/git/jeremy/xen.git;a=blobdiff;f=drivers/xen/blktap/device.c;h=8650f028485a6ac10bcb89ebfeaa5756ff2204e6;hp=a50b622ef018c0db88f0d491ce389768c570a4bc;hb=b34e5649ea3fc0f70f87e104c9b08a0dc013b6bb;hpb=4b647759e9f4afd46c23e26e54241a9dfa7d8f4b
  

        while ((req = blk_peek_request(rq)) != NULL) {
                if (!blk_fs_request(req)) {
+                       blk_start_request(req);
                        __blk_end_request_cur(req, 0);
                        continue;
                }
 
                if (blk_barrier_rq(req)) {
+                       blk_start_request(req);
                        __blk_end_request_cur(req, 0);
                        continue;
                }

Looks correct to me. What are you looking at?

Cheers,
Daniel



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel