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]

Message-ID: <20090331060143.GF5178@kernel.dk>
Date:	Tue, 31 Mar 2009 08:01:43 +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, Jens Axboe wrote:
> > > 
> > > It has _nothing_ to do with 'reckless'. It has everything to do with 'you 
> > > can't do anything about it'.
> > 
> > No, but you better damn well inform of such a discovery!
> 
> Well, if that's the issue, then just add a printk to that 
> 'blkdev_issue_flush()', and now you have that informational message in 
> _one_ place, instead of havign each filesystem having to do it over and 
> over again.

Right, that's exactly what I want :-)

> > > No. Returning an error just means that now the box is useless. Nobody can 
> > > do anything about it. Not the admin, not the driver writer, not anybody. 
> > 
> > What, that's nonsense. The admin can certainly check whether it's an
> > issue or not, and he should.
> 
> If it's just informational, then again - why should the filesystem care?
> 
> Returning an error to the caller is never the right thing to do. The 
> caller can't do anything sane about it.
> 
> If you argue that the admin wants to know, then sure, make that
> 
>                 if (bio_flagged(bio, BIO_EOPNOTSUPP))
>         -               ret = -EOPNOTSUPP;
>         +               set_queue_noflush(q);
> 
> "set_queue_noflush()" function print a warning message when it sets the 
> bit.
> 
> Problem solved.
> 
> > That's different from handling it in the kernel or in the application, 
> > but you have to inform about it. I honestly cannot fathom why you don't 
> > think that is important.
> 
> I cannot fathom why you can _possibly_ think that this is something that 
> can and must be done something about in the caller. When the caller 
> obviously has no real option except to ignore the error _anyway_.
> 
> That was always my point. Returning an error is INSANE, because ther is no 
> valid thing that the caller can possibly do.
> 
> If you want it logged, fine. But THAT DOES NOT CHANGE ANYTHING. It would 
> still be wrong to return the error, since the caller _still_ can't do 
> anything about it.

I don't want to return -EOPNOTSUPP, I think this thread has become
increasingly confusing. And it's probably largely due to me mixing write
barriers into it, if we stick purely to blkdev_issue_flush(), then
logging a warning and returning 0 is perfectly fine with me. My
objection was to ignoring the "I can't flush" error in the first place,
not returning 0.

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