linux-ext4 - Re: [PATCHv3 1/1] fs: ext4: Don't use CMA for buffer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20240913105310.enqz47nmcn24ux45@quack3>
Date: Fri, 13 Sep 2024 12:53:10 +0200
From: Jan Kara <jack@...e.cz>
To: Zhaoyang Huang <huangzhaoyang@...il.com>
Cc: "zhaoyang.huang" <zhaoyang.huang@...soc.com>, Jan Kara <jack@...e.cz>,
	Theodore Ts'o <tytso@....edu>,
	Andreas Dilger <adilger.kernel@...ger.ca>,
	linux-ext4@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>,
	康纪滨 (Steve Kang) <steve.kang@...soc.com>
Subject: Re: [PATCHv3 1/1] fs: ext4: Don't use CMA for buffer_head

On Fri 13-09-24 10:02:40, Zhaoyang Huang wrote:
> loop in jan kara
> 
> On Wed, Sep 4, 2024 at 3:54 PM zhaoyang.huang <zhaoyang.huang@...soc.com> wrote:
> >
> > From: Zhaoyang Huang <zhaoyang.huang@...soc.com>
> >
> > cma_alloc() keep failed in our system which thanks to a jh->bh->b_page
> > can not be migrated out of CMA area[1] as the jh has one cp_transaction
> > pending on it because of j_free > j_max_transaction_buffers[2][3][4][5][6].
> > We temporarily solve this by launching jbd2_log_do_checkpoint forcefully
> > somewhere. Since journal is common mechanism to all JFSs and
> > cp_transaction has a little fewer opportunity to be launched, the
> > cma_alloc() could be affected under the same scenario. This patch
> > would like to have buffer_head of ext4 not use CMA pages when doing
> > sb_getblk.
> >
> > [1]
> > crash_arm64_v8.0.4++> kmem -p|grep ffffff808f0aa150(sb->s_bdev->bd_inode->i_mapping)
> > fffffffe01a51c00  e9470000 ffffff808f0aa150        3  2 8000000008020 lru,private
> > fffffffe03d189c0 174627000 ffffff808f0aa150        4  2 2004000000008020 lru,private
> > fffffffe03d88e00 176238000 ffffff808f0aa150      3f9  2 2008000000008020 lru,private
> > fffffffe03d88e40 176239000 ffffff808f0aa150        6  2 2008000000008020 lru,private
> > fffffffe03d88e80 17623a000 ffffff808f0aa150        5  2 2008000000008020 lru,private
> > fffffffe03d88ec0 17623b000 ffffff808f0aa150        1  2 2008000000008020 lru,private
> > fffffffe03d88f00 17623c000 ffffff808f0aa150        0  2 2008000000008020 lru,private
> > fffffffe040e6540 183995000 ffffff808f0aa150      3f4  2 2004000000008020 lru,private
> >
> > [2] page -> buffer_head
> > crash_arm64_v8.0.4++> struct page.private fffffffe01a51c00 -x
> >       private = 0xffffff802fca0c00
> >
> > [3] buffer_head -> journal_head
> > crash_arm64_v8.0.4++> struct buffer_head.b_private 0xffffff802fca0c00
> >   b_private = 0xffffff8041338e10,
> >
> > [4] journal_head -> b_cp_transaction
> > crash_arm64_v8.0.4++> struct journal_head.b_cp_transaction 0xffffff8041338e10 -x
> >   b_cp_transaction = 0xffffff80410f1900,
> >
> > [5] transaction_t -> journal
> > crash_arm64_v8.0.4++> struct transaction_t.t_journal 0xffffff80410f1900 -x
> >   t_journal = 0xffffff80e70f3000,
> >
> > [6] j_free & j_max_transaction_buffers
> > crash_arm64_v8.0.4++> struct journal_t.j_free,j_max_transaction_buffers 0xffffff80e70f3000 -x
> >   j_free = 0x3f1,
> >   j_max_transaction_buffers = 0x100,
> >
> > Suggested-by: Theodore Ts'o <tytso@....edu>
> > Signed-off-by: Zhaoyang Huang <zhaoyang.huang@...soc.com>

I think there are better fixes to the problem but as a quick stopgap
solution I have no problem with this since this is trivial and influences
only CMA users on ext4. So feel free to add:

Reviewed-by: Jan Kara <jack@...e.cz>

								Honza

> > ---
> > v3: switch to use getblk_unmoveable as suggested by Theodore Ts'o
> > ---
> > ---
> >  fs/ext4/inode.c | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> > index 941c1c0d5c6e..a0f48840c5c1 100644
> > --- a/fs/ext4/inode.c
> > +++ b/fs/ext4/inode.c
> > @@ -869,7 +869,14 @@ struct buffer_head *ext4_getblk(handle_t *handle, struct inode *inode,
> >         if (nowait)
> >                 return sb_find_get_block(inode->i_sb, map.m_pblk);
> >
> > -       bh = sb_getblk(inode->i_sb, map.m_pblk);
> > +       /*
> > +        * Since bh could introduce extra ref count such as referred by
> > +        * journal_head etc. Try to avoid using __GFP_MOVABLE here
> > +        * as it may fail the migration when journal_head remains.
> > +        */
> > +       bh = getblk_unmovable(inode->i_sb->s_bdev, map.m_pblk,
> > +                               inode->i_sb->s_blocksize);
> > +
> >         if (unlikely(!bh))
> >                 return ERR_PTR(-ENOMEM);
> >         if (map.m_flags & EXT4_MAP_NEW) {
> > --
> > 2.25.1
> >
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR