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.
diff --git a/block/blk-barrier.c b/block/blk-barrier.c
index e544813..988b634 100644
--- a/block/blk-barrier.c
+++ b/block/blk-barrier.c
@@ -372,7 +372,7 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
nr_sects = 0;
}
bio_get(bio);
- submit_bio(WRITE_DISCARD, bio);
+ submit_bio(DISCARD_BARRIER, bio);
/* Check if it failed immediately */
if (bio_flagged(bio, BIO_EOPNOTSUPP))
diff --git a/block/blk-core.c b/block/blk-core.c
index 0c8ed97..e7f2419 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1075,12 +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;
+ 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;
@@ -1112,12 +1113,13 @@ 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)) {
+ discard = bio_discard(bio);
+ if (unlikely(barrier) && !discard &&
+ (q->next_ordered == QUEUE_ORDERED_NONE)) {
err = -EOPNOTSUPP;
goto end_io;
}
- discard = bio_discard(bio);
if (unlikely(discard) && !q->prepare_discard_fn) {
err = -EOPNOTSUPP;
goto end_io;
diff --git a/block/ioctl.c b/block/ioctl.c
index 890f003..6f34c6c 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -161,7 +161,7 @@ static int blk_ioctl_discard(struct block_device *bdev, uint64_t start,
bio->bi_size = len << 9;
len = 0;
}
- submit_bio(WRITE_DISCARD, bio);
+ submit_bio(DISCARD_NOBARRIER, bio);
wait_for_completion(&wait);
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 88358ca..860689f 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -87,7 +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 WRITE_DISCARD (WRITE | (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
