Re: [PATCH] dm-crypt: set max_io_len as chunk_sectors of zoned device
On 2024/11/25 14:11, Christoph Hellwig wrote: On Mon, Nov 25, 2024 at 03:07:24PM +0900, Damien Le Moal wrote: 100% agree. But in this case, dm-crypt sees a single BIO which gets split when submitted to the underlying device. But the split issues the requests in reverse order, which breaks write ordering. I'd like to see verification of this. People who send these kinds of odd old kernel reports tend to have weird patches applied to their tree as well. Without an upstream reproducer that actually shows issues I think this can be safely ignored. Thanks for your suggestions. I will try to make another patch of fault injection to reproduce it on qemu.
Re: [PATCH] dm-crypt: set max_io_len as chunk_sectors of zoned device
On 2024/11/25 14:01, Christoph Hellwig wrote: On Mon, Nov 25, 2024 at 05:58:22AM +, 韦龙平(Groot) wrote: What is ZWL? And why mention 6.6? And why do you care about ordering? ZWL is Zone-Write-Locking and it was replaced by Zone-Write-Plugging on 6.12. That's not a generally accepted acronym. But more importantly it's all gone now, so completely irrelevant. There is no real ordering requirement in the block layer. According to the design constraints of zoned device, write requests in the same zone must be dispatched in order of address and there can be no holes in the middle __bio_split_to_limits will split read bios with max_sectors too, split read bios in dm-level won't increase the number of requests dispatched to scsi devices. Yes. But none of that matters at the device mapper layer. Either the submitter needs to ensure ordering, or you must use zone append. Just like what was said in "dm crypt: Fix zoned block device support": Zone append BIOs (REQ_OP_ZONE_APPEND) always specify the start sector of the zone to be written instead of the actual sector location to write. The write location is determined by the device and returned to the host upon completion of the operation. This interface, while simple and efficient for writing into sequential zones of a zoned block device, is incompatible with the use of sector values to calculate a cypher block IV. All data written in a zone end up using the same IV values corresponding to the first sectors of the zone, but read operation will specify any sector within the zone resulting in an IV mismatch between encryption and decryption. To solve this problem, report to DM core that zone append operations are not supported. This result in the zone append operations being emulated using regular write operations. Zone append writes to zoned device will be converted to normal write requests and submitted one by one by zone-write-locking OR zone-write-plugging. This patch just want zone-append-emulation which was implemented in dm-level could works fine even if dm-crypt got a write or zone-append bio with size greater than max_sectors.
Re: [PATCH] dm-crypt: set max_io_len as chunk_sectors of zoned device
On Mon, Nov 25, 2024 at 03:07:24PM +0900, Damien Le Moal wrote: > 100% agree. But in this case, dm-crypt sees a single BIO which gets split when > submitted to the underlying device. But the split issues the requests in > reverse order, which breaks write ordering. I'd like to see verification of this. People who send these kinds of odd old kernel reports tend to have weird patches applied to their tree as well. Without an upstream reproducer that actually shows issues I think this can be safely ignored.
Re: [PATCH] dm-crypt: set max_io_len as chunk_sectors of zoned device
On 11/25/24 3:01 PM, Christoph Hellwig wrote: > On Mon, Nov 25, 2024 at 05:58:22AM +, 韦龙平(Groot) wrote: >>> What is ZWL? And why mention 6.6? And why do you care about ordering? >> ZWL is Zone-Write-Locking and it was replaced by Zone-Write-Plugging on 6.12. > > That's not a generally accepted acronym. But more importantly it's all > gone now, so completely irrelevant. > >> >>> There is no real ordering requirement in the block layer. >> According to the design constraints of zoned device, write requests in the >> same >> zone must be dispatched in order of address and there can be no holes in the >> middle > > Yes. But none of that matters at the device mapper layer. Either the > submitter needs to ensure ordering, or you must use zone append. 100% agree. But in this case, dm-crypt sees a single BIO which gets split when submitted to the underlying device. But the split issues the requests in reverse order, which breaks write ordering. So this seems like an issue with the block layer rather than dm-crypt. Though I do not understand why I have never seen this issue before, despite all the testing we do with all the zone block devices, dm and FS combinations we run every week (including f2fs). So I am not sure that the story is complete here... Never had to play with that dm max_io_len before to get things running correctly. -- Damien Le Moal Western Digital Research
Re: [PATCH] dm-crypt: set max_io_len as chunk_sectors of zoned device
On 11/25/24 2:58 PM, 韦龙平(Groot) wrote: > On Sun, 24 Nov 2024 21:00:52 -0800, Christoph Hellwig wrote: >> On Mon, Nov 25, 2024 at 11:22:51AM +0800, LongPing Wei wrote: >>> When dm level submit a bio with bi_size > max_sectors_kb of lower device, >>> blk_mq_submit_bio will split it into several ones with limited bi_size. >>> It may be a potential unordered issue as it breaks the ZWL design in dm >>> on 6.6. >> >> What is ZWL? And why mention 6.6? And why do you care about ordering? > ZWL is Zone-Write-Locking and it was replaced by Zone-Write-Plugging on 6.12. 6.10. >> There is no real ordering requirement in the block layer. > According to the design constraints of zoned device, write requests in the > same > zone must be dispatched in order of address and there can be no holes in the > middle -- Damien Le Moal Western Digital Research
Re: [PATCH] dm-crypt: set max_io_len as chunk_sectors of zoned device
On 11/25/24 2:00 PM, Christoph Hellwig wrote: > On Mon, Nov 25, 2024 at 11:22:51AM +0800, LongPing Wei wrote: >> When dm level submit a bio with bi_size > max_sectors_kb of lower device, >> blk_mq_submit_bio will split it into several ones with limited bi_size. >> It may be a potential unordered issue as it breaks the ZWL design in dm >> on 6.6. > > What is ZWL? And why mention 6.6? And why do you care about ordering? > There is no real ordering requirement in the block layer. > ZWL == zone write locking. That is, the mq-deadline request-based write request ordering control we had before zone write plugging. This is about ordering of write requests resulting from a split BIO. -- Damien Le Moal Western Digital Research
Re: [PATCH] dm-crypt: set max_io_len as chunk_sectors of zoned device
On Mon, Nov 25, 2024 at 05:58:22AM +, 韦龙平(Groot) wrote: > > What is ZWL? And why mention 6.6? And why do you care about ordering? > ZWL is Zone-Write-Locking and it was replaced by Zone-Write-Plugging on 6.12. That's not a generally accepted acronym. But more importantly it's all gone now, so completely irrelevant. > > > There is no real ordering requirement in the block layer. > According to the design constraints of zoned device, write requests in the > same > zone must be dispatched in order of address and there can be no holes in the > middle Yes. But none of that matters at the device mapper layer. Either the submitter needs to ensure ordering, or you must use zone append.
Re: [PATCH] dm-crypt: set max_io_len as chunk_sectors of zoned device
On Sun, 24 Nov 2024 21:00:52 -0800, Christoph Hellwig wrote: > On Mon, Nov 25, 2024 at 11:22:51AM +0800, LongPing Wei wrote: > > When dm level submit a bio with bi_size > max_sectors_kb of lower device, > > blk_mq_submit_bio will split it into several ones with limited bi_size. > > It may be a potential unordered issue as it breaks the ZWL design in dm > > on 6.6. > > What is ZWL? And why mention 6.6? And why do you care about ordering? ZWL is Zone-Write-Locking and it was replaced by Zone-Write-Plugging on 6.12. > There is no real ordering requirement in the block layer. According to the design constraints of zoned device, write requests in the same zone must be dispatched in order of address and there can be no holes in the middle
Re: [PATCH] dm-crypt: set max_io_len as chunk_sectors of zoned device
On Mon, Nov 25, 2024 at 11:22:51AM +0800, LongPing Wei wrote: > When dm level submit a bio with bi_size > max_sectors_kb of lower device, > blk_mq_submit_bio will split it into several ones with limited bi_size. > It may be a potential unordered issue as it breaks the ZWL design in dm > on 6.6. What is ZWL? And why mention 6.6? And why do you care about ordering? There is no real ordering requirement in the block layer.
