Re: Block regression since 3.1-rc3

[Christophe Saout]

Hi Shaohua,

> >>> Looks the dm request based flush logic is broken.
> >>>
> >>> saved_make_request_fn
> >>>   __make_request
> >>>     blk_insert_flush
> >>> but blk_insert_flush doesn't put the original request to list, instead, the
> >>> q->flush_rq is in list.
> >>> then
> >>> dm_request_fn
> >>>   blk_peek_request
> >>>     dm_prep_fn
> >>>       clone_rq
> >>>   map_request
> >>>     blk_insert_cloned_request
> >>> so q->flush_rq is cloned, and get dispatched. but we can't clone q->flush_rq
> >>> and use it to do flush. map_request even could assign a different blockdev to
> >>> the cloned request.
> >>
> >> You haven't explained why cloning q->flush_rq is broken.  What is the
> >> problem with map_request changing the blockdev?  For the purposes of
> >> request-based DM the flush machinery has already managed the processing
> >> of the flush at the higher level request_queue.
> > hmm, looks I overlook the issue. cloned flush_rq has some problems but can
> > be fixed.
> > 1. it doesn't set requet->bio, request->bio_tail
> > 2. REQ_CLONE_MASK should set REQ_FLUSH_SEQ
> oh, don't need set REQ_FLUSH_SEQ, the low level queue will set it
> anyway. sorry for
> the noise. Jeff's patch looks ok then.

Just for the record: I tried your latest patch and it indeed makes the
crasher go away. However, things are getting horribly slow (hangs for
like 20 seconds when writing), at least for GFS2. This is probably
because the queue isn't kicked, like you stated in your most recent
comment, I guess.

Also for the record: Jeff's patch causes other crashes on my machine
(already posted), so while it might be fine from a technical point of
view, it still has issues in its current form. Jeff is working on it.

Thanks,
	Christophe


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