| Message ID | 158157930219.111879.12072477040351921368.stgit@localhost.localdomain (mailing list archive) |
|---|---|
| Headers | show |
| Series | block: Introduce REQ_ALLOCATE flag for REQ_OP_WRITE_ZEROES | expand |
Hi, Jens, could you please provide some comments on this? I sent v1 two months ago, and it would be great to know your vision of the functionality and the approach and whether it is going to go to block tree. Thanks, Kirill On 13.02.2020 10:39, Kirill Tkhai wrote: > (was "[PATCH block v2 0/3] block: Introduce REQ_NOZERO flag > for REQ_OP_WRITE_ZEROES operation"; > was "[PATCH RFC 0/3] block,ext4: Introduce REQ_OP_ASSIGN_RANGE > to reflect extents allocation in block device internals") > > v7: Two comments changed. > > v6: req_op() cosmetic change. > > v5: Kill dm|md patch, which disables REQ_ALLOCATE for these devices. > Disable REQ_ALLOCATE for all stacking devices instead of this. > > v4: Correct argument for mddev_check_write_zeroes(). > > v3: Rename REQ_NOZERO to REQ_ALLOCATE. > Split helpers to separate patches. > Add a patch, disabling max_allocate_sectors inheritance for dm. > > v2: Introduce new flag for REQ_OP_WRITE_ZEROES instead of > introduction a new operation as suggested by Martin K. Petersen. > Removed ext4-related patch to focus on block changes > for now. > > Information about continuous extent placement may be useful > for some block devices. Say, distributed network filesystems, > which provide block device interface, may use this information > for better blocks placement over the nodes in their cluster, > and for better performance. Block devices, which map a file > on another filesystem (loop), may request the same length extent > on underlining filesystem for less fragmentation and for batching > allocation requests. Also, hypervisors like QEMU may use this > information for optimization of cluster allocations. > > This patchset introduces REQ_ALLOCATE flag for REQ_OP_WRITE_ZEROES, > which makes a block device to allocate blocks instead of actual > blocks zeroing. This may be used for forwarding user's fallocate(0) > requests into block device internals. E.g., in loop driver this > will result in allocation extents in backing-file, so subsequent > write won't fail by the reason of no available space. Distributed > network filesystems will be able to assign specific servers for > specific extents, so subsequent write will be more efficient. > > Patches [1-3/6] are preparation on helper functions, patch [4/6] > introduces REQ_ALLOCATE flag and implements all the logic, > patch [5/6] adds one more helper, patch [6/6] adds loop > as the first user of the flag. > > Note, that here is only block-related patches, example of usage > for ext4 with a performance numbers may be seen in [1]. > > [1] https://lore.kernel.org/linux-ext4/157599697369.12112.10138136904533871162.stgit@localhost.localdomain/T/#me5bdd5cc313e14de615d81bea214f355ae975db0 > --- > > Kirill Tkhai (6): > block: Add @flags argument to bdev_write_zeroes_sectors() > block: Pass op_flags into blk_queue_get_max_sectors() > block: Introduce blk_queue_get_max_write_zeroes_sectors() > block: Add support for REQ_ALLOCATE flag > block: Add blk_queue_max_allocate_sectors() > loop: Add support for REQ_ALLOCATE > > > block/blk-core.c | 6 +++--- > block/blk-lib.c | 17 ++++++++++------- > block/blk-merge.c | 9 ++++++--- > block/blk-settings.c | 17 +++++++++++++++++ > drivers/block/loop.c | 20 +++++++++++++------- > drivers/md/dm-kcopyd.c | 2 +- > drivers/target/target_core_iblock.c | 4 ++-- > fs/block_dev.c | 4 ++++ > include/linux/blk_types.h | 6 ++++++ > include/linux/blkdev.h | 34 ++++++++++++++++++++++++++-------- > 10 files changed, 88 insertions(+), 31 deletions(-) > > -- > Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com> > Reviewed-by: Bob Liu <bob.liu@oracle.com> > -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
ping On 13.02.2020 10:55, Kirill Tkhai wrote: > Hi, Jens, > > could you please provide some comments on this? I sent v1 two months ago, > and it would be great to know your vision of the functionality and > the approach and whether it is going to go to block tree. > > Thanks, > Kirill > > On 13.02.2020 10:39, Kirill Tkhai wrote: >> (was "[PATCH block v2 0/3] block: Introduce REQ_NOZERO flag >> for REQ_OP_WRITE_ZEROES operation"; >> was "[PATCH RFC 0/3] block,ext4: Introduce REQ_OP_ASSIGN_RANGE >> to reflect extents allocation in block device internals") >> >> v7: Two comments changed. >> >> v6: req_op() cosmetic change. >> >> v5: Kill dm|md patch, which disables REQ_ALLOCATE for these devices. >> Disable REQ_ALLOCATE for all stacking devices instead of this. >> >> v4: Correct argument for mddev_check_write_zeroes(). >> >> v3: Rename REQ_NOZERO to REQ_ALLOCATE. >> Split helpers to separate patches. >> Add a patch, disabling max_allocate_sectors inheritance for dm. >> >> v2: Introduce new flag for REQ_OP_WRITE_ZEROES instead of >> introduction a new operation as suggested by Martin K. Petersen. >> Removed ext4-related patch to focus on block changes >> for now. >> >> Information about continuous extent placement may be useful >> for some block devices. Say, distributed network filesystems, >> which provide block device interface, may use this information >> for better blocks placement over the nodes in their cluster, >> and for better performance. Block devices, which map a file >> on another filesystem (loop), may request the same length extent >> on underlining filesystem for less fragmentation and for batching >> allocation requests. Also, hypervisors like QEMU may use this >> information for optimization of cluster allocations. >> >> This patchset introduces REQ_ALLOCATE flag for REQ_OP_WRITE_ZEROES, >> which makes a block device to allocate blocks instead of actual >> blocks zeroing. This may be used for forwarding user's fallocate(0) >> requests into block device internals. E.g., in loop driver this >> will result in allocation extents in backing-file, so subsequent >> write won't fail by the reason of no available space. Distributed >> network filesystems will be able to assign specific servers for >> specific extents, so subsequent write will be more efficient. >> >> Patches [1-3/6] are preparation on helper functions, patch [4/6] >> introduces REQ_ALLOCATE flag and implements all the logic, >> patch [5/6] adds one more helper, patch [6/6] adds loop >> as the first user of the flag. >> >> Note, that here is only block-related patches, example of usage >> for ext4 with a performance numbers may be seen in [1]. >> >> [1] https://lore.kernel.org/linux-ext4/157599697369.12112.10138136904533871162.stgit@localhost.localdomain/T/#me5bdd5cc313e14de615d81bea214f355ae975db0 >> --- >> >> Kirill Tkhai (6): >> block: Add @flags argument to bdev_write_zeroes_sectors() >> block: Pass op_flags into blk_queue_get_max_sectors() >> block: Introduce blk_queue_get_max_write_zeroes_sectors() >> block: Add support for REQ_ALLOCATE flag >> block: Add blk_queue_max_allocate_sectors() >> loop: Add support for REQ_ALLOCATE >> >> >> block/blk-core.c | 6 +++--- >> block/blk-lib.c | 17 ++++++++++------- >> block/blk-merge.c | 9 ++++++--- >> block/blk-settings.c | 17 +++++++++++++++++ >> drivers/block/loop.c | 20 +++++++++++++------- >> drivers/md/dm-kcopyd.c | 2 +- >> drivers/target/target_core_iblock.c | 4 ++-- >> fs/block_dev.c | 4 ++++ >> include/linux/blk_types.h | 6 ++++++ >> include/linux/blkdev.h | 34 ++++++++++++++++++++++++++-------- >> 10 files changed, 88 insertions(+), 31 deletions(-) >> >> -- >> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com> >> Reviewed-by: Bob Liu <bob.liu@oracle.com> >> > -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
I just don't understand the reason nothing happens :( I see newly-sent patches comes fast into block tree. But there is only silence... I grepped over Documentation, and there is no special rules about block tree. So, it looks like standard rules should be applyable. Some comments? Some requests for reworking? Some personal reasons to ignore my patches? On 06.03.2020 12:11, Kirill Tkhai wrote: > ping > > On 13.02.2020 10:55, Kirill Tkhai wrote: >> Hi, Jens, >> >> could you please provide some comments on this? I sent v1 two months ago, >> and it would be great to know your vision of the functionality and >> the approach and whether it is going to go to block tree. >> >> Thanks, >> Kirill >> >> On 13.02.2020 10:39, Kirill Tkhai wrote: >>> (was "[PATCH block v2 0/3] block: Introduce REQ_NOZERO flag >>> for REQ_OP_WRITE_ZEROES operation"; >>> was "[PATCH RFC 0/3] block,ext4: Introduce REQ_OP_ASSIGN_RANGE >>> to reflect extents allocation in block device internals") >>> >>> v7: Two comments changed. >>> >>> v6: req_op() cosmetic change. >>> >>> v5: Kill dm|md patch, which disables REQ_ALLOCATE for these devices. >>> Disable REQ_ALLOCATE for all stacking devices instead of this. >>> >>> v4: Correct argument for mddev_check_write_zeroes(). >>> >>> v3: Rename REQ_NOZERO to REQ_ALLOCATE. >>> Split helpers to separate patches. >>> Add a patch, disabling max_allocate_sectors inheritance for dm. >>> >>> v2: Introduce new flag for REQ_OP_WRITE_ZEROES instead of >>> introduction a new operation as suggested by Martin K. Petersen. >>> Removed ext4-related patch to focus on block changes >>> for now. >>> >>> Information about continuous extent placement may be useful >>> for some block devices. Say, distributed network filesystems, >>> which provide block device interface, may use this information >>> for better blocks placement over the nodes in their cluster, >>> and for better performance. Block devices, which map a file >>> on another filesystem (loop), may request the same length extent >>> on underlining filesystem for less fragmentation and for batching >>> allocation requests. Also, hypervisors like QEMU may use this >>> information for optimization of cluster allocations. >>> >>> This patchset introduces REQ_ALLOCATE flag for REQ_OP_WRITE_ZEROES, >>> which makes a block device to allocate blocks instead of actual >>> blocks zeroing. This may be used for forwarding user's fallocate(0) >>> requests into block device internals. E.g., in loop driver this >>> will result in allocation extents in backing-file, so subsequent >>> write won't fail by the reason of no available space. Distributed >>> network filesystems will be able to assign specific servers for >>> specific extents, so subsequent write will be more efficient. >>> >>> Patches [1-3/6] are preparation on helper functions, patch [4/6] >>> introduces REQ_ALLOCATE flag and implements all the logic, >>> patch [5/6] adds one more helper, patch [6/6] adds loop >>> as the first user of the flag. >>> >>> Note, that here is only block-related patches, example of usage >>> for ext4 with a performance numbers may be seen in [1]. >>> >>> [1] https://lore.kernel.org/linux-ext4/157599697369.12112.10138136904533871162.stgit@localhost.localdomain/T/#me5bdd5cc313e14de615d81bea214f355ae975db0 >>> --- >>> >>> Kirill Tkhai (6): >>> block: Add @flags argument to bdev_write_zeroes_sectors() >>> block: Pass op_flags into blk_queue_get_max_sectors() >>> block: Introduce blk_queue_get_max_write_zeroes_sectors() >>> block: Add support for REQ_ALLOCATE flag >>> block: Add blk_queue_max_allocate_sectors() >>> loop: Add support for REQ_ALLOCATE >>> >>> >>> block/blk-core.c | 6 +++--- >>> block/blk-lib.c | 17 ++++++++++------- >>> block/blk-merge.c | 9 ++++++--- >>> block/blk-settings.c | 17 +++++++++++++++++ >>> drivers/block/loop.c | 20 +++++++++++++------- >>> drivers/md/dm-kcopyd.c | 2 +- >>> drivers/target/target_core_iblock.c | 4 ++-- >>> fs/block_dev.c | 4 ++++ >>> include/linux/blk_types.h | 6 ++++++ >>> include/linux/blkdev.h | 34 ++++++++++++++++++++++++++-------- >>> 10 files changed, 88 insertions(+), 31 deletions(-) >>> >>> -- >>> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com> >>> Reviewed-by: Bob Liu <bob.liu@oracle.com> >>> >> > -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
On Fri, Mar 13, 2020 at 04:08:58PM +0300, Kirill Tkhai wrote: > I just don't understand the reason nothing happens :( > I see newly-sent patches comes fast into block tree. > But there is only silence... I grepped over Documentation, > and there is no special rules about block tree. So, > it looks like standard rules should be applyable. > > Some comments? Some requests for reworking? Some personal reasons to ignore my patches? I'm still completely opposed to the magic overloading using a flag. That is just a bad design waiting for trouble to happen. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
On 19.03.2020 13:28, Christoph Hellwig wrote: > On Fri, Mar 13, 2020 at 04:08:58PM +0300, Kirill Tkhai wrote: >> I just don't understand the reason nothing happens :( >> I see newly-sent patches comes fast into block tree. >> But there is only silence... I grepped over Documentation, >> and there is no special rules about block tree. So, >> it looks like standard rules should be applyable. >> >> Some comments? Some requests for reworking? Some personal reasons to ignore my patches? > > I'm still completely opposed to the magic overloading using a flag. > That is just a bad design waiting for trouble to happen. This flag is suggested by Martin Petersen, while the first version of the patchset was based on a separate operation. Since I see only Jens in MAINTAINERS: BLOCK LAYER M: Jens Axboe <axboe@kernel.dk> L: linux-block@vger.kernel.org T: git git://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git S: Maintained F: block/ F: drivers/block/ F: kernel/trace/blktrace.c F: lib/sbitmap.c I expect his comments about final design of this, because both you and Martin are maintainers of another subsystems. I don't want rework this many times until Jens says he wants some third design. I think I'm pretty polite and patient in my waiting, while Jens completely ignores me by some reasons, which are completely unclear for me. I don't think this is completely polite in relation to me. Kirill -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Christoph, >> Some comments? Some requests for reworking? Some personal reasons to >> ignore my patches? > > I'm still completely opposed to the magic overloading using a flag. > That is just a bad design waiting for trouble to happen. The observation was that Kirill's original patch set was a line-for-line carbon copy of the write zeroes handling throughout the stack. The only difference between the two was at the bottom. Instead of duplicating all that code it seemed cleaner to use shared plumbing since these operations need to be split and merged exactly the same way in the block layer. Also, we already have REQ_NOUNMAP, not sure why an additional handling flag would lead to trouble? Note that I suggested renaming REQ_OP_WRITE_ZEROES to something else to separate the semantics from the plumbing. We need to be able to express: - zero & allocate block range (REQ_OP_WRITE_ZEROES, REQ_NOUNMAP) - zero & deallocate block range (REQ_OP_WRITE_ZEROES, !REQ_NOUNMAP) - allocate block range (?, don't care about zeroing) - deallocate block range (REQ_OP_DISCARD, don't care about zeroing) It just seems like a full-fledged REQ_OP_ALLOCATE is an overkill to fill that gap. That said, I do think that we have traditionally put emphasis on the wrong part of these operations. All we ever talk about wrt. discard and friends is the zeroing aspect. But I actually think that, semantically, the act of allocating and deallocating blocks is more important. And that zeroing is an optional second order effect of those operations. So if we could go back in time and nuke multi-range DSM TRIM/UNMAP, I would like to have REQ_OP_ALLOCATE/REQ_OP_DEALLOCATE with an optional REQ_ZERO flag. I think that would be cleaner. I have a much easier time wrapping my head around "allocate this block and zero it if you can" than "zero this block and do not deallocate it". But maybe that's just me.
On Thu, Mar 19, 2020 at 09:03:40AM -0400, Martin K. Petersen wrote: > > Christoph, > > >> Some comments? Some requests for reworking? Some personal reasons to > >> ignore my patches? > > > > I'm still completely opposed to the magic overloading using a flag. > > That is just a bad design waiting for trouble to happen. > > The observation was that Kirill's original patch set was a line-for-line > carbon copy of the write zeroes handling throughout the stack. The only > difference between the two was at the bottom. Instead of duplicating all > that code it seemed cleaner to use shared plumbing since these > operations need to be split and merged exactly the same way in the block > layer. > > Also, we already have REQ_NOUNMAP, not sure why an additional handling > flag would lead to trouble? Note that I suggested renaming > REQ_OP_WRITE_ZEROES to something else to separate the semantics from the > plumbing. > > We need to be able to express: > > - zero & allocate block range (REQ_OP_WRITE_ZEROES, REQ_NOUNMAP) > - zero & deallocate block range (REQ_OP_WRITE_ZEROES, !REQ_NOUNMAP) > - allocate block range (?, don't care about zeroing) > - deallocate block range (REQ_OP_DISCARD, don't care about zeroing) > > It just seems like a full-fledged REQ_OP_ALLOCATE is an overkill to fill > that gap. > > That said, I do think that we have traditionally put emphasis on the > wrong part of these operations. All we ever talk about wrt. discard and > friends is the zeroing aspect. But I actually think that, semantically, > the act of allocating and deallocating blocks is more important. And > that zeroing is an optional second order effect of those operations. So > if we could go back in time and nuke multi-range DSM TRIM/UNMAP, I would > like to have REQ_OP_ALLOCATE/REQ_OP_DEALLOCATE with an optional REQ_ZERO > flag. I think that would be cleaner. I have a much easier time wrapping > my head around "allocate this block and zero it if you can" than "zero > this block and do not deallocate it". But maybe that's just me. I'd love to transition to that. My brain is not good at following all the inverse logic that NOUNMAP spread everywhere. I have a difficult time following what the blockdev fallocate code does, which is sad since hch and I are the primary stuckees^Wmeddlers^Wauthors of that function. :/ --D > -- > Martin K. Petersen Oracle Linux Engineering -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
On Wed, Mar 25, 2020 at 09:26:56AM -0700, Darrick J. Wong wrote: > > That said, I do think that we have traditionally put emphasis on the > > wrong part of these operations. All we ever talk about wrt. discard and > > friends is the zeroing aspect. But I actually think that, semantically, > > the act of allocating and deallocating blocks is more important. And > > that zeroing is an optional second order effect of those operations. So > > if we could go back in time and nuke multi-range DSM TRIM/UNMAP, I would > > like to have REQ_OP_ALLOCATE/REQ_OP_DEALLOCATE with an optional REQ_ZERO > > flag. I think that would be cleaner. I have a much easier time wrapping > > my head around "allocate this block and zero it if you can" than "zero > > this block and do not deallocate it". But maybe that's just me. > > I'd love to transition to that. My brain is not good at following all > the inverse logic that NOUNMAP spread everywhere. I have a difficult > time following what the blockdev fallocate code does, which is sad since > hch and I are the primary stuckees^Wmeddlers^Wauthors of that function. :/ I am very much against that for the following reason: - the current REQ_OP_DISCARD is purely a hint, and implementations can (and do) choose to ignore it - REQ_OP_WRITE_ZEROES is an actual data integrity operation with everything that entails Going back to mixing these two will lead to a disaster sooner or later. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Christoph, > I am very much against that for the following reason: > > - the current REQ_OP_DISCARD is purely a hint, and implementations can > (and do) choose to ignore it > > - REQ_OP_WRITE_ZEROES is an actual data integrity operation with > everything that entails If you want to keep emphasis on the "integrity operation" instead of the provisioning aspect, would you expect REQ_ALLOCATE (which may or may not zero blocks) to be considered a deterministic operation or a non-deterministic one? Should this depend on whether the device guarantees zeroing when provisioning blocks or not?
On Wed, Mar 25, 2020 at 01:23:33PM -0400, Martin K. Petersen wrote: > > Christoph, > > > I am very much against that for the following reason: > > > > - the current REQ_OP_DISCARD is purely a hint, and implementations can > > (and do) choose to ignore it > > > > - REQ_OP_WRITE_ZEROES is an actual data integrity operation with > > everything that entails > > If you want to keep emphasis on the "integrity operation" instead of the > provisioning aspect, would you expect REQ_ALLOCATE (which may or may not > zero blocks) to be considered a deterministic operation or a > non-deterministic one? Should this depend on whether the device > guarantees zeroing when provisioning blocks or not? That's why I don't like the whole flags game very much. I'd rather have REQ_OP_WRITE_ZEROES as the integrity operation that gurantees zeroing, and a REQ_ALLOCATE that doesn't guarantee zeroing, just some deterministic state of the blocks. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Christoph, > That's why I don't like the whole flags game very much. I'd rather > have REQ_OP_WRITE_ZEROES as the integrity operation that gurantees > zeroing, and a REQ_ALLOCATE that doesn't guarantee zeroing, just some > deterministic state of the blocks. I just worry about the proliferation of identical merging and splitting code throughout the block stack as we add additional single-range, no payload operations (Verify, etc.). I prefer to enforce the semantics in the LLD and not in the plumbing. But I won't object to a separate REQ_OP_ALLOCATE if you find the resulting code duplication acceptable.
On Thu, Mar 26, 2020 at 10:34:42AM -0400, Martin K. Petersen wrote: > I just worry about the proliferation of identical merging and splitting > code throughout the block stack as we add additional single-range, no > payload operations (Verify, etc.). I prefer to enforce the semantics in > the LLD and not in the plumbing. But I won't object to a separate > REQ_OP_ALLOCATE if you find the resulting code duplication acceptable. I find it acceptable for now. And I think we should find some way (e.g. by being table driven) to share code between differnet opcodes. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
On 03/26/2020 07:46 AM, Christoph Hellwig wrote: > On Thu, Mar 26, 2020 at 10:34:42AM -0400, Martin K. Petersen wrote: >>> I just worry about the proliferation of identical merging and >>> splitting code throughout the block stack as we add additional >>> single-range, no payload operations (Verify, etc.). I prefer to >>> enforce the semantics in the LLD and not in the plumbing. But I >>> won't object to a separate REQ_OP_ALLOCATE if you find the >>> resulting code duplication acceptable. > I find it acceptable for now. And I think we should find some way > (e.g. by being table driven) to share code between differnet > opcodes. > With reference to Martin's comment (verify etc) there is a significant advantage when using payloadless bio to offload the functionality to the directly attached device and over the fabrics when dealing with larger disks. How about we create a helper which is independent of the operations can accept req_op and issues the payloadless bios. Something like following totally untested :- diff --git a/block/blk-lib.c b/block/blk-lib.c index cf9e75a730b4..d3fccd3211cc 100644 --- a/block/blk-lib.c +++ b/block/blk-lib.c @@ -209,6 +209,33 @@ int blkdev_issue_write_same(struct block_device *bdev, sector_t sector, } EXPORT_SYMBOL(blkdev_issue_write_same); +static void __blkdev_issue_payloadless(struct block_device *bdev, unsigned op, + sector_t sector, sector_t nr_sects, gfp_t gfp_mask, + struct bio **biop, unsigned bio_opf, unsigned int max_sectors) +{ + struct bio *bio = *biop; + + while (nr_sects) { + bio = blk_next_bio(bio, 0, gfp_mask); + bio->bi_iter.bi_sector = sector; + bio_set_dev(bio, bdev); + bio->bi_opf = op; + bio->bi_opf |= bio_opf; + + if (nr_sects > max_sectors) { + bio->bi_iter.bi_size = max_sectors << 9; + nr_sects -= max_sectors; + sector += max_sectors; + } else { + bio->bi_iter.bi_size = nr_sects << 9; + nr_sects = 0; + } + cond_resched(); + } + + *biop = bio; +} + static int __blkdev_issue_write_zeroes(struct block_device *bdev, sector_t sector, sector_t nr_sects, gfp_t gfp_mask, struct bio **biop, unsigned flags) @@ -216,6 +243,7 @@ static int __blkdev_issue_write_zeroes(struct block_device *bdev, struct bio *bio = *biop; unsigned int max_write_zeroes_sectors; struct request_queue *q = bdev_get_queue(bdev); + unsigned int unmap = (flags & BLKDEV_ZERO_NOUNMAP) ? REQ_NOUNMAP : 0; if (!q) return -ENXIO; @@ -229,24 +257,8 @@ static int __blkdev_issue_write_zeroes(struct block_device *bdev, if (max_write_zeroes_sectors == 0) return -EOPNOTSUPP; - while (nr_sects) { - bio = blk_next_bio(bio, 0, gfp_mask); - bio->bi_iter.bi_sector = sector; - bio_set_dev(bio, bdev); - bio->bi_opf = REQ_OP_WRITE_ZEROES; - if (flags & BLKDEV_ZERO_NOUNMAP) - bio->bi_opf |= REQ_NOUNMAP; - - if (nr_sects > max_write_zeroes_sectors) { - bio->bi_iter.bi_size = max_write_zeroes_sectors << 9; - nr_sects -= max_write_zeroes_sectors; - sector += max_write_zeroes_sectors; - } else { - bio->bi_iter.bi_size = nr_sects << 9; - nr_sects = 0; - } - cond_resched(); - } + __blkdev_issue_payloadless(bdev, REQ_OP_WRITE_ZEROES, sector, nr_sects, + gfp_mask, biop, unmap, max_write_zeroes_sectors); *biop = bio; return 0; I'll be happy to send out a well tested patch based on the above suggestion or any feedback I get and re-spin this series or OP can re-spin this series whatever works. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
(was "[PATCH block v2 0/3] block: Introduce REQ_NOZERO flag for REQ_OP_WRITE_ZEROES operation"; was "[PATCH RFC 0/3] block,ext4: Introduce REQ_OP_ASSIGN_RANGE to reflect extents allocation in block device internals") v7: Two comments changed. v6: req_op() cosmetic change. v5: Kill dm|md patch, which disables REQ_ALLOCATE for these devices. Disable REQ_ALLOCATE for all stacking devices instead of this. v4: Correct argument for mddev_check_write_zeroes(). v3: Rename REQ_NOZERO to REQ_ALLOCATE. Split helpers to separate patches. Add a patch, disabling max_allocate_sectors inheritance for dm. v2: Introduce new flag for REQ_OP_WRITE_ZEROES instead of introduction a new operation as suggested by Martin K. Petersen. Removed ext4-related patch to focus on block changes for now. Information about continuous extent placement may be useful for some block devices. Say, distributed network filesystems, which provide block device interface, may use this information for better blocks placement over the nodes in their cluster, and for better performance. Block devices, which map a file on another filesystem (loop), may request the same length extent on underlining filesystem for less fragmentation and for batching allocation requests. Also, hypervisors like QEMU may use this information for optimization of cluster allocations. This patchset introduces REQ_ALLOCATE flag for REQ_OP_WRITE_ZEROES, which makes a block device to allocate blocks instead of actual blocks zeroing. This may be used for forwarding user's fallocate(0) requests into block device internals. E.g., in loop driver this will result in allocation extents in backing-file, so subsequent write won't fail by the reason of no available space. Distributed network filesystems will be able to assign specific servers for specific extents, so subsequent write will be more efficient. Patches [1-3/6] are preparation on helper functions, patch [4/6] introduces REQ_ALLOCATE flag and implements all the logic, patch [5/6] adds one more helper, patch [6/6] adds loop as the first user of the flag. Note, that here is only block-related patches, example of usage for ext4 with a performance numbers may be seen in [1]. [1] https://lore.kernel.org/linux-ext4/157599697369.12112.10138136904533871162.stgit@localhost.localdomain/T/#me5bdd5cc313e14de615d81bea214f355ae975db0 --- Kirill Tkhai (6): block: Add @flags argument to bdev_write_zeroes_sectors() block: Pass op_flags into blk_queue_get_max_sectors() block: Introduce blk_queue_get_max_write_zeroes_sectors() block: Add support for REQ_ALLOCATE flag block: Add blk_queue_max_allocate_sectors() loop: Add support for REQ_ALLOCATE block/blk-core.c | 6 +++--- block/blk-lib.c | 17 ++++++++++------- block/blk-merge.c | 9 ++++++--- block/blk-settings.c | 17 +++++++++++++++++ drivers/block/loop.c | 20 +++++++++++++------- drivers/md/dm-kcopyd.c | 2 +- drivers/target/target_core_iblock.c | 4 ++-- fs/block_dev.c | 4 ++++ include/linux/blk_types.h | 6 ++++++ include/linux/blkdev.h | 34 ++++++++++++++++++++++++++-------- 10 files changed, 88 insertions(+), 31 deletions(-) -- Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com> Reviewed-by: Bob Liu <bob.liu@oracle.com> -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel