On Thursday, April 5, 2018 6:17:45 PM IST Theodore Y. Ts'o wrote: > On Thu, Apr 05, 2018 at 12:33:26PM +0530, Chandan Rajendra wrote: > > > > I encountered a problem when refactoring the code to get fscrypt layer to > > encrypt all the blocks of a page by internally calling > > fscrypt_encrypt_block(). > > > > It is the filesystem which knows which subset of blocks mapped by a page that > > needs to be encrypted. > > That's not quite correct. All blocks in a file are either always > encrypted, or not. So that's not really the problem. Sorry, I wasn't clear enough with my explaination. I actually meant to say that not all blocks mapped by a page might be dirty and hence only a subset of the dirty blocks might need to be written to the disk. I understand that in such cases we could still invoke fscrypt_encrypt_page() and encrypt the contents of all the blocks (irrespective of whether a block is dirty or not). I wanted to optimize this and avoid the encryption of "clean blocks". > > > For example, ext4_bio_write_page() marks such blocks > > with "Async Write" flag and later in another pass, it encrypts and also adds > > these blocks to a bio. > > The tricky bits with ext4_bio_write_page() all are in handling the > case where page_size > block_size. In that case, where there are multiple > file system blocks covering a page, we need to know the on-disk > block numbers are for the blocks covering the page, and the encryption > is intertwined with the I/O submission path, which is file system > specific -- mainly because how the completion callback and the > parameters which need to be passed *into* the the callback function is > file system specific. > > However, none of that is needed or relevant to the encryption > operation. It's an accident of code development history that > fscrypt_encrypt_page was placed where it was. > > That is, none of work done in the first pass (starting with the > comment "In the first loop we prepare and mark buffers to submit....") > is needed to be done before we call fscrypt_encrypt_page(). That call: > > data_page = fscrypt_encrypt_page(inode, page, PAGE_SIZE, 0, > page->index, gfp_flags); > > ... could easily be moved to the beginning of ext4_bio_write_page(). > > I can do that to make the function easier to understand, but that > particular cleanup is merely cosmetic. It doesn't what you would need > to do order to make fscrypt_encrypt_page() iterate over the page as it > calls fscrypt_encrypt_buffer(). > -- chandan
