linux-kernel - Re: [patch] blk-flush: fix flush policy calculation

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:	Thu, 4 Aug 2011 12:16:24 +0200
From:	Tejun Heo <tj@...nel.org>
To:	Jeff Moyer <jmoyer@...hat.com>
Cc:	linux-kernel@...r.kernel.org, Tejun Heo <tj@...nle.org>,
	Jens Axboe <jaxboe@...ionio.com>
Subject: Re: [patch] blk-flush: fix flush policy calculation

Hello,

On Mon, Aug 01, 2011 at 04:32:17PM -0400, Jeff Moyer wrote:
>         /*
>          * If there's data but flush is not necessary, the request can be
>          * processed directly without going through flush machinery.  Queue
>          * for normal execution.
>          */
>         if ((policy & REQ_FSEQ_DATA) &&
>             !(policy & (REQ_FSEQ_PREFLUSH | REQ_FSEQ_POSTFLUSH))) {
>                 list_add_tail(&rq->queuelist, &q->queue_head);
>                 return;
>         }
> 
> Given the code as it stands, I don't think the body of this if statement
> will ever be executed.

It does if the device has a WB cache and filesystem sends down a FUA
(but no FLUSH) request.

> I've attached a fix for this below.  It seems
> like this could be both a performance and a correctness issue, though
> I've not run into any problems I can directly attribute to this (perhaps
> due to file systems not issuing flushes when support is not advertised?).
> 
> Signed-off-by: Jeff Moyer <jmoyer@...hat.com>

Hmmm... yes, this can become a correctness issue if (and only if)
blk_queue_flush() is called to change q->flush_flags while requests
are in-flight; otherwise, requests wouldn't reach the function at all.
Also, I think it would be a generally good idea to always set
FSEQ_DATA if the request has data.

 Acked-by: Tejun Heo <tj@...nel.org>

but can you please update the description?

Thank you.

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