On Wednesday, March 28, 2018 1:10:56 AM IST Theodore Y. Ts'o wrote: > On Mon, Mar 26, 2018 at 01:52:54PM +0530, Chandan Rajendra wrote: > > > Also, it looks like when you renamed the *_page fscrypt functions to > > > *_blocks, on the write side, a bounce page is still being used for > > > each block. So so an an architecture which has 64k pages, and we are > > > writing to a file sytem with 4k blocks, to write a 64k page, the > > > fscrypt layer will have to allocate 16 64k bounce pages to write a > > > single 64k page to an encrypted file. Am I missing something? > > > > > > > ext4_bio_write_page() invokes the new fscrypt_encrypt_block() function for > > each block of the page that has been marked with "Async write". For all blocks > > of the page that needs to be written to the disk, we pass the same bounce page > > as an argument to fscrypt_encrypt_block(). > > Thanks for the explanation. I do wonder if the proper thing to export > from the fscrypt layer is fscrypt_encrypt_page(), since for all file > systems, the only thing which really makes sense is to read and write > a full page at a time, since we cache things at the page cache a full > page a time. So instead of teaching each file system how to use > fscrypt_{encrypt,decrypt}_block, maybe push that into the fscrypt > layer, and implement a new fscrypt_encrypt_page() which calls > fs_encrypt_block()? > 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. 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. So IMHO, I think fscrypt layer should limit itself to encrypting/decrypting file data in block size units. -- chandan