Re: [PATCH 1/2] ext4: hoist ext4_block_write_begin and replace the __block_write_begin

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Eric Biggers <ebiggers@xxxxxxxxxx> 于2024年8月29日周四 08:03写道:
>
> On Fri, Aug 23, 2024 at 09:33:28AM +0800, 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. Narrow the scope of CONFIG_FS_ENCRYPTION so as
> > to allow ext4_block_write_begin() to function like __block_write_begin
> > when the config is disabled.
> > And hoist the ext4_block_write_begin so that it can be used in other
> > files.
> >
> > Suggested-by: Jan Kara <jack@xxxxxxx>
> > Signed-off-by: Shida Zhang <zhangshida@xxxxxxxxxx>
> > ---
> >  fs/ext4/ext4.h   |  2 ++
> >  fs/ext4/inline.c | 10 +++++-----
> >  fs/ext4/inode.c  | 23 ++++++-----------------
> >  3 files changed, 13 insertions(+), 22 deletions(-)
>
> Thanks for cleaning this up.
>
> There are still some comments in fs/ext4/inode.c that reference
> __block_write_begin.  Can you update them too?
>
> One more thing below.
>
> > @@ -1119,7 +1118,9 @@ static int ext4_block_write_begin(struct folio *folio, loff_t pos, unsigned len,
> >       }
> >       if (unlikely(err)) {
> >               folio_zero_new_buffers(folio, from, to);
> > -     } else if (fscrypt_inode_uses_fs_layer_crypto(inode)) {
> > +     }
> > +#ifdef CONFIG_FS_ENCRYPTION
> > +     else if (fscrypt_inode_uses_fs_layer_crypto(inode)) {
> >               for (i = 0; i < nr_wait; i++) {
> >                       int err2;
> >
> > @@ -1131,10 +1132,10 @@ static int ext4_block_write_begin(struct folio *folio, loff_t pos, unsigned len,
> >                       }
> >               }
> >       }
> > +#endif
>
> This #ifdef isn't necessary since fscrypt_inode_uses_fs_layer_crypto() returns
> false (and it's known at compile time) when !CONFIG_FS_ENCRYPTION.
>

Okay. Will make another version.

Thanks,
Stephen

> - Eric





[Index of Archives]     [Reiser Filesystem Development]     [Ceph FS]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite National Park]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Media]

  Powered by Linux