| Message ID | 20201029183220.GA88284@xps-13-7390 |
|---|---|
| State | New |
| Headers | show |
| Series | [Unstable] ext4: properly check for dirty state in ext4_inode_datasync_dirty() | expand |
On 29/10/2020 18:32, Andrea Righi wrote: > ext4_inode_datasync_dirty() needs to return 'true' if the inode is > dirty, 'false' otherwise, but the logic seems to be incorrectly changed > by commit aa75f4d3daae ("ext4: main fast-commit commit path"). > > This introduces a problem with swap files that are always failing to be > activated, showing this error in dmesg: > > [ 34.406479] swapon: file is not committed > > Simple test case to reproduce the problem: > > # fallocate -l 8G swapfile > # chmod 0600 swapfile > # mkswap swapfile > # swapon swapfile > > Fix the logic to return the proper state of the inode. > > Link: https://lore.kernel.org/lkml/20201024131333.GA32124@xps-13-7390 > Fixes: aa75f4d3daae ("ext4: main fast-commit commit path") > Signed-off-by: Andrea Righi <andrea.righi@canonical.com> > Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com> > (cherry picked from commit a316082fa331d086dd8dea9d1e10c7fac3cbbad6 https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git) > Signed-off-by: Andrea Righi <andrea.righi@canonical.com> > --- > fs/ext4/inode.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c > index 03c2253005f0..520a0209451e 100644 > --- a/fs/ext4/inode.c > +++ b/fs/ext4/inode.c > @@ -3307,10 +3307,12 @@ static bool ext4_inode_datasync_dirty(struct inode *inode) > > if (journal) { > if (jbd2_transaction_committed(journal, > - EXT4_I(inode)->i_datasync_tid)) > - return true; > - return atomic_read(&EXT4_SB(inode->i_sb)->s_fc_subtid) >= > - EXT4_I(inode)->i_fc_committed_subtid; > + EXT4_I(inode)->i_datasync_tid)) > + return false; > + if (test_opt2(inode->i_sb, JOURNAL_FAST_COMMIT)) > + return atomic_read(&EXT4_SB(inode->i_sb)->s_fc_subtid) < > + EXT4_I(inode)->i_fc_committed_subtid; > + return true; > } > > /* Any metadata buffers to write? */ > I had to re-read the commit aa75f4d3daae a few times before I could understand this fix. Looks OK and it addresses the issue, so.. Acked-by: Colin Ian King <colin.king@canonical.com>
On Thu, Oct 29, 2020 at 07:32:22PM +0100, Andrea Righi wrote: > ext4_inode_datasync_dirty() needs to return 'true' if the inode is > dirty, 'false' otherwise, but the logic seems to be incorrectly changed > by commit aa75f4d3daae ("ext4: main fast-commit commit path"). > > This introduces a problem with swap files that are always failing to be > activated, showing this error in dmesg: > > [ 34.406479] swapon: file is not committed > > Simple test case to reproduce the problem: > > # fallocate -l 8G swapfile > # chmod 0600 swapfile > # mkswap swapfile > # swapon swapfile > > Fix the logic to return the proper state of the inode. > > Link: https://lore.kernel.org/lkml/20201024131333.GA32124@xps-13-7390 > Fixes: aa75f4d3daae ("ext4: main fast-commit commit path") > Signed-off-by: Andrea Righi <andrea.righi@canonical.com> > Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com> > (cherry picked from commit a316082fa331d086dd8dea9d1e10c7fac3cbbad6 https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git) > Signed-off-by: Andrea Righi <andrea.righi@canonical.com> > --- > fs/ext4/inode.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c > index 03c2253005f0..520a0209451e 100644 > --- a/fs/ext4/inode.c > +++ b/fs/ext4/inode.c > @@ -3307,10 +3307,12 @@ static bool ext4_inode_datasync_dirty(struct inode *inode) > > if (journal) { > if (jbd2_transaction_committed(journal, > - EXT4_I(inode)->i_datasync_tid)) > - return true; > - return atomic_read(&EXT4_SB(inode->i_sb)->s_fc_subtid) >= > - EXT4_I(inode)->i_fc_committed_subtid; > + EXT4_I(inode)->i_datasync_tid)) > + return false; > + if (test_opt2(inode->i_sb, JOURNAL_FAST_COMMIT)) > + return atomic_read(&EXT4_SB(inode->i_sb)->s_fc_subtid) < > + EXT4_I(inode)->i_fc_committed_subtid; > + return true; > } > > /* Any metadata buffers to write? */ > -- > 2.27.0 >
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c index 03c2253005f0..520a0209451e 100644 --- a/fs/ext4/inode.c +++ b/fs/ext4/inode.c @@ -3307,10 +3307,12 @@ static bool ext4_inode_datasync_dirty(struct inode *inode) if (journal) { if (jbd2_transaction_committed(journal, - EXT4_I(inode)->i_datasync_tid)) - return true; - return atomic_read(&EXT4_SB(inode->i_sb)->s_fc_subtid) >= - EXT4_I(inode)->i_fc_committed_subtid; + EXT4_I(inode)->i_datasync_tid)) + return false; + if (test_opt2(inode->i_sb, JOURNAL_FAST_COMMIT)) + return atomic_read(&EXT4_SB(inode->i_sb)->s_fc_subtid) < + EXT4_I(inode)->i_fc_committed_subtid; + return true; } /* Any metadata buffers to write? */
ext4_inode_datasync_dirty() needs to return 'true' if the inode is dirty, 'false' otherwise, but the logic seems to be incorrectly changed by commit aa75f4d3daae ("ext4: main fast-commit commit path"). This introduces a problem with swap files that are always failing to be activated, showing this error in dmesg: [ 34.406479] swapon: file is not committed Simple test case to reproduce the problem: # fallocate -l 8G swapfile # chmod 0600 swapfile # mkswap swapfile # swapon swapfile Fix the logic to return the proper state of the inode. Link: https://lore.kernel.org/lkml/20201024131333.GA32124@xps-13-7390 Fixes: aa75f4d3daae ("ext4: main fast-commit commit path") Signed-off-by: Andrea Righi <andrea.righi@canonical.com> Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com> (cherry picked from commit a316082fa331d086dd8dea9d1e10c7fac3cbbad6 https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git) Signed-off-by: Andrea Righi <andrea.righi@canonical.com> --- fs/ext4/inode.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)