linux-kernel - Re: [PATCH block:for-3.3/core] cfq: merged request shouldn't jump to a different cfqq

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]

Message-ID: <20120106033012.GE6276@google.com>
Date:	Thu, 5 Jan 2012 19:30:12 -0800
From:	Tejun Heo <tj@...nel.org>
To:	Jens Axboe <axboe@...nel.dk>, Hugh Dickins <hughd@...gle.com>,
	Shaohua Li <shaohua.li@...el.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Stephen Rothwell <sfr@...b.auug.org.au>,
	linux-next@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>,
	linux-scsi@...r.kernel.org, linux-ide@...r.kernel.org,
	x86@...nel.org
Subject: Re: [PATCH block:for-3.3/core] cfq: merged request shouldn't jump
 to a different cfqq

Ummmm... I've been looking at the code and currently I think the best
option is yank out plug merging for this merge window.

Bypassing rqhash for request merging doesn't seem like a good idea.
rqhash itself is, at this point, redundant and the limitation that it
can only find requests by the ending sectors lead to weird designs.
The single ->last_merge was okay but the recursive back merging is
just ugly.  What it should be doing is trying back merge and then
front merge once for each request insertion as the usual merge path
does.

We can't do attempt_back/front_merge() for INSERT_MERGE at this point
because elv_latter/former_request() only works for requests which are
already on elevator and, unfortunately, putting a request onto cfq may
kick the queue directly and the request might already be gone by the
time we try to merge it.

For this merge window, I think we better just disable INSERT_MERGE.

A mid-term solution could be changing elevator interface such that
elevator_add_req_fn() doesn't kick the queue directly but notify
elevator core that the queue needs kicking via return value, so that
merging can happen before kicking the queue.  Note that there's a
caveat here.  Merging might make kicking unnecessary or wrong.

Better solution would be changing elevator merge logic so that it has
"give me request closest to this offset" interface and then use
prev/next from there to find out merge candidates for both bio-rq and
rq-rq merges and just kill rqhash.

Thanks.

-- 
tejun
--
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/