On Tue, 2008-08-12 at 14:53 +0200, Jens Axboe wrote:
> On Tue, Aug 12 2008, David Woodhouse wrote:
> > On Tue, 2008-08-12 at 12:16 +0100, David Woodhouse wrote:
> > > There's a check in __make_request() which will return -EOPNOTSUPP for
> > > bios with BIO_RW_BARRIER set, in some circumstances. Is that OK too?
> >
> > A: No, it breaks submission of DISCARD|BARRIER requests. And I have to
> > change blk_empty_barrier() to explicitly omit discards too, to avoid the
> > check at line 1419 (in __generic_make_request()) returning -EOPNOTSUPP
> > before we even get there.
> >
> > Using BIO_RW_BARRIER seems conceptually cleaner, but in practice it's
> > messier. I think this version will work, but I'm inclined to prefer the
> > previous one I posted, using the R/W bit.
>
> Or just match the check before -EOPNOTSUPP with bio_has_data(), since it
> only applies to a barrier that carries data.
Like this, you mean? Empty barriers don't get to there?
I still prefer the R/W version, but I'll commit it this way if you
want...
diff --git a/block/blk-core.c b/block/blk-core.c
index be2fe52..1b38994 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1075,19 +1075,13 @@ void init_request_from_bio(struct request *req, struct bio *bio)
/*
* REQ_BARRIER implies no merging, but lets make it explicit
*/
- if (unlikely(bio_barrier(bio)))
- req->cmd_flags |= (REQ_HARDBARRIER | REQ_NOMERGE);
if (unlikely(bio_discard(bio))) {
req->cmd_flags |= REQ_DISCARD;
- /*
- * READ discard requests are soft barriers; WRITE
- * indicates the file system will take care of that
- * for itself.
- */
- if (!bio_data_dir(bio))
+ if (bio_barrier(bio))
req->cmd_flags |= REQ_SOFTBARRIER;
req->q->prepare_discard_fn(req->q, req);
- }
+ } else if (unlikely(bio_barrier(bio)))
+ req->cmd_flags |= (REQ_HARDBARRIER | REQ_NOMERGE);
if (bio_sync(bio))
req->cmd_flags |= REQ_RW_SYNC;
@@ -1119,7 +1113,8 @@ static int __make_request(struct request_queue *q, struct bio *bio)
blk_queue_bounce(q, &bio);
barrier = bio_barrier(bio);
- if (unlikely(barrier) && (q->next_ordered == QUEUE_ORDERED_NONE)) {
+ if (unlikely(barrier) && bio_has_data(bio) &&
+ (q->next_ordered == QUEUE_ORDERED_NONE)) {
err = -EOPNOTSUPP;
goto end_io;
}
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 1fdfc56..33c3947 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -188,8 +188,8 @@ struct bio {
#define bio_failfast(bio) ((bio)->bi_rw & (1 << BIO_RW_FAILFAST))
#define bio_rw_ahead(bio) ((bio)->bi_rw & (1 << BIO_RW_AHEAD))
#define bio_rw_meta(bio) ((bio)->bi_rw & (1 << BIO_RW_META))
-#define bio_empty_barrier(bio) (bio_barrier(bio) && !bio_has_data(bio))
#define bio_discard(bio) ((bio)->bi_rw & (1 << BIO_RW_DISCARD))
+#define bio_empty_barrier(bio) (bio_barrier(bio) && !bio_has_data(bio) && !bio_discard(bio))
static inline unsigned int bio_cur_sectors(struct bio *bio)
{
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 5cfacb3..860689f 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -87,8 +87,8 @@ extern int dir_notify_enable;
#define WRITE_SYNC (WRITE | (1 << BIO_RW_SYNC))
#define SWRITE_SYNC (SWRITE | (1 << BIO_RW_SYNC))
#define WRITE_BARRIER (WRITE | (1 << BIO_RW_BARRIER))
-#define DISCARD_NOBARRIER (WRITE | (1 << BIO_RW_DISCARD))
-#define DISCARD_BARRIER (1 << BIO_RW_DISCARD)
+#define DISCARD_NOBARRIER (1 << BIO_RW_DISCARD)
+#define DISCARD_BARRIER ((1 << BIO_RW_DISCARD) | (1 << BIO_RW_BARRIER))
#define SEL_IN 1
#define SEL_OUT 2
--
David Woodhouse Open Source Technology Centre
David.Woodhouse@xxxxxxxxx Intel Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
