Re: [PATCH] xprtrdma: add missing curly braces, set rc to zero on non-zero

[Anna Schumaker]

On 11/24/2015 06:11 PM, Colin King wrote:
> From: Colin Ian King 
> 
> Add the missing curly braces so that rc is only set to zero when
> it is non-zero.  Without this minor fix, rc is set to zero even
> when it is zero, which is slightly redundant.
> 
> Detected with smatch static analysis.

Thanks for the patch, but I've already have a patch from Dan Carpenter queued 
up to fix this.

Anna

> 
> Signed-off-by: Colin Ian King 
> ---
>  net/sunrpc/xprtrdma/verbs.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
> index eadd1655..2cc1014 100644
> --- a/net/sunrpc/xprtrdma/verbs.c
> +++ b/net/sunrpc/xprtrdma/verbs.c
> @@ -852,10 +852,11 @@ retry:
>  
>   if (extras) {
>   rc = rpcrdma_ep_post_extra_recv(r_xprt, extras);
> - if (rc)
> + if (rc) {
>   pr_warn("%s: rpcrdma_ep_post_extra_recv: %i\n",
>   __func__, rc);
>   rc = 0;
> + }
>   }
>   }
>  
> 

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] sunrpc: Remove unnecessary variable

[Anna Schumaker]

On 08/08/2016 05:13 AM, Amitoj Kaur Chawla wrote:
> The variable `err` is not used anywhere and just returns the
> predefined value `0` at the end of the function. Hence, remove the
> variable and return 0 explicitly.

Makes sense to me.  Thanks!

Anna

> 
> Signed-off-by: Amitoj Kaur Chawla 
> ---
>  net/sunrpc/clnt.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
> index b7f2104..0a775fa 100644
> --- a/net/sunrpc/clnt.c
> +++ b/net/sunrpc/clnt.c
> @@ -184,7 +184,6 @@ static int __rpc_clnt_handle_event(struct rpc_clnt *clnt, 
> unsigned long event,
>  struct super_block *sb)
>  {
>   struct dentry *dentry;
> - int err = 0;
>  
>   switch (event) {
>   case RPC_PIPEFS_MOUNT:
> @@ -201,7 +200,7 @@ static int __rpc_clnt_handle_event(struct rpc_clnt *clnt, 
> unsigned long event,
>   printk(KERN_ERR "%s: unknown event: %ld\n", __func__, event);
>   return -ENOTSUPP;
>   }
> - return err;
> + return 0;
>  }
>  
>  static int __rpc_pipefs_event(struct rpc_clnt *clnt, unsigned long event,
> 

Re: [PATCH 4.10-rc3 01/13] net: sunrpc: fix build errors when linux/phy*.h is removed from net/dsa.h

[Anna Schumaker]

Hi Russell,

On 01/31/2017 02:18 PM, Russell King wrote:
> Removing linux/phy.h from net/dsa.h reveals a build error in the sunrpc
> code:
> 
> net/sunrpc/xprtrdma/svc_rdma_backchannel.c: In function 'xprt_rdma_bc_put':
> net/sunrpc/xprtrdma/svc_rdma_backchannel.c:277:2: error: implicit declaration 
> of function 'module_put' [-Werror=implicit-function-declaration]
> net/sunrpc/xprtrdma/svc_rdma_backchannel.c: In function 'xprt_setup_rdma_bc':
> net/sunrpc/xprtrdma/svc_rdma_backchannel.c:348:7: error: implicit declaration 
> of function 'try_module_get' [-Werror=implicit-function-declaration]
> 
> Fix this by adding linux/module.h to svc_rdma_backchannel.c
> 
> Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>

This patch looks okay to me:

Acked-by: Anna Schumaker <anna.schuma...@netapp.com>

> ---
>  net/sunrpc/xprtrdma/svc_rdma_backchannel.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/sunrpc/xprtrdma/svc_rdma_backchannel.c 
> b/net/sunrpc/xprtrdma/svc_rdma_backchannel.c
> index 288e35c2d8f4..cb1e48e54eb1 100644
> --- a/net/sunrpc/xprtrdma/svc_rdma_backchannel.c
> +++ b/net/sunrpc/xprtrdma/svc_rdma_backchannel.c
> @@ -4,6 +4,7 @@
>   * Support for backward direction RPCs on RPC/RDMA (server-side).
>   */
>  
> +#include 
>  #include 
>  #include "xprt_rdma.h"
>  
> 

Re: [PATCH] sunrpc: queue work on system_power_efficient_wq

[Anna Schumaker]

On 09/01/2016 03:30 AM, Chunyan Zhang wrote:
> From: Ke Wang 
> 
> sunrpc uses workqueue to clean cache regulary. There is no real dependency
> of executing work on the cpu which queueing it.
> 
> On a idle system, especially for a heterogeneous systems like big.LITTLE,
> it is observed that the big idle cpu was woke up many times just to service
> this work, which against the principle of power saving. It would be better
> if we can schedule it on a cpu which the scheduler believes to be the most
> appropriate one.
> 
> After apply this patch, system_wq will be replaced by
> system_power_efficient_wq for sunrpc. This functionality is enabled when
> CONFIG_WQ_POWER_EFFICIENT is selected.

Makes sense to me, but I'm a little surprised that there isn't a 
"schedule_delayed_power_efficient_work()" function to match how the normal 
workqueue is used.

Thanks,
Anna

> 
> Signed-off-by: Ke Wang 
> ---
>  net/sunrpc/cache.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
> index 4d8e11f..8aabe12 100644
> --- a/net/sunrpc/cache.c
> +++ b/net/sunrpc/cache.c
> @@ -353,7 +353,7 @@ void sunrpc_init_cache_detail(struct cache_detail *cd)
>   spin_unlock(_list_lock);
>  
>   /* start the cleaning process */
> - schedule_delayed_work(_cleaner, 0);
> + queue_delayed_work(system_power_efficient_wq, _cleaner, 0);
>  }
>  EXPORT_SYMBOL_GPL(sunrpc_init_cache_detail);
>  
> @@ -476,7 +476,8 @@ static void do_cache_clean(struct work_struct *work)
>   delay = 0;
>  
>   if (delay)
> - schedule_delayed_work(_cleaner, delay);
> + queue_delayed_work(system_power_efficient_wq,
> +_cleaner, delay);
>  }
>  
>  
> 

Re: 答复: [PATCH] sunrpc: queue work on system_power_efficient_wq

[Anna Schumaker]

Hi,

I took a second look at the patch and it still looks okay to me.  It's in my 
git tree for 4.9.

Thanks,
Anna

On 09/19/2016 10:33 PM, Ke Wang (王科) wrote:
> May I have any comments for this patch?
> or
> This patch can be merged directly into next release?
> 
> Thanks,
> Ke
> ____
> 发件人: Anna Schumaker <anna.schuma...@netapp.com>
> 发送时间: 2016年9月2日 2:46
> 收件人: Chunyan Zhang; trond.mykleb...@primarydata.com; 
> anna.schuma...@netapp.com; da...@davemloft.net
> 抄送: linux-...@vger.kernel.org; netdev@vger.kernel.org; 
> linux-ker...@vger.kernel.org; Ke Wang (王科)
> 主题: Re: [PATCH] sunrpc: queue work on system_power_efficient_wq
> 
> On 09/01/2016 03:30 AM, Chunyan Zhang wrote:
>> From: Ke Wang <ke.w...@spreadtrum.com>
>>
>> sunrpc uses workqueue to clean cache regulary. There is no real dependency
>> of executing work on the cpu which queueing it.
>>
>> On a idle system, especially for a heterogeneous systems like big.LITTLE,
>> it is observed that the big idle cpu was woke up many times just to service
>> this work, which against the principle of power saving. It would be better
>> if we can schedule it on a cpu which the scheduler believes to be the most
>> appropriate one.
>>
>> After apply this patch, system_wq will be replaced by
>> system_power_efficient_wq for sunrpc. This functionality is enabled when
>> CONFIG_WQ_POWER_EFFICIENT is selected.
> 
> Makes sense to me, but I'm a little surprised that there isn't a 
> "schedule_delayed_power_efficient_work()" function to match how the normal 
> workqueue is used.
> 
> Thanks,
> Anna
> 
>>
>> Signed-off-by: Ke Wang <ke.w...@spreadtrum.com>
>> ---
>>  net/sunrpc/cache.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
>> index 4d8e11f..8aabe12 100644
>> --- a/net/sunrpc/cache.c
>> +++ b/net/sunrpc/cache.c
>> @@ -353,7 +353,7 @@ void sunrpc_init_cache_detail(struct cache_detail *cd)
>>   spin_unlock(_list_lock);
>>
>>   /* start the cleaning process */
>> - schedule_delayed_work(_cleaner, 0);
>> + queue_delayed_work(system_power_efficient_wq, _cleaner, 0);
>>  }
>>  EXPORT_SYMBOL_GPL(sunrpc_init_cache_detail);
>>
>> @@ -476,7 +476,8 @@ static void do_cache_clean(struct work_struct *work)
>>   delay = 0;
>>
>>   if (delay)
>> - schedule_delayed_work(_cleaner, delay);
>> + queue_delayed_work(system_power_efficient_wq,
>> +_cleaner, delay);
>>  }
>>
>>
>>
> 

Re: nfs NULL-dereferencing in net-next

[Anna Schumaker]

On 10/25/2016 01:19 PM, Yotam Gigi wrote:
> 
>> -Original Message-
>> From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org] On
>> Behalf Of Jakub Kicinski
>> Sent: Monday, October 17, 2016 10:20 PM
>> To: Andy Adamson <and...@netapp.com>; Anna Schumaker
>> <anna.schuma...@netapp.com>; linux-...@vger.kernel.org
>> Cc: netdev@vger.kernel.org; Trond Myklebust <trond.mykleb...@netapp.com>
>> Subject: nfs NULL-dereferencing in net-next
>>
>> Hi!
>>
>> I'm hitting this reliably on net-next, HEAD at 3f3177bb680f
>> ("fsl/fman: fix error return code in mac_probe()").
> 
> 
> I see the same thing. It happens constantly on some of my machines, making 
> them
> completely unusable.
> 
> I bisected it and got to the commit:
> 
> commit 04ea1b3e6d8ed4978bb608c1748530af3de8c274
> Author: Andy Adamson <and...@netapp.com>
> Date:   Fri Sep 9 09:22:27 2016 -0400
> 
> NFS add xprt switch addrs test to match client
> 
> Signed-off-by: Andy Adamson <and...@netapp.com>
> Signed-off-by: Anna Schumaker <anna.schuma...@netapp.com>

Thanks for reporting on this everyone!  Does this patch help?

>From 96376ca1dd4077a1d341bdcb9cc86426ee3844f1 Mon Sep 17 00:00:00 2001
From: Anna Schumaker <anna.schuma...@netapp.com>
Date: Wed, 26 Oct 2016 10:33:31 -0400
Subject: [PATCH] SUNRPC: Fix suspicious RCU usage

We need to hold the rcu_read_lock() when calling rcu_dereference(),
otherwise we can't guarantee that the object being dereferenced still
exists.

Signed-off-by: Anna Schumaker <anna.schuma...@netapp.com>
---
 net/sunrpc/clnt.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 34dd7b2..62a4827 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -2753,14 +2753,18 @@ EXPORT_SYMBOL_GPL(rpc_cap_max_reconnect_timeout);

 void rpc_clnt_xprt_switch_put(struct rpc_clnt *clnt)
 {
+   rcu_read_lock();
xprt_switch_put(rcu_dereference(clnt->cl_xpi.xpi_xpswitch));
+   rcu_read_unlock();
 }
 EXPORT_SYMBOL_GPL(rpc_clnt_xprt_switch_put);

 void rpc_clnt_xprt_switch_add_xprt(struct rpc_clnt *clnt, struct rpc_xprt 
*xprt)
 {
+   rcu_read_lock();
rpc_xprt_switch_add_xprt(rcu_dereference(clnt->cl_xpi.xpi_xpswitch),
 xprt);
+   rcu_read_unlock();
 }
 EXPORT_SYMBOL_GPL(rpc_clnt_xprt_switch_add_xprt);

@@ -2770,9 +2774,8 @@ bool rpc_clnt_xprt_switch_has_addr(struct rpc_clnt *clnt,
struct rpc_xprt_switch *xps;
bool ret;

-   xps = rcu_dereference(clnt->cl_xpi.xpi_xpswitch);
-
rcu_read_lock();
+   xps = rcu_dereference(clnt->cl_xpi.xpi_xpswitch);
ret = rpc_xprt_switch_has_addr(xps, sap);
rcu_read_unlock();
return ret;
--
2.10.1

> 
> 
>>
>> [   23.409633] BUG: unable to handle kernel NULL pointer dereference at
>> 0172
>> [   23.418716] IP: [] 
>> rpc_clnt_xprt_switch_has_addr+0xc/0x40
>> [sunrpc]
>> [   23.427574] PGD 859020067 [   23.430472] PUD 858f2d067
>> PMD 0 [   23.434311]
>> [   23.436133] Oops:  [#1] PREEMPT SMP
>> [   23.440506] Modules linked in: nfsv4 ip6table_filter ip6_tables 
>> iptable_filter
>> ip_tables ebtable_nat ebtables x_tables intel_ri
>> [   23.505915] CPU: 1 PID: 1067 Comm: mount.nfs Not tainted 4.8.0-perf-13951-
>> g3f3177bb680f #51
>> [   23.515363] Hardware name: Dell Inc. PowerEdge T630/0W9WXC, BIOS 1.2.10
>> 03/10/2015
>> [   23.523937] task: 983e9086ea00 task.stack: ac6c0a57c000
>> [   23.530641] RIP: 0010:[]  []
>> rpc_clnt_xprt_switch_has_addr+0xc/0x40 [sunrpc]
>> [   23.542229] RSP: 0018:ac6c0a57fb28  EFLAGS: 00010a97
>> [   23.548255] RAX: c80214ac RBX: 983e97c7b000 RCX: 
>> 983e9b3bc180
>> [   23.556320] RDX: 0001 RSI: 983e9928ed28 RDI: 
>> ffea
>> [   23.564386] RBP: ac6c0a57fb38 R08: 983e97090630 R09: 
>> 983e9928ed30
>> [   23.572452] R10: ac6c0a57fba0 R11: 0010 R12: 
>> ac6c0a57fba0
>> [   23.580517] R13: 983e9928ed28 R14:  R15: 
>> 983e91360560
>> [   23.588585] FS:  7f4c348aa880() GS:983e9f24()
>> knlGS:
>> [   23.597742] CS:  0010 DS:  ES:  CR0: 80050033
>> [   23.604251] CR2: 0172 CR3: 000850a5f000 CR4:
>> 001406e0
>> [   23.612316] Stack:
>> [   23.614648]  983e97c7b000 ac6c0a57fba0 ac6c0a57fb90 
>> c04d38c3
>> [   23.623331]  983e91360500 983e9928ed30 c0b9e560
>> 983e913605b8
>> [

Re: net/sunrpc/clnt.c:2773 suspicious rcu_dereference_check() usage!

[Anna Schumaker]

On 11/08/2016 12:43 PM, Ross Zwisler wrote:
> On Tue, Nov 08, 2016 at 07:42:59AM -0500, Anna Schumaker wrote:
>> On 11/08/2016 07:09 AM, Jeff Layton wrote:
>>> On Tue, 2016-11-08 at 06:53 -0500, Jeff Layton wrote:
>>>> On Mon, 2016-11-07 at 22:42 -0700, Ross Zwisler wrote:
>>>>>
>>>>> I've got a virtual machine that has some NFS mounts, and with a newly 
>>>>> compiled
>>>>> kernel based on v4.9-rc3 I see the following warning/info message:
>>>>>
>>>>> [   42.750181] ===
>>>>> [   42.750192] [ INFO: suspicious RCU usage. ]
>>>>> [   42.750203] 4.9.0-rc3-2-g7b6e7de #3 Not tainted
>>>>> [   42.750213] ---
>>>>> [   42.750225] net/sunrpc/clnt.c:2773 suspicious rcu_dereference_check() 
>>>>> usage!
>>>>> [   42.750235] 
>>>>> [   42.750235] other info that might help us debug this:
>>>>> [   42.750235] 
>>>>> [   42.750246] 
>>>>> [   42.750246] rcu_scheduler_active = 1, debug_locks = 0
>>>>> [   42.750257] 1 lock held by mount.nfs4/6440:
>>>>> [   42.750278]  #0: 
>>>>> [   42.750299]  (
>>>>> [   42.750319] &(>nfs_client_lock)->rlock
>>>>> [   42.750340] ){+.+...}
>>>>> [   42.750362] , at: 
>>>>> [   42.750372] [] nfs_get_client+0x105/0x5e0
>>>>> [   42.750383] 
>>>>> [   42.750383] stack backtrace:
>>>>> [   42.750394] CPU: 0 PID: 6440 Comm: mount.nfs4 Not tainted 
>>>>> 4.9.0-rc3-2-g7b6e7de #3
>>>>> [   42.750406] Hardware name: Intel Corporation PURLEY/PURLEY, BIOS 
>>>>> PLYDCRB1.MBH.0096.D23.1608240105 08/24/2016
>>>>> [   42.750429]  c992fa68 8150730f 88014ec8da40 
>>>>> 0001
>>>>> [   42.750452]  c992fa98 810bc3f7 880150b0b228 
>>>>> 88015068dbb0
>>>>> [   42.750475]  c992fb38 88014fc99180 c992fac0 
>>>>> 81b243e5
>>>>> [   42.750486] Call Trace:
>>>>> [   42.750498]  [] dump_stack+0x67/0x98
>>>>> [   42.750511]  [] lockdep_rcu_suspicious+0xe7/0x120
>>>>> [   42.750524]  [] 
>>>>> rpc_clnt_xprt_switch_has_addr+0x115/0x150
>>>>> [   42.750536]  [] nfs_get_client+0x244/0x5e0
>>>>> [   42.750549]  [] ? nfs_get_client+0xfc/0x5e0
>>>>> [   42.750561]  [] nfs4_set_client+0x98/0x130
>>>>> [   42.750574]  [] nfs4_create_server+0x13e/0x390
>>>>> [   42.750588]  [] nfs4_remote_mount+0x2e/0x60
>>>>> [   42.750600]  [] mount_fs+0x39/0x170
>>>>> [   42.750614]  [] vfs_kern_mount+0x6b/0x150
>>>>> [   42.750626]  [] ? nfs_do_root_mount+0x3c/0xc0
>>>>> [   42.750639]  [] nfs_do_root_mount+0x86/0xc0
>>>>> [   42.750652]  [] nfs4_try_mount+0x44/0xc0
>>>>> [   42.750664]  [] ? get_nfs_version+0x27/0x90
>>>>> [   42.750677]  [] nfs_fs_mount+0x4ac/0xd80
>>>>> [   42.750689]  [] ? lockdep_init_map+0x88/0x1f0
>>>>> [   42.750701]  [] ? nfs_clone_super+0x130/0x130
>>>>> [   42.750713]  [] ? param_set_portnr+0x70/0x70
>>>>> [   42.750726]  [] mount_fs+0x39/0x170
>>>>> [   42.750740]  [] vfs_kern_mount+0x6b/0x150
>>>>> [   42.750752]  [] do_mount+0x1f1/0xd10
>>>>> [   42.750765]  [] ? copy_mount_options+0xa1/0x140
>>>>> [   42.750777]  [] SyS_mount+0x83/0xd0
>>>>> [   42.750790]  [] do_syscall_64+0x5c/0x130
>>>>> [   42.750802]  [] entry_SYSCALL64_slow_path+0x25/0x25
>>>>>
>>>>> This rcu_dereference_check() was introduced by the following commit:
>>>>>
>>>>> commit 39e5d2df959dd4aea81fa33d765d2a5cc67a0512
>>>>> Author: Andy Adamson <and...@netapp.com>
>>>>> Date:   Fri Sep 9 09:22:25 2016 -0400
>>>>>
>>>>> SUNRPC search xprt switch for sockaddr
>>>>> 
>>>>> Signed-off-by: Andy Adamson <and...@netapp.com>
>>>>> Signed-off-by: Anna Schumaker <anna.schuma...@netapp.com>
>>>>>
>>>>> Thanks,
>>>>> - Ross
>>>>
>>>> Thanks Ross,
>>
>> Hi Ross,
>>
>> Can you try this patch and let me know if it helps:
>>
>> http://git.linux-nfs.org/?p=anna/linux-nfs.git;a=commitdiff;h=bb29dd84333a96f309c6d0f88b285b5b78927058
>>
>> I'm planning on sending it to Linus soon, so it should be in rc5.
> 
> Hi Anna,
> 
> Yep, this patch makes the warning go away in my setup.

Great!  Thanks for testing!

Anna

> 
> Thanks,
> - Ross
> 

Re: net/sunrpc/clnt.c:2773 suspicious rcu_dereference_check() usage!

[Anna Schumaker]

On 11/08/2016 07:09 AM, Jeff Layton wrote:
> On Tue, 2016-11-08 at 06:53 -0500, Jeff Layton wrote:
>> On Mon, 2016-11-07 at 22:42 -0700, Ross Zwisler wrote:
>>>
>>> I've got a virtual machine that has some NFS mounts, and with a newly 
>>> compiled
>>> kernel based on v4.9-rc3 I see the following warning/info message:
>>>
>>> [   42.750181] ===
>>> [   42.750192] [ INFO: suspicious RCU usage. ]
>>> [   42.750203] 4.9.0-rc3-2-g7b6e7de #3 Not tainted
>>> [   42.750213] ---
>>> [   42.750225] net/sunrpc/clnt.c:2773 suspicious rcu_dereference_check() 
>>> usage!
>>> [   42.750235] 
>>> [   42.750235] other info that might help us debug this:
>>> [   42.750235] 
>>> [   42.750246] 
>>> [   42.750246] rcu_scheduler_active = 1, debug_locks = 0
>>> [   42.750257] 1 lock held by mount.nfs4/6440:
>>> [   42.750278]  #0: 
>>> [   42.750299]  (
>>> [   42.750319] &(>nfs_client_lock)->rlock
>>> [   42.750340] ){+.+...}
>>> [   42.750362] , at: 
>>> [   42.750372] [] nfs_get_client+0x105/0x5e0
>>> [   42.750383] 
>>> [   42.750383] stack backtrace:
>>> [   42.750394] CPU: 0 PID: 6440 Comm: mount.nfs4 Not tainted 
>>> 4.9.0-rc3-2-g7b6e7de #3
>>> [   42.750406] Hardware name: Intel Corporation PURLEY/PURLEY, BIOS 
>>> PLYDCRB1.MBH.0096.D23.1608240105 08/24/2016
>>> [   42.750429]  c992fa68 8150730f 88014ec8da40 
>>> 0001
>>> [   42.750452]  c992fa98 810bc3f7 880150b0b228 
>>> 88015068dbb0
>>> [   42.750475]  c992fb38 88014fc99180 c992fac0 
>>> 81b243e5
>>> [   42.750486] Call Trace:
>>> [   42.750498]  [] dump_stack+0x67/0x98
>>> [   42.750511]  [] lockdep_rcu_suspicious+0xe7/0x120
>>> [   42.750524]  [] 
>>> rpc_clnt_xprt_switch_has_addr+0x115/0x150
>>> [   42.750536]  [] nfs_get_client+0x244/0x5e0
>>> [   42.750549]  [] ? nfs_get_client+0xfc/0x5e0
>>> [   42.750561]  [] nfs4_set_client+0x98/0x130
>>> [   42.750574]  [] nfs4_create_server+0x13e/0x390
>>> [   42.750588]  [] nfs4_remote_mount+0x2e/0x60
>>> [   42.750600]  [] mount_fs+0x39/0x170
>>> [   42.750614]  [] vfs_kern_mount+0x6b/0x150
>>> [   42.750626]  [] ? nfs_do_root_mount+0x3c/0xc0
>>> [   42.750639]  [] nfs_do_root_mount+0x86/0xc0
>>> [   42.750652]  [] nfs4_try_mount+0x44/0xc0
>>> [   42.750664]  [] ? get_nfs_version+0x27/0x90
>>> [   42.750677]  [] nfs_fs_mount+0x4ac/0xd80
>>> [   42.750689]  [] ? lockdep_init_map+0x88/0x1f0
>>> [   42.750701]  [] ? nfs_clone_super+0x130/0x130
>>> [   42.750713]  [] ? param_set_portnr+0x70/0x70
>>> [   42.750726]  [] mount_fs+0x39/0x170
>>> [   42.750740]  [] vfs_kern_mount+0x6b/0x150
>>> [   42.750752]  [] do_mount+0x1f1/0xd10
>>> [   42.750765]  [] ? copy_mount_options+0xa1/0x140
>>> [   42.750777]  [] SyS_mount+0x83/0xd0
>>> [   42.750790]  [] do_syscall_64+0x5c/0x130
>>> [   42.750802]  [] entry_SYSCALL64_slow_path+0x25/0x25
>>>
>>> This rcu_dereference_check() was introduced by the following commit:
>>>
>>> commit 39e5d2df959dd4aea81fa33d765d2a5cc67a0512
>>> Author: Andy Adamson <and...@netapp.com>
>>> Date:   Fri Sep 9 09:22:25 2016 -0400
>>>
>>> SUNRPC search xprt switch for sockaddr
>>> 
>>> Signed-off-by: Andy Adamson <and...@netapp.com>
>>> Signed-off-by: Anna Schumaker <anna.schuma...@netapp.com>
>>>
>>> Thanks,
>>> - Ross
>>
>> Thanks Ross,

Hi Ross,

Can you try this patch and let me know if it helps:

http://git.linux-nfs.org/?p=anna/linux-nfs.git;a=commitdiff;h=bb29dd84333a96f309c6d0f88b285b5b78927058

I'm planning on sending it to Linus soon, so it should be in rc5.

Anna

>>
>> --8<--
>> bool rpc_clnt_xprt_switch_has_addr(struct rpc_clnt *clnt,
>>const struct sockaddr *sap)
>> {
>> struct rpc_xprt_switch *xps;
>> bool ret;
>>
>> xps = rcu_dereference(clnt->cl_xpi.xpi_xpswitch);
>>
>> rcu_read_lock();
>> ret = rpc_xprt_switch_has_addr(xps, sap);
>> rcu_read_unlock();
>> return ret;
>> }
>> --8<--
>>
>> Looks like the simple fix is to just move that rcu_dereference call
>> inside the rcu_read_lock there.
>>
> 
> Hmm...that said though, there are some other suspicious accesses
> of xpi_xpswitch. Looks like these are called without the rcu_read_lock
> clearly being held:
> 
> rpc_clnt_xprt_switch_add_xprt
> rpc_clnt_xprt_switch_put
> 
> ...though it's possible I missed something there.
> 

Re: nfs NULL-dereferencing in net-next

[Anna Schumaker]

On 11/10/2016 09:47 AM, Olaf Hering wrote:
> On Thu, Nov 03, Anna Schumaker wrote:
> 
>> Aww, I was hoping that patch would work.  It still seemed to fix some
>> issues for me when mounting multiple servers, so I'm planning to keep
>> it.  Unfortunately I'm out of town this week, so I haven't had much of
>> a chance to keep poking at this issue.  I should be able to get back
>> to it next week!
> 
> Is this supposed to be fixed already? I get an oops in
> rpc_clnt_xprt_switch_has_addr+0xc/0x40 with 4.9.0-rc4.

Not yet, sorry.  I'm waiting on one bugfix patch before sending my pull request.

Thanks for waiting patiently,
Anna

> 
> Olaf
> 

Re: nfs NULL-dereferencing in net-next

[Anna Schumaker]

On 10/31/2016 11:25 AM, Jakub Kicinski wrote:
> On Thu, 27 Oct 2016 06:50:22 +, Yotam Gigi wrote:
>>> -Original Message-
>>> From: Anna Schumaker [mailto:anna.schuma...@netapp.com]
>>> Sent: Wednesday, October 26, 2016 9:17 PM
>>> To: Jakub Kicinski <kubak...@wp.pl>
>>> Cc: Yotam Gigi <yot...@mellanox.com>; Andy Adamson <and...@netapp.com>;
>>> linux-...@vger.kernel.org; netdev@vger.kernel.org; Trond Myklebust
>>> <trond.mykleb...@netapp.com>; Yotam Gigi <yotam...@gmail.com>; mlxsw
>>> <ml...@mellanox.com>
>>> Subject: Re: nfs NULL-dereferencing in net-next
>>>
>>> On 10/26/2016 02:08 PM, Jakub Kicinski wrote:  
>>>> On Wed, 26 Oct 2016 16:15:24 +, Yotam Gigi wrote:  
>>>>>> -Original Message-
>>>>>> From: Anna Schumaker [mailto:anna.schuma...@netapp.com]
>>>>>> Sent: Wednesday, October 26, 2016 5:40 PM
>>>>>> To: Yotam Gigi <yot...@mellanox.com>; Jakub Kicinski <kubak...@wp.pl>;  
>>> Andy  
>>>>>> Adamson <and...@netapp.com>; Anna Schumaker
>>>>>> <anna.schuma...@netapp.com>; linux-...@vger.kernel.org
>>>>>> Cc: netdev@vger.kernel.org; Trond Myklebust  
>>> <trond.mykleb...@netapp.com>;  
>>>>>> Yotam Gigi <yotam...@gmail.com>; mlxsw <ml...@mellanox.com>
>>>>>> Subject: Re: nfs NULL-dereferencing in net-next
>>>>>>
>>>>>> On 10/25/2016 01:19 PM, Yotam Gigi wrote:  
>>>>>>>  
>>>>>>>> -Original Message-
>>>>>>>> From: netdev-ow...@vger.kernel.org [mailto:netdev-  
>>> ow...@vger.kernel.org]  
>>>>>> On  
>>>>>>>> Behalf Of Jakub Kicinski
>>>>>>>> Sent: Monday, October 17, 2016 10:20 PM
>>>>>>>> To: Andy Adamson <and...@netapp.com>; Anna Schumaker
>>>>>>>> <anna.schuma...@netapp.com>; linux-...@vger.kernel.org
>>>>>>>> Cc: netdev@vger.kernel.org; Trond Myklebust  
>>>>>> <trond.mykleb...@netapp.com>  
>>>>>>>> Subject: nfs NULL-dereferencing in net-next
>>>>>>>>
>>>>>>>> Hi!
>>>>>>>>
>>>>>>>> I'm hitting this reliably on net-next, HEAD at 3f3177bb680f
>>>>>>>> ("fsl/fman: fix error return code in mac_probe()").  
>>>>>>>
>>>>>>>
>>>>>>> I see the same thing. It happens constantly on some of my machines, 
>>>>>>> making  
>>>>>> them  
>>>>>>> completely unusable.
>>>>>>>
>>>>>>> I bisected it and got to the commit:
>>>>>>>
>>>>>>> commit 04ea1b3e6d8ed4978bb608c1748530af3de8c274
>>>>>>> Author: Andy Adamson <and...@netapp.com>
>>>>>>> Date:   Fri Sep 9 09:22:27 2016 -0400
>>>>>>>
>>>>>>> NFS add xprt switch addrs test to match client
>>>>>>>
>>>>>>> Signed-off-by: Andy Adamson <and...@netapp.com>
>>>>>>> Signed-off-by: Anna Schumaker <anna.schuma...@netapp.com>  
>>>>>>
>>>>>> Thanks for reporting on this everyone!  Does this patch help?  
>>>>>
>>>>> Actually, I still see the same bug with the same trace.  
>>>
>>> Well, it was worth a shot.  I'll keep poking at it.
>>>  
>>>>
>>>> I rebuild the latest net-next and I'm not seeing the trace any more...
>>>> I'm only seeing this (with or without your patch):
>>>>
>>>> [   23.465877] NFS: set_pnfs_layoutdriver: cl_exchange_flags 0x0
>>>> [   23.473784] NFS: set_pnfs_layoutdriver: cl_exchange_flags 0x0
>>>> [   23.588890] NFS: set_pnfs_layoutdriver: cl_exchange_flags 0x0
>>>> [   23.596746] NFS: set_pnfs_layoutdriver: cl_exchange_flags 0x0
>>>> [   23.781574] NFS: set_pnfs_layoutdriver: cl_exchange_flags 0x0
>>>> [   23.789599] NFS: set_pnfs_layoutdriver: cl_exchange_flags 0x0  
>>>
>>> Interesting, I get that too when I try to use NFS v4.1.  It's weird that 
>>> the crash would
>>> stop happening like that, so maybe something is racy in this area.
>>>
>>> Thanks for testing, Yotam and Jakub!
>>> Anna  
>>
>> I just found out that it happens on any of my machines, once I put two nfs 
>> entries in
>> my fstab. If I put only one, I don't see the problem. 
>>
>> I hope it might be helpful :)
> 
> Hi Anna,
> 
> any updates on this one?  The crash came back half an hour after I
> reported that it was gone...

Aww, I was hoping that patch would work.  It still seemed to fix some issues 
for me when mounting multiple servers, so I'm planning to keep it.  
Unfortunately I'm out of town this week, so I haven't had much of a chance to 
keep poking at this issue.  I should be able to get back to it next week!

Thanks for the update!
Anna

> 
> Over the weekend David Miller rebased net-next on top of 4.9.0-rc3 and
> the bug is still there :(  FWIW I also have multiple nfs mounts on my
> setup, 2 in fstab and one in a startup script.  Following Yotam I
> dropped one of the fstab entries and things seem to be working (even
> though I still have multiple mounts, the other one just comes a bit
> later).
> 

Re: nfs NULL-dereferencing in net-next

[Anna Schumaker]

On 10/26/2016 02:08 PM, Jakub Kicinski wrote:
> On Wed, 26 Oct 2016 16:15:24 +, Yotam Gigi wrote:
>>> -Original Message-
>>> From: Anna Schumaker [mailto:anna.schuma...@netapp.com]
>>> Sent: Wednesday, October 26, 2016 5:40 PM
>>> To: Yotam Gigi <yot...@mellanox.com>; Jakub Kicinski <kubak...@wp.pl>; Andy
>>> Adamson <and...@netapp.com>; Anna Schumaker
>>> <anna.schuma...@netapp.com>; linux-...@vger.kernel.org
>>> Cc: netdev@vger.kernel.org; Trond Myklebust <trond.mykleb...@netapp.com>;
>>> Yotam Gigi <yotam...@gmail.com>; mlxsw <ml...@mellanox.com>
>>> Subject: Re: nfs NULL-dereferencing in net-next
>>>
>>> On 10/25/2016 01:19 PM, Yotam Gigi wrote:  
>>>>  
>>>>> -Original Message-
>>>>> From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org]  
>>> On  
>>>>> Behalf Of Jakub Kicinski
>>>>> Sent: Monday, October 17, 2016 10:20 PM
>>>>> To: Andy Adamson <and...@netapp.com>; Anna Schumaker
>>>>> <anna.schuma...@netapp.com>; linux-...@vger.kernel.org
>>>>> Cc: netdev@vger.kernel.org; Trond Myklebust  
>>> <trond.mykleb...@netapp.com>  
>>>>> Subject: nfs NULL-dereferencing in net-next
>>>>>
>>>>> Hi!
>>>>>
>>>>> I'm hitting this reliably on net-next, HEAD at 3f3177bb680f
>>>>> ("fsl/fman: fix error return code in mac_probe()").  
>>>>
>>>>
>>>> I see the same thing. It happens constantly on some of my machines, making 
>>>>  
>>> them  
>>>> completely unusable.
>>>>
>>>> I bisected it and got to the commit:
>>>>
>>>> commit 04ea1b3e6d8ed4978bb608c1748530af3de8c274
>>>> Author: Andy Adamson <and...@netapp.com>
>>>> Date:   Fri Sep 9 09:22:27 2016 -0400
>>>>
>>>> NFS add xprt switch addrs test to match client
>>>>
>>>> Signed-off-by: Andy Adamson <and...@netapp.com>
>>>> Signed-off-by: Anna Schumaker <anna.schuma...@netapp.com>  
>>>
>>> Thanks for reporting on this everyone!  Does this patch help?  
>>
>> Actually, I still see the same bug with the same trace.

Well, it was worth a shot.  I'll keep poking at it.

> 
> I rebuild the latest net-next and I'm not seeing the trace any more...
> I'm only seeing this (with or without your patch):
> 
> [   23.465877] NFS: set_pnfs_layoutdriver: cl_exchange_flags 0x0
> [   23.473784] NFS: set_pnfs_layoutdriver: cl_exchange_flags 0x0
> [   23.588890] NFS: set_pnfs_layoutdriver: cl_exchange_flags 0x0
> [   23.596746] NFS: set_pnfs_layoutdriver: cl_exchange_flags 0x0
> [   23.781574] NFS: set_pnfs_layoutdriver: cl_exchange_flags 0x0
> [   23.789599] NFS: set_pnfs_layoutdriver: cl_exchange_flags 0x0

Interesting, I get that too when I try to use NFS v4.1.  It's weird that the 
crash would stop happening like that, so maybe something is racy in this area.

Thanks for testing, Yotam and Jakub!
Anna

> 
> HTH
> 

Re: [PATCH 0/4] make function arg and structures as const

[Anna Schumaker]

On 11/09/2017 09:21 PM, J. Bruce Fields wrote:
> On Tue, Oct 17, 2017 at 12:40:27PM -0400, Jeff Layton wrote:
>> On Tue, 2017-10-17 at 18:14 +0200, Bhumika Goyal wrote:
>>> Make the function argument as const. After thing change, make
>>> the cache_detail structures as const.
>>>
>>> Bhumika Goyal (4):
>>>   sunrpc: make the function arg as const
>>>   NFS: make cache_detail structures const
>>>   NFSD: make cache_detail structures const
>>>   SUNRPC: make cache_detail structures const
>>>
>>>  fs/nfs/dns_resolve.c  | 2 +-
>>>  fs/nfsd/export.c  | 4 ++--
>>>  fs/nfsd/nfs4idmap.c   | 4 ++--
>>>  include/linux/sunrpc/cache.h  | 2 +-
>>>  net/sunrpc/auth_gss/svcauth_gss.c | 4 ++--
>>>  net/sunrpc/cache.c| 2 +-
>>>  net/sunrpc/svcauth_unix.c | 4 ++--
>>>  7 files changed, 11 insertions(+), 11 deletions(-)
>>>
>>
>> Looks pretty straightforward. You can add this to the set:
>>
>> Reviewed-by: Jeff Layton 
> 
> Thanks, I've applied 1, 3, and 4 and could take #2 as well if it's OK
> with Trond/Anna.

I don't mind taking #2, it's already in my branch :)

Anna

> 
> --b.
> 

Re: [PATCH net-next nfs 1/6] net: Convert rpcsec_gss_net_ops

[Anna Schumaker]

On 03/13/2018 06:49 AM, Kirill Tkhai wrote:
> These pernet_operations initialize and destroy sunrpc_net_id
> refered per-net items. Only used global list is cache_list,
> and accesses already serialized.
> 
> sunrpc_destroy_cache_detail() check for list_empty() without
> cache_list_lock, but when it's called from unregister_pernet_subsys(),
> there can't be callers in parallel, so we won't miss list_empty()
> in this case.
> 
> Signed-off-by: Kirill Tkhai <ktk...@virtuozzo.com>

It might make sense to take these and the other NFS patches through the net 
tree, since the pernet_operations don't yet have the async field in my tree 
(and I therefore can't compile once these are applied).

Acked-by: Anna Schumaker <anna.schuma...@netapp.com>

> ---
>  net/sunrpc/auth_gss/auth_gss.c |1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c
> index 9463af4b32e8..44f939cb6bc8 100644
> --- a/net/sunrpc/auth_gss/auth_gss.c
> +++ b/net/sunrpc/auth_gss/auth_gss.c
> @@ -2063,6 +2063,7 @@ static __net_exit void rpcsec_gss_exit_net(struct net 
> *net)
>  static struct pernet_operations rpcsec_gss_net_ops = {
>   .init = rpcsec_gss_init_net,
>   .exit = rpcsec_gss_exit_net,
> + .async = true,
>  };
>  
>  /*
> 

Re: [PATCH net-next nfs 1/6] net: Convert rpcsec_gss_net_ops

[Anna Schumaker]

On 03/13/2018 06:49 AM, Kirill Tkhai wrote:
> These pernet_operations initialize and destroy sunrpc_net_id
> refered per-net items. Only used global list is cache_list,
> and accesses already serialized.
> 
> sunrpc_destroy_cache_detail() check for list_empty() without
> cache_list_lock, but when it's called from unregister_pernet_subsys(),
> there can't be callers in parallel, so we won't miss list_empty()
> in this case.
> 
> Signed-off-by: Kirill Tkhai <ktk...@virtuozzo.com>

It might make sense to take these and the other NFS patches through the net 
tree, since the pernet_operations don't yet have the async field in my tree 
(and I therefore can't compile once these are applied).

Acked-by: Anna Schumaker <anna.schuma...@netapp.com>

> ---
>  net/sunrpc/auth_gss/auth_gss.c |1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c
> index 9463af4b32e8..44f939cb6bc8 100644
> --- a/net/sunrpc/auth_gss/auth_gss.c
> +++ b/net/sunrpc/auth_gss/auth_gss.c
> @@ -2063,6 +2063,7 @@ static __net_exit void rpcsec_gss_exit_net(struct net 
> *net)
>  static struct pernet_operations rpcsec_gss_net_ops = {
>   .init = rpcsec_gss_init_net,
>   .exit = rpcsec_gss_exit_net,
> + .async = true,
>  };
>  
>  /*
> 

Re: [PATCH net-next nfs 4/6] net: Convert nfs4_dns_resolver_ops

[Anna Schumaker]

On 03/13/2018 06:49 AM, Kirill Tkhai wrote:
> These pernet_operations look similar to rpcsec_gss_net_ops,
> they just create and destroy another cache. Also they create
> and destroy directory. So, they also look safe to be async.
> 
> Signed-off-by: Kirill Tkhai <ktk...@virtuozzo.com>

Acked-by: Anna Schumaker <anna.schuma...@netapp.com>

> ---
>  fs/nfs/dns_resolve.c |1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/nfs/dns_resolve.c b/fs/nfs/dns_resolve.c
> index 060c658eab66..e90bd69ab653 100644
> --- a/fs/nfs/dns_resolve.c
> +++ b/fs/nfs/dns_resolve.c
> @@ -410,6 +410,7 @@ static void nfs4_dns_net_exit(struct net *net)
>  static struct pernet_operations nfs4_dns_resolver_ops = {
>   .init = nfs4_dns_net_init,
>   .exit = nfs4_dns_net_exit,
> + .async = true,
>  };
>  
>  static int rpc_pipefs_event(struct notifier_block *nb, unsigned long event,
> 

Re: [PATCH net-next nfs 2/6] net: Convert sunrpc_net_ops

[Anna Schumaker]

On 03/13/2018 06:49 AM, Kirill Tkhai wrote:
> These pernet_operations look similar to rpcsec_gss_net_ops,
> they just create and destroy another caches. So, they also
> can be async.
> 
> Signed-off-by: Kirill Tkhai <ktk...@virtuozzo.com>

Acked-by: Anna Schumaker <anna.schuma...@netapp.com>

> ---
>  net/sunrpc/sunrpc_syms.c |1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/sunrpc/sunrpc_syms.c b/net/sunrpc/sunrpc_syms.c
> index 56f9eff74150..68287e921847 100644
> --- a/net/sunrpc/sunrpc_syms.c
> +++ b/net/sunrpc/sunrpc_syms.c
> @@ -79,6 +79,7 @@ static struct pernet_operations sunrpc_net_ops = {
>   .exit = sunrpc_exit_net,
>   .id = _net_id,
>   .size = sizeof(struct sunrpc_net),
> + .async = true,
>  };
>  
>  static int __init
> 

Re: [PATCH net-next nfs 5/6] net: Convert nfs4blocklayout_net_ops

[Anna Schumaker]

On 03/13/2018 06:49 AM, Kirill Tkhai wrote:
> These pernet_operations create and destroy per-net pipe
> and dentry, and they seem safe to be marked as async.
> 
> Signed-off-by: Kirill Tkhai <ktk...@virtuozzo.com>

Acked-by: Anna Schumaker <anna.schuma...@netapp.com>

> ---
>  fs/nfs/blocklayout/rpc_pipefs.c |1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/nfs/blocklayout/rpc_pipefs.c b/fs/nfs/blocklayout/rpc_pipefs.c
> index 9fb067a6f7e0..ef9fa111b009 100644
> --- a/fs/nfs/blocklayout/rpc_pipefs.c
> +++ b/fs/nfs/blocklayout/rpc_pipefs.c
> @@ -261,6 +261,7 @@ static void nfs4blocklayout_net_exit(struct net *net)
>  static struct pernet_operations nfs4blocklayout_net_ops = {
>   .init = nfs4blocklayout_net_init,
>   .exit = nfs4blocklayout_net_exit,
> + .async = true,
>  };
>  
>  int __init bl_init_pipefs(void)
>