linux-kernel - Re: Block regression since 3.1-rc3

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 09 Oct 2011 10:12:03 +0200
From:	Christophe Saout <christophe@...ut.de>
To:	Shaohua Li <shli@...nel.org>
Cc:	Mike Snitzer <snitzer@...hat.com>, Jeff Moyer <jmoyer@...hat.com>,
	Jens Axboe <axboe@...nel.dk>, Tejun Heo <tj@...nel.org>,
	device-mapper development <dm-devel@...hat.com>,
	linux-kernel@...r.kernel.org
Subject: Re: Block regression since 3.1-rc3

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/