diff mbox

[02/35] block: add REQ_OP definitions and bi_op/op fields

Message ID 1452027218-32303-3-git-send-email-mchristi@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Mike Christie Jan. 5, 2016, 8:53 p.m. UTC 
From: Mike Christie <mchristi@redhat.com>

The following patches separate the operation (write, read, discard,
etc) from the flags in bi_rw/cmd_flags. This patch adds definitions
for request/bio operations, adds fields to the request/bio to set
them, and some temporary compat code so the kernel/modules can use
either one. In the final patches this compat code will be removed
when everything is converted.

Also, in this patch the REQ_OPs match the REQ rq_flag_bits ones
for compat reasons while all the code is converted in this set. In the
last patches that will also be removed.

Signed-off-by: Mike Christie <mchristi@redhat.com>
---
 block/blk-core.c          | 19 ++++++++++++++++---
 include/linux/blk_types.h | 15 ++++++++++++++-
 include/linux/blkdev.h    |  1 +
 include/linux/fs.h        | 37 +++++++++++++++++++++++++++++++++++--
 4 files changed, 66 insertions(+), 6 deletions(-)

Comments

Martin K. Petersen Jan. 6, 2016, 11:17 p.m. UTC | #1 
>>>>> "Mike" == mchristi  <mchristi@redhat.com> writes:

+enum req_op {
+	REQ_OP_READ,
+	REQ_OP_WRITE		= REQ_WRITE,
+	REQ_OP_DISCARD		= REQ_DISCARD,
+	REQ_OP_WRITE_SAME	= REQ_WRITE_SAME,
+};
+

I have been irked by the REQ_ prefix in bios since the flags were
consolidated a while back. When I attempted to fix the READ/WRITE mess I
used a BLK_ prefix as a result.

Anyway. Just bikeshedding...
Christoph Hellwig Jan. 9, 2016, 1:37 p.m. UTC | #2 
Seems like this is missing REQ_OP_FLUSH, which still hides as a write?

--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mike Christie Jan. 10, 2016, 12:56 a.m. UTC | #3 
On 01/09/2016 07:37 AM, Christoph Hellwig wrote:
> Seems like this is missing REQ_OP_FLUSH, which still hides as a write?
> 

I might have misunderstood what you wanted or am misunderstanding you now.

I did not change the flush related code until the last patches. I added
REQ_OP_FLUSH in patch:

[PATCH 33/35] block, drivers: add REQ_OP_FLUSH operation
commit 18dea20363ba245a47bc1bb54f6465b8a05b19af
Author: Mike Christie <mchristi@redhat.com>
Date:   Tue Jan 5 21:02:17 2016 -0600

    block, drivers: add REQ_OP_FLUSH operation


and I added the REQ_PREFLUSH flag in


[PATCH 35/35] block, drivers, fs: rename REQ_FLUSH to REQ_PREFLUSH
commit 303dc6a7cc3673065538ba041562fcd833a619af
Author: Mike Christie <mchristi@redhat.com>
Date:   Thu Jan 7 18:17:03 2016 -0600

    block, drivers, fs: rename REQ_FLUSH to REQ_PREFLUSH


I did them in separate patches, because I was not sure if they were
correct with what you requested before, and because they were a little
more tricky because of how request_fn drivers, make_request_fn drivers,
and dm-multipath handle flushes differently.
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mike Christie Jan. 10, 2016, 1:21 a.m. UTC | #4 
On 01/09/2016 06:56 PM, Mike Christie wrote:
> On 01/09/2016 07:37 AM, Christoph Hellwig wrote:
>> Seems like this is missing REQ_OP_FLUSH, which still hides as a write?
>>

Oh yeah, to answer the second part of your question, REQ_OP_FLUSH is
only a flush operation like what request_fn drivers wanted.

REQ_PREFLUSH can be set with a REQ_OP_WRITE bio when filesystems want to
do both.

There is then the case where filesystems and blkdev_issue_flush could
just want to request a flush. I left them as a REQ_PREFLUSH with
REQ_OP_WRITE set, so there would be a single code path.

> 
> I might have misunderstood what you wanted or am misunderstanding you now.
> 
> I did not change the flush related code until the last patches. I added
> REQ_OP_FLUSH in patch:
> 
> [PATCH 33/35] block, drivers: add REQ_OP_FLUSH operation
> commit 18dea20363ba245a47bc1bb54f6465b8a05b19af
> Author: Mike Christie <mchristi@redhat.com>
> Date:   Tue Jan 5 21:02:17 2016 -0600
> 
>     block, drivers: add REQ_OP_FLUSH operation
> 
> 
> and I added the REQ_PREFLUSH flag in
> 
> 
> [PATCH 35/35] block, drivers, fs: rename REQ_FLUSH to REQ_PREFLUSH
> commit 303dc6a7cc3673065538ba041562fcd833a619af
> Author: Mike Christie <mchristi@redhat.com>
> Date:   Thu Jan 7 18:17:03 2016 -0600
> 
>     block, drivers, fs: rename REQ_FLUSH to REQ_PREFLUSH
> 
> 
> I did them in separate patches, because I was not sure if they were
> correct with what you requested before, and because they were a little
> more tricky because of how request_fn drivers, make_request_fn drivers,
> and dm-multipath handle flushes differently.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Christoph Hellwig Jan. 10, 2016, 7:14 a.m. UTC | #5 
On Sat, Jan 09, 2016 at 06:56:14PM -0600, Mike Christie wrote:
> I did not change the flush related code until the last patches. I added
> REQ_OP_FLUSH in patch:

Oh, I missed that.
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Christoph Hellwig Jan. 10, 2016, 7:15 a.m. UTC | #6 
On Sat, Jan 09, 2016 at 07:21:12PM -0600, Mike Christie wrote:
> Oh yeah, to answer the second part of your question, REQ_OP_FLUSH is
> only a flush operation like what request_fn drivers wanted.

And that's the odd part that trips me up.

> 
> REQ_PREFLUSH can be set with a REQ_OP_WRITE bio when filesystems want to
> do both.
> 
> There is then the case where filesystems and blkdev_issue_flush could
> just want to request a flush. I left them as a REQ_PREFLUSH with
> REQ_OP_WRITE set, so there would be a single code path.

But the pure flush without data transfer case is pretty different,
so it seems rather odd to handle it like that.  But I suspec we could
just fix that up later.
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/block/blk-core.c b/block/blk-core.c
index 9b887e3..954a450 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1697,7 +1697,8 @@  void init_request_from_bio(struct request *req, struct bio *bio)
 {
 	req->cmd_type = REQ_TYPE_FS;
 
-	req->cmd_flags |= bio->bi_rw & REQ_COMMON_MASK;
+	/* tmp compat. Allow users to set bi_op or bi_rw */
+	req->cmd_flags |= (bio->bi_rw | bio->bi_op) & REQ_COMMON_MASK;
 	if (bio->bi_rw & REQ_RAHEAD)
 		req->cmd_flags |= REQ_FAILFAST_MASK;
 
@@ -2032,6 +2033,12 @@  blk_qc_t generic_make_request(struct bio *bio)
 	struct bio_list bio_list_on_stack;
 	blk_qc_t ret = BLK_QC_T_NONE;
 
+	/* tmp compat. Allow users to set either one or both.
+	 * This will be removed when we have converted
+	 * everyone in the next patches.
+	 */
+	bio->bi_rw |= bio->bi_op;
+
 	if (!generic_make_request_checks(bio))
 		goto out;
 
@@ -2101,6 +2108,12 @@  EXPORT_SYMBOL(generic_make_request);
  */
 blk_qc_t submit_bio(struct bio *bio)
 {
+	/* tmp compat. Allow users to set either one or both.
+	 * This will be removed when we have converted
+	 * everyone in the next patches.
+	 */
+	bio->bi_rw |= bio->bi_op;
+
 	/*
 	 * If it's a regular read/write or a barrier with data attached,
 	 * go through the normal accounting stuff before submission.
@@ -2972,8 +2985,8 @@  EXPORT_SYMBOL_GPL(__blk_end_request_err);
 void blk_rq_bio_prep(struct request_queue *q, struct request *rq,
 		     struct bio *bio)
 {
-	/* Bit 0 (R/W) is identical in rq->cmd_flags and bio->bi_rw */
-	rq->cmd_flags |= bio->bi_rw & REQ_WRITE;
+	/* tmp compat. Allow users to set bi_op or bi_rw */
+	rq->cmd_flags |= bio_data_dir(bio);
 
 	if (bio_has_data(bio))
 		rq->nr_phys_segments = bio_phys_segments(q, bio);
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 86a38ea..6e49c91 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -48,9 +48,15 @@  struct bio {
 	struct block_device	*bi_bdev;
 	unsigned int		bi_flags;	/* status, command, etc */
 	int			bi_error;
-	unsigned long		bi_rw;		/* bottom bits READ/WRITE,
+	unsigned long		bi_rw;		/* bottom bits rq_flags_bits
 						 * top bits priority
 						 */
+	/*
+	 * this will be a u8 in the next patches and bi_rw can be shrunk to
+	 * a u32. For compat in these transistional patches op is a int here.
+	 */
+	int			bi_op;		/* REQ_OP */
+
 
 	struct bvec_iter	bi_iter;
 
@@ -242,6 +248,13 @@  enum rq_flag_bits {
 #define REQ_HASHED		(1ULL << __REQ_HASHED)
 #define REQ_MQ_INFLIGHT		(1ULL << __REQ_MQ_INFLIGHT)
 
+enum req_op {
+	REQ_OP_READ,
+	REQ_OP_WRITE		= REQ_WRITE,
+	REQ_OP_DISCARD		= REQ_DISCARD,
+	REQ_OP_WRITE_SAME	= REQ_WRITE_SAME,
+};
+
 typedef unsigned int blk_qc_t;
 #define BLK_QC_T_NONE	-1U
 #define BLK_QC_T_SHIFT	16
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 29189ae..35b9eb3 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -96,6 +96,7 @@  struct request {
 	struct request_queue *q;
 	struct blk_mq_ctx *mq_ctx;
 
+	int op;
 	u64 cmd_flags;
 	unsigned cmd_type;
 	unsigned long atomic_flags;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 3b4e751..fb9e516 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2405,15 +2405,48 @@  extern void make_bad_inode(struct inode *);
 extern bool is_bad_inode(struct inode *);
 
 #ifdef CONFIG_BLOCK
+
+static inline bool op_is_write(int op)
+{
+	switch (op) {
+	case REQ_OP_WRITE:
+	case REQ_OP_WRITE_SAME:
+	case REQ_OP_DISCARD:
+		return true;
+	default:
+		return false;
+	}
+}
+
 /*
  * return READ, READA, or WRITE
  */
-#define bio_rw(bio)		((bio)->bi_rw & (RW_MASK | RWA_MASK))
+static inline int bio_rw(struct bio *bio)
+{
+	/*
+	 * tmp cpmpat. Allow users to set either op or rw, until
+	 * all code is converted in the next patches.
+	 */
+	if (op_is_write(bio->bi_op))
+		return WRITE;
+
+	return bio->bi_rw & (RW_MASK | RWA_MASK);
+}
 
 /*
  * return data direction, READ or WRITE
  */
-#define bio_data_dir(bio)	((bio)->bi_rw & 1)
+static inline int bio_data_dir(struct bio *bio)
+{
+	/*
+	 * tmp cpmpat. Allow users to set either op or rw, until
+	 * all code is converted in the next patches.
+	 */
+	if (op_is_write(bio->bi_op))
+		return WRITE;
+
+	return bio->bi_rw & 1;
+}
 
 extern void check_disk_size_change(struct gendisk *disk,
 				   struct block_device *bdev);