lkml.org 
[lkml]   [2009]   [Mar]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 1/7] block: Add block_flush_device()
    On Tue, Mar 31 2009, Linus Torvalds wrote:
    >
    >
    > 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.

    FUA we should be able to reliably detect, it's really the cache flush
    operation itself that has caused headaches in the past. The -EOPNOTSUPP
    really comes from the block layer, not from the device driver. That's
    mainly due to the fact that we only send down the actual barrier, if the
    driver already said it supported them. If they do fail them, we probably
    need to pick up the -EIO bits and pieces and pretend it didn't happen as
    well. So it definitely needs more looking into, auditing, and testing.
    I'll do that tomorrow.

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

    The empty flush is special and it is easy to fix that by itself. That
    should probably be the first patch in the series. But the retry logic
    and such for actual write barriers are the majority of the problems
    involved with supporting barriers, and those I want to get rid of.

    I think it'll be more clear when I post a real patch series with the
    individual steps outlined.

    --
    Jens Axboe



    \
     
     \ /
      Last update: 2009-03-31 19:21    [W:2.588 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site