On Thu 16-10-14 18:09:13, Wu Fengguang wrote:
> Hi Jan,
>
> Your patch gives a warning on the xfs code path. :)
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
> commit be330474e2d0533a7a6185e567f3654fec096dbd ("vfs: fix data corruption when blocksize < pagesize for mmaped data")
...
> <5>[ 25.956576] XFS (sda1): Mounting V4 Filesystem
> <6>[ 26.194468] XFS (sda1): Ending clean mount
> <4>[ 27.258450] ------------[ cut here ]------------
> <4>[ 27.258789] WARNING: CPU: 3 PID: 2339 at mm/truncate.c:758 pagecache_isize_extended+0xdd/0x120()
Ah, I believe this is:
WARN_ON(!mutex_is_locked(&inode->i_mutex));
because XFS doesn't use i_mutex but its private rwlock. I forgot to check
dmesg after running xfstests for XFS and didn't notice this. Anyway, the
solution is to just remove the assertion since it doesn't hold for all
filesystems. Patch for that is attached. Thanks Fengguang for catching
this.
Honza
> <4>[ 27.259443] Modules linked in: ipmi_watchdog ipmi_msghandler btrfs xor raid6_pq sg sr_mod cdrom sd_mod firewire_ohci firewire_core crc_itu_t snd_hda_codec_realtek pcspkr snd_hda_codec_generic ahci libahci libata snd_hda_intel i2c_i801 snd_hda_controller parport_pc parport snd_hda_codec snd_hwdep snd_pcm snd_timer shpchp snd x38_edac edac_core soundcore acpi_cpufreq
> <4>[ 27.262734] CPU: 3 PID: 2339 Comm: fallocate Not tainted 3.17.0-gda9a9f1 #1
> <4>[ 27.263153] Hardware name: / , BIOS VVRBLI9J.86A.2891.2007.0511.1144 05/11/2007
> <4>[ 27.263780] 0000000000000009 ffff88007a43fd88 ffffffff81859ea6 0000000000000000
> <4>[ 27.264492] ffff88007a43fdc0 ffffffff8106ef0d 0000000000001000 ffff88005b6f05a8
> <4>[ 27.265199] 0000000000000000 ffff88005b6f05a8 0000000004000000 ffff88007a43fdd0
> <4>[ 27.265906] Call Trace:
> <4>[ 27.266165] [<ffffffff81859ea6>] dump_stack+0x4d/0x66
> <4>[ 27.266511] [<ffffffff8106ef0d>] warn_slowpath_common+0x7d/0xa0
> <4>[ 27.266900] [<ffffffff8106efea>] warn_slowpath_null+0x1a/0x20
> <4>[ 27.267286] [<ffffffff8117020d>] pagecache_isize_extended+0xdd/0x120
> <4>[ 27.267690] [<ffffffff811712b7>] truncate_setsize+0x27/0x40
> <4>[ 27.268068] [<ffffffff8133eab7>] xfs_setattr_size+0x157/0x3a0
> <4>[ 27.268442] [<ffffffff8134c827>] ? xfs_trans_commit+0x157/0x250
> <4>[ 27.268821] [<ffffffff813336df>] xfs_file_fallocate+0x2df/0x300
> <4>[ 27.269215] [<ffffffff811dbb09>] ? __sb_start_write+0x49/0xf0
> <4>[ 27.269596] [<ffffffff813923b4>] ? selinux_file_permission+0xc4/0x120
> <4>[ 27.270009] [<ffffffff811d7563>] do_fallocate+0x123/0x1b0
> <4>[ 27.270380] [<ffffffff811d7633>] SyS_fallocate+0x43/0x70
> <4>[ 27.270738] [<ffffffff81862c69>] system_call_fastpath+0x16/0x1b
> <4>[ 27.271119] ---[ end trace 6a3b1350ad399610 ]---
> <4>[ 27.274498] ------------[ cut here ]------------
On Thu, Oct 16, 2014 at 01:01:27PM +0200, Jan Kara wrote:
> From de3426d6495f4b44b14c09b7c7202e9a86d864b9 Mon Sep 17 00:00:00 2001
> From: Jan Kara <[email protected]>
> Date: Thu, 16 Oct 2014 12:58:42 +0200
> Subject: [PATCH] mm: Remove false WARN_ON from pagecache_isize_extended()
>
> The WARN_ON checking whether i_mutex is held in
> pagecache_isize_extended() was wrong because some filesystems (e.g.
> XFS) use different locks for serialization of truncates / writes. So
> just remove the check.
>
> Signed-off-by: Jan Kara <[email protected]>
> ---
> mm/truncate.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/mm/truncate.c b/mm/truncate.c
> index 261eaf6e5a19..c646084e5eec 100644
> --- a/mm/truncate.c
> +++ b/mm/truncate.c
> @@ -755,7 +755,6 @@ void pagecache_isize_extended(struct inode *inode, loff_t from, loff_t to)
> struct page *page;
> pgoff_t index;
>
> - WARN_ON(!mutex_is_locked(&inode->i_mutex));
> WARN_ON(to > inode->i_size);
>
> if (from >= to || bsize == PAGE_CACHE_SIZE)
Jan, Have you sent this patch upstream yet? I'm seeing it fire in
my testing in 3.18-rc1 kernels, so I was wondering what your plans
are for this...
Cheers,
Dave.
--
Dave Chinner
[email protected]
On Mon 27-10-14 12:04:22, Dave Chinner wrote:
> On Thu, Oct 16, 2014 at 01:01:27PM +0200, Jan Kara wrote:
> > From de3426d6495f4b44b14c09b7c7202e9a86d864b9 Mon Sep 17 00:00:00 2001
> > From: Jan Kara <[email protected]>
> > Date: Thu, 16 Oct 2014 12:58:42 +0200
> > Subject: [PATCH] mm: Remove false WARN_ON from pagecache_isize_extended()
> >
> > The WARN_ON checking whether i_mutex is held in
> > pagecache_isize_extended() was wrong because some filesystems (e.g.
> > XFS) use different locks for serialization of truncates / writes. So
> > just remove the check.
> >
> > Signed-off-by: Jan Kara <[email protected]>
> > ---
> > mm/truncate.c | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/mm/truncate.c b/mm/truncate.c
> > index 261eaf6e5a19..c646084e5eec 100644
> > --- a/mm/truncate.c
> > +++ b/mm/truncate.c
> > @@ -755,7 +755,6 @@ void pagecache_isize_extended(struct inode *inode, loff_t from, loff_t to)
> > struct page *page;
> > pgoff_t index;
> >
> > - WARN_ON(!mutex_is_locked(&inode->i_mutex));
> > WARN_ON(to > inode->i_size);
> >
> > if (from >= to || bsize == PAGE_CACHE_SIZE)
>
> Jan, Have you sent this patch upstream yet? I'm seeing it fire in
> my testing in 3.18-rc1 kernels, so I was wondering what your plans
> are for this...
I did send it but it got lost somewhere. I'll resend it.
Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR
On Mon, Oct 27, 2014 at 10:58:12AM +0100, Jan Kara wrote:
> On Mon 27-10-14 12:04:22, Dave Chinner wrote:
> > On Thu, Oct 16, 2014 at 01:01:27PM +0200, Jan Kara wrote:
> > > From de3426d6495f4b44b14c09b7c7202e9a86d864b9 Mon Sep 17 00:00:00 2001
> > > From: Jan Kara <[email protected]>
> > > Date: Thu, 16 Oct 2014 12:58:42 +0200
> > > Subject: [PATCH] mm: Remove false WARN_ON from pagecache_isize_extended()
> > >
> > > The WARN_ON checking whether i_mutex is held in
> > > pagecache_isize_extended() was wrong because some filesystems (e.g.
> > > XFS) use different locks for serialization of truncates / writes. So
> > > just remove the check.
> > >
> > > Signed-off-by: Jan Kara <[email protected]>
> > > ---
> > > mm/truncate.c | 1 -
> > > 1 file changed, 1 deletion(-)
> > >
> > > diff --git a/mm/truncate.c b/mm/truncate.c
> > > index 261eaf6e5a19..c646084e5eec 100644
> > > --- a/mm/truncate.c
> > > +++ b/mm/truncate.c
> > > @@ -755,7 +755,6 @@ void pagecache_isize_extended(struct inode *inode, loff_t from, loff_t to)
> > > struct page *page;
> > > pgoff_t index;
> > >
> > > - WARN_ON(!mutex_is_locked(&inode->i_mutex));
> > > WARN_ON(to > inode->i_size);
> > >
> > > if (from >= to || bsize == PAGE_CACHE_SIZE)
> >
> > Jan, Have you sent this patch upstream yet? I'm seeing it fire in
> > my testing in 3.18-rc1 kernels, so I was wondering what your plans
> > are for this...
> I did send it but it got lost somewhere. I'll resend it.
If it comes to it, I can push it through the XFS tree - I've got a
few patches I need to send for -rc3...
Cheers,
Dave.
--
Dave Chinner
[email protected]
still in rc2
------------[ cut here ]------------
WARNING: CPU: 0 PID: 18152 at mm/truncate.c:758
pagecache_isize_extended+0xfd/0x110()
Modules linked in: btrfs xor zlib_deflate raid6_pq ntfs fuse
x86_pkg_temp_thermal wmi radeon e1000e ttm
CPU: 0 PID: 18152 Comm: Cache I/O Tainted: G W 3.18.0-rc2 #1
Hardware name: Hewlett-Packard HP Compaq Elite 8300 MT/3397, BIOS K01
v02.05 05/07/2012
0000000000000009 ffff8800d1b3bd38 ffffffff81863216 0000000000000000
0000000000000000 ffff8800d1b3bd78 ffffffff8105046c ffffea0002e0b600
0000000000001000 ffff8800bb9a49a8 0000000000000000 0000000000031ada
Call Trace:
[<ffffffff81863216>] dump_stack+0x46/0x58
[<ffffffff8105046c>] warn_slowpath_common+0x7c/0xa0
[<ffffffff81050535>] warn_slowpath_null+0x15/0x20
[<ffffffff8111ebbd>] pagecache_isize_extended+0xfd/0x110
[<ffffffff812ca780>] ? __xfs_get_blocks+0x4d0/0x4d0
[<ffffffff8111f8a2>] truncate_setsize+0x22/0x40
[<ffffffff812df5db>] xfs_setattr_size+0x10b/0x3c0
[<ffffffff812ecb3e>] ? xfs_trans_commit+0x13e/0x220
[<ffffffff812d4c57>] xfs_file_fallocate+0x2e7/0x300
[<ffffffff81167374>] ? __sb_start_write+0x44/0xd0
[<ffffffff811630da>] do_fallocate+0x12a/0x1d0
[<ffffffff811631c3>] SyS_fallocate+0x43/0x70
[<ffffffff8186ce29>] system_call_fastpath+0x12/0x17
---[ end trace abf71ba1f4096942 ]---
2014-10-28 5:05 GMT+08:00 Dave Chinner <[email protected]>:
> On Mon, Oct 27, 2014 at 10:58:12AM +0100, Jan Kara wrote:
>> On Mon 27-10-14 12:04:22, Dave Chinner wrote:
>> > On Thu, Oct 16, 2014 at 01:01:27PM +0200, Jan Kara wrote:
>> > > From de3426d6495f4b44b14c09b7c7202e9a86d864b9 Mon Sep 17 00:00:00 2001
>> > > From: Jan Kara <[email protected]>
>> > > Date: Thu, 16 Oct 2014 12:58:42 +0200
>> > > Subject: [PATCH] mm: Remove false WARN_ON from pagecache_isize_extended()
>> > >
>> > > The WARN_ON checking whether i_mutex is held in
>> > > pagecache_isize_extended() was wrong because some filesystems (e.g.
>> > > XFS) use different locks for serialization of truncates / writes. So
>> > > just remove the check.
>> > >
>> > > Signed-off-by: Jan Kara <[email protected]>
>> > > ---
>> > > mm/truncate.c | 1 -
>> > > 1 file changed, 1 deletion(-)
>> > >
>> > > diff --git a/mm/truncate.c b/mm/truncate.c
>> > > index 261eaf6e5a19..c646084e5eec 100644
>> > > --- a/mm/truncate.c
>> > > +++ b/mm/truncate.c
>> > > @@ -755,7 +755,6 @@ void pagecache_isize_extended(struct inode *inode, loff_t from, loff_t to)
>> > > struct page *page;
>> > > pgoff_t index;
>> > >
>> > > - WARN_ON(!mutex_is_locked(&inode->i_mutex));
>> > > WARN_ON(to > inode->i_size);
>> > >
>> > > if (from >= to || bsize == PAGE_CACHE_SIZE)
>> >
>> > Jan, Have you sent this patch upstream yet? I'm seeing it fire in
>> > my testing in 3.18-rc1 kernels, so I was wondering what your plans
>> > are for this...
>> I did send it but it got lost somewhere. I'll resend it.
>
> If it comes to it, I can push it through the XFS tree - I've got a
> few patches I need to send for -rc3...
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> [email protected]
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/