| Message ID | 131a7e4a0e020a94c719994867484ba248316d13.1681188927.git.ritesh.list@gmail.com |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | ext2: DIO to use iomap | expand |
On Tue, Apr 11, 2023 at 10:51:49AM +0530, Ritesh Harjani (IBM) wrote: > PAGE_ALIGN(x) macro gives the next highest value which is multiple of > pagesize. But if x is already page aligned then it simply returns x. > So, if x passed is 0 in dax_zero_range() function, that means the > length gets passed as 0 to ->iomap_begin(). > > In ext2 it then calls ext2_get_blocks -> max_blocks as 0 and hits bug_on > here in ext2_get_blocks(). > BUG_ON(maxblocks == 0); > > Instead we should be calling dax_truncate_page() here which takes > care of it. i.e. it only calls dax_zero_range if the offset is not > page/block aligned. > > This can be easily triggered with following on fsdax mounted pmem > device. > > dd if=/dev/zero of=file count=1 bs=512 > truncate -s 0 file > > [79.525838] EXT2-fs (pmem0): DAX enabled. Warning: EXPERIMENTAL, use at your own risk > [79.529376] ext2 filesystem being mounted at /mnt1/test supports timestamps until 2038 (0x7fffffff) > [93.793207] ------------[ cut here ]------------ > [93.795102] kernel BUG at fs/ext2/inode.c:637! > [93.796904] invalid opcode: 0000 [#1] PREEMPT SMP PTI > [93.798659] CPU: 0 PID: 1192 Comm: truncate Not tainted 6.3.0-rc2-xfstests-00056-g131086faa369 #139 > [93.806459] RIP: 0010:ext2_get_blocks.constprop.0+0x524/0x610 > <...> > [93.835298] Call Trace: > [93.836253] <TASK> > [93.837103] ? lock_acquire+0xf8/0x110 > [93.838479] ? d_lookup+0x69/0xd0 > [93.839779] ext2_iomap_begin+0xa7/0x1c0 > [93.841154] iomap_iter+0xc7/0x150 > [93.842425] dax_zero_range+0x6e/0xa0 > [93.843813] ext2_setsize+0x176/0x1b0 > [93.845164] ext2_setattr+0x151/0x200 > [93.846467] notify_change+0x341/0x4e0 > [93.847805] ? lock_acquire+0xf8/0x110 > [93.849143] ? do_truncate+0x74/0xe0 > [93.850452] ? do_truncate+0x84/0xe0 > [93.851739] do_truncate+0x84/0xe0 > [93.852974] do_sys_ftruncate+0x2b4/0x2f0 > [93.854404] do_syscall_64+0x3f/0x90 > [93.855789] entry_SYSCALL_64_after_hwframe+0x72/0xdc > > Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com> Would seem to make sense... Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > --- > fs/ext2/inode.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c > index 26f135e7ffce..dc76147e7b07 100644 > --- a/fs/ext2/inode.c > +++ b/fs/ext2/inode.c > @@ -1259,9 +1259,8 @@ static int ext2_setsize(struct inode *inode, loff_t newsize) > inode_dio_wait(inode); > > if (IS_DAX(inode)) > - error = dax_zero_range(inode, newsize, > - PAGE_ALIGN(newsize) - newsize, NULL, > - &ext2_iomap_ops); > + error = dax_truncate_page(inode, newsize, NULL, > + &ext2_iomap_ops); > else > error = block_truncate_page(inode->i_mapping, > newsize, ext2_get_block); > -- > 2.39.2 >
diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c index 26f135e7ffce..dc76147e7b07 100644 --- a/fs/ext2/inode.c +++ b/fs/ext2/inode.c @@ -1259,9 +1259,8 @@ static int ext2_setsize(struct inode *inode, loff_t newsize) inode_dio_wait(inode); if (IS_DAX(inode)) - error = dax_zero_range(inode, newsize, - PAGE_ALIGN(newsize) - newsize, NULL, - &ext2_iomap_ops); + error = dax_truncate_page(inode, newsize, NULL, + &ext2_iomap_ops); else error = block_truncate_page(inode->i_mapping, newsize, ext2_get_block);
PAGE_ALIGN(x) macro gives the next highest value which is multiple of pagesize. But if x is already page aligned then it simply returns x. So, if x passed is 0 in dax_zero_range() function, that means the length gets passed as 0 to ->iomap_begin(). In ext2 it then calls ext2_get_blocks -> max_blocks as 0 and hits bug_on here in ext2_get_blocks(). BUG_ON(maxblocks == 0); Instead we should be calling dax_truncate_page() here which takes care of it. i.e. it only calls dax_zero_range if the offset is not page/block aligned. This can be easily triggered with following on fsdax mounted pmem device. dd if=/dev/zero of=file count=1 bs=512 truncate -s 0 file [79.525838] EXT2-fs (pmem0): DAX enabled. Warning: EXPERIMENTAL, use at your own risk [79.529376] ext2 filesystem being mounted at /mnt1/test supports timestamps until 2038 (0x7fffffff) [93.793207] ------------[ cut here ]------------ [93.795102] kernel BUG at fs/ext2/inode.c:637! [93.796904] invalid opcode: 0000 [#1] PREEMPT SMP PTI [93.798659] CPU: 0 PID: 1192 Comm: truncate Not tainted 6.3.0-rc2-xfstests-00056-g131086faa369 #139 [93.806459] RIP: 0010:ext2_get_blocks.constprop.0+0x524/0x610 <...> [93.835298] Call Trace: [93.836253] <TASK> [93.837103] ? lock_acquire+0xf8/0x110 [93.838479] ? d_lookup+0x69/0xd0 [93.839779] ext2_iomap_begin+0xa7/0x1c0 [93.841154] iomap_iter+0xc7/0x150 [93.842425] dax_zero_range+0x6e/0xa0 [93.843813] ext2_setsize+0x176/0x1b0 [93.845164] ext2_setattr+0x151/0x200 [93.846467] notify_change+0x341/0x4e0 [93.847805] ? lock_acquire+0xf8/0x110 [93.849143] ? do_truncate+0x74/0xe0 [93.850452] ? do_truncate+0x84/0xe0 [93.851739] do_truncate+0x84/0xe0 [93.852974] do_sys_ftruncate+0x2b4/0x2f0 [93.854404] do_syscall_64+0x3f/0x90 [93.855789] entry_SYSCALL_64_after_hwframe+0x72/0xdc Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com> --- fs/ext2/inode.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)