On Thu 29-08-24 11:12:50, Jan Kara wrote: > On Thu 29-08-24 16:54:06, zhangshida wrote: > > From: Shida Zhang <zhangshida@xxxxxxxxxx> > > > > Using __block_write_begin() make it inconvenient to journal the > > user data dirty process. We can't tell the block layer maintainer, > > ‘Hey, we want to trace the dirty user data in ext4, can we add some > > special code for ext4 in __block_write_begin?’:P > > > > So use ext4_block_write_begin() instead. > > > > The two functions are basically doing the same thing except for the > > fscrypt related code. Remove the unnecessary #ifdef since > > fscrypt_inode_uses_fs_layer_crypto() returns false (and it's known at > > compile time) when !CONFIG_FS_ENCRYPTION. > > > > And hoist the ext4_block_write_begin so that it can be used in other > > files. > > > > Suggested-by: Jan Kara <jack@xxxxxxx> > > Suggested-by: Eric Biggers <ebiggers@xxxxxxxxxx> > > Signed-off-by: Shida Zhang <zhangshida@xxxxxxxxxx> > > I think I've given my Reviewed-by on this already in previous version - you > can keep those tags unless the patch significantly changes. Anyway: feel > free to add: > > Reviewed-by: Jan Kara <jack@xxxxxxx> I've realized the patch slightly changed so that's likely why you've dropped the Reviewed-by so I retract my comment :) I've also realized one thing: > > --- a/fs/ext4/inode.c > > +++ b/fs/ext4/inode.c > > @@ -1024,10 +1024,10 @@ int do_journal_get_write_access(handle_t *handle, struct inode *inode, > > if (!buffer_mapped(bh) || buffer_freed(bh)) > > return 0; > > /* > > - * __block_write_begin() could have dirtied some buffers. Clean > > + * ext4_block_write_begin() could have dirtied some buffers. Clean > > * the dirty bit as jbd2_journal_get_write_access() could complain > > * otherwise about fs integrity issues. Setting of the dirty bit > > - * by __block_write_begin() isn't a real problem here as we clear > > + * by ext4_block_write_begin() isn't a real problem here as we clear > > * the bit before releasing a page lock and thus writeback cannot > > * ever write the buffer. > > */ This comment and the special buffer_dirty() handling in this function can be removed after patch 3 of this series. Nice additional cleanup :). Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR