linux-kernel - Re: [PATCH] blk-mq: don't use rw_is

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: <20141201185934.GA52328@kernel.org>
Date:	Mon, 1 Dec 2014 10:59:34 -0800
From:	Shaohua Li <shli@...nel.org>
To:	Jens Axboe <axboe@...com>
Cc:	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] blk-mq: don't use rw_is_sync() to determine sync request

On Sun, Nov 30, 2014 at 07:57:12PM -0800, Shaohua Li wrote:
> On Sun, Nov 30, 2014 at 06:35:11PM -0700, Jens Axboe wrote:
> > On 11/30/2014 05:01 PM, Shaohua Li wrote:
> > >Buffer read is counted as sync in rw_is_sync(). If we use it,
> > >blk_sq_make_request() will not do per-process plug any more.
> > >
> > >I haven't changed blk_mq_make_request() yet. It makes sense to dispatch
> > >REQ_SYNC request immediately. But for buffer read, it's weird not to do
> > >per-process plug, as buffer read doesn't need low latency.
> > >blk_mq_merge_queue_io() isn't very helpful, as we don't have delay mechanism
> > >there, the queue is immediately flushed, which makes the merge very
> > >superficial.
> > 
> > A read is sync, buffered or not. A buffered read is every bit as
> > latency sensitive as an O_DIRECT read. I think it'd be fine to
> > modify rw_is_sync() to disregard REQ_AHEAD as sync (and ensure it's
> > carried forward in the request flags, too). At least to the extent
> > that we process plug and get the merging, since for streamed reads
> > we'd soon be waiting on them anyway.
> 
> A quick search shows nobody uses REQ_AHEAD. For stream reads, only first several
> reads are waited I suppose, later reads are read ahead. Maybe only counts
> REQ_META read as sync?

Changing rw_is_sync() sounds risky, as it will change behavior of other parts,
like CFQ. REQ_META/REQ_PRIO isn't an option, metadata does readahead too.
And nobody uses REQ_AHEAD. explictly checking REQ_SYNC in blk_sq_make_request()
sounds better, which is just for pluging and we use it for ages in
blk_queue_bio().

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