MIME-Version: 1.0
References: <20211201095258.1966-1-yinxin.x@bytedance.com> <20211217204322.1000035-1-harshads@google.com>
In-Reply-To: <20211217204322.1000035-1-harshads@google.com>
From:   Xin Yin <yinxin.x@bytedance.com>
Date:   Sun, 19 Dec 2021 12:45:09 +0800
Message-ID: <CAK896s6_kfs9AoDSq=Wfgb9Rx4zKmvoC-YYBr5gnwBCAejL7xA@mail.gmail.com>
Subject: Re: [External] Re: [PATCH] ext4: fix fast commit may miss tracking
 range for FALLOC_FL_ZERO_RANGE
To:     Harshad Shirwadkar <harshadshirwadkar@gmail.com>
Cc:     adilger.kernel@dilger.ca, linux-ext4@vger.kernel.org,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Sat, Dec 18, 2021 at 4:43 AM Harshad Shirwadkar
<harshadshirwadkar@gmail.com> wrote:
>
> > fs/ext4/extents.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> >diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> >index 9229ab1f99c5..4108896d471b 100644
> >--- a/fs/ext4/extents.c
> >+++ b/fs/ext4/extents.c
> >@@ -4433,6 +4433,8 @@ static int ext4_alloc_file_blocks(struct file *file, ext4_lblk_t offset,
> >                       ret2 = ext4_journal_stop(handle);
> >                       break;
> >               }
> >+              ext4_fc_track_range(handle, inode, map.m_lblk,
> >+                                      map.m_lblk + map.m_len - 1);
>
> ext4_alloc_file_blocks() calls ext4_map_blocks(), inside which we do
> call ext4_fc_track_range(). However, we are doing that only if
> map.m_flags & EXT4_MAP_MAPPED is true. So, unwritten flag is set we
> are not calling track_range there. Perhaps the right fix is to call
> ext4_fc_track_range() from ext4_map_blocks() if MAPPED or UNWRITTEN
> flag is set?

Thanks, you are right, this should be better. I will test this way and
resend a v2 patch for this issue.

>
> >               map.m_lblk += ret;
> >               map.m_len = len = len - ret;
> >               epos = (loff_t)map.m_lblk << inode->i_blkbits;
> >@@ -4599,8 +4601,6 @@ static long ext4_zero_range(struct file *file, loff_t offset,
> >       ret = ext4_mark_inode_dirty(handle, inode);
> >       if (unlikely(ret))
> >               goto out_handle;
> >-      ext4_fc_track_range(handle, inode, offset >> inode->i_sb->s_blocksize_bits,
> >-                      (offset + len - 1) >> inode->i_sb->s_blocksize_bits);
> >       /* Zero out partial block at the edges of the range */
> >       ret = ext4_zero_partial_blocks(handle, inode, offset, len);
> >       if (ret >= 0)
> >--