linux-kernel - Re: [PATCH 1/7] block: Add block_flush

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:	Mon, 30 Mar 2009 19:55:44 +0200
From:	Jens Axboe <jens.axboe@...cle.com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Fernando Luis Vázquez Cao 
	<fernando@....ntt.co.jp>, Jeff Garzik <jeff@...zik.org>,
	Christoph Hellwig <hch@...radead.org>,
	Theodore Tso <tytso@....edu>, Ingo Molnar <mingo@...e.hu>,
	Alan Cox <alan@...rguk.ukuu.org.uk>,
	Arjan van de Ven <arjan@...radead.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Nick Piggin <npiggin@...e.de>, David Rees <drees76@...il.com>,
	Jesper Krogh <jesper@...gh.cc>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	chris.mason@...cle.com, david@...morbit.com, tj@...nel.org
Subject: Re: [PATCH 1/7] block: Add block_flush_device()

On Mon, Mar 30 2009, Linus Torvalds wrote:
> 
> 
> On Mon, 30 Mar 2009, Fernando Luis Vázquez Cao wrote:
> > +	int ret = 0;
> > +
> > +	ret = blkdev_issue_flush(bdev, NULL);
> > +
> > +	return (ret == -EOPNOTSUPP) ? 0 : ret;
> 
> Btw, why do we do that silly EOPNOTSUPP at all?
> 
> If the device doesn't support flushing, we should
> 
>  - set a flag in the device saying so, and not ever try to flush again on
>    that device (who knows how long it took for the device to say "I can't
>    do this"? We don't want to keep on doing it)
> 
>  - return "done". There's nothing sane the caller can do with the error
>    code anyway, it just has to assume that the device basically doesn't
>    reorder writes.
> 
> So wouldn't it be better to just fix blkdev_issue_flush() to not do those
> crazy error codes?

The problem is that we may not know upfront, so it sort-of has to be
this trial approach where the first barrier issued will notice and fail
with -EOPNOTSUPP. Sure, we could cache this value, but it's pretty
pointless since the filesystem will stop sending barriers in this case.
As it also modifed fs behaviour, we need to pass the info back.

For blkdev_issue_flush() it may not be very interesting, since there's
not much we can do about that. Just seems like very bad style to NOT
return an error in such a case. You can assume that ordering is fine,
but it definitely wont be in all case (eg devices that have write back
caching on by default and don't support flush). So the nice thing to do
there is actually tell the caller about it. So the same error is reused
as we do for actualy write barriers that have data attached.

> [ The same thing probably goes for those ENXIO errors, btw. If we don't
>   have a bd_disk or a queue, why would the caller care about it? ]

Right, that is pretty pointless.

-- 
Jens Axboe

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