Skip to contentSkip to conversation

rbd: implement REQ_OP_WRITE_ZEROES

PATCH In review Ilya Dryomov 4 participants 8 messages May 23, 2017 – May 29, 2017

Subsystems (2)
Block layer RADOS block device (RBD)
lore
Share & tools
lore

Ilya Dryomov

author maintainer
#0 lore raw

PATCH 1/1 rbd: implement REQ_OP_WRITE_ZEROES

Commit 93c1defedcae ("rbd: remove the discard_zeroes_data flag") explicitly didn't implement REQ_OP_WRITE_ZEROES for rbd, while the following commit 48920ff2a5a9 ("block: remove the discard_zeroes_data flag") dropped ->discard_zeroes_data in favor of REQ_OP_WRITE_ZEROES.

rbd does support efficient zeroing via CEPH_OSD_OP_ZERO opcode and will release either some or all blocks depending on whether the zeroing request is rbd_obj_bytes() aligned. This is how we currently implement discards, so REQ_OP_WRITE_ZEROES can be identical to REQ_OP_DISCARD for now. Caveats:

- REQ_NOUNMAP is ignored, but AFAICT that's true of at least two other current implementations - nvme and loop

- there is no ->write_zeroes_alignment and blk_bio_write_zeroes_split() is hence less helpful than blk_bio_discard_split(), but this can (and should) be fixed on the rbd side

In the future we will split these into two code paths to respect REQ_NOUNMAP on zeroout and save on zeroing blocks that couldn't be released on discard.

Fixes 93c1defedcae ("rbd: remove the discard_zeroes_data flag")
Signed-off-by Ilya Dryomov <[email protected]>
---
 drivers/block/rbd.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/block/rbd.c +2 -0
index 454bf9c34882..c16f74547804 100644
@@ -4023,6 +4023,7 @@ static void rbd_queue_workfn(struct work_struct *work)
 
 	switch (req_op(rq)) {
 	case REQ_OP_DISCARD:
+	case REQ_OP_WRITE_ZEROES:
 		op_type = OBJ_OP_DISCARD;
 		break;
 	case REQ_OP_WRITE:

@@ -4420,6 +4421,7 @@ static int rbd_init_disk(struct rbd_device *rbd_dev)
 	q->limits.discard_granularity = segment_size;
 	q->limits.discard_alignment = segment_size;
 	blk_queue_max_discard_sectors(q, segment_size / SECTOR_SIZE);
+	blk_queue_max_write_zeroes_sectors(q, segment_size / SECTOR_SIZE);
 
 	if (!ceph_test_opt(rbd_dev->rbd_client->client, NOCRC))
 		q->backing_dev_info->capabilities |= BDI_CAP_STABLE_WRITES;
Signature · 1 line
2.4.3
Reviewed-by on v1 1/1
#1 lore raw

lgtm

Reviewed-by Jason Dillaman <[email protected]>
Quoted 55 lines · Ilya Dryomov

Commit 93c1defedcae ("rbd: remove the discard_zeroes_data flag") explicitly didn't implement REQ_OP_WRITE_ZEROES for rbd, while the following commit 48920ff2a5a9 ("block: remove the discard_zeroes_data flag") dropped ->discard_zeroes_data in favor of REQ_OP_WRITE_ZEROES.

rbd does support efficient zeroing via CEPH_OSD_OP_ZERO opcode and will release either some or all blocks depending on whether the zeroing request is rbd_obj_bytes() aligned. This is how we currently implement discards, so REQ_OP_WRITE_ZEROES can be identical to REQ_OP_DISCARD for now. Caveats:

- REQ_NOUNMAP is ignored, but AFAICT that's true of at least two other current implementations - nvme and loop

- there is no ->write_zeroes_alignment and blk_bio_write_zeroes_split() is hence less helpful than blk_bio_discard_split(), but this can (and should) be fixed on the rbd side

In the future we will split these into two code paths to respect REQ_NOUNMAP on zeroout and save on zeroing blocks that couldn't be released on discard.

Fixes 93c1defedcae ("rbd: remove the discard_zeroes_data flag")
Signed-off-by Ilya Dryomov <[email protected]>
---
 drivers/block/rbd.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/block/rbd.c +2 -1
index 454bf9c34882..c16f74547804 100644
@@ -4023,6 +4023,7 @@ static void rbd_queue_workfn(struct work_struct *work)
 
        switch (req_op(rq)) {
        case REQ_OP_DISCARD:
+       case REQ_OP_WRITE_ZEROES:
                op_type = OBJ_OP_DISCARD;
                break;
        case REQ_OP_WRITE:

@@ -4420,6 +4421,7 @@ static int rbd_init_disk(struct rbd_device *rbd_dev)
        q->limits.discard_granularity = segment_size;
        q->limits.discard_alignment = segment_size;
        blk_queue_max_discard_sectors(q, segment_size / SECTOR_SIZE);
+       blk_queue_max_write_zeroes_sectors(q, segment_size / SECTOR_SIZE);
 
        if (!ceph_test_opt(rbd_dev->rbd_client->client, NOCRC))
                q->backing_dev_info->capabilities |= BDI_CAP_STABLE_WRITES;
--

2.4.3

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 1 line
Jason
Reviewed-by Jason Dillaman
#2 lore raw

Hi,

is it planned to implement write zeroes in qemu rbd block driver soon ? (bdrv_co_write_zeroes)

It's really missing currently, as qemu drive-mirror need it to have sparse images on copy.

Ref from my discussion with Paolo from redhat in 2014 about this:
https://lists.gnu.org/archive/html/qemu-devel/2014-10/msg01274.html

REgards,

Alexandre

----- Mail original -----
De: "Jason Dillaman" <[email protected]>
À: "Ilya Dryomov" <[email protected]>
Cc: "ceph-devel" <[email protected]>, "Christoph Hellwig" <[email protected]>, "Hannes Reinecke" <[email protected]> Envoyé: Mardi 23 Mai 2017 20:28:00
Objet: Re: [PATCH] rbd: implement REQ_OP_WRITE_ZEROES

lgtm

Reviewed-by Jason Dillaman <[email protected]>
Quoted 55 lines · Ilya Dryomov

Commit 93c1defedcae ("rbd: remove the discard_zeroes_data flag") explicitly didn't implement REQ_OP_WRITE_ZEROES for rbd, while the following commit 48920ff2a5a9 ("block: remove the discard_zeroes_data flag") dropped ->discard_zeroes_data in favor of REQ_OP_WRITE_ZEROES.

rbd does support efficient zeroing via CEPH_OSD_OP_ZERO opcode and will release either some or all blocks depending on whether the zeroing request is rbd_obj_bytes() aligned. This is how we currently implement discards, so REQ_OP_WRITE_ZEROES can be identical to REQ_OP_DISCARD for now. Caveats:

- REQ_NOUNMAP is ignored, but AFAICT that's true of at least two other current implementations - nvme and loop

- there is no ->write_zeroes_alignment and blk_bio_write_zeroes_split() is hence less helpful than blk_bio_discard_split(), but this can (and should) be fixed on the rbd side

In the future we will split these into two code paths to respect REQ_NOUNMAP on zeroout and save on zeroing blocks that couldn't be released on discard.

Fixes 93c1defedcae ("rbd: remove the discard_zeroes_data flag")
Signed-off-by Ilya Dryomov <[email protected]>

---
drivers/block/rbd.c | 2 ++

1 file changed, 2 insertions(+)
drivers/block/rbd.c +0 -0
index 454bf9c34882..c16f74547804 100644 
@@ -4023,6 +4023,7 @@ static void rbd_queue_workfn(struct work_struct *work) 
 
 switch (req_op(rq)) {

case REQ_OP_DISCARD:
+ case REQ_OP_WRITE_ZEROES:
op_type = OBJ_OP_DISCARD;
break;
case REQ_OP_WRITE:

drivers/block/rbd.c +1 -0
@@ -4420,6 +4421,7 @@ static int rbd_init_disk(struct rbd_device *rbd_dev) 
 q->limits.discard_granularity = segment_size; 
 q->limits.discard_alignment = segment_size; 
 blk_queue_max_discard_sectors(q, segment_size / SECTOR_SIZE); 
+ blk_queue_max_write_zeroes_sectors(q, segment_size / SECTOR_SIZE); 
 
 if (!ceph_test_opt(rbd_dev->rbd_client->client, NOCRC)) 
 q->backing_dev_info->capabilities |= BDI_CAP_STABLE_WRITES;
Signature · 6 lines
2.4.3 

-- 
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in 
the body of a message to [email protected] 
More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 5 lines
Jason 
-- 
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in 
the body of a message to [email protected] 
More majordomo info at http://vger.kernel.org/majordomo-info.html
#3 lore raw

I just opened a tracker ticket for this [1] -- let me know if you have any other QEMU improvement ideas.

[1] http://tracker.ceph.com/issues/20070

Quoted 96 lines · Alexandre DERUMIER

Hi,

is it planned to implement write zeroes in qemu rbd block driver soon ? (bdrv_co_write_zeroes)

It's really missing currently, as qemu drive-mirror need it to have sparse images on copy.

Ref from my discussion with Paolo from redhat in 2014 about this:
https://lists.gnu.org/archive/html/qemu-devel/2014-10/msg01274.html

REgards,

Alexandre

----- Mail original -----
De: "Jason Dillaman" <[email protected]>
À: "Ilya Dryomov" <[email protected]>
Cc: "ceph-devel" <[email protected]>, "Christoph Hellwig" <[email protected]>, "Hannes Reinecke" <[email protected]> Envoyé: Mardi 23 Mai 2017 20:28:00
Objet: Re: [PATCH] rbd: implement REQ_OP_WRITE_ZEROES

lgtm

Reviewed-by Jason Dillaman <[email protected]>
Quoted 51 lines · Ilya Dryomov

Commit 93c1defedcae ("rbd: remove the discard_zeroes_data flag") explicitly didn't implement REQ_OP_WRITE_ZEROES for rbd, while the following commit 48920ff2a5a9 ("block: remove the discard_zeroes_data flag") dropped ->discard_zeroes_data in favor of REQ_OP_WRITE_ZEROES.

rbd does support efficient zeroing via CEPH_OSD_OP_ZERO opcode and will release either some or all blocks depending on whether the zeroing request is rbd_obj_bytes() aligned. This is how we currently implement discards, so REQ_OP_WRITE_ZEROES can be identical to REQ_OP_DISCARD for now. Caveats:

- REQ_NOUNMAP is ignored, but AFAICT that's true of at least two other current implementations - nvme and loop

- there is no ->write_zeroes_alignment and blk_bio_write_zeroes_split() is hence less helpful than blk_bio_discard_split(), but this can (and should) be fixed on the rbd side

In the future we will split these into two code paths to respect REQ_NOUNMAP on zeroout and save on zeroing blocks that couldn't be released on discard.

Fixes 93c1defedcae ("rbd: remove the discard_zeroes_data flag")
Signed-off-by Ilya Dryomov <[email protected]>

---
drivers/block/rbd.c | 2 ++

1 file changed, 2 insertions(+)
drivers/block/rbd.c +0 -0
index 454bf9c34882..c16f74547804 100644
@@ -4023,6 +4023,7 @@ static void rbd_queue_workfn(struct work_struct *work)
 
 switch (req_op(rq)) {

case REQ_OP_DISCARD:
+ case REQ_OP_WRITE_ZEROES:
op_type = OBJ_OP_DISCARD;
break;
case REQ_OP_WRITE:

drivers/block/rbd.c +1 -1
@@ -4420,6 +4421,7 @@ static int rbd_init_disk(struct rbd_device *rbd_dev)
 q->limits.discard_granularity = segment_size;
 q->limits.discard_alignment = segment_size;
 blk_queue_max_discard_sectors(q, segment_size / SECTOR_SIZE);
+ blk_queue_max_write_zeroes_sectors(q, segment_size / SECTOR_SIZE);
 
 if (!ceph_test_opt(rbd_dev->rbd_client->client, NOCRC))
 q->backing_dev_info->capabilities |= BDI_CAP_STABLE_WRITES;
--

2.4.3

Quoted 4 lines

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Jason
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 1 line
Jason
#4 lore raw
Quoted 3 lines
Quoted 3 lines

I just opened a tracker ticket for this [1]

[1] http://tracker.ceph.com/issues/20070

Thanks Jason!

Quoted 2 lines
Quoted 2 lines

-- let me know if you have
any other QEMU improvement ideas.

For the moment, the bigger limitation is cpu usage of librbd,
as qemu can only 1 thread, I can't reach more than around 70000 iops by disk. (3,1ghz cpu, disabling debug, rbd_cache, using jemalloc).

So any improvement to reduce cpu usage could be great :)

Also, in the future, I think qemu will support multiple iothreads by disk, I don't known if librbd is already ready for this ?

----- Mail original -----
De: "Jason Dillaman" <[email protected]>
À: "aderumier" <[email protected]>
Cc: "Ilya Dryomov" <[email protected]>, "ceph-devel" <[email protected]>, "Christoph Hellwig" <[email protected]>, "Hannes Reinecke" <[email protected]> Envoyé: Mercredi 24 Mai 2017 13:53:40
Objet: Re: [PATCH] rbd: implement REQ_OP_WRITE_ZEROES

I just opened a tracker ticket for this [1] -- let me know if you have any other QEMU improvement ideas.

[1] http://tracker.ceph.com/issues/20070

Quoted 96 lines · Alexandre DERUMIER

Hi,

is it planned to implement write zeroes in qemu rbd block driver soon ? (bdrv_co_write_zeroes)

It's really missing currently, as qemu drive-mirror need it to have sparse images on copy.

Ref from my discussion with Paolo from redhat in 2014 about this:
https://lists.gnu.org/archive/html/qemu-devel/2014-10/msg01274.html

REgards,

Alexandre

----- Mail original -----
De: "Jason Dillaman" <[email protected]>
À: "Ilya Dryomov" <[email protected]>
Cc: "ceph-devel" <[email protected]>, "Christoph Hellwig" <[email protected]>, "Hannes Reinecke" <[email protected]> Envoyé: Mardi 23 Mai 2017 20:28:00
Objet: Re: [PATCH] rbd: implement REQ_OP_WRITE_ZEROES

lgtm

Reviewed-by Jason Dillaman <[email protected]>
Quoted 55 lines · Ilya Dryomov

Commit 93c1defedcae ("rbd: remove the discard_zeroes_data flag") explicitly didn't implement REQ_OP_WRITE_ZEROES for rbd, while the following commit 48920ff2a5a9 ("block: remove the discard_zeroes_data flag") dropped ->discard_zeroes_data in favor of REQ_OP_WRITE_ZEROES.

rbd does support efficient zeroing via CEPH_OSD_OP_ZERO opcode and will release either some or all blocks depending on whether the zeroing request is rbd_obj_bytes() aligned. This is how we currently implement discards, so REQ_OP_WRITE_ZEROES can be identical to REQ_OP_DISCARD for now. Caveats:

- REQ_NOUNMAP is ignored, but AFAICT that's true of at least two other current implementations - nvme and loop

- there is no ->write_zeroes_alignment and blk_bio_write_zeroes_split() is hence less helpful than blk_bio_discard_split(), but this can (and should) be fixed on the rbd side

In the future we will split these into two code paths to respect REQ_NOUNMAP on zeroout and save on zeroing blocks that couldn't be released on discard.

Fixes 93c1defedcae ("rbd: remove the discard_zeroes_data flag")
Signed-off-by Ilya Dryomov <[email protected]>

---
drivers/block/rbd.c | 2 ++

1 file changed, 2 insertions(+)
drivers/block/rbd.c +0 -0
index 454bf9c34882..c16f74547804 100644 
@@ -4023,6 +4023,7 @@ static void rbd_queue_workfn(struct work_struct *work) 
 
 switch (req_op(rq)) {

case REQ_OP_DISCARD:
+ case REQ_OP_WRITE_ZEROES:
op_type = OBJ_OP_DISCARD;
break;
case REQ_OP_WRITE:

drivers/block/rbd.c +1 -0
@@ -4420,6 +4421,7 @@ static int rbd_init_disk(struct rbd_device *rbd_dev) 
 q->limits.discard_granularity = segment_size; 
 q->limits.discard_alignment = segment_size; 
 blk_queue_max_discard_sectors(q, segment_size / SECTOR_SIZE); 
+ blk_queue_max_write_zeroes_sectors(q, segment_size / SECTOR_SIZE); 
 
 if (!ceph_test_opt(rbd_dev->rbd_client->client, NOCRC)) 
 q->backing_dev_info->capabilities |= BDI_CAP_STABLE_WRITES;
Signature · 6 lines
2.4.3 

-- 
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in 
the body of a message to [email protected] 
More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 10 lines
Jason 
-- 
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in 
the body of a message to [email protected] 
More majordomo info at http://vger.kernel.org/majordomo-info.html 

-- 
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in 
the body of a message to [email protected] 
More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 1 line
Jason
#5 lore raw
Quoted 6 lines · Alexandre DERUMIER
Quoted 3 lines
Quoted 3 lines

I just opened a tracker ticket for this [1]

[1] http://tracker.ceph.com/issues/20070

Thanks Jason!

Quoted 8 lines
Quoted 2 lines
Quoted 2 lines

-- let me know if you have
any other QEMU improvement ideas.

For the moment, the bigger limitation is cpu usage of librbd,
as qemu can only 1 thread, I can't reach more than around 70000 iops by disk. (3,1ghz cpu, disabling debug, rbd_cache, using jemalloc).

So any improvement to reduce cpu usage could be great :)

We've had some interest expressed in volunteering time and code contributions to reduce the CPU usage of librbd / librados. I am hoping some of these improvements will be included in the Mimic release.

Quoted 2 lines

Also, in the future, I think qemu will support multiple iothreads by disk, I don't known if librbd is already ready for this ?

This is hopefully another improvement we can make in the Mimic release timeframe. librbd already has an architecture to support multiple internal IO threads, but we have it disabled for now due to lack of thorough testing -- and for the fact that librados needs to utilize multiple IO threads as well.

Quoted 117 lines

----- Mail original -----
De: "Jason Dillaman" <[email protected]>
À: "aderumier" <[email protected]>
Cc: "Ilya Dryomov" <[email protected]>, "ceph-devel" <[email protected]>, "Christoph Hellwig" <[email protected]>, "Hannes Reinecke" <[email protected]> Envoyé: Mercredi 24 Mai 2017 13:53:40
Objet: Re: [PATCH] rbd: implement REQ_OP_WRITE_ZEROES

I just opened a tracker ticket for this [1] -- let me know if you have any other QEMU improvement ideas.

[1] http://tracker.ceph.com/issues/20070

Quoted 92 lines · Alexandre DERUMIER

Hi,

is it planned to implement write zeroes in qemu rbd block driver soon ? (bdrv_co_write_zeroes)

It's really missing currently, as qemu drive-mirror need it to have sparse images on copy.

Ref from my discussion with Paolo from redhat in 2014 about this:
https://lists.gnu.org/archive/html/qemu-devel/2014-10/msg01274.html

REgards,

Alexandre

----- Mail original -----
De: "Jason Dillaman" <[email protected]>
À: "Ilya Dryomov" <[email protected]>
Cc: "ceph-devel" <[email protected]>, "Christoph Hellwig" <[email protected]>, "Hannes Reinecke" <[email protected]> Envoyé: Mardi 23 Mai 2017 20:28:00
Objet: Re: [PATCH] rbd: implement REQ_OP_WRITE_ZEROES

lgtm

Reviewed-by Jason Dillaman <[email protected]>
Quoted 51 lines · Ilya Dryomov

Commit 93c1defedcae ("rbd: remove the discard_zeroes_data flag") explicitly didn't implement REQ_OP_WRITE_ZEROES for rbd, while the following commit 48920ff2a5a9 ("block: remove the discard_zeroes_data flag") dropped ->discard_zeroes_data in favor of REQ_OP_WRITE_ZEROES.

rbd does support efficient zeroing via CEPH_OSD_OP_ZERO opcode and will release either some or all blocks depending on whether the zeroing request is rbd_obj_bytes() aligned. This is how we currently implement discards, so REQ_OP_WRITE_ZEROES can be identical to REQ_OP_DISCARD for now. Caveats:

- REQ_NOUNMAP is ignored, but AFAICT that's true of at least two other current implementations - nvme and loop

- there is no ->write_zeroes_alignment and blk_bio_write_zeroes_split() is hence less helpful than blk_bio_discard_split(), but this can (and should) be fixed on the rbd side

In the future we will split these into two code paths to respect REQ_NOUNMAP on zeroout and save on zeroing blocks that couldn't be released on discard.

Fixes 93c1defedcae ("rbd: remove the discard_zeroes_data flag")
Signed-off-by Ilya Dryomov <[email protected]>

---
drivers/block/rbd.c | 2 ++

1 file changed, 2 insertions(+)
drivers/block/rbd.c +0 -0
index 454bf9c34882..c16f74547804 100644
@@ -4023,6 +4023,7 @@ static void rbd_queue_workfn(struct work_struct *work)
 
 switch (req_op(rq)) {

case REQ_OP_DISCARD:
+ case REQ_OP_WRITE_ZEROES:
op_type = OBJ_OP_DISCARD;
break;
case REQ_OP_WRITE:

drivers/block/rbd.c +1 -1
@@ -4420,6 +4421,7 @@ static int rbd_init_disk(struct rbd_device *rbd_dev)
 q->limits.discard_granularity = segment_size;
 q->limits.discard_alignment = segment_size;
 blk_queue_max_discard_sectors(q, segment_size / SECTOR_SIZE);
+ blk_queue_max_write_zeroes_sectors(q, segment_size / SECTOR_SIZE);
 
 if (!ceph_test_opt(rbd_dev->rbd_client->client, NOCRC))
 q->backing_dev_info->capabilities |= BDI_CAP_STABLE_WRITES;
--

2.4.3

Quoted 4 lines

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Jason
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 4 lines

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Jason

Signature · 1 line
Jason
author maintainer
#6 lore raw
In reply to Ilya Dryomov
Quoted 41 lines · Ilya Dryomov

Commit 93c1defedcae ("rbd: remove the discard_zeroes_data flag") explicitly didn't implement REQ_OP_WRITE_ZEROES for rbd, while the following commit 48920ff2a5a9 ("block: remove the discard_zeroes_data flag") dropped ->discard_zeroes_data in favor of REQ_OP_WRITE_ZEROES.

rbd does support efficient zeroing via CEPH_OSD_OP_ZERO opcode and will release either some or all blocks depending on whether the zeroing request is rbd_obj_bytes() aligned. This is how we currently implement discards, so REQ_OP_WRITE_ZEROES can be identical to REQ_OP_DISCARD for now. Caveats:

- REQ_NOUNMAP is ignored, but AFAICT that's true of at least two other current implementations - nvme and loop

- there is no ->write_zeroes_alignment and blk_bio_write_zeroes_split() is hence less helpful than blk_bio_discard_split(), but this can (and should) be fixed on the rbd side

In the future we will split these into two code paths to respect REQ_NOUNMAP on zeroout and save on zeroing blocks that couldn't be released on discard.

Fixes 93c1defedcae ("rbd: remove the discard_zeroes_data flag")
Signed-off-by Ilya Dryomov <[email protected]>
---
 drivers/block/rbd.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/block/rbd.c +1 -0
index 454bf9c34882..c16f74547804 100644
@@ -4023,6 +4023,7 @@ static void rbd_queue_workfn(struct work_struct *work)
 
        switch (req_op(rq)) {
        case REQ_OP_DISCARD:
+       case REQ_OP_WRITE_ZEROES:
                op_type = OBJ_OP_DISCARD;
                break;
        case REQ_OP_WRITE:

drivers/block/rbd.c static int rbd_init_disk(struct rbd_device *rbd_dev) · line 4427

Quoted 7 lines
drivers/block/rbd.c +1 -0
@@ -4420,6 +4421,7 @@ static int rbd_init_disk(struct rbd_device *rbd_dev)
        q->limits.discard_granularity = segment_size;
        q->limits.discard_alignment = segment_size;
        blk_queue_max_discard_sectors(q, segment_size / SECTOR_SIZE);
+       blk_queue_max_write_zeroes_sectors(q, segment_size / SECTOR_SIZE);
 
        if (!ceph_test_opt(rbd_dev->rbd_client->client, NOCRC))
                q->backing_dev_info->capabilities |= BDI_CAP_STABLE_WRITES;

Hi Christoph,

I'm planning to merge this into 4.12-rc because it fixes a 93c1defedcae regression, but I'm not quite sure what you meant by "rbd only supports discarding on large alignments, so the zeroing code would always fall back to explicit writings of zeroes". Care to take a look and ack?

Thanks,

Ilya

Reviewed-by on v1 1/1
#7 lore raw
In reply to Ilya Dryomov
Quoted 5 lines · Ilya Dryomov

Commit 93c1defedcae ("rbd: remove the discard_zeroes_data flag") explicitly didn't implement REQ_OP_WRITE_ZEROES for rbd, while the following commit 48920ff2a5a9 ("block: remove the discard_zeroes_data flag") dropped ->discard_zeroes_data in favor of REQ_OP_WRITE_ZEROES.

Quoted 8 lines

rbd does support efficient zeroing via CEPH_OSD_OP_ZERO opcode and will release either some or all blocks depending on whether the zeroing request is rbd_obj_bytes() aligned. This is how we currently implement discards, so REQ_OP_WRITE_ZEROES can be identical to REQ_OP_DISCARD for now. Caveats:

- REQ_NOUNMAP is ignored, but AFAICT that's true of at least two other current implementations - nvme and loop

That's no problem, it's just a hint in case the device differenciates the cases.

Quoted 3 lines

- there is no ->write_zeroes_alignment and blk_bio_write_zeroes_split() is hence less helpful than blk_bio_discard_split(), but this can (and should) be fixed on the rbd side

Yes.

Quoted 6 lines

In the future we will split these into two code paths to respect REQ_NOUNMAP on zeroout and save on zeroing blocks that couldn't be released on discard.

Fixes 93c1defedcae ("rbd: remove the discard_zeroes_data flag")
Signed-off-by Ilya Dryomov <[email protected]>

Looks fine,

Reviewed-by Christoph Hellwig <[email protected]>