On 2020/09/25 4:14, Sudhakar Panneerselvam wrote: >> >> On Thu, 24 Sep 2020, Sudhakar Panneerselvam wrote: >> >>>> By copying it to a temporary aligned buffer and issuing I/O on this >>>> buffer. >>> >>> I don't like this idea. Because, you need to allocate additional pages >>> for the entire I/O size(for the misaligned case, if you think through >> >> You can break the I/O to smaller pieces. You can use mempool for >> pre-allocation of the pages. > > Assuming we do this, how is this code simpler(based on your > comment below) than the fix in dm-crypt? In fact, this approach > would make the code change look bad in vhost, at the same time > having performance penalty. By doing this, we are just moving the > responsibility to other unrelated component. Because vhost is at the top of the block-io food chain. Fixing the unaligned segments there will ensure that it does not matter what device is under it. It will work. I am still baffled that the unaligned segments go through in the first place... Do we have something missing in the BIO code ? > >> >>> carefully, you will know why we have to allocate a temporary buffer that >>> is as big as the original IO) and on top of it, copying the buffer from >>> original to temporary buffer which is all unnecessary while it can >>> simply be fixed in dm-crypt without any of these additional overheads. >>> >>>> >>>>> Only other >>>>> possibility I see is to have windows fix it by always sending 512 byte >>>>> aligned buffer lengths, but going with my earlier point that every other >>>>> component in the Linux IO path handles this case well except for >>>>> dm-crypt, so it make more sense to fix it in dm-crypt. >>>>> >>>>> Thanks >>>>> Sudhakar >>>> >>>> Are you sure that the problem is only with dm-crypt? You haven't tried >> all >>>> the existing block device drivers, have you? >>> >>> My question is, why dm-crypt worries about alignment requirement of >>> other layers? Is there anything that impacts dm-crypt if the segment >>> lengths are not aligned?(I believe this case is just not handled so far >> >> Because the code is simpler if it assumes aligned buffers. > > Did you get a chance to review my changes? If you want more documentation, > improve the code, etc, let me know, I can do that if there is scope for that. > >> >>> in dm-crypt and my patch addresses it). Should dm-crypt not just pass on >>> all those I/O requests to those respective layers to handle it which >>> will be more graceful? >>> >>> -Sudhakar >> >> So, suppose that we change dm-crypt to handle your workload - what are >> you >> going to do with other block device drivers that assume aligned bio vector >> length? How will you find all the other drivers that need to be changed? >> >> You are claiming that "every other component in the Linux IO path handles >> this case well except for dm-crypt", but this claim is wrong. There are >> other driver that don't handle misaligned length as well. > > I should not have said, "every other component", I take that back, sorry. How > about doing something like this in crypt_convert_block_skcipher(): > > Add a check that looks at the alignment requirement of the low-level driver > and reject the I/O if it doesn't meet that requirement. This means, we still > need to handle the case in this function where the low lever driver support > unaligned buffer lengths, that means, my other changes in this function > would still be needed. > > Is this acceptable to everyone? > > -Sudhakar > >> >> Mikulas >> >> -- >> dm-devel mailing list >> dm-devel@xxxxxxxxxx >> https://www.redhat.com/mailman/listinfo/dm-devel >> > -- Damien Le Moal Western Digital Research _______________________________________________ dm-crypt mailing list dm-crypt@xxxxxxxx https://www.saout.de/mailman/listinfo/dm-crypt
