All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] xfs: actually report xattr extents via iomap
@ 2017-04-06 16:03 Darrick J. Wong
  2017-04-06 16:58 ` Brian Foster
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Darrick J. Wong @ 2017-04-06 16:03 UTC (permalink / raw)
  To: xfs; +Cc: Christoph Hellwig, Eryu Guan

Apparently FIEMAP for xattrs has been broken since we switched to
the iomap backend because of an incorrect check for xattr presence.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_iomap.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 288ee5b..4f118a1 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -1173,7 +1173,7 @@ xfs_xattr_iomap_begin(
 	lockmode = xfs_ilock_data_map_shared(ip);
 
 	/* if there are no attribute fork or extents, return ENOENT */
-	if (XFS_IFORK_Q(ip) || !ip->i_d.di_anextents) {
+	if (!XFS_IFORK_Q(ip) || !ip->i_d.di_anextents) {
 		error = -ENOENT;
 		goto out_unlock;
 	}

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] xfs: actually report xattr extents via iomap
  2017-04-06 16:03 [PATCH] xfs: actually report xattr extents via iomap Darrick J. Wong
@ 2017-04-06 16:58 ` Brian Foster
  2017-04-06 18:37 ` Eric Sandeen
  2017-04-07  7:20 ` Eryu Guan
  2 siblings, 0 replies; 8+ messages in thread
From: Brian Foster @ 2017-04-06 16:58 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: xfs, Christoph Hellwig, Eryu Guan

On Thu, Apr 06, 2017 at 09:03:56AM -0700, Darrick J. Wong wrote:
> Apparently FIEMAP for xattrs has been broken since we switched to
> the iomap backend because of an incorrect check for xattr presence.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/xfs_iomap.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index 288ee5b..4f118a1 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -1173,7 +1173,7 @@ xfs_xattr_iomap_begin(
>  	lockmode = xfs_ilock_data_map_shared(ip);
>  
>  	/* if there are no attribute fork or extents, return ENOENT */
> -	if (XFS_IFORK_Q(ip) || !ip->i_d.di_anextents) {
> +	if (!XFS_IFORK_Q(ip) || !ip->i_d.di_anextents) {
>  		error = -ENOENT;
>  		goto out_unlock;
>  	}
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] xfs: actually report xattr extents via iomap
  2017-04-06 16:03 [PATCH] xfs: actually report xattr extents via iomap Darrick J. Wong
  2017-04-06 16:58 ` Brian Foster
@ 2017-04-06 18:37 ` Eric Sandeen
  2017-04-06 18:38   ` Eric Sandeen
  2017-04-07  7:20 ` Eryu Guan
  2 siblings, 1 reply; 8+ messages in thread
From: Eric Sandeen @ 2017-04-06 18:37 UTC (permalink / raw)
  To: Darrick J. Wong, xfs; +Cc: Christoph Hellwig, Eryu Guan

On 4/6/17 11:03 AM, Darrick J. Wong wrote:
> Apparently FIEMAP for xattrs has been broken since we switched to
> the iomap backend because of an incorrect check for xattr presence.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

nothing tests this?  eek.  Who wants to fix that? :)

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
>  fs/xfs/xfs_iomap.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index 288ee5b..4f118a1 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -1173,7 +1173,7 @@ xfs_xattr_iomap_begin(
>  	lockmode = xfs_ilock_data_map_shared(ip);
>  
>  	/* if there are no attribute fork or extents, return ENOENT */
> -	if (XFS_IFORK_Q(ip) || !ip->i_d.di_anextents) {
> +	if (!XFS_IFORK_Q(ip) || !ip->i_d.di_anextents) {
>  		error = -ENOENT;
>  		goto out_unlock;
>  	}
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] xfs: actually report xattr extents via iomap
  2017-04-06 18:37 ` Eric Sandeen
@ 2017-04-06 18:38   ` Eric Sandeen
  0 siblings, 0 replies; 8+ messages in thread
From: Eric Sandeen @ 2017-04-06 18:38 UTC (permalink / raw)
  To: Darrick J. Wong, xfs; +Cc: Christoph Hellwig, Eryu Guan



On 4/6/17 1:37 PM, Eric Sandeen wrote:
> On 4/6/17 11:03 AM, Darrick J. Wong wrote:
>> Apparently FIEMAP for xattrs has been broken since we switched to
>> the iomap backend because of an incorrect check for xattr presence.
>>
>> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> 
> nothing tests this?  eek.  Who wants to fix that? :)

Oh, you fixed that.  thanks :)

-Eric

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] xfs: actually report xattr extents via iomap
  2017-04-06 16:03 [PATCH] xfs: actually report xattr extents via iomap Darrick J. Wong
  2017-04-06 16:58 ` Brian Foster
  2017-04-06 18:37 ` Eric Sandeen
@ 2017-04-07  7:20 ` Eryu Guan
  2017-04-07  7:23   ` Christoph Hellwig
  2017-04-07 16:01   ` Darrick J. Wong
  2 siblings, 2 replies; 8+ messages in thread
From: Eryu Guan @ 2017-04-07  7:20 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: xfs, Christoph Hellwig

On Thu, Apr 06, 2017 at 09:03:56AM -0700, Darrick J. Wong wrote:
> Apparently FIEMAP for xattrs has been broken since we switched to
> the iomap backend because of an incorrect check for xattr presence.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/xfs_iomap.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index 288ee5b..4f118a1 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -1173,7 +1173,7 @@ xfs_xattr_iomap_begin(
>  	lockmode = xfs_ilock_data_map_shared(ip);
>  
>  	/* if there are no attribute fork or extents, return ENOENT */
> -	if (XFS_IFORK_Q(ip) || !ip->i_d.di_anextents) {
> +	if (!XFS_IFORK_Q(ip) || !ip->i_d.di_anextents) {
>  		error = -ENOENT;
>  		goto out_unlock;
>  	}

Surprisingly, after applying this patch on top of 4.11-rc4 kernel, your
test case crashed debug built XFS when testing with 512b block fs, XFS
with other block sizes passed the test.

The three new patches from tags/xfs-4.11-fixes-3 won't help either.

Thanks,
Eryu

[  188.954656] run fstests generic/425 at 2017-04-07 14:51:54
[  189.311170] XFS (sdc2): Unmounting Filesystem
[  189.488020] XFS (sdc2): Mounting V4 Filesystem
[  189.533595] XFS (sdc2): Ending clean mount
[  189.669876] XFS (sdc2): Unmounting Filesystem
[  189.689625] XFS (sdc2): Mounting V4 Filesystem
[  189.768990] XFS (sdc2): Ending clean mount
[  189.780954] XFS: Assertion failed: xfs_isilocked(ip, XFS_ILOCK_EXCL), file: fs/xfs/libxfs/xfs_inode_fork.c, line: 501
[  189.791575] ------------[ cut here ]------------
[  189.796191] kernel BUG at fs/xfs/xfs_message.c:113!
[  189.801067] invalid opcode: 0000 [#1] SMP
[  189.805074] Modules linked in: xfs(OE) binfmt_misc ip6t_rpfilter ipt_REJECT nf_reject_ipv4 nf_conntrack_ipv4 nf_defrag_ipv4 ip6t_REJECT nf_reject_ipv6 nf_conntrack_ipv6 nf_defrag_ipv6 xt_conntrack nf_conntrack libcrc32c ip_set nfnetlink ebtable_nat ebtable_broute bridge stp llc ip6table_mangle ip6table_security ip6table_raw iptable_mangle iptable_security iptable_raw ebtable_filter ebtables ip6table_filter ip6_tables iptable_filter btrfs intel_powerclamp coretemp kvm_intel xor kvm raid6_pq irqbypass crc32_pclmul ghash_clmulni_intel iTCO_wdt pcbc iTCO_vendor_support aesni_intel cdc_ether crypto_simd lpc_ich usbnet glue_helper cryptd mii pcspkr mfd_core i7core_edac i2c_i801 nfsd ipmi_si sg ipmi_devintf shpchp edac_core ioatdma ipmi_msghandler dca acpi_cpufreq auth_rpcgss nfs_acl lockd grace sunrpc
[  189.875423]  ip_tables ext4 jbd2 mbcache sr_mod cdrom sd_mod mgag200 i2c_algo_bit drm_kms_helper syscopyarea sysfillrect ata_generic sysimgblt pata_acpi fb_sys_fops ttm drm ata_piix libata crc32c_intel megaraid_sas i2c_core bnx2 dm_mirror dm_region_hash dm_log dm_mod [last unloaded: xfs]
[  189.900855] CPU: 1 PID: 12226 Comm: xfs_io Tainted: G           OE   4.11.0-rc4 #82
[  189.908500] Hardware name: IBM System x3550 M3 -[7944OEJ]-/90Y4784     , BIOS -[D6E150CUS-1.11]- 02/08/2011
[  189.918226] task: ffff880279460000 task.stack: ffffc90002340000
[  189.924198] RIP: 0010:assfail+0x22/0x30 [xfs]
[  189.928553] RSP: 0018:ffffc90002343c00 EFLAGS: 00010282
[  189.933775] RAX: 00000000ffffffea RBX: ffff88016fe4ad00 RCX: 0000000000000021
[  189.940901] RDX: ffffc90002343b28 RSI: 000000000000000a RDI: ffffffffa0a73fb2
[  189.948028] RBP: ffffc90002343c00 R08: 0000000000000000 R09: 0000000000000000
[  189.955154] R10: 000000000000000a R11: f000000000000000 R12: 0000000000000005
[  189.962281] R13: 0000000000000001 R14: ffff8802767cc000 R15: 0000000000000000
[  189.969408] FS:  00007f1cbd156740(0000) GS:ffff88017ba40000(0000) knlGS:0000000000000000
[  189.977489] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  189.983228] CR2: 00007f1cbd162000 CR3: 0000000179ce1000 CR4: 00000000000006e0
[  189.990354] Call Trace:
[  189.992836]  xfs_iread_extents+0x164/0x170 [xfs]
[  189.997477]  xfs_bmapi_read+0x187/0x3b0 [xfs]
[  190.001867]  ? xfs_ilock+0xf1/0x1f0 [xfs]
[  190.005911]  xfs_xattr_iomap_begin+0xbe/0x170 [xfs]
[  190.010790]  ? __alloc_pages_nodemask+0x11d/0x250
[  190.015494]  iomap_apply+0x58/0x100
[  190.018983]  iomap_fiemap+0xa7/0xf0
[  190.022473]  ? iomap_write_actor+0x170/0x170
[  190.026776]  xfs_vn_fiemap+0x5c/0x80 [xfs]
[  190.030873]  do_vfs_ioctl+0x409/0x5b0
[  190.034534]  SyS_ioctl+0x79/0x90
[  190.037767]  do_syscall_64+0x67/0x180
[  190.041431]  entry_SYSCALL64_slow_path+0x25/0x25
[  190.046044] RIP: 0033:0x7f1cbca56507
[  190.049618] RSP: 002b:00007ffd2ef2a358 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[  190.057178] RAX: ffffffffffffffda RBX: 0000000000000005 RCX: 00007f1cbca56507
[  190.064308] RDX: 0000000002020600 RSI: 00000000c020660b RDI: 0000000000000003
[  190.071434] RBP: 0000000000000005 R08: 00007f1cbc9b0938 R09: 0000000000000017
[  190.078560] R10: 00007ffd2ef2a0e0 R11: 0000000000000246 R12: 0000000000000000
[  190.085685] R13: 000000000201f8b0 R14: cccccccccccccccd R15: 0000000002020600
[  190.092813] Code: 2e 0f 1f 84 00 00 00 00 00 66 66 66 66 90 55 48 89 f1 41 89 d0 48 c7 c6 98 04 a8 a0 48 89 fa 31 c0 48 89 e5 31 ff e8 ce f9 ff ff <0f> 0b 66 90 66 2e 0f 1f 84 00 00 00 00 00 66 66 66 66 90 55 48
[  190.111699] RIP: assfail+0x22/0x30 [xfs] RSP: ffffc90002343c00
[  190.117558] ---[ end trace e3b5a0987ffe392b ]---

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] xfs: actually report xattr extents via iomap
  2017-04-07  7:20 ` Eryu Guan
@ 2017-04-07  7:23   ` Christoph Hellwig
  2017-04-07 16:21     ` Darrick J. Wong
  2017-04-07 16:01   ` Darrick J. Wong
  1 sibling, 1 reply; 8+ messages in thread
From: Christoph Hellwig @ 2017-04-07  7:23 UTC (permalink / raw)
  To: Eryu Guan; +Cc: Darrick J. Wong, xfs, Christoph Hellwig

Which just established my point that we should not even try to
support  FIEMAP for xattrs.   There is no use case for it, and it's
untestested.  I wanted to remove it when doing the iomap conversion
and added it back on Dave's insistance..

On Fri, Apr 07, 2017 at 03:20:29PM +0800, Eryu Guan wrote:
> On Thu, Apr 06, 2017 at 09:03:56AM -0700, Darrick J. Wong wrote:
> > Apparently FIEMAP for xattrs has been broken since we switched to
> > the iomap backend because of an incorrect check for xattr presence.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  fs/xfs/xfs_iomap.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> > index 288ee5b..4f118a1 100644
> > --- a/fs/xfs/xfs_iomap.c
> > +++ b/fs/xfs/xfs_iomap.c
> > @@ -1173,7 +1173,7 @@ xfs_xattr_iomap_begin(
> >  	lockmode = xfs_ilock_data_map_shared(ip);
> >  
> >  	/* if there are no attribute fork or extents, return ENOENT */
> > -	if (XFS_IFORK_Q(ip) || !ip->i_d.di_anextents) {
> > +	if (!XFS_IFORK_Q(ip) || !ip->i_d.di_anextents) {
> >  		error = -ENOENT;
> >  		goto out_unlock;
> >  	}
> 
> Surprisingly, after applying this patch on top of 4.11-rc4 kernel, your
> test case crashed debug built XFS when testing with 512b block fs, XFS
> with other block sizes passed the test.
> 
> The three new patches from tags/xfs-4.11-fixes-3 won't help either.
> 
> Thanks,
> Eryu
> 
> [  188.954656] run fstests generic/425 at 2017-04-07 14:51:54
> [  189.311170] XFS (sdc2): Unmounting Filesystem
> [  189.488020] XFS (sdc2): Mounting V4 Filesystem
> [  189.533595] XFS (sdc2): Ending clean mount
> [  189.669876] XFS (sdc2): Unmounting Filesystem
> [  189.689625] XFS (sdc2): Mounting V4 Filesystem
> [  189.768990] XFS (sdc2): Ending clean mount
> [  189.780954] XFS: Assertion failed: xfs_isilocked(ip, XFS_ILOCK_EXCL), file: fs/xfs/libxfs/xfs_inode_fork.c, line: 501
> [  189.791575] ------------[ cut here ]------------
> [  189.796191] kernel BUG at fs/xfs/xfs_message.c:113!
> [  189.801067] invalid opcode: 0000 [#1] SMP
> [  189.805074] Modules linked in: xfs(OE) binfmt_misc ip6t_rpfilter ipt_REJECT nf_reject_ipv4 nf_conntrack_ipv4 nf_defrag_ipv4 ip6t_REJECT nf_reject_ipv6 nf_conntrack_ipv6 nf_defrag_ipv6 xt_conntrack nf_conntrack libcrc32c ip_set nfnetlink ebtable_nat ebtable_broute bridge stp llc ip6table_mangle ip6table_security ip6table_raw iptable_mangle iptable_security iptable_raw ebtable_filter ebtables ip6table_filter ip6_tables iptable_filter btrfs intel_powerclamp coretemp kvm_intel xor kvm raid6_pq irqbypass crc32_pclmul ghash_clmulni_intel iTCO_wdt pcbc iTCO_vendor_support aesni_intel cdc_ether crypto_simd lpc_ich usbnet glue_helper cryptd mii pcspkr mfd_core i7core_edac i2c_i801 nfsd ipmi_si sg ipmi_devintf shpchp edac_core ioatdma ipmi_msghandler dca acpi_cpufreq auth_rpcgss nfs_acl lockd grace sunrpc
> [  189.875423]  ip_tables ext4 jbd2 mbcache sr_mod cdrom sd_mod mgag200 i2c_algo_bit drm_kms_helper syscopyarea sysfillrect ata_generic sysimgblt pata_acpi fb_sys_fops ttm drm ata_piix libata crc32c_intel megaraid_sas i2c_core bnx2 dm_mirror dm_region_hash dm_log dm_mod [last unloaded: xfs]
> [  189.900855] CPU: 1 PID: 12226 Comm: xfs_io Tainted: G           OE   4.11.0-rc4 #82
> [  189.908500] Hardware name: IBM System x3550 M3 -[7944OEJ]-/90Y4784     , BIOS -[D6E150CUS-1.11]- 02/08/2011
> [  189.918226] task: ffff880279460000 task.stack: ffffc90002340000
> [  189.924198] RIP: 0010:assfail+0x22/0x30 [xfs]
> [  189.928553] RSP: 0018:ffffc90002343c00 EFLAGS: 00010282
> [  189.933775] RAX: 00000000ffffffea RBX: ffff88016fe4ad00 RCX: 0000000000000021
> [  189.940901] RDX: ffffc90002343b28 RSI: 000000000000000a RDI: ffffffffa0a73fb2
> [  189.948028] RBP: ffffc90002343c00 R08: 0000000000000000 R09: 0000000000000000
> [  189.955154] R10: 000000000000000a R11: f000000000000000 R12: 0000000000000005
> [  189.962281] R13: 0000000000000001 R14: ffff8802767cc000 R15: 0000000000000000
> [  189.969408] FS:  00007f1cbd156740(0000) GS:ffff88017ba40000(0000) knlGS:0000000000000000
> [  189.977489] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  189.983228] CR2: 00007f1cbd162000 CR3: 0000000179ce1000 CR4: 00000000000006e0
> [  189.990354] Call Trace:
> [  189.992836]  xfs_iread_extents+0x164/0x170 [xfs]
> [  189.997477]  xfs_bmapi_read+0x187/0x3b0 [xfs]
> [  190.001867]  ? xfs_ilock+0xf1/0x1f0 [xfs]
> [  190.005911]  xfs_xattr_iomap_begin+0xbe/0x170 [xfs]
> [  190.010790]  ? __alloc_pages_nodemask+0x11d/0x250
> [  190.015494]  iomap_apply+0x58/0x100
> [  190.018983]  iomap_fiemap+0xa7/0xf0
> [  190.022473]  ? iomap_write_actor+0x170/0x170
> [  190.026776]  xfs_vn_fiemap+0x5c/0x80 [xfs]
> [  190.030873]  do_vfs_ioctl+0x409/0x5b0
> [  190.034534]  SyS_ioctl+0x79/0x90
> [  190.037767]  do_syscall_64+0x67/0x180
> [  190.041431]  entry_SYSCALL64_slow_path+0x25/0x25
> [  190.046044] RIP: 0033:0x7f1cbca56507
> [  190.049618] RSP: 002b:00007ffd2ef2a358 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> [  190.057178] RAX: ffffffffffffffda RBX: 0000000000000005 RCX: 00007f1cbca56507
> [  190.064308] RDX: 0000000002020600 RSI: 00000000c020660b RDI: 0000000000000003
> [  190.071434] RBP: 0000000000000005 R08: 00007f1cbc9b0938 R09: 0000000000000017
> [  190.078560] R10: 00007ffd2ef2a0e0 R11: 0000000000000246 R12: 0000000000000000
> [  190.085685] R13: 000000000201f8b0 R14: cccccccccccccccd R15: 0000000002020600
> [  190.092813] Code: 2e 0f 1f 84 00 00 00 00 00 66 66 66 66 90 55 48 89 f1 41 89 d0 48 c7 c6 98 04 a8 a0 48 89 fa 31 c0 48 89 e5 31 ff e8 ce f9 ff ff <0f> 0b 66 90 66 2e 0f 1f 84 00 00 00 00 00 66 66 66 66 90 55 48
> [  190.111699] RIP: assfail+0x22/0x30 [xfs] RSP: ffffc90002343c00
> [  190.117558] ---[ end trace e3b5a0987ffe392b ]---
---end quoted text---

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] xfs: actually report xattr extents via iomap
  2017-04-07  7:20 ` Eryu Guan
  2017-04-07  7:23   ` Christoph Hellwig
@ 2017-04-07 16:01   ` Darrick J. Wong
  1 sibling, 0 replies; 8+ messages in thread
From: Darrick J. Wong @ 2017-04-07 16:01 UTC (permalink / raw)
  To: Eryu Guan; +Cc: xfs, Christoph Hellwig

On Fri, Apr 07, 2017 at 03:20:29PM +0800, Eryu Guan wrote:
> On Thu, Apr 06, 2017 at 09:03:56AM -0700, Darrick J. Wong wrote:
> > Apparently FIEMAP for xattrs has been broken since we switched to
> > the iomap backend because of an incorrect check for xattr presence.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  fs/xfs/xfs_iomap.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> > index 288ee5b..4f118a1 100644
> > --- a/fs/xfs/xfs_iomap.c
> > +++ b/fs/xfs/xfs_iomap.c
> > @@ -1173,7 +1173,7 @@ xfs_xattr_iomap_begin(
> >  	lockmode = xfs_ilock_data_map_shared(ip);
> >  
> >  	/* if there are no attribute fork or extents, return ENOENT */
> > -	if (XFS_IFORK_Q(ip) || !ip->i_d.di_anextents) {
> > +	if (!XFS_IFORK_Q(ip) || !ip->i_d.di_anextents) {
> >  		error = -ENOENT;
> >  		goto out_unlock;
> >  	}
> 
> Surprisingly, after applying this patch on top of 4.11-rc4 kernel, your
> test case crashed debug built XFS when testing with 512b block fs, XFS
> with other block sizes passed the test.

The xfs_ilock_data_map_shared should be xfs_ilock_attr_map_shared.
Will send updated patch, thank you for catching this.

> The three new patches from tags/xfs-4.11-fixes-3 won't help either.

Fortunately, without this patch we never bother looking for results,
so we don't trip the assert.

--D

> 
> Thanks,
> Eryu
> 
> [  188.954656] run fstests generic/425 at 2017-04-07 14:51:54
> [  189.311170] XFS (sdc2): Unmounting Filesystem
> [  189.488020] XFS (sdc2): Mounting V4 Filesystem
> [  189.533595] XFS (sdc2): Ending clean mount
> [  189.669876] XFS (sdc2): Unmounting Filesystem
> [  189.689625] XFS (sdc2): Mounting V4 Filesystem
> [  189.768990] XFS (sdc2): Ending clean mount
> [  189.780954] XFS: Assertion failed: xfs_isilocked(ip, XFS_ILOCK_EXCL), file: fs/xfs/libxfs/xfs_inode_fork.c, line: 501
> [  189.791575] ------------[ cut here ]------------
> [  189.796191] kernel BUG at fs/xfs/xfs_message.c:113!
> [  189.801067] invalid opcode: 0000 [#1] SMP
> [  189.805074] Modules linked in: xfs(OE) binfmt_misc ip6t_rpfilter ipt_REJECT nf_reject_ipv4 nf_conntrack_ipv4 nf_defrag_ipv4 ip6t_REJECT nf_reject_ipv6 nf_conntrack_ipv6 nf_defrag_ipv6 xt_conntrack nf_conntrack libcrc32c ip_set nfnetlink ebtable_nat ebtable_broute bridge stp llc ip6table_mangle ip6table_security ip6table_raw iptable_mangle iptable_security iptable_raw ebtable_filter ebtables ip6table_filter ip6_tables iptable_filter btrfs intel_powerclamp coretemp kvm_intel xor kvm raid6_pq irqbypass crc32_pclmul ghash_clmulni_intel iTCO_wdt pcbc iTCO_vendor_support aesni_intel cdc_ether crypto_simd lpc_ich usbnet glue_helper cryptd mii pcspkr mfd_core i7core_edac i2c_i801 nfsd ipmi_si sg ipmi_devintf shpchp edac_core ioatdma ipmi_msghandler dca acpi_cpufreq auth_rpcgss nfs_acl lockd grace sunrpc
> [  189.875423]  ip_tables ext4 jbd2 mbcache sr_mod cdrom sd_mod mgag200 i2c_algo_bit drm_kms_helper syscopyarea sysfillrect ata_generic sysimgblt pata_acpi fb_sys_fops ttm drm ata_piix libata crc32c_intel megaraid_sas i2c_core bnx2 dm_mirror dm_region_hash dm_log dm_mod [last unloaded: xfs]
> [  189.900855] CPU: 1 PID: 12226 Comm: xfs_io Tainted: G           OE   4.11.0-rc4 #82
> [  189.908500] Hardware name: IBM System x3550 M3 -[7944OEJ]-/90Y4784     , BIOS -[D6E150CUS-1.11]- 02/08/2011
> [  189.918226] task: ffff880279460000 task.stack: ffffc90002340000
> [  189.924198] RIP: 0010:assfail+0x22/0x30 [xfs]
> [  189.928553] RSP: 0018:ffffc90002343c00 EFLAGS: 00010282
> [  189.933775] RAX: 00000000ffffffea RBX: ffff88016fe4ad00 RCX: 0000000000000021
> [  189.940901] RDX: ffffc90002343b28 RSI: 000000000000000a RDI: ffffffffa0a73fb2
> [  189.948028] RBP: ffffc90002343c00 R08: 0000000000000000 R09: 0000000000000000
> [  189.955154] R10: 000000000000000a R11: f000000000000000 R12: 0000000000000005
> [  189.962281] R13: 0000000000000001 R14: ffff8802767cc000 R15: 0000000000000000
> [  189.969408] FS:  00007f1cbd156740(0000) GS:ffff88017ba40000(0000) knlGS:0000000000000000
> [  189.977489] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  189.983228] CR2: 00007f1cbd162000 CR3: 0000000179ce1000 CR4: 00000000000006e0
> [  189.990354] Call Trace:
> [  189.992836]  xfs_iread_extents+0x164/0x170 [xfs]
> [  189.997477]  xfs_bmapi_read+0x187/0x3b0 [xfs]
> [  190.001867]  ? xfs_ilock+0xf1/0x1f0 [xfs]
> [  190.005911]  xfs_xattr_iomap_begin+0xbe/0x170 [xfs]
> [  190.010790]  ? __alloc_pages_nodemask+0x11d/0x250
> [  190.015494]  iomap_apply+0x58/0x100
> [  190.018983]  iomap_fiemap+0xa7/0xf0
> [  190.022473]  ? iomap_write_actor+0x170/0x170
> [  190.026776]  xfs_vn_fiemap+0x5c/0x80 [xfs]
> [  190.030873]  do_vfs_ioctl+0x409/0x5b0
> [  190.034534]  SyS_ioctl+0x79/0x90
> [  190.037767]  do_syscall_64+0x67/0x180
> [  190.041431]  entry_SYSCALL64_slow_path+0x25/0x25
> [  190.046044] RIP: 0033:0x7f1cbca56507
> [  190.049618] RSP: 002b:00007ffd2ef2a358 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> [  190.057178] RAX: ffffffffffffffda RBX: 0000000000000005 RCX: 00007f1cbca56507
> [  190.064308] RDX: 0000000002020600 RSI: 00000000c020660b RDI: 0000000000000003
> [  190.071434] RBP: 0000000000000005 R08: 00007f1cbc9b0938 R09: 0000000000000017
> [  190.078560] R10: 00007ffd2ef2a0e0 R11: 0000000000000246 R12: 0000000000000000
> [  190.085685] R13: 000000000201f8b0 R14: cccccccccccccccd R15: 0000000002020600
> [  190.092813] Code: 2e 0f 1f 84 00 00 00 00 00 66 66 66 66 90 55 48 89 f1 41 89 d0 48 c7 c6 98 04 a8 a0 48 89 fa 31 c0 48 89 e5 31 ff e8 ce f9 ff ff <0f> 0b 66 90 66 2e 0f 1f 84 00 00 00 00 00 66 66 66 66 90 55 48
> [  190.111699] RIP: assfail+0x22/0x30 [xfs] RSP: ffffc90002343c00
> [  190.117558] ---[ end trace e3b5a0987ffe392b ]---
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] xfs: actually report xattr extents via iomap
  2017-04-07  7:23   ` Christoph Hellwig
@ 2017-04-07 16:21     ` Darrick J. Wong
  0 siblings, 0 replies; 8+ messages in thread
From: Darrick J. Wong @ 2017-04-07 16:21 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Eryu Guan, xfs

On Fri, Apr 07, 2017 at 12:23:45AM -0700, Christoph Hellwig wrote:
> Which just established my point that we should not even try to
> support  FIEMAP for xattrs.   There is no use case for it, and it's
> untestested.  I wanted to remove it when doing the iomap conversion
> and added it back on Dave's insistance..

XFS (and ext4) have historically reported results for xattr blocks, so I
consider this a behavioral regression.

I understand that FIEMAP returns potentially stale results, has bitten
people in the past, and that they should use SEEK_{HOLE,DATA}.  That
alone could argue for withdrawing FIEMAP entirely.

However, if the use case for FIEMAP is to enable us to debug fs space
management behaviors, then I think attr fork reporting should stay.
Frankly, that's the /only/ use case I can think of for FIEMAP, even
though it doesn't handle multi-device filesystems like btrfs and XFS.

--D

> 
> On Fri, Apr 07, 2017 at 03:20:29PM +0800, Eryu Guan wrote:
> > On Thu, Apr 06, 2017 at 09:03:56AM -0700, Darrick J. Wong wrote:
> > > Apparently FIEMAP for xattrs has been broken since we switched to
> > > the iomap backend because of an incorrect check for xattr presence.
> > > 
> > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > > ---
> > >  fs/xfs/xfs_iomap.c |    2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> > > index 288ee5b..4f118a1 100644
> > > --- a/fs/xfs/xfs_iomap.c
> > > +++ b/fs/xfs/xfs_iomap.c
> > > @@ -1173,7 +1173,7 @@ xfs_xattr_iomap_begin(
> > >  	lockmode = xfs_ilock_data_map_shared(ip);
> > >  
> > >  	/* if there are no attribute fork or extents, return ENOENT */
> > > -	if (XFS_IFORK_Q(ip) || !ip->i_d.di_anextents) {
> > > +	if (!XFS_IFORK_Q(ip) || !ip->i_d.di_anextents) {
> > >  		error = -ENOENT;
> > >  		goto out_unlock;
> > >  	}
> > 
> > Surprisingly, after applying this patch on top of 4.11-rc4 kernel, your
> > test case crashed debug built XFS when testing with 512b block fs, XFS
> > with other block sizes passed the test.
> > 
> > The three new patches from tags/xfs-4.11-fixes-3 won't help either.
> > 
> > Thanks,
> > Eryu
> > 
> > [  188.954656] run fstests generic/425 at 2017-04-07 14:51:54
> > [  189.311170] XFS (sdc2): Unmounting Filesystem
> > [  189.488020] XFS (sdc2): Mounting V4 Filesystem
> > [  189.533595] XFS (sdc2): Ending clean mount
> > [  189.669876] XFS (sdc2): Unmounting Filesystem
> > [  189.689625] XFS (sdc2): Mounting V4 Filesystem
> > [  189.768990] XFS (sdc2): Ending clean mount
> > [  189.780954] XFS: Assertion failed: xfs_isilocked(ip, XFS_ILOCK_EXCL), file: fs/xfs/libxfs/xfs_inode_fork.c, line: 501
> > [  189.791575] ------------[ cut here ]------------
> > [  189.796191] kernel BUG at fs/xfs/xfs_message.c:113!
> > [  189.801067] invalid opcode: 0000 [#1] SMP
> > [  189.805074] Modules linked in: xfs(OE) binfmt_misc ip6t_rpfilter ipt_REJECT nf_reject_ipv4 nf_conntrack_ipv4 nf_defrag_ipv4 ip6t_REJECT nf_reject_ipv6 nf_conntrack_ipv6 nf_defrag_ipv6 xt_conntrack nf_conntrack libcrc32c ip_set nfnetlink ebtable_nat ebtable_broute bridge stp llc ip6table_mangle ip6table_security ip6table_raw iptable_mangle iptable_security iptable_raw ebtable_filter ebtables ip6table_filter ip6_tables iptable_filter btrfs intel_powerclamp coretemp kvm_intel xor kvm raid6_pq irqbypass crc32_pclmul ghash_clmulni_intel iTCO_wdt pcbc iTCO_vendor_support aesni_intel cdc_ether crypto_simd lpc_ich usbnet glue_helper cryptd mii pcspkr mfd_core i7core_edac i2c_i801 nfsd ipmi_si sg ipmi_devintf shpchp edac_core ioatdma ipmi_msghandler dca acpi_cpufreq auth_rpcgss nfs_acl lockd grace sunrpc
> > [  189.875423]  ip_tables ext4 jbd2 mbcache sr_mod cdrom sd_mod mgag200 i2c_algo_bit drm_kms_helper syscopyarea sysfillrect ata_generic sysimgblt pata_acpi fb_sys_fops ttm drm ata_piix libata crc32c_intel megaraid_sas i2c_core bnx2 dm_mirror dm_region_hash dm_log dm_mod [last unloaded: xfs]
> > [  189.900855] CPU: 1 PID: 12226 Comm: xfs_io Tainted: G           OE   4.11.0-rc4 #82
> > [  189.908500] Hardware name: IBM System x3550 M3 -[7944OEJ]-/90Y4784     , BIOS -[D6E150CUS-1.11]- 02/08/2011
> > [  189.918226] task: ffff880279460000 task.stack: ffffc90002340000
> > [  189.924198] RIP: 0010:assfail+0x22/0x30 [xfs]
> > [  189.928553] RSP: 0018:ffffc90002343c00 EFLAGS: 00010282
> > [  189.933775] RAX: 00000000ffffffea RBX: ffff88016fe4ad00 RCX: 0000000000000021
> > [  189.940901] RDX: ffffc90002343b28 RSI: 000000000000000a RDI: ffffffffa0a73fb2
> > [  189.948028] RBP: ffffc90002343c00 R08: 0000000000000000 R09: 0000000000000000
> > [  189.955154] R10: 000000000000000a R11: f000000000000000 R12: 0000000000000005
> > [  189.962281] R13: 0000000000000001 R14: ffff8802767cc000 R15: 0000000000000000
> > [  189.969408] FS:  00007f1cbd156740(0000) GS:ffff88017ba40000(0000) knlGS:0000000000000000
> > [  189.977489] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [  189.983228] CR2: 00007f1cbd162000 CR3: 0000000179ce1000 CR4: 00000000000006e0
> > [  189.990354] Call Trace:
> > [  189.992836]  xfs_iread_extents+0x164/0x170 [xfs]
> > [  189.997477]  xfs_bmapi_read+0x187/0x3b0 [xfs]
> > [  190.001867]  ? xfs_ilock+0xf1/0x1f0 [xfs]
> > [  190.005911]  xfs_xattr_iomap_begin+0xbe/0x170 [xfs]
> > [  190.010790]  ? __alloc_pages_nodemask+0x11d/0x250
> > [  190.015494]  iomap_apply+0x58/0x100
> > [  190.018983]  iomap_fiemap+0xa7/0xf0
> > [  190.022473]  ? iomap_write_actor+0x170/0x170
> > [  190.026776]  xfs_vn_fiemap+0x5c/0x80 [xfs]
> > [  190.030873]  do_vfs_ioctl+0x409/0x5b0
> > [  190.034534]  SyS_ioctl+0x79/0x90
> > [  190.037767]  do_syscall_64+0x67/0x180
> > [  190.041431]  entry_SYSCALL64_slow_path+0x25/0x25
> > [  190.046044] RIP: 0033:0x7f1cbca56507
> > [  190.049618] RSP: 002b:00007ffd2ef2a358 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> > [  190.057178] RAX: ffffffffffffffda RBX: 0000000000000005 RCX: 00007f1cbca56507
> > [  190.064308] RDX: 0000000002020600 RSI: 00000000c020660b RDI: 0000000000000003
> > [  190.071434] RBP: 0000000000000005 R08: 00007f1cbc9b0938 R09: 0000000000000017
> > [  190.078560] R10: 00007ffd2ef2a0e0 R11: 0000000000000246 R12: 0000000000000000
> > [  190.085685] R13: 000000000201f8b0 R14: cccccccccccccccd R15: 0000000002020600
> > [  190.092813] Code: 2e 0f 1f 84 00 00 00 00 00 66 66 66 66 90 55 48 89 f1 41 89 d0 48 c7 c6 98 04 a8 a0 48 89 fa 31 c0 48 89 e5 31 ff e8 ce f9 ff ff <0f> 0b 66 90 66 2e 0f 1f 84 00 00 00 00 00 66 66 66 66 90 55 48
> > [  190.111699] RIP: assfail+0x22/0x30 [xfs] RSP: ffffc90002343c00
> > [  190.117558] ---[ end trace e3b5a0987ffe392b ]---
> ---end quoted text---
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-04-07 16:21 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-06 16:03 [PATCH] xfs: actually report xattr extents via iomap Darrick J. Wong
2017-04-06 16:58 ` Brian Foster
2017-04-06 18:37 ` Eric Sandeen
2017-04-06 18:38   ` Eric Sandeen
2017-04-07  7:20 ` Eryu Guan
2017-04-07  7:23   ` Christoph Hellwig
2017-04-07 16:21     ` Darrick J. Wong
2017-04-07 16:01   ` Darrick J. Wong

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.