All of lore.kernel.org
 help / color / mirror / Atom feed
* Kernel OPS when using xattr
@ 2020-12-03  8:20 Mkrtchyan, Tigran
  2020-12-03 14:07 ` Olga Kornievskaia
  2020-12-03 15:07 ` Trond Myklebust
  0 siblings, 2 replies; 15+ messages in thread
From: Mkrtchyan, Tigran @ 2020-12-03  8:20 UTC (permalink / raw)
  To: linux-nfs; +Cc: trondmy, Frank van der Linden


Dear NFS folk,

this is I got while accessing xattrs over NFS with 5.10.0-rc6 kernel
from Trond's testing tree (8102e956f22e710eecb3913cdd236282213812cf).
The 5.9 kernel works as expected.

Tigran.

[ 2803.765467] nfs4flexfilelayout_init: NFSv4 Flexfile Layout Driver Registering...
[59837.811426] general protection fault, probably for non-canonical address 0x5088000000ffc: 0000 [#1] SMP PTI
[59837.811433] CPU: 3 PID: 3858 Comm: attr Not tainted 5.10.0-rc6+ #60
[59837.811435] Hardware name: Dell Inc. Latitude E6520/0J4TFW, BIOS A06 07/11/2011
[59837.811442] RIP: 0010:__memmove+0xe2/0x1a0
[59837.811445] Code: 1f 84 00 00 00 00 00 90 48 83 fa 20 72 50 48 81 fa a8 02 00 00 72 05 40 38 fe 74 bc 48 01 d6 48 01 d7 48 83 ea 20 48 83 ea 20 <4c> 8b 5e f8 4c 8b 56 f0 4c 8b 4e e8 4c 8b 46 e0 48 8d 76 e0 4c 89
[59837.811447] RSP: 0018:ffffc90002b4f870 EFLAGS: 00010202
[59837.811451] RAX: 0005088000000004 RBX: 0000000000000000 RCX: 000000000000fffc
[59837.811453] RDX: 0000000000000fbc RSI: 0005088000000ffc RDI: 0005088000001000
[59837.811455] RBP: ffff88810be80000 R08: 0005088000000ffc R09: ffff888235aec550
[59837.811457] R10: 0000000000000356 R11: 000000000000016c R12: 0000000000000004
[59837.811459] R13: 000000000000fffc R14: ffff88810be80000 R15: ffffc90002b4fc58
[59837.811462] FS:  00007fd9303ff740(0000) GS:ffff888235ac0000(0000) knlGS:0000000000000000
[59837.811465] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[59837.811467] CR2: 00007fd9305d2000 CR3: 000000011fc10005 CR4: 00000000000606e0
[59837.811469] Call Trace:
[59837.811505]  _shift_data_right_pages+0x11e/0x150 [sunrpc]
[59837.811531]  xdr_shrink_bufhead+0x151/0x170 [sunrpc]
[59837.811555]  xdr_realign_pages+0x4c/0xa0 [sunrpc]
[59837.811578]  xdr_align_pages+0x49/0x120 [sunrpc]
[59837.811601]  xdr_read_pages+0x23/0xb0 [sunrpc]
[59837.811626]  nfs4_xdr_dec_getxattr+0xfa/0x120 [nfsv4]
[59837.811643]  call_decode+0x199/0x1f0 [sunrpc]
[59837.811659]  ? rpc_decode_header+0x4e0/0x4e0 [sunrpc]
[59837.811678]  __rpc_execute+0x71/0x420 [sunrpc]
[59837.811700]  ? xprt_iter_default_rewind+0x10/0x10 [sunrpc]
[59837.811721]  ? xprt_iter_get_next+0x4a/0x60 [sunrpc]
[59837.811737]  rpc_run_task+0x14c/0x180 [sunrpc]
[59837.811756]  nfs4_do_call_sync+0x6e/0xb0 [nfsv4]
[59837.811785]  _nfs42_proc_getxattr+0xb7/0x170 [nfsv4]
[59837.811809]  ? xprt_iter_get_next+0x4a/0x60 [sunrpc]
[59837.811830]  nfs42_proc_getxattr+0x86/0xb0 [nfsv4]
[59837.811844]  nfs4_xattr_get_nfs4_user+0xc9/0xe0 [nfsv4]
[59837.811849]  vfs_getxattr+0x161/0x1a0
[59837.811852]  getxattr+0x14f/0x230
[59837.811856]  ? filename_lookup+0x123/0x1b0
[59837.811861]  ? _cond_resched+0x16/0x40
[59837.811864]  ? kmem_cache_alloc+0x3c4/0x4b0
[59837.811866]  ? getname_flags.part.0+0x45/0x1a0
[59837.811869]  path_getxattr+0x62/0xb0
[59837.811873]  do_syscall_64+0x33/0x40
[59837.811876]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[59837.811879] RIP: 0033:0x7fd93050162e
[59837.811883] Code: 48 8b 0d 4d 48 0c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 49 89 ca b8 c0 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 1a 48 0c 00 f7 d8 64 89 01 48
[59837.811884] RSP: 002b:00007ffe178ad1c8 EFLAGS: 00000202 ORIG_RAX: 00000000000000c0
[59837.811887] RAX: ffffffffffffffda RBX: 00007ffe178ad330 RCX: 00007fd93050162e
[59837.811889] RDX: 0000000000000000 RSI: 00007ffe178ad330 RDI: 00007ffe178bf525
[59837.811890] RBP: 0000000000000000 R08: 00007ffe178ad210 R09: 0000000000000000
[59837.811892] R10: 0000000000000000 R11: 0000000000000202 R12: 0000000000000001
[59837.811894] R13: 00007ffe178ad210 R14: 00007ffe178bf525 R15: 0000000000000000
[59837.811896] Modules linked in: nfs_layout_flexfiles rpcsec_gss_krb5 auth_rpcgss nfsv4 dns_resolver nfs lockd grace nfs_ssc fscache nf_conntrack_netbios_ns nf_conntrack_broadcast nft_ct nf_tables ebtable_nat ebtable_broute ip6table_nat ip6table_mangle ip6table_raw ip6table_security iptable_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 iptable_mangle iptable_raw iptable_security ip_set nfnetlink ebtable_filter ebtables ip6table_filter ip6_tables sunrpc snd_hda_codec_idt snd_hda_codec_generic ledtrig_audio nouveau i915 iwldvm mac80211 btrfs snd_hda_codec_hdmi snd_hda_intel snd_intel_ds

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

* Re: Kernel OPS when using xattr
  2020-12-03  8:20 Kernel OPS when using xattr Mkrtchyan, Tigran
@ 2020-12-03 14:07 ` Olga Kornievskaia
  2020-12-03 15:43   ` Mkrtchyan, Tigran
  2020-12-03 15:07 ` Trond Myklebust
  1 sibling, 1 reply; 15+ messages in thread
From: Olga Kornievskaia @ 2020-12-03 14:07 UTC (permalink / raw)
  To: Mkrtchyan, Tigran; +Cc: linux-nfs, trondmy, Frank van der Linden

On Thu, Dec 3, 2020 at 3:22 AM Mkrtchyan, Tigran
<tigran.mkrtchyan@desy.de> wrote:
>
>
> Dear NFS folk,
>
> this is I got while accessing xattrs over NFS with 5.10.0-rc6 kernel
> from Trond's testing tree (8102e956f22e710eecb3913cdd236282213812cf).
> The 5.9 kernel works as expected.

Is that with Frank's getxattr patch?
https://www.spinics.net/lists/linux-nfs/msg81183.html

>
> Tigran.
>
> [ 2803.765467] nfs4flexfilelayout_init: NFSv4 Flexfile Layout Driver Registering...
> [59837.811426] general protection fault, probably for non-canonical address 0x5088000000ffc: 0000 [#1] SMP PTI
> [59837.811433] CPU: 3 PID: 3858 Comm: attr Not tainted 5.10.0-rc6+ #60
> [59837.811435] Hardware name: Dell Inc. Latitude E6520/0J4TFW, BIOS A06 07/11/2011
> [59837.811442] RIP: 0010:__memmove+0xe2/0x1a0
> [59837.811445] Code: 1f 84 00 00 00 00 00 90 48 83 fa 20 72 50 48 81 fa a8 02 00 00 72 05 40 38 fe 74 bc 48 01 d6 48 01 d7 48 83 ea 20 48 83 ea 20 <4c> 8b 5e f8 4c 8b 56 f0 4c 8b 4e e8 4c 8b 46 e0 48 8d 76 e0 4c 89
> [59837.811447] RSP: 0018:ffffc90002b4f870 EFLAGS: 00010202
> [59837.811451] RAX: 0005088000000004 RBX: 0000000000000000 RCX: 000000000000fffc
> [59837.811453] RDX: 0000000000000fbc RSI: 0005088000000ffc RDI: 0005088000001000
> [59837.811455] RBP: ffff88810be80000 R08: 0005088000000ffc R09: ffff888235aec550
> [59837.811457] R10: 0000000000000356 R11: 000000000000016c R12: 0000000000000004
> [59837.811459] R13: 000000000000fffc R14: ffff88810be80000 R15: ffffc90002b4fc58
> [59837.811462] FS:  00007fd9303ff740(0000) GS:ffff888235ac0000(0000) knlGS:0000000000000000
> [59837.811465] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [59837.811467] CR2: 00007fd9305d2000 CR3: 000000011fc10005 CR4: 00000000000606e0
> [59837.811469] Call Trace:
> [59837.811505]  _shift_data_right_pages+0x11e/0x150 [sunrpc]
> [59837.811531]  xdr_shrink_bufhead+0x151/0x170 [sunrpc]
> [59837.811555]  xdr_realign_pages+0x4c/0xa0 [sunrpc]
> [59837.811578]  xdr_align_pages+0x49/0x120 [sunrpc]
> [59837.811601]  xdr_read_pages+0x23/0xb0 [sunrpc]
> [59837.811626]  nfs4_xdr_dec_getxattr+0xfa/0x120 [nfsv4]
> [59837.811643]  call_decode+0x199/0x1f0 [sunrpc]
> [59837.811659]  ? rpc_decode_header+0x4e0/0x4e0 [sunrpc]
> [59837.811678]  __rpc_execute+0x71/0x420 [sunrpc]
> [59837.811700]  ? xprt_iter_default_rewind+0x10/0x10 [sunrpc]
> [59837.811721]  ? xprt_iter_get_next+0x4a/0x60 [sunrpc]
> [59837.811737]  rpc_run_task+0x14c/0x180 [sunrpc]
> [59837.811756]  nfs4_do_call_sync+0x6e/0xb0 [nfsv4]
> [59837.811785]  _nfs42_proc_getxattr+0xb7/0x170 [nfsv4]
> [59837.811809]  ? xprt_iter_get_next+0x4a/0x60 [sunrpc]
> [59837.811830]  nfs42_proc_getxattr+0x86/0xb0 [nfsv4]
> [59837.811844]  nfs4_xattr_get_nfs4_user+0xc9/0xe0 [nfsv4]
> [59837.811849]  vfs_getxattr+0x161/0x1a0
> [59837.811852]  getxattr+0x14f/0x230
> [59837.811856]  ? filename_lookup+0x123/0x1b0
> [59837.811861]  ? _cond_resched+0x16/0x40
> [59837.811864]  ? kmem_cache_alloc+0x3c4/0x4b0
> [59837.811866]  ? getname_flags.part.0+0x45/0x1a0
> [59837.811869]  path_getxattr+0x62/0xb0
> [59837.811873]  do_syscall_64+0x33/0x40
> [59837.811876]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
> [59837.811879] RIP: 0033:0x7fd93050162e
> [59837.811883] Code: 48 8b 0d 4d 48 0c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 49 89 ca b8 c0 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 1a 48 0c 00 f7 d8 64 89 01 48
> [59837.811884] RSP: 002b:00007ffe178ad1c8 EFLAGS: 00000202 ORIG_RAX: 00000000000000c0
> [59837.811887] RAX: ffffffffffffffda RBX: 00007ffe178ad330 RCX: 00007fd93050162e
> [59837.811889] RDX: 0000000000000000 RSI: 00007ffe178ad330 RDI: 00007ffe178bf525
> [59837.811890] RBP: 0000000000000000 R08: 00007ffe178ad210 R09: 0000000000000000
> [59837.811892] R10: 0000000000000000 R11: 0000000000000202 R12: 0000000000000001
> [59837.811894] R13: 00007ffe178ad210 R14: 00007ffe178bf525 R15: 0000000000000000
> [59837.811896] Modules linked in: nfs_layout_flexfiles rpcsec_gss_krb5 auth_rpcgss nfsv4 dns_resolver nfs lockd grace nfs_ssc fscache nf_conntrack_netbios_ns nf_conntrack_broadcast nft_ct nf_tables ebtable_nat ebtable_broute ip6table_nat ip6table_mangle ip6table_raw ip6table_security iptable_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 iptable_mangle iptable_raw iptable_security ip_set nfnetlink ebtable_filter ebtables ip6table_filter ip6_tables sunrpc snd_hda_codec_idt snd_hda_codec_generic ledtrig_audio nouveau i915 iwldvm mac80211 btrfs snd_hda_codec_hdmi snd_hda_intel snd_intel_ds

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

* Re: Kernel OPS when using xattr
  2020-12-03  8:20 Kernel OPS when using xattr Mkrtchyan, Tigran
  2020-12-03 14:07 ` Olga Kornievskaia
@ 2020-12-03 15:07 ` Trond Myklebust
  2020-12-03 16:04   ` Mkrtchyan, Tigran
  1 sibling, 1 reply; 15+ messages in thread
From: Trond Myklebust @ 2020-12-03 15:07 UTC (permalink / raw)
  To: tigran.mkrtchyan, linux-nfs; +Cc: fllinden

Hi Tigran,

On Thu, 2020-12-03 at 09:20 +0100, Mkrtchyan, Tigran wrote:
> 
> Dear NFS folk,
> 
> this is I got while accessing xattrs over NFS with 5.10.0-rc6 kernel
> from Trond's testing tree (8102e956f22e710eecb3913cdd236282213812cf).
> The 5.9 kernel works as expected.
> 
> Tigran.
> 
> [ 2803.765467] nfs4flexfilelayout_init: NFSv4 Flexfile Layout Driver
> Registering...
> [59837.811426] general protection fault, probably for non-canonical
> address 0x5088000000ffc: 0000 [#1] SMP PTI
> [59837.811433] CPU: 3 PID: 3858 Comm: attr Not tainted 5.10.0-rc6+
> #60
> [59837.811435] Hardware name: Dell Inc. Latitude E6520/0J4TFW, BIOS
> A06 07/11/2011
> [59837.811442] RIP: 0010:__memmove+0xe2/0x1a0
> [59837.811445] Code: 1f 84 00 00 00 00 00 90 48 83 fa 20 72 50 48 81
> fa a8 02 00 00 72 05 40 38 fe 74 bc 48 01 d6 48 01 d7 48 83 ea 20 48
> 83 ea 20 <4c> 8b 5e f8 4c 8b 56 f0 4c 8b 4e e8 4c 8b 46 e0 48 8d 76
> e0 4c 89
> [59837.811447] RSP: 0018:ffffc90002b4f870 EFLAGS: 00010202
> [59837.811451] RAX: 0005088000000004 RBX: 0000000000000000 RCX:
> 000000000000fffc
> [59837.811453] RDX: 0000000000000fbc RSI: 0005088000000ffc RDI:
> 0005088000001000
> [59837.811455] RBP: ffff88810be80000 R08: 0005088000000ffc R09:
> ffff888235aec550
> [59837.811457] R10: 0000000000000356 R11: 000000000000016c R12:
> 0000000000000004
> [59837.811459] R13: 000000000000fffc R14: ffff88810be80000 R15:
> ffffc90002b4fc58
> [59837.811462] FS:  00007fd9303ff740(0000) GS:ffff888235ac0000(0000)
> knlGS:0000000000000000
> [59837.811465] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [59837.811467] CR2: 00007fd9305d2000 CR3: 000000011fc10005 CR4:
> 00000000000606e0
> [59837.811469] Call Trace:
> [59837.811505]  _shift_data_right_pages+0x11e/0x150 [sunrpc]
> [59837.811531]  xdr_shrink_bufhead+0x151/0x170 [sunrpc]
> [59837.811555]  xdr_realign_pages+0x4c/0xa0 [sunrpc]
> [59837.811578]  xdr_align_pages+0x49/0x120 [sunrpc]
> [59837.811601]  xdr_read_pages+0x23/0xb0 [sunrpc]
> [59837.811626]  nfs4_xdr_dec_getxattr+0xfa/0x120 [nfsv4]
> [59837.811643]  call_decode+0x199/0x1f0 [sunrpc]
> [59837.811659]  ? rpc_decode_header+0x4e0/0x4e0 [sunrpc]
> [59837.811678]  __rpc_execute+0x71/0x420 [sunrpc]
> [59837.811700]  ? xprt_iter_default_rewind+0x10/0x10 [sunrpc]
> [59837.811721]  ? xprt_iter_get_next+0x4a/0x60 [sunrpc]
> [59837.811737]  rpc_run_task+0x14c/0x180 [sunrpc]
> [59837.811756]  nfs4_do_call_sync+0x6e/0xb0 [nfsv4]
> [59837.811785]  _nfs42_proc_getxattr+0xb7/0x170 [nfsv4]
> [59837.811809]  ? xprt_iter_get_next+0x4a/0x60 [sunrpc]
> [59837.811830]  nfs42_proc_getxattr+0x86/0xb0 [nfsv4]
> [59837.811844]  nfs4_xattr_get_nfs4_user+0xc9/0xe0 [nfsv4]
> [59837.811849]  vfs_getxattr+0x161/0x1a0
> [59837.811852]  getxattr+0x14f/0x230
> [59837.811856]  ? filename_lookup+0x123/0x1b0
> [59837.811861]  ? _cond_resched+0x16/0x40
> [59837.811864]  ? kmem_cache_alloc+0x3c4/0x4b0
> [59837.811866]  ? getname_flags.part.0+0x45/0x1a0
> [59837.811869]  path_getxattr+0x62/0xb0
> [59837.811873]  do_syscall_64+0x33/0x40
> [59837.811876]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
> [59837.811879] RIP: 0033:0x7fd93050162e
> [59837.811883] Code: 48 8b 0d 4d 48 0c 00 f7 d8 64 89 01 48 83 c8 ff
> c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 49 89 ca b8 c0 00 00
> 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 1a 48 0c 00 f7 d8 64
> 89 01 48
> [59837.811884] RSP: 002b:00007ffe178ad1c8 EFLAGS: 00000202 ORIG_RAX:
> 00000000000000c0
> [59837.811887] RAX: ffffffffffffffda RBX: 00007ffe178ad330 RCX:
> 00007fd93050162e
> [59837.811889] RDX: 0000000000000000 RSI: 00007ffe178ad330 RDI:
> 00007ffe178bf525
> [59837.811890] RBP: 0000000000000000 R08: 00007ffe178ad210 R09:
> 0000000000000000
> [59837.811892] R10: 0000000000000000 R11: 0000000000000202 R12:
> 0000000000000001
> [59837.811894] R13: 00007ffe178ad210 R14: 00007ffe178bf525 R15:
> 0000000000000000
> [59837.811896] Modules linked in: nfs_layout_flexfiles
> rpcsec_gss_krb5 auth_rpcgss nfsv4 dns_resolver nfs lockd grace
> nfs_ssc fscache nf_conntrack_netbios_ns nf_conntrack_broadcast nft_ct
> nf_tables ebtable_nat ebtable_broute ip6table_nat ip6table_mangle
> ip6table_raw ip6table_security iptable_nat nf_nat nf_conntrack
> nf_defrag_ipv6 nf_defrag_ipv4 iptable_mangle iptable_raw
> iptable_security ip_set nfnetlink ebtable_filter ebtables
> ip6table_filter ip6_tables sunrpc snd_hda_codec_idt
> snd_hda_codec_generic ledtrig_audio nouveau i915 iwldvm mac80211
> btrfs snd_hda_codec_hdmi snd_hda_intel snd_intel_ds

Does this fix it?

8<---------------------------------------------
From 013ee77c43a4dcd468becaf2f234624433cc6fb2 Mon Sep 17 00:00:00 2001
From: Trond Myklebust <trond.myklebust@hammerspace.com>
Date: Thu, 3 Dec 2020 09:16:15 -0500
Subject: [PATCH] SUNRPC: xs_alloc_sparse_pages() should set buf-
>page_len

If the page buffer allocated in xs_alloc_sparse_pages() ends up being
shorter than the predicted buf->page_len, then we should truncate the
latter so that later calls to xdr_read_pages() doesn't get confused and
trigger an Oops.

Reported-by: "Mkrtchyan, Tigran" <tigran.mkrtchyan@desy.de>
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
 net/sunrpc/xprtsock.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index c93ff70da3f9..32054176786e 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -327,21 +327,28 @@ static void xs_free_peer_addresses(struct
rpc_xprt *xprt)
 static size_t
 xs_alloc_sparse_pages(struct xdr_buf *buf, size_t want, gfp_t gfp)
 {
-	size_t i,n;
+	size_t i,n, len;
 
-	if (!want || !(buf->flags & XDRBUF_SPARSE_PAGES))
+	if (!(buf->flags & XDRBUF_SPARSE_PAGES))
 		return want;
 	n = (buf->page_base + want + PAGE_SIZE - 1) >> PAGE_SHIFT;
 	for (i = 0; i < n; i++) {
 		if (buf->pages[i])
 			continue;
 		buf->bvec[i].bv_page = buf->pages[i] =
alloc_page(gfp);
-		if (!buf->pages[i]) {
-			i *= PAGE_SIZE;
-			return i > buf->page_base ? i - buf->page_base
: 0;
-		}
+		if (!buf->pages[i])
+			break;
+	}
+	len = i << PAGE_SHIFT;
+	if (len > buf->page_base)
+		len -= buf->page_base;
+	else
+		len = 0;
+	if (buf->page_len > len) {
+		buf->buflen -= buf->page_len - len;
+		buf->page_len = len;
 	}
-	return want;
+	return want <= len ? want : len;
 }
 
 static ssize_t
-- 
2.28.0


-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com



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

* Re: Kernel OPS when using xattr
  2020-12-03 14:07 ` Olga Kornievskaia
@ 2020-12-03 15:43   ` Mkrtchyan, Tigran
  2020-12-03 16:18     ` Frank van der Linden
  0 siblings, 1 reply; 15+ messages in thread
From: Mkrtchyan, Tigran @ 2020-12-03 15:43 UTC (permalink / raw)
  To: Olga Kornievskaia; +Cc: linux-nfs, trondmy, Frank van der Linden


Hi Olga,

Franks patches are not applied. I will check with Trond's patch and
then will try those as well.

Regards,
   Tigran.

----- Original Message -----
> From: "Olga Kornievskaia" <aglo@umich.edu>
> To: "Tigran Mkrtchyan" <tigran.mkrtchyan@desy.de>
> Cc: "linux-nfs" <linux-nfs@vger.kernel.org>, "trondmy" <trondmy@hammerspace.com>, "Frank van der Linden"
> <fllinden@amazon.com>
> Sent: Thursday, 3 December, 2020 15:07:55
> Subject: Re: Kernel OPS when using xattr

> On Thu, Dec 3, 2020 at 3:22 AM Mkrtchyan, Tigran
> <tigran.mkrtchyan@desy.de> wrote:
>>
>>
>> Dear NFS folk,
>>
>> this is I got while accessing xattrs over NFS with 5.10.0-rc6 kernel
>> from Trond's testing tree (8102e956f22e710eecb3913cdd236282213812cf).
>> The 5.9 kernel works as expected.
> 
> Is that with Frank's getxattr patch?
> https://www.spinics.net/lists/linux-nfs/msg81183.html
> 
>>
>> Tigran.
>>
>> [ 2803.765467] nfs4flexfilelayout_init: NFSv4 Flexfile Layout Driver
>> Registering...
>> [59837.811426] general protection fault, probably for non-canonical address
>> 0x5088000000ffc: 0000 [#1] SMP PTI
>> [59837.811433] CPU: 3 PID: 3858 Comm: attr Not tainted 5.10.0-rc6+ #60
>> [59837.811435] Hardware name: Dell Inc. Latitude E6520/0J4TFW, BIOS A06
>> 07/11/2011
>> [59837.811442] RIP: 0010:__memmove+0xe2/0x1a0
>> [59837.811445] Code: 1f 84 00 00 00 00 00 90 48 83 fa 20 72 50 48 81 fa a8 02 00
>> 00 72 05 40 38 fe 74 bc 48 01 d6 48 01 d7 48 83 ea 20 48 83 ea 20 <4c> 8b 5e f8
>> 4c 8b 56 f0 4c 8b 4e e8 4c 8b 46 e0 48 8d 76 e0 4c 89
>> [59837.811447] RSP: 0018:ffffc90002b4f870 EFLAGS: 00010202
>> [59837.811451] RAX: 0005088000000004 RBX: 0000000000000000 RCX: 000000000000fffc
>> [59837.811453] RDX: 0000000000000fbc RSI: 0005088000000ffc RDI: 0005088000001000
>> [59837.811455] RBP: ffff88810be80000 R08: 0005088000000ffc R09: ffff888235aec550
>> [59837.811457] R10: 0000000000000356 R11: 000000000000016c R12: 0000000000000004
>> [59837.811459] R13: 000000000000fffc R14: ffff88810be80000 R15: ffffc90002b4fc58
>> [59837.811462] FS:  00007fd9303ff740(0000) GS:ffff888235ac0000(0000)
>> knlGS:0000000000000000
>> [59837.811465] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [59837.811467] CR2: 00007fd9305d2000 CR3: 000000011fc10005 CR4: 00000000000606e0
>> [59837.811469] Call Trace:
>> [59837.811505]  _shift_data_right_pages+0x11e/0x150 [sunrpc]
>> [59837.811531]  xdr_shrink_bufhead+0x151/0x170 [sunrpc]
>> [59837.811555]  xdr_realign_pages+0x4c/0xa0 [sunrpc]
>> [59837.811578]  xdr_align_pages+0x49/0x120 [sunrpc]
>> [59837.811601]  xdr_read_pages+0x23/0xb0 [sunrpc]
>> [59837.811626]  nfs4_xdr_dec_getxattr+0xfa/0x120 [nfsv4]
>> [59837.811643]  call_decode+0x199/0x1f0 [sunrpc]
>> [59837.811659]  ? rpc_decode_header+0x4e0/0x4e0 [sunrpc]
>> [59837.811678]  __rpc_execute+0x71/0x420 [sunrpc]
>> [59837.811700]  ? xprt_iter_default_rewind+0x10/0x10 [sunrpc]
>> [59837.811721]  ? xprt_iter_get_next+0x4a/0x60 [sunrpc]
>> [59837.811737]  rpc_run_task+0x14c/0x180 [sunrpc]
>> [59837.811756]  nfs4_do_call_sync+0x6e/0xb0 [nfsv4]
>> [59837.811785]  _nfs42_proc_getxattr+0xb7/0x170 [nfsv4]
>> [59837.811809]  ? xprt_iter_get_next+0x4a/0x60 [sunrpc]
>> [59837.811830]  nfs42_proc_getxattr+0x86/0xb0 [nfsv4]
>> [59837.811844]  nfs4_xattr_get_nfs4_user+0xc9/0xe0 [nfsv4]
>> [59837.811849]  vfs_getxattr+0x161/0x1a0
>> [59837.811852]  getxattr+0x14f/0x230
>> [59837.811856]  ? filename_lookup+0x123/0x1b0
>> [59837.811861]  ? _cond_resched+0x16/0x40
>> [59837.811864]  ? kmem_cache_alloc+0x3c4/0x4b0
>> [59837.811866]  ? getname_flags.part.0+0x45/0x1a0
>> [59837.811869]  path_getxattr+0x62/0xb0
>> [59837.811873]  do_syscall_64+0x33/0x40
>> [59837.811876]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
>> [59837.811879] RIP: 0033:0x7fd93050162e
>> [59837.811883] Code: 48 8b 0d 4d 48 0c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f
>> 1f 84 00 00 00 00 00 90 f3 0f 1e fa 49 89 ca b8 c0 00 00 00 0f 05 <48> 3d 01 f0
>> ff ff 73 01 c3 48 8b 0d 1a 48 0c 00 f7 d8 64 89 01 48
>> [59837.811884] RSP: 002b:00007ffe178ad1c8 EFLAGS: 00000202 ORIG_RAX:
>> 00000000000000c0
>> [59837.811887] RAX: ffffffffffffffda RBX: 00007ffe178ad330 RCX: 00007fd93050162e
>> [59837.811889] RDX: 0000000000000000 RSI: 00007ffe178ad330 RDI: 00007ffe178bf525
>> [59837.811890] RBP: 0000000000000000 R08: 00007ffe178ad210 R09: 0000000000000000
>> [59837.811892] R10: 0000000000000000 R11: 0000000000000202 R12: 0000000000000001
>> [59837.811894] R13: 00007ffe178ad210 R14: 00007ffe178bf525 R15: 0000000000000000
>> [59837.811896] Modules linked in: nfs_layout_flexfiles rpcsec_gss_krb5
>> auth_rpcgss nfsv4 dns_resolver nfs lockd grace nfs_ssc fscache
>> nf_conntrack_netbios_ns nf_conntrack_broadcast nft_ct nf_tables ebtable_nat
>> ebtable_broute ip6table_nat ip6table_mangle ip6table_raw ip6table_security
>> iptable_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 iptable_mangle
>> iptable_raw iptable_security ip_set nfnetlink ebtable_filter ebtables
>> ip6table_filter ip6_tables sunrpc snd_hda_codec_idt snd_hda_codec_generic
>> ledtrig_audio nouveau i915 iwldvm mac80211 btrfs snd_hda_codec_hdmi
> > snd_hda_intel snd_intel_ds

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

* Re: Kernel OPS when using xattr
  2020-12-03 15:07 ` Trond Myklebust
@ 2020-12-03 16:04   ` Mkrtchyan, Tigran
  2020-12-03 17:13     ` Trond Myklebust
  0 siblings, 1 reply; 15+ messages in thread
From: Mkrtchyan, Tigran @ 2020-12-03 16:04 UTC (permalink / raw)
  To: trondmy; +Cc: linux-nfs, Frank van der Linden



Hi Trond,

unfortunately the same result. Here the output of gdb, if it helps.

(gdb) list *0x00000000000252be
0x252be is in _shift_data_right_pages (net/sunrpc/xdr.c:344).
339			if (*pgto != *pgfrom) {
340				vfrom = kmap_atomic(*pgfrom);
341				memcpy(vto + pgto_base, vfrom + pgfrom_base, copy);
342				kunmap_atomic(vfrom);
343			} else
344				memmove(vto + pgto_base, vto + pgfrom_base, copy);
345			flush_dcache_page(*pgto);
346			kunmap_atomic(vto);
347
348		} while ((len -= copy) != 0);
(gdb)


Tigran.


----- Original Message -----
> From: "trondmy" <trondmy@hammerspace.com>
> To: "Tigran Mkrtchyan" <tigran.mkrtchyan@desy.de>, "linux-nfs" <linux-nfs@vger.kernel.org>
> Cc: "Frank van der Linden" <fllinden@amazon.com>
> Sent: Thursday, 3 December, 2020 16:07:53
> Subject: Re: Kernel OPS when using xattr

> Hi Tigran,
> 
> On Thu, 2020-12-03 at 09:20 +0100, Mkrtchyan, Tigran wrote:
>> 
>> Dear NFS folk,
>> 
>> this is I got while accessing xattrs over NFS with 5.10.0-rc6 kernel
>> from Trond's testing tree (8102e956f22e710eecb3913cdd236282213812cf).
>> The 5.9 kernel works as expected.
>> 
>> Tigran.
>> 
>> [ 2803.765467] nfs4flexfilelayout_init: NFSv4 Flexfile Layout Driver
>> Registering...
>> [59837.811426] general protection fault, probably for non-canonical
>> address 0x5088000000ffc: 0000 [#1] SMP PTI
>> [59837.811433] CPU: 3 PID: 3858 Comm: attr Not tainted 5.10.0-rc6+
>> #60
>> [59837.811435] Hardware name: Dell Inc. Latitude E6520/0J4TFW, BIOS
>> A06 07/11/2011
>> [59837.811442] RIP: 0010:__memmove+0xe2/0x1a0
>> [59837.811445] Code: 1f 84 00 00 00 00 00 90 48 83 fa 20 72 50 48 81
>> fa a8 02 00 00 72 05 40 38 fe 74 bc 48 01 d6 48 01 d7 48 83 ea 20 48
>> 83 ea 20 <4c> 8b 5e f8 4c 8b 56 f0 4c 8b 4e e8 4c 8b 46 e0 48 8d 76
>> e0 4c 89
>> [59837.811447] RSP: 0018:ffffc90002b4f870 EFLAGS: 00010202
>> [59837.811451] RAX: 0005088000000004 RBX: 0000000000000000 RCX:
>> 000000000000fffc
>> [59837.811453] RDX: 0000000000000fbc RSI: 0005088000000ffc RDI:
>> 0005088000001000
>> [59837.811455] RBP: ffff88810be80000 R08: 0005088000000ffc R09:
>> ffff888235aec550
>> [59837.811457] R10: 0000000000000356 R11: 000000000000016c R12:
>> 0000000000000004
>> [59837.811459] R13: 000000000000fffc R14: ffff88810be80000 R15:
>> ffffc90002b4fc58
>> [59837.811462] FS:  00007fd9303ff740(0000) GS:ffff888235ac0000(0000)
>> knlGS:0000000000000000
>> [59837.811465] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [59837.811467] CR2: 00007fd9305d2000 CR3: 000000011fc10005 CR4:
>> 00000000000606e0
>> [59837.811469] Call Trace:
>> [59837.811505]  _shift_data_right_pages+0x11e/0x150 [sunrpc]
>> [59837.811531]  xdr_shrink_bufhead+0x151/0x170 [sunrpc]
>> [59837.811555]  xdr_realign_pages+0x4c/0xa0 [sunrpc]
>> [59837.811578]  xdr_align_pages+0x49/0x120 [sunrpc]
>> [59837.811601]  xdr_read_pages+0x23/0xb0 [sunrpc]
>> [59837.811626]  nfs4_xdr_dec_getxattr+0xfa/0x120 [nfsv4]
>> [59837.811643]  call_decode+0x199/0x1f0 [sunrpc]
>> [59837.811659]  ? rpc_decode_header+0x4e0/0x4e0 [sunrpc]
>> [59837.811678]  __rpc_execute+0x71/0x420 [sunrpc]
>> [59837.811700]  ? xprt_iter_default_rewind+0x10/0x10 [sunrpc]
>> [59837.811721]  ? xprt_iter_get_next+0x4a/0x60 [sunrpc]
>> [59837.811737]  rpc_run_task+0x14c/0x180 [sunrpc]
>> [59837.811756]  nfs4_do_call_sync+0x6e/0xb0 [nfsv4]
>> [59837.811785]  _nfs42_proc_getxattr+0xb7/0x170 [nfsv4]
>> [59837.811809]  ? xprt_iter_get_next+0x4a/0x60 [sunrpc]
>> [59837.811830]  nfs42_proc_getxattr+0x86/0xb0 [nfsv4]
>> [59837.811844]  nfs4_xattr_get_nfs4_user+0xc9/0xe0 [nfsv4]
>> [59837.811849]  vfs_getxattr+0x161/0x1a0
>> [59837.811852]  getxattr+0x14f/0x230
>> [59837.811856]  ? filename_lookup+0x123/0x1b0
>> [59837.811861]  ? _cond_resched+0x16/0x40
>> [59837.811864]  ? kmem_cache_alloc+0x3c4/0x4b0
>> [59837.811866]  ? getname_flags.part.0+0x45/0x1a0
>> [59837.811869]  path_getxattr+0x62/0xb0
>> [59837.811873]  do_syscall_64+0x33/0x40
>> [59837.811876]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
>> [59837.811879] RIP: 0033:0x7fd93050162e
>> [59837.811883] Code: 48 8b 0d 4d 48 0c 00 f7 d8 64 89 01 48 83 c8 ff
>> c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 49 89 ca b8 c0 00 00
>> 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 1a 48 0c 00 f7 d8 64
>> 89 01 48
>> [59837.811884] RSP: 002b:00007ffe178ad1c8 EFLAGS: 00000202 ORIG_RAX:
>> 00000000000000c0
>> [59837.811887] RAX: ffffffffffffffda RBX: 00007ffe178ad330 RCX:
>> 00007fd93050162e
>> [59837.811889] RDX: 0000000000000000 RSI: 00007ffe178ad330 RDI:
>> 00007ffe178bf525
>> [59837.811890] RBP: 0000000000000000 R08: 00007ffe178ad210 R09:
>> 0000000000000000
>> [59837.811892] R10: 0000000000000000 R11: 0000000000000202 R12:
>> 0000000000000001
>> [59837.811894] R13: 00007ffe178ad210 R14: 00007ffe178bf525 R15:
>> 0000000000000000
>> [59837.811896] Modules linked in: nfs_layout_flexfiles
>> rpcsec_gss_krb5 auth_rpcgss nfsv4 dns_resolver nfs lockd grace
>> nfs_ssc fscache nf_conntrack_netbios_ns nf_conntrack_broadcast nft_ct
>> nf_tables ebtable_nat ebtable_broute ip6table_nat ip6table_mangle
>> ip6table_raw ip6table_security iptable_nat nf_nat nf_conntrack
>> nf_defrag_ipv6 nf_defrag_ipv4 iptable_mangle iptable_raw
>> iptable_security ip_set nfnetlink ebtable_filter ebtables
>> ip6table_filter ip6_tables sunrpc snd_hda_codec_idt
>> snd_hda_codec_generic ledtrig_audio nouveau i915 iwldvm mac80211
>> btrfs snd_hda_codec_hdmi snd_hda_intel snd_intel_ds
> 
> Does this fix it?
> 
> 8<---------------------------------------------
> From 013ee77c43a4dcd468becaf2f234624433cc6fb2 Mon Sep 17 00:00:00 2001
> From: Trond Myklebust <trond.myklebust@hammerspace.com>
> Date: Thu, 3 Dec 2020 09:16:15 -0500
> Subject: [PATCH] SUNRPC: xs_alloc_sparse_pages() should set buf-
>>page_len
> 
> If the page buffer allocated in xs_alloc_sparse_pages() ends up being
> shorter than the predicted buf->page_len, then we should truncate the
> latter so that later calls to xdr_read_pages() doesn't get confused and
> trigger an Oops.
> 
> Reported-by: "Mkrtchyan, Tigran" <tigran.mkrtchyan@desy.de>
> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> ---
> net/sunrpc/xprtsock.c | 21 ++++++++++++++-------
> 1 file changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> index c93ff70da3f9..32054176786e 100644
> --- a/net/sunrpc/xprtsock.c
> +++ b/net/sunrpc/xprtsock.c
> @@ -327,21 +327,28 @@ static void xs_free_peer_addresses(struct
> rpc_xprt *xprt)
> static size_t
> xs_alloc_sparse_pages(struct xdr_buf *buf, size_t want, gfp_t gfp)
> {
> -	size_t i,n;
> +	size_t i,n, len;
> 
> -	if (!want || !(buf->flags & XDRBUF_SPARSE_PAGES))
> +	if (!(buf->flags & XDRBUF_SPARSE_PAGES))
> 		return want;
> 	n = (buf->page_base + want + PAGE_SIZE - 1) >> PAGE_SHIFT;
> 	for (i = 0; i < n; i++) {
> 		if (buf->pages[i])
> 			continue;
> 		buf->bvec[i].bv_page = buf->pages[i] =
> alloc_page(gfp);
> -		if (!buf->pages[i]) {
> -			i *= PAGE_SIZE;
> -			return i > buf->page_base ? i - buf->page_base
> : 0;
> -		}
> +		if (!buf->pages[i])
> +			break;
> +	}
> +	len = i << PAGE_SHIFT;
> +	if (len > buf->page_base)
> +		len -= buf->page_base;
> +	else
> +		len = 0;
> +	if (buf->page_len > len) {
> +		buf->buflen -= buf->page_len - len;
> +		buf->page_len = len;
> 	}
> -	return want;
> +	return want <= len ? want : len;
> }
> 
> static ssize_t
> --
> 2.28.0
> 
> 
> --
> Trond Myklebust
> Linux NFS client maintainer, Hammerspace
> trond.myklebust@hammerspace.com

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

* Re: Kernel OPS when using xattr
  2020-12-03 15:43   ` Mkrtchyan, Tigran
@ 2020-12-03 16:18     ` Frank van der Linden
  2020-12-03 16:36       ` Mkrtchyan, Tigran
  2020-12-03 16:59       ` Chuck Lever
  0 siblings, 2 replies; 15+ messages in thread
From: Frank van der Linden @ 2020-12-03 16:18 UTC (permalink / raw)
  To: Mkrtchyan, Tigran; +Cc: Olga Kornievskaia, linux-nfs, trondmy

On Thu, Dec 03, 2020 at 04:43:12PM +0100, Mkrtchyan, Tigran wrote:
> 
> Hi Olga,
> 
> Franks patches are not applied. I will check with Trond's patch and
> then will try those as well.
> 
> Regards,
>    Tigran.

Since my change no longer uses SPARSE_PAGES, it'll probably avoid the
oops, so give it a try.

Having said that, fixing SPARSE_PAGES seems like a better option.. My
ideal outcome would be to have a working SPARSE_PAGES for all transports.
That would allow GETXATTR and LISTXATTRS to just always specify a max
size array of pages, giving it maximum flexibility to cache the received
result no matter what, and avoiding allocations that are too large.

For now, though, I'm happy with the v2 patch I sent in.

- Frank

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

* Re: Kernel OPS when using xattr
  2020-12-03 16:18     ` Frank van der Linden
@ 2020-12-03 16:36       ` Mkrtchyan, Tigran
  2020-12-03 16:38         ` Mkrtchyan, Tigran
  2020-12-03 16:59       ` Chuck Lever
  1 sibling, 1 reply; 15+ messages in thread
From: Mkrtchyan, Tigran @ 2020-12-03 16:36 UTC (permalink / raw)
  To: Frank van der Linden; +Cc: Olga Kornievskaia, linux-nfs, trondmy

The patch from Frank works.
( you can attribute with Tested-by if needed)

Tigran.

----- Original Message -----
> From: "Frank van der Linden" <fllinden@amazon.com>
> To: "Tigran Mkrtchyan" <tigran.mkrtchyan@desy.de>
> Cc: "Olga Kornievskaia" <aglo@umich.edu>, "linux-nfs" <linux-nfs@vger.kernel.org>, "trondmy" <trondmy@hammerspace.com>
> Sent: Thursday, 3 December, 2020 17:18:58
> Subject: Re: Kernel OPS when using xattr

> On Thu, Dec 03, 2020 at 04:43:12PM +0100, Mkrtchyan, Tigran wrote:
>> 
>> Hi Olga,
>> 
>> Franks patches are not applied. I will check with Trond's patch and
>> then will try those as well.
>> 
>> Regards,
>>    Tigran.
> 
> Since my change no longer uses SPARSE_PAGES, it'll probably avoid the
> oops, so give it a try.
> 
> Having said that, fixing SPARSE_PAGES seems like a better option.. My
> ideal outcome would be to have a working SPARSE_PAGES for all transports.
> That would allow GETXATTR and LISTXATTRS to just always specify a max
> size array of pages, giving it maximum flexibility to cache the received
> result no matter what, and avoiding allocations that are too large.
> 
> For now, though, I'm happy with the v2 patch I sent in.
> 
> - Frank

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

* Re: Kernel OPS when using xattr
  2020-12-03 16:36       ` Mkrtchyan, Tigran
@ 2020-12-03 16:38         ` Mkrtchyan, Tigran
  2020-12-03 16:57           ` Mkrtchyan, Tigran
  0 siblings, 1 reply; 15+ messages in thread
From: Mkrtchyan, Tigran @ 2020-12-03 16:38 UTC (permalink / raw)
  To: Frank van der Linden; +Cc: Olga Kornievskaia, linux-nfs, trondmy

Oh. I tested V1.... Let me check v2 as well.

Tigran.

----- Original Message -----
> From: "Tigran Mkrtchyan" <tigran.mkrtchyan@desy.de>
> To: "Frank van der Linden" <fllinden@amazon.com>
> Cc: "Olga Kornievskaia" <aglo@umich.edu>, "linux-nfs" <linux-nfs@vger.kernel.org>, "trondmy" <trondmy@hammerspace.com>
> Sent: Thursday, 3 December, 2020 17:36:07
> Subject: Re: Kernel OPS when using xattr

> The patch from Frank works.
> ( you can attribute with Tested-by if needed)
> 
> Tigran.
> 
> ----- Original Message -----
>> From: "Frank van der Linden" <fllinden@amazon.com>
>> To: "Tigran Mkrtchyan" <tigran.mkrtchyan@desy.de>
>> Cc: "Olga Kornievskaia" <aglo@umich.edu>, "linux-nfs"
>> <linux-nfs@vger.kernel.org>, "trondmy" <trondmy@hammerspace.com>
>> Sent: Thursday, 3 December, 2020 17:18:58
>> Subject: Re: Kernel OPS when using xattr
> 
>> On Thu, Dec 03, 2020 at 04:43:12PM +0100, Mkrtchyan, Tigran wrote:
>>> 
>>> Hi Olga,
>>> 
>>> Franks patches are not applied. I will check with Trond's patch and
>>> then will try those as well.
>>> 
>>> Regards,
>>>    Tigran.
>> 
>> Since my change no longer uses SPARSE_PAGES, it'll probably avoid the
>> oops, so give it a try.
>> 
>> Having said that, fixing SPARSE_PAGES seems like a better option.. My
>> ideal outcome would be to have a working SPARSE_PAGES for all transports.
>> That would allow GETXATTR and LISTXATTRS to just always specify a max
>> size array of pages, giving it maximum flexibility to cache the received
>> result no matter what, and avoiding allocations that are too large.
>> 
>> For now, though, I'm happy with the v2 patch I sent in.
>> 
> > - Frank

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

* Re: Kernel OPS when using xattr
  2020-12-03 16:38         ` Mkrtchyan, Tigran
@ 2020-12-03 16:57           ` Mkrtchyan, Tigran
  0 siblings, 0 replies; 15+ messages in thread
From: Mkrtchyan, Tigran @ 2020-12-03 16:57 UTC (permalink / raw)
  To: Frank van der Linden; +Cc: Olga Kornievskaia, linux-nfs, trondmy

Frank's v2 works as well. 

Tigran.

----- Original Message -----
> From: "Tigran Mkrtchyan" <tigran.mkrtchyan@desy.de>
> To: "Frank van der Linden" <fllinden@amazon.com>
> Cc: "Olga Kornievskaia" <aglo@umich.edu>, "linux-nfs" <linux-nfs@vger.kernel.org>, "trondmy" <trondmy@hammerspace.com>
> Sent: Thursday, 3 December, 2020 17:38:12
> Subject: Re: Kernel OPS when using xattr

> Oh. I tested V1.... Let me check v2 as well.
> 
> Tigran.
> 
> ----- Original Message -----
>> From: "Tigran Mkrtchyan" <tigran.mkrtchyan@desy.de>
>> To: "Frank van der Linden" <fllinden@amazon.com>
>> Cc: "Olga Kornievskaia" <aglo@umich.edu>, "linux-nfs"
>> <linux-nfs@vger.kernel.org>, "trondmy" <trondmy@hammerspace.com>
>> Sent: Thursday, 3 December, 2020 17:36:07
>> Subject: Re: Kernel OPS when using xattr
> 
>> The patch from Frank works.
>> ( you can attribute with Tested-by if needed)
>> 
>> Tigran.
>> 
>> ----- Original Message -----
>>> From: "Frank van der Linden" <fllinden@amazon.com>
>>> To: "Tigran Mkrtchyan" <tigran.mkrtchyan@desy.de>
>>> Cc: "Olga Kornievskaia" <aglo@umich.edu>, "linux-nfs"
>>> <linux-nfs@vger.kernel.org>, "trondmy" <trondmy@hammerspace.com>
>>> Sent: Thursday, 3 December, 2020 17:18:58
>>> Subject: Re: Kernel OPS when using xattr
>> 
>>> On Thu, Dec 03, 2020 at 04:43:12PM +0100, Mkrtchyan, Tigran wrote:
>>>> 
>>>> Hi Olga,
>>>> 
>>>> Franks patches are not applied. I will check with Trond's patch and
>>>> then will try those as well.
>>>> 
>>>> Regards,
>>>>    Tigran.
>>> 
>>> Since my change no longer uses SPARSE_PAGES, it'll probably avoid the
>>> oops, so give it a try.
>>> 
>>> Having said that, fixing SPARSE_PAGES seems like a better option.. My
>>> ideal outcome would be to have a working SPARSE_PAGES for all transports.
>>> That would allow GETXATTR and LISTXATTRS to just always specify a max
>>> size array of pages, giving it maximum flexibility to cache the received
>>> result no matter what, and avoiding allocations that are too large.
>>> 
>>> For now, though, I'm happy with the v2 patch I sent in.
>>> 
> > > - Frank

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

* Re: Kernel OPS when using xattr
  2020-12-03 16:18     ` Frank van der Linden
  2020-12-03 16:36       ` Mkrtchyan, Tigran
@ 2020-12-03 16:59       ` Chuck Lever
  1 sibling, 0 replies; 15+ messages in thread
From: Chuck Lever @ 2020-12-03 16:59 UTC (permalink / raw)
  To: Frank van der Linden
  Cc: Mkrtchyan, Tigran, Olga Kornievskaia, Linux NFS Mailing List, trondmy



> On Dec 3, 2020, at 11:18 AM, Frank van der Linden <fllinden@amazon.com> wrote:
> 
> On Thu, Dec 03, 2020 at 04:43:12PM +0100, Mkrtchyan, Tigran wrote:
>> 
>> Hi Olga,
>> 
>> Franks patches are not applied. I will check with Trond's patch and
>> then will try those as well.
>> 
>> Regards,
>>   Tigran.
> 
> Since my change no longer uses SPARSE_PAGES, it'll probably avoid the
> oops, so give it a try.
> 
> Having said that, fixing SPARSE_PAGES seems like a better option.. My
> ideal outcome would be to have a working SPARSE_PAGES for all transports.
> That would allow GETXATTR and LISTXATTRS to just always specify a max
> size array of pages, giving it maximum flexibility to cache the received
> result no matter what, and avoiding allocations that are too large.

SPARSE_PAGES (at least for RDMA) always allocates the full set of
pages, because it has to set the receive buffer up before the request
is sent and it has no way to know the actual size of the reply. 

There is really no savings when the transport does it, and it is less
reliable because the transport runs in a context where GFP_KERNEL
allocations are not allowed.


> For now, though, I'm happy with the v2 patch I sent in.
> 
> - Frank

--
Chuck Lever




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

* Re: Kernel OPS when using xattr
  2020-12-03 16:04   ` Mkrtchyan, Tigran
@ 2020-12-03 17:13     ` Trond Myklebust
  2020-12-03 17:47       ` Frank van der Linden
  2020-12-03 17:49       ` Mkrtchyan, Tigran
  0 siblings, 2 replies; 15+ messages in thread
From: Trond Myklebust @ 2020-12-03 17:13 UTC (permalink / raw)
  To: tigran.mkrtchyan; +Cc: fllinden, linux-nfs

On Thu, 2020-12-03 at 17:04 +0100, Mkrtchyan, Tigran wrote:
> 
> 
> Hi Trond,
> 
> unfortunately the same result. Here the output of gdb, if it helps.
> 
> (gdb) list *0x00000000000252be
> 0x252be is in _shift_data_right_pages (net/sunrpc/xdr.c:344).
> 339                     if (*pgto != *pgfrom) {
> 340                             vfrom = kmap_atomic(*pgfrom);
> 341                             memcpy(vto + pgto_base, vfrom +
> pgfrom_base, copy);
> 342                             kunmap_atomic(vfrom);
> 343                     } else
> 344                             memmove(vto + pgto_base, vto +
> pgfrom_base, copy);
> 345                     flush_dcache_page(*pgto);
> 346                     kunmap_atomic(vto);
> 347
> 348             } while ((len -= copy) != 0);
> (gdb)
> 

You probably need this one in addition to the first patch.
8<----------------------------------------------------
From fec77469f373fbccb292c2d522f2ebee3b9011a8 Mon Sep 17 00:00:00 2001
From: Trond Myklebust <trond.myklebust@hammerspace.com>
Date: Thu, 3 Dec 2020 12:04:51 -0500
Subject: [PATCH] NFSv4.2: Fix up the get/listxattr calls to
 rpc_prepare_reply_pages()

Ensure that both getxattr and listxattr page array are correctly
aligned, and that getxattr correctly accounts for the page padding
word.

Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
 fs/nfs/nfs42xdr.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/fs/nfs/nfs42xdr.c b/fs/nfs/nfs42xdr.c
index 8432bd6b95f0..103978ff76c9 100644
--- a/fs/nfs/nfs42xdr.c
+++ b/fs/nfs/nfs42xdr.c
@@ -191,7 +191,7 @@
 
 #define encode_getxattr_maxsz   (op_encode_hdr_maxsz + 1 + \
 				 nfs4_xattr_name_maxsz)
-#define decode_getxattr_maxsz   (op_decode_hdr_maxsz + 1 + 1)
+#define decode_getxattr_maxsz   (op_decode_hdr_maxsz + 1 +
pagepad_maxsz)
 #define encode_setxattr_maxsz   (op_encode_hdr_maxsz + \
 				 1 + nfs4_xattr_name_maxsz + 1)
 #define decode_setxattr_maxsz   (op_decode_hdr_maxsz +
decode_change_info_maxsz)
@@ -1476,17 +1476,18 @@ static void nfs4_xdr_enc_getxattr(struct
rpc_rqst *req, struct xdr_stream *xdr,
 	struct compound_hdr hdr = {
 		.minorversion = nfs4_xdr_minorversion(&args-
>seq_args),
 	};
+	uint32_t replen;
 	size_t plen;
 
 	encode_compound_hdr(xdr, req, &hdr);
 	encode_sequence(xdr, &args->seq_args, &hdr);
 	encode_putfh(xdr, args->fh, &hdr);
+	replen = hdr.replen + op_decode_hdr_maxsz + 1;
 	encode_getxattr(xdr, args->xattr_name, &hdr);
 
 	plen = args->xattr_len ? args->xattr_len : XATTR_SIZE_MAX;
 
-	rpc_prepare_reply_pages(req, args->xattr_pages, 0, plen,
-	    hdr.replen);
+	rpc_prepare_reply_pages(req, args->xattr_pages, 0, plen,
replen);
 	req->rq_rcv_buf.flags |= XDRBUF_SPARSE_PAGES;
 
 	encode_nops(&hdr);
@@ -1520,14 +1521,15 @@ static void nfs4_xdr_enc_listxattrs(struct
rpc_rqst *req,
 	struct compound_hdr hdr = {
 		.minorversion = nfs4_xdr_minorversion(&args-
>seq_args),
 	};
+	uint32_t replen;
 
 	encode_compound_hdr(xdr, req, &hdr);
 	encode_sequence(xdr, &args->seq_args, &hdr);
 	encode_putfh(xdr, args->fh, &hdr);
+	replen = hdr.replen + op_decode_hdr_maxsz + 2 + 1;
 	encode_listxattrs(xdr, args, &hdr);
 
-	rpc_prepare_reply_pages(req, args->xattr_pages, 0, args-
>count,
-	    hdr.replen);
+	rpc_prepare_reply_pages(req, args->xattr_pages, 0, args-
>count, replen);
 
 	encode_nops(&hdr);
 }
-- 
2.28.0


-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com



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

* Re: Kernel OPS when using xattr
  2020-12-03 17:13     ` Trond Myklebust
@ 2020-12-03 17:47       ` Frank van der Linden
  2020-12-03 18:23         ` Trond Myklebust
  2020-12-03 17:49       ` Mkrtchyan, Tigran
  1 sibling, 1 reply; 15+ messages in thread
From: Frank van der Linden @ 2020-12-03 17:47 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: tigran.mkrtchyan, linux-nfs

On Thu, Dec 03, 2020 at 05:13:26PM +0000, Trond Myklebust wrote:
[...]
> You probably need this one in addition to the first patch.
> 8<----------------------------------------------------
> From fec77469f373fbccb292c2d522f2ebee3b9011a8 Mon Sep 17 00:00:00 2001
> From: Trond Myklebust <trond.myklebust@hammerspace.com>
> Date: Thu, 3 Dec 2020 12:04:51 -0500
> Subject: [PATCH] NFSv4.2: Fix up the get/listxattr calls to
>  rpc_prepare_reply_pages()
> 
> Ensure that both getxattr and listxattr page array are correctly
> aligned, and that getxattr correctly accounts for the page padding
> word.
> 
> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> ---
>  fs/nfs/nfs42xdr.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/nfs/nfs42xdr.c b/fs/nfs/nfs42xdr.c
> index 8432bd6b95f0..103978ff76c9 100644
> --- a/fs/nfs/nfs42xdr.c
> +++ b/fs/nfs/nfs42xdr.c
> @@ -191,7 +191,7 @@
> 
>  #define encode_getxattr_maxsz   (op_encode_hdr_maxsz + 1 + \
>                                  nfs4_xattr_name_maxsz)
> -#define decode_getxattr_maxsz   (op_decode_hdr_maxsz + 1 + 1)
> +#define decode_getxattr_maxsz   (op_decode_hdr_maxsz + 1 +
> pagepad_maxsz)
>  #define encode_setxattr_maxsz   (op_encode_hdr_maxsz + \
>                                  1 + nfs4_xattr_name_maxsz + 1)
>  #define decode_setxattr_maxsz   (op_decode_hdr_maxsz +
> decode_change_info_maxsz)
> @@ -1476,17 +1476,18 @@ static void nfs4_xdr_enc_getxattr(struct
> rpc_rqst *req, struct xdr_stream *xdr,
>         struct compound_hdr hdr = {
>                 .minorversion = nfs4_xdr_minorversion(&args-
> >seq_args),
>         };
> +       uint32_t replen;
>         size_t plen;
> 
>         encode_compound_hdr(xdr, req, &hdr);
>         encode_sequence(xdr, &args->seq_args, &hdr);
>         encode_putfh(xdr, args->fh, &hdr);
> +       replen = hdr.replen + op_decode_hdr_maxsz + 1;
>         encode_getxattr(xdr, args->xattr_name, &hdr);
> 
>         plen = args->xattr_len ? args->xattr_len : XATTR_SIZE_MAX;
> 
> -       rpc_prepare_reply_pages(req, args->xattr_pages, 0, plen,
> -           hdr.replen);
> +       rpc_prepare_reply_pages(req, args->xattr_pages, 0, plen,
> replen);
>         req->rq_rcv_buf.flags |= XDRBUF_SPARSE_PAGES;
> 
>         encode_nops(&hdr);
> @@ -1520,14 +1521,15 @@ static void nfs4_xdr_enc_listxattrs(struct
> rpc_rqst *req,
>         struct compound_hdr hdr = {
>                 .minorversion = nfs4_xdr_minorversion(&args-
> >seq_args),
>         };
> +       uint32_t replen;
> 
>         encode_compound_hdr(xdr, req, &hdr);
>         encode_sequence(xdr, &args->seq_args, &hdr);
>         encode_putfh(xdr, args->fh, &hdr);
> +       replen = hdr.replen + op_decode_hdr_maxsz + 2 + 1;
>         encode_listxattrs(xdr, args, &hdr);
> 
> -       rpc_prepare_reply_pages(req, args->xattr_pages, 0, args-
> >count,
> -           hdr.replen);
> +       rpc_prepare_reply_pages(req, args->xattr_pages, 0, args-
> >count, replen);
> 
>         encode_nops(&hdr);
>  }
> --
> 2.28.0

Hm.. that doesn't seem to match other, similar functionality.
Why is the additional padding and op_decode_hdr_maxsz needed?

- Frank

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

* Re: Kernel OPS when using xattr
  2020-12-03 17:13     ` Trond Myklebust
  2020-12-03 17:47       ` Frank van der Linden
@ 2020-12-03 17:49       ` Mkrtchyan, Tigran
  1 sibling, 0 replies; 15+ messages in thread
From: Mkrtchyan, Tigran @ 2020-12-03 17:49 UTC (permalink / raw)
  To: trondmy; +Cc: Frank van der Linden, linux-nfs

Hi Trond,

I can list and read attributes, but still get
stack traces (and getdeviceinfo still broken)


[  111.887664] ------------[ cut here ]------------
[  111.887696] WARNING: CPU: 4 PID: 1313 at net/sunrpc/clnt.c:2478 call_decode+0x1a1/0x1f0 [sunrpc]
[  111.887697] Modules linked in: nfs_layout_flexfiles rpcsec_gss_krb5 auth_rpcgss nfsv4(E) dns_resolver nfs(E) lockd grace nfs_ssc fscache nf_conntrack_netbios_ns nf_conntrack_broadcast nft_ct nf_tables ebtable_nat ebtable_broute ip6table_nat ip6table_mangle ip6table_raw ip6table_security iptable_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 iptable_mangle iptable_raw iptable_security ip_set nfnetlink ebtable_filter ebtables ip6table_filter ip6_tables sunrpc(E) snd_hda_codec_idt snd_hda_codec_generic ledtrig_audio nouveau i915 iwldvm mac80211 btrfs intel_rapl_msr snd_hda_codec_hdmi intel_rapl_common snd_hda_intel x86_pkg_temp_thermal uvcvideo snd_intel_dspcfg libarc4 snd_hda_codec coretemp kvm_intel videobuf2_vmalloc iwlwifi snd_hda_core videobuf2_memops videobuf2_v4l2 snd_hwdep snd_seq blake2b_generic videobuf2_common kvm at24 iTCO_wdt xor snd_seq_device videodev regmap_i2c dcdbas zstd_decompress ppdev iTCO_vendor_support wmi_bmof mxm_wmi snd_pcm cfg80211 dell_smm_hwmon ttm
[  111.887754]  zstd_compress i2c_algo_bit e1000e mc irqbypass snd_timer mei_me drm_kms_helper snd raid6_pq pcspkr i2c_i801 joydev ptp lpc_ich rfkill i2c_smbus mei soundcore pps_core mfd_core wmi parport_pc parport video dell_smo8800 drm sdhci_pci crct10dif_pclmul cqhci crc32_pclmul crc32c_intel sdhci ghash_clmulni_intel mmc_core firewire_ohci serio_raw firewire_core crc_itu_t fuse
[  111.887785] CPU: 4 PID: 1313 Comm: attr Tainted: G            E     5.10.0-rc6+ #60
[  111.887787] Hardware name: Dell Inc. Latitude E6520/0J4TFW, BIOS A06 07/11/2011
[  111.887803] RIP: 0010:call_decode+0x1a1/0x1f0 [sunrpc]
[  111.887805] Code: df 02 00 0f b7 85 dc 00 00 00 e9 cc fe ff ff 48 c7 45 20 90 d5 da a0 48 89 e6 48 89 ef e8 17 8c 01 00 89 45 04 e9 ed fe ff ff <0f> 0b e9 3d ff ff ff 65 8b 05 e1 5f 27 5f 89 c0 48 0f a3 05 87 09
[  111.887807] RSP: 0018:ffffc9000171b9f0 EFLAGS: 00010286
[  111.887809] RAX: 00000000fffffff0 RBX: ffff888103f11200 RCX: 0000000000000010
[  111.887810] RDX: 00000000fffffff0 RSI: 0000000000000048 RDI: ffff888103f11250
[  111.887811] RBP: ffff888101435500 R08: ffff888103f11330 R09: ffff888235b2c550
[  111.887812] R10: 0000000000000362 R11: 0000000000000327 R12: ffff888103f11250
[  111.887813] R13: ffff888101435530 R14: 0000000000000000 R15: ffff888101e76e18
[  111.887815] FS:  00007f4d48875740(0000) GS:ffff888235b00000(0000) knlGS:0000000000000000
[  111.887817] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  111.887818] CR2: 00007f4d48a48000 CR3: 000000011c72e002 CR4: 00000000000606e0
[  111.887819] Call Trace:
[  111.887829]  ? out_of_line_wait_on_bit+0x92/0xb0
[  111.887844]  ? rpc_decode_header+0x4e0/0x4e0 [sunrpc]
[  111.887863]  __rpc_execute+0x71/0x420 [sunrpc]
[  111.887885]  ? xprt_iter_default_rewind+0x10/0x10 [sunrpc]
[  111.887907]  ? xprt_iter_get_next+0x4a/0x60 [sunrpc]
[  111.887922]  rpc_run_task+0x14c/0x180 [sunrpc]
[  111.887940]  nfs4_do_call_sync+0x6e/0xb0 [nfsv4]
[  111.887961]  _nfs42_proc_getxattr+0xb7/0x170 [nfsv4]
[  111.887984]  ? xprt_iter_get_next+0x4a/0x60 [sunrpc]
[  111.888005]  nfs42_proc_getxattr+0x86/0xb0 [nfsv4]
[  111.888018]  nfs4_xattr_get_nfs4_user+0xc9/0xe0 [nfsv4]
[  111.888023]  vfs_getxattr+0x161/0x1a0
[  111.888026]  getxattr+0x14f/0x230
[  111.888028]  ? filename_lookup+0x123/0x1b0
[  111.888031]  ? _cond_resched+0x16/0x40
[  111.888034]  ? kmem_cache_alloc+0x3c4/0x4b0
[  111.888036]  ? getname_flags.part.0+0x45/0x1a0
[  111.888039]  path_getxattr+0x62/0xb0
[  111.888043]  do_syscall_64+0x33/0x40
[  111.888046]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[  111.888048] RIP: 0033:0x7f4d4897762e
[  111.888050] Code: 48 8b 0d 4d 48 0c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 49 89 ca b8 c0 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 1a 48 0c 00 f7 d8 64 89 01 48
[  111.888052] RSP: 002b:00007fff01750768 EFLAGS: 00000202 ORIG_RAX: 00000000000000c0
[  111.888054] RAX: ffffffffffffffda RBX: 00007fff017508d0 RCX: 00007f4d4897762e
[  111.888055] RDX: 0000000000000000 RSI: 00007fff017508d0 RDI: 00007fff0176243a
[  111.888056] RBP: 0000000000000000 R08: 00007fff017507b0 R09: 0000000000000000
[  111.888057] R10: 0000000000000000 R11: 0000000000000202 R12: 0000000000000001
[  111.888058] R13: 00007fff017507b0 R14: 00007fff0176243a R15: 0000000000000000
[  111.888060] ---[ end trace fdd032a781f2160d ]---
[  111.889050] ------------[ cut here ]------------


Tigran.

----- Original Message -----
> From: "trondmy" <trondmy@hammerspace.com>
> To: "Tigran Mkrtchyan" <tigran.mkrtchyan@desy.de>
> Cc: "Frank van der Linden" <fllinden@amazon.com>, "linux-nfs" <linux-nfs@vger.kernel.org>
> Sent: Thursday, 3 December, 2020 18:13:26
> Subject: Re: Kernel OPS when using xattr

> On Thu, 2020-12-03 at 17:04 +0100, Mkrtchyan, Tigran wrote:
>> 
>> 
>> Hi Trond,
>> 
>> unfortunately the same result. Here the output of gdb, if it helps.
>> 
>> (gdb) list *0x00000000000252be
>> 0x252be is in _shift_data_right_pages (net/sunrpc/xdr.c:344).
>> 339                     if (*pgto != *pgfrom) {
>> 340                             vfrom = kmap_atomic(*pgfrom);
>> 341                             memcpy(vto + pgto_base, vfrom +
>> pgfrom_base, copy);
>> 342                             kunmap_atomic(vfrom);
>> 343                     } else
>> 344                             memmove(vto + pgto_base, vto +
>> pgfrom_base, copy);
>> 345                     flush_dcache_page(*pgto);
>> 346                     kunmap_atomic(vto);
>> 347
>> 348             } while ((len -= copy) != 0);
>> (gdb)
>> 
> 
> You probably need this one in addition to the first patch.
> 8<----------------------------------------------------
> From fec77469f373fbccb292c2d522f2ebee3b9011a8 Mon Sep 17 00:00:00 2001
> From: Trond Myklebust <trond.myklebust@hammerspace.com>
> Date: Thu, 3 Dec 2020 12:04:51 -0500
> Subject: [PATCH] NFSv4.2: Fix up the get/listxattr calls to
> rpc_prepare_reply_pages()
> 
> Ensure that both getxattr and listxattr page array are correctly
> aligned, and that getxattr correctly accounts for the page padding
> word.
> 
> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> ---
> fs/nfs/nfs42xdr.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/nfs/nfs42xdr.c b/fs/nfs/nfs42xdr.c
> index 8432bd6b95f0..103978ff76c9 100644
> --- a/fs/nfs/nfs42xdr.c
> +++ b/fs/nfs/nfs42xdr.c
> @@ -191,7 +191,7 @@
> 
> #define encode_getxattr_maxsz   (op_encode_hdr_maxsz + 1 + \
> 				 nfs4_xattr_name_maxsz)
> -#define decode_getxattr_maxsz   (op_decode_hdr_maxsz + 1 + 1)
> +#define decode_getxattr_maxsz   (op_decode_hdr_maxsz + 1 +
> pagepad_maxsz)
> #define encode_setxattr_maxsz   (op_encode_hdr_maxsz + \
> 				 1 + nfs4_xattr_name_maxsz + 1)
> #define decode_setxattr_maxsz   (op_decode_hdr_maxsz +
> decode_change_info_maxsz)
> @@ -1476,17 +1476,18 @@ static void nfs4_xdr_enc_getxattr(struct
> rpc_rqst *req, struct xdr_stream *xdr,
> 	struct compound_hdr hdr = {
> 		.minorversion = nfs4_xdr_minorversion(&args-
>>seq_args),
> 	};
> +	uint32_t replen;
> 	size_t plen;
> 
> 	encode_compound_hdr(xdr, req, &hdr);
> 	encode_sequence(xdr, &args->seq_args, &hdr);
> 	encode_putfh(xdr, args->fh, &hdr);
> +	replen = hdr.replen + op_decode_hdr_maxsz + 1;
> 	encode_getxattr(xdr, args->xattr_name, &hdr);
> 
> 	plen = args->xattr_len ? args->xattr_len : XATTR_SIZE_MAX;
> 
> -	rpc_prepare_reply_pages(req, args->xattr_pages, 0, plen,
> -	    hdr.replen);
> +	rpc_prepare_reply_pages(req, args->xattr_pages, 0, plen,
> replen);
> 	req->rq_rcv_buf.flags |= XDRBUF_SPARSE_PAGES;
> 
> 	encode_nops(&hdr);
> @@ -1520,14 +1521,15 @@ static void nfs4_xdr_enc_listxattrs(struct
> rpc_rqst *req,
> 	struct compound_hdr hdr = {
> 		.minorversion = nfs4_xdr_minorversion(&args-
>>seq_args),
> 	};
> +	uint32_t replen;
> 
> 	encode_compound_hdr(xdr, req, &hdr);
> 	encode_sequence(xdr, &args->seq_args, &hdr);
> 	encode_putfh(xdr, args->fh, &hdr);
> +	replen = hdr.replen + op_decode_hdr_maxsz + 2 + 1;
> 	encode_listxattrs(xdr, args, &hdr);
> 
> -	rpc_prepare_reply_pages(req, args->xattr_pages, 0, args-
>>count,
> -	    hdr.replen);
> +	rpc_prepare_reply_pages(req, args->xattr_pages, 0, args-
>>count, replen);
> 
> 	encode_nops(&hdr);
> }
> --
> 2.28.0
> 
> 
> --
> Trond Myklebust
> Linux NFS client maintainer, Hammerspace
> trond.myklebust@hammerspace.com

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

* Re: Kernel OPS when using xattr
  2020-12-03 17:47       ` Frank van der Linden
@ 2020-12-03 18:23         ` Trond Myklebust
  2020-12-03 18:25           ` Frank van der Linden
  0 siblings, 1 reply; 15+ messages in thread
From: Trond Myklebust @ 2020-12-03 18:23 UTC (permalink / raw)
  To: fllinden; +Cc: tigran.mkrtchyan, linux-nfs

On Thu, 2020-12-03 at 17:47 +0000, Frank van der Linden wrote:
> On Thu, Dec 03, 2020 at 05:13:26PM +0000, Trond Myklebust wrote:
> [...]
> > You probably need this one in addition to the first patch.
> > 8<----------------------------------------------------
> > From fec77469f373fbccb292c2d522f2ebee3b9011a8 Mon Sep 17 00:00:00
> > 2001
> > From: Trond Myklebust <trond.myklebust@hammerspace.com>
> > Date: Thu, 3 Dec 2020 12:04:51 -0500
> > Subject: [PATCH] NFSv4.2: Fix up the get/listxattr calls to
> >  rpc_prepare_reply_pages()
> > 
> > Ensure that both getxattr and listxattr page array are correctly
> > aligned, and that getxattr correctly accounts for the page padding
> > word.
> > 
> > Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> > ---
> >  fs/nfs/nfs42xdr.c | 12 +++++++-----
> >  1 file changed, 7 insertions(+), 5 deletions(-)
> > 
> > diff --git a/fs/nfs/nfs42xdr.c b/fs/nfs/nfs42xdr.c
> > index 8432bd6b95f0..103978ff76c9 100644
> > --- a/fs/nfs/nfs42xdr.c
> > +++ b/fs/nfs/nfs42xdr.c
> > @@ -191,7 +191,7 @@
> > 
> >  #define encode_getxattr_maxsz   (op_encode_hdr_maxsz + 1 + \
> >                                  nfs4_xattr_name_maxsz)
> > -#define decode_getxattr_maxsz   (op_decode_hdr_maxsz + 1 + 1)
> > +#define decode_getxattr_maxsz   (op_decode_hdr_maxsz + 1 +
> > pagepad_maxsz)
> >  #define encode_setxattr_maxsz   (op_encode_hdr_maxsz + \
> >                                  1 + nfs4_xattr_name_maxsz + 1)
> >  #define decode_setxattr_maxsz   (op_decode_hdr_maxsz +
> > decode_change_info_maxsz)
> > @@ -1476,17 +1476,18 @@ static void nfs4_xdr_enc_getxattr(struct
> > rpc_rqst *req, struct xdr_stream *xdr,
> >         struct compound_hdr hdr = {
> >                 .minorversion = nfs4_xdr_minorversion(&args-
> > > seq_args),
> >         };
> > +       uint32_t replen;
> >         size_t plen;
> > 
> >         encode_compound_hdr(xdr, req, &hdr);
> >         encode_sequence(xdr, &args->seq_args, &hdr);
> >         encode_putfh(xdr, args->fh, &hdr);
> > +       replen = hdr.replen + op_decode_hdr_maxsz + 1;
> >         encode_getxattr(xdr, args->xattr_name, &hdr);
> > 
> >         plen = args->xattr_len ? args->xattr_len : XATTR_SIZE_MAX;
> > 
> > -       rpc_prepare_reply_pages(req, args->xattr_pages, 0, plen,
> > -           hdr.replen);
> > +       rpc_prepare_reply_pages(req, args->xattr_pages, 0, plen,
> > replen);
> >         req->rq_rcv_buf.flags |= XDRBUF_SPARSE_PAGES;
> > 
> >         encode_nops(&hdr);
> > @@ -1520,14 +1521,15 @@ static void nfs4_xdr_enc_listxattrs(struct
> > rpc_rqst *req,
> >         struct compound_hdr hdr = {
> >                 .minorversion = nfs4_xdr_minorversion(&args-
> > > seq_args),
> >         };
> > +       uint32_t replen;
> > 
> >         encode_compound_hdr(xdr, req, &hdr);
> >         encode_sequence(xdr, &args->seq_args, &hdr);
> >         encode_putfh(xdr, args->fh, &hdr);
> > +       replen = hdr.replen + op_decode_hdr_maxsz + 2 + 1;
> >         encode_listxattrs(xdr, args, &hdr);
> > 
> > -       rpc_prepare_reply_pages(req, args->xattr_pages, 0, args-
> > > count,
> > -           hdr.replen);
> > +       rpc_prepare_reply_pages(req, args->xattr_pages, 0, args-
> > > count, replen);
> > 
> >         encode_nops(&hdr);
> >  }
> > --
> > 2.28.0
> 
> Hm.. that doesn't seem to match other, similar functionality.
> Why is the additional padding and op_decode_hdr_maxsz needed?
> 

It isn't extra padding. It is the same padding, but after the cleanup
that removes the confusing behaviour of rpc_prepare_reply_pages(), the
caller is supposed to supply the actual offset for the beginning of the
page data instead of adding the padding to that offset:
https://git.linux-nfs.org/?p=trondmy/linux-nfs.git;a=commitdiff;h=9ed5af268e88f6e5b65376be98d652b37cb20d7b

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com



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

* Re: Kernel OPS when using xattr
  2020-12-03 18:23         ` Trond Myklebust
@ 2020-12-03 18:25           ` Frank van der Linden
  0 siblings, 0 replies; 15+ messages in thread
From: Frank van der Linden @ 2020-12-03 18:25 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: tigran.mkrtchyan, linux-nfs

On Thu, Dec 03, 2020 at 06:23:24PM +0000, Trond Myklebust wrote:
> 
> 
> On Thu, 2020-12-03 at 17:47 +0000, Frank van der Linden wrote:
> > On Thu, Dec 03, 2020 at 05:13:26PM +0000, Trond Myklebust wrote:
> > [...]
> > > You probably need this one in addition to the first patch.
> > > 8<----------------------------------------------------
> > > From fec77469f373fbccb292c2d522f2ebee3b9011a8 Mon Sep 17 00:00:00
> > > 2001
> > > From: Trond Myklebust <trond.myklebust@hammerspace.com>
> > > Date: Thu, 3 Dec 2020 12:04:51 -0500
> > > Subject: [PATCH] NFSv4.2: Fix up the get/listxattr calls to
> > >  rpc_prepare_reply_pages()
> > >
> > > Ensure that both getxattr and listxattr page array are correctly
> > > aligned, and that getxattr correctly accounts for the page padding
> > > word.
> > >
> > > Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> > > ---
> > >  fs/nfs/nfs42xdr.c | 12 +++++++-----
> > >  1 file changed, 7 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/fs/nfs/nfs42xdr.c b/fs/nfs/nfs42xdr.c
> > > index 8432bd6b95f0..103978ff76c9 100644
> > > --- a/fs/nfs/nfs42xdr.c
> > > +++ b/fs/nfs/nfs42xdr.c
> > > @@ -191,7 +191,7 @@
> > >
> > >  #define encode_getxattr_maxsz   (op_encode_hdr_maxsz + 1 + \
> > >                                  nfs4_xattr_name_maxsz)
> > > -#define decode_getxattr_maxsz   (op_decode_hdr_maxsz + 1 + 1)
> > > +#define decode_getxattr_maxsz   (op_decode_hdr_maxsz + 1 +
> > > pagepad_maxsz)
> > >  #define encode_setxattr_maxsz   (op_encode_hdr_maxsz + \
> > >                                  1 + nfs4_xattr_name_maxsz + 1)
> > >  #define decode_setxattr_maxsz   (op_decode_hdr_maxsz +
> > > decode_change_info_maxsz)
> > > @@ -1476,17 +1476,18 @@ static void nfs4_xdr_enc_getxattr(struct
> > > rpc_rqst *req, struct xdr_stream *xdr,
> > >         struct compound_hdr hdr = {
> > >                 .minorversion = nfs4_xdr_minorversion(&args-
> > > > seq_args),
> > >         };
> > > +       uint32_t replen;
> > >         size_t plen;
> > >
> > >         encode_compound_hdr(xdr, req, &hdr);
> > >         encode_sequence(xdr, &args->seq_args, &hdr);
> > >         encode_putfh(xdr, args->fh, &hdr);
> > > +       replen = hdr.replen + op_decode_hdr_maxsz + 1;
> > >         encode_getxattr(xdr, args->xattr_name, &hdr);
> > >
> > >         plen = args->xattr_len ? args->xattr_len : XATTR_SIZE_MAX;
> > >
> > > -       rpc_prepare_reply_pages(req, args->xattr_pages, 0, plen,
> > > -           hdr.replen);
> > > +       rpc_prepare_reply_pages(req, args->xattr_pages, 0, plen,
> > > replen);
> > >         req->rq_rcv_buf.flags |= XDRBUF_SPARSE_PAGES;
> > >
> > >         encode_nops(&hdr);
> > > @@ -1520,14 +1521,15 @@ static void nfs4_xdr_enc_listxattrs(struct
> > > rpc_rqst *req,
> > >         struct compound_hdr hdr = {
> > >                 .minorversion = nfs4_xdr_minorversion(&args-
> > > > seq_args),
> > >         };
> > > +       uint32_t replen;
> > >
> > >         encode_compound_hdr(xdr, req, &hdr);
> > >         encode_sequence(xdr, &args->seq_args, &hdr);
> > >         encode_putfh(xdr, args->fh, &hdr);
> > > +       replen = hdr.replen + op_decode_hdr_maxsz + 2 + 1;
> > >         encode_listxattrs(xdr, args, &hdr);
> > >
> > > -       rpc_prepare_reply_pages(req, args->xattr_pages, 0, args-
> > > > count,
> > > -           hdr.replen);
> > > +       rpc_prepare_reply_pages(req, args->xattr_pages, 0, args-
> > > > count, replen);
> > >
> > >         encode_nops(&hdr);
> > >  }
> > > --
> > > 2.28.0
> >
> > Hm.. that doesn't seem to match other, similar functionality.
> > Why is the additional padding and op_decode_hdr_maxsz needed?
> >
> 
> It isn't extra padding. It is the same padding, but after the cleanup
> that removes the confusing behaviour of rpc_prepare_reply_pages(), the
> caller is supposed to supply the actual offset for the beginning of the
> page data instead of adding the padding to that offset:
> https://git.linux-nfs.org/?p=trondmy/linux-nfs.git;a=commitdiff;h=9ed5af268e88f6e5b65376be98d652b37cb20d7b

Ahh, that makes a lot of sense, thanks for fixing rpc_prepare_reply_pages.

- Frank

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

end of thread, other threads:[~2020-12-03 18:26 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-03  8:20 Kernel OPS when using xattr Mkrtchyan, Tigran
2020-12-03 14:07 ` Olga Kornievskaia
2020-12-03 15:43   ` Mkrtchyan, Tigran
2020-12-03 16:18     ` Frank van der Linden
2020-12-03 16:36       ` Mkrtchyan, Tigran
2020-12-03 16:38         ` Mkrtchyan, Tigran
2020-12-03 16:57           ` Mkrtchyan, Tigran
2020-12-03 16:59       ` Chuck Lever
2020-12-03 15:07 ` Trond Myklebust
2020-12-03 16:04   ` Mkrtchyan, Tigran
2020-12-03 17:13     ` Trond Myklebust
2020-12-03 17:47       ` Frank van der Linden
2020-12-03 18:23         ` Trond Myklebust
2020-12-03 18:25           ` Frank van der Linden
2020-12-03 17:49       ` Mkrtchyan, Tigran

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.