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

[Jens Axboe]

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/