Re: [PATCH 01/11] blk: remove bio_set arg from blk_queue_split()

[NeilBrown]

On Sat, Apr 22 2017, Javier González wrote:

>> On 20 Apr 2017, at 07.38, NeilBrown  wrote:
>> 
>> blk_queue_split() is always called with the last arg being q->bio_split,
>> where 'q' is the first arg.
>> 
>> Also blk_queue_split() sometimes uses the passed-in 'bs' and sometimes uses
>> q->bio_split.
>> 
>> This is inconsistent and unnecessary.  Remove the last arg and always use
>> q->bio_split inside blk_queue_split()
>> 
>> Signed-off-by: NeilBrown 
>> ---
>> block/blk-core.c  |2 +-
>> block/blk-merge.c |9 -
>> block/blk-mq.c|2 +-
>> drivers/block/drbd/drbd_req.c |2 +-
>> drivers/block/pktcdvd.c   |2 +-
>> drivers/block/ps3vram.c   |2 +-
>> drivers/block/rsxx/dev.c  |2 +-
>> drivers/block/umem.c  |2 +-
>> drivers/block/zram/zram_drv.c |2 +-
>> drivers/lightnvm/rrpc.c   |2 +-
>> drivers/md/md.c   |2 +-
>> drivers/s390/block/dcssblk.c  |2 +-
>> drivers/s390/block/xpram.c|2 +-
>> include/linux/blkdev.h|3 +--
>> 14 files changed, 17 insertions(+), 19 deletions(-)
>> 
>
> It most probably made it to Jens' tree after you made these changes, but
> in drivers/lightnvm/pblk-init.c we also use blk_queue_split() in a
> couple of places inside pblk_rw_io().

Ahh, yes - thanks.  That will need to same small change.

Jens, do you prefer a resend, or and update patch, or to just add this
change manually?

Thanks,
NeilBrown


signature.asc
Description: PGP signature

Re: [PATCH 01/11] blk: remove bio_set arg from blk_queue_split()

[Javier González]

> On 20 Apr 2017, at 07.38, NeilBrown  wrote:
> 
> blk_queue_split() is always called with the last arg being q->bio_split,
> where 'q' is the first arg.
> 
> Also blk_queue_split() sometimes uses the passed-in 'bs' and sometimes uses
> q->bio_split.
> 
> This is inconsistent and unnecessary.  Remove the last arg and always use
> q->bio_split inside blk_queue_split()
> 
> Signed-off-by: NeilBrown 
> ---
> block/blk-core.c  |2 +-
> block/blk-merge.c |9 -
> block/blk-mq.c|2 +-
> drivers/block/drbd/drbd_req.c |2 +-
> drivers/block/pktcdvd.c   |2 +-
> drivers/block/ps3vram.c   |2 +-
> drivers/block/rsxx/dev.c  |2 +-
> drivers/block/umem.c  |2 +-
> drivers/block/zram/zram_drv.c |2 +-
> drivers/lightnvm/rrpc.c   |2 +-
> drivers/md/md.c   |2 +-
> drivers/s390/block/dcssblk.c  |2 +-
> drivers/s390/block/xpram.c|2 +-
> include/linux/blkdev.h|3 +--
> 14 files changed, 17 insertions(+), 19 deletions(-)
> 

It most probably made it to Jens' tree after you made these changes, but
in drivers/lightnvm/pblk-init.c we also use blk_queue_split() in a
couple of places inside pblk_rw_io().

Thanks,
Javier.


signature.asc
Description: Message signed with OpenPGP

Re: [PATCH 01/11] blk: remove bio_set arg from blk_queue_split()

[Ming Lei]

On Thu, Apr 20, 2017 at 1:38 PM, NeilBrown  wrote:
> blk_queue_split() is always called with the last arg being q->bio_split,
> where 'q' is the first arg.
>
> Also blk_queue_split() sometimes uses the passed-in 'bs' and sometimes uses
> q->bio_split.
>
> This is inconsistent and unnecessary.  Remove the last arg and always use
> q->bio_split inside blk_queue_split()
>
> Signed-off-by: NeilBrown 

Reviewed-by: Ming Lei 

Thanks,

Re: [PATCH 01/11] blk: remove bio_set arg from blk_queue_split()

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig 

[PATCH 01/11] blk: remove bio_set arg from blk_queue_split()

[NeilBrown]

blk_queue_split() is always called with the last arg being q->bio_split,
where 'q' is the first arg.

Also blk_queue_split() sometimes uses the passed-in 'bs' and sometimes uses
q->bio_split.

This is inconsistent and unnecessary.  Remove the last arg and always use
q->bio_split inside blk_queue_split()

Signed-off-by: NeilBrown 
---
 block/blk-core.c  |2 +-
 block/blk-merge.c |9 -
 block/blk-mq.c|2 +-
 drivers/block/drbd/drbd_req.c |2 +-
 drivers/block/pktcdvd.c   |2 +-
 drivers/block/ps3vram.c   |2 +-
 drivers/block/rsxx/dev.c  |2 +-
 drivers/block/umem.c  |2 +-
 drivers/block/zram/zram_drv.c |2 +-
 drivers/lightnvm/rrpc.c   |2 +-
 drivers/md/md.c   |2 +-
 drivers/s390/block/dcssblk.c  |2 +-
 drivers/s390/block/xpram.c|2 +-
 include/linux/blkdev.h|3 +--
 14 files changed, 17 insertions(+), 19 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 25aea293ee98..f5d64ad75b36 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1662,7 +1662,7 @@ static blk_qc_t blk_queue_bio(struct request_queue *q, 
struct bio *bio)
 */
blk_queue_bounce(q, &bio);
 
-   blk_queue_split(q, &bio, q->bio_split);
+   blk_queue_split(q, &bio);
 
if (bio_integrity_enabled(bio) && bio_integrity_prep(bio)) {
bio->bi_error = -EIO;
diff --git a/block/blk-merge.c b/block/blk-merge.c
index 3990ae406341..d59074556703 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -202,8 +202,7 @@ static struct bio *blk_bio_segment_split(struct 
request_queue *q,
return do_split ? new : NULL;
 }
 
-void blk_queue_split(struct request_queue *q, struct bio **bio,
-struct bio_set *bs)
+void blk_queue_split(struct request_queue *q, struct bio **bio)
 {
struct bio *split, *res;
unsigned nsegs;
@@ -211,13 +210,13 @@ void blk_queue_split(struct request_queue *q, struct bio 
**bio,
switch (bio_op(*bio)) {
case REQ_OP_DISCARD:
case REQ_OP_SECURE_ERASE:
-   split = blk_bio_discard_split(q, *bio, bs, &nsegs);
+   split = blk_bio_discard_split(q, *bio, q->bio_split, &nsegs);
break;
case REQ_OP_WRITE_ZEROES:
-   split = blk_bio_write_zeroes_split(q, *bio, bs, &nsegs);
+   split = blk_bio_write_zeroes_split(q, *bio, q->bio_split, 
&nsegs);
break;
case REQ_OP_WRITE_SAME:
-   split = blk_bio_write_same_split(q, *bio, bs, &nsegs);
+   split = blk_bio_write_same_split(q, *bio, q->bio_split, &nsegs);
break;
default:
split = blk_bio_segment_split(q, *bio, q->bio_split, &nsegs);
diff --git a/block/blk-mq.c b/block/blk-mq.c
index c496692ecc5b..365cb17308e5 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1549,7 +1549,7 @@ static blk_qc_t blk_mq_make_request(struct request_queue 
*q, struct bio *bio)
return BLK_QC_T_NONE;
}
 
-   blk_queue_split(q, &bio, q->bio_split);
+   blk_queue_split(q, &bio);
 
if (!is_flush_fua && !blk_queue_nomerges(q) &&
blk_attempt_plug_merge(q, bio, &request_count, &same_queue_rq))
diff --git a/drivers/block/drbd/drbd_req.c b/drivers/block/drbd/drbd_req.c
index b5730e17b455..fa62dd8a4d46 100644
--- a/drivers/block/drbd/drbd_req.c
+++ b/drivers/block/drbd/drbd_req.c
@@ -1557,7 +1557,7 @@ blk_qc_t drbd_make_request(struct request_queue *q, 
struct bio *bio)
struct drbd_device *device = (struct drbd_device *) q->queuedata;
unsigned long start_jif;
 
-   blk_queue_split(q, &bio, q->bio_split);
+   blk_queue_split(q, &bio);
 
start_jif = jiffies;
 
diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index 66d846ba85a9..98394d034c29 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -2414,7 +2414,7 @@ static blk_qc_t pkt_make_request(struct request_queue *q, 
struct bio *bio)
 
blk_queue_bounce(q, &bio);
 
-   blk_queue_split(q, &bio, q->bio_split);
+   blk_queue_split(q, &bio);
 
pd = q->queuedata;
if (!pd) {
diff --git a/drivers/block/ps3vram.c b/drivers/block/ps3vram.c
index 456b4fe21559..48072c0c1010 100644
--- a/drivers/block/ps3vram.c
+++ b/drivers/block/ps3vram.c
@@ -606,7 +606,7 @@ static blk_qc_t ps3vram_make_request(struct request_queue 
*q, struct bio *bio)
 
dev_dbg(&dev->core, "%s\n", __func__);
 
-   blk_queue_split(q, &bio, q->bio_split);
+   blk_queue_split(q, &bio);
 
spin_lock_irq(&priv->lock);
busy = !bio_list_empty(&priv->list);
diff --git a/drivers/block/rsxx/dev.c b/drivers/block/rsxx/dev.c
index 9c566364ac9c..01624eaefcba 100644
--- a/drivers/block/rsxx/dev.c
+++ b/drivers/block/rsxx/dev.c
@@ -151,7 +151,7 @@ static blk_qc_t rsxx_make_request(struct request_queue *q, 
struct bio *bio)
struct rsxx_b