Re: [PATCH 1/7] block: Add block_flush_device()

Jens Axboe
2009-03-31T06:02:01Z

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