Messages in this thread | | | Date | Tue, 31 Mar 2009 09:57:18 -0700 (PDT) | From | Linus Torvalds <> | Subject | Re: [PATCH 1/7] block: Add block_flush_device() |
| |
On Tue, 31 Mar 2009, Jens Axboe wrote: > > So here's a test patch that attempts to just ignore such a failure to > flush the caches.
I suspect you should not do it like this.
> diff --git a/fs/bio.c b/fs/bio.c > index a040cde..79e3cec 100644 > --- a/fs/bio.c > +++ b/fs/bio.c > @@ -1380,7 +1380,17 @@ void bio_check_pages_dirty(struct bio *bio) > **/ > void bio_endio(struct bio *bio, int error) > { > - if (error) > + /* > + * Special case here - hide the -EOPNOTSUPP from the driver or > + * block layer, dump a warning the first time this happens so that > + * the admin knows that we may not provide the ordering guarantees > + * that are needed. Don't clear the uptodate bit. > + */ > + if (error == -EOPNOTSUPP && bio_barrier(bio)) { > + set_bit(BIO_EOPNOTSUPP, &bio->bi_flags); > + blk_queue_set_noflush(bio->bi_bdev); > + error = 0; > + } else if (error)
I suspect this part is just wrong.
I could easily imagine a driver that returns EOPNOTSUPP only for a certain _kind_ of bio.
For example, if the drive doesn't support FUA, then you cannot do a serialized IO operation, but you can still mostly do a serialized op without any IO attached to it.
IOW, the "empty flush" really _is_ special. An this check should not be in the generic "bio_endio()" case, it should only be in the special blkdev_issue_flush() case.
I think. No?
Linus
| |