Skip to contentSkip to conversation

nfs: delegation: Replace rcu_assign_pointer() with RCU_INIT_POINTER()

PATCH In review Andreea-Cristina Bernat 2 participants 2 messages Aug 22, 2014

Subsystems (2)
NFS client NFS server
lore
Share & tools
lore

Andreea-Cristina Bernat

author
#0 lore raw

PATCH 1/1 nfs: delegation: Replace rcu_assign_pointer() with RCU_INIT_POINTER()

The use of "rcu_assign_pointer()" is NULLing out the pointer. According to RCU_INIT_POINTER()'s block comment:
"1. This use of RCU_INIT_POINTER() is NULLing out the pointer"
it is better to use it instead of rcu_assign_pointer() because it has a smaller overhead.

The following Coccinelle semantic patch was used:
@@
@@

- rcu_assign_pointer
+ RCU_INIT_POINTER
(..., NULL)

Signed-off-by Andreea-Cristina Bernat <[email protected]>
---
 fs/nfs/delegation.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/nfs/delegation.c +1 -1
index 5d8ccec..349e63b 100644
@@ -249,7 +249,7 @@ nfs_detach_delegation_locked(struct nfs_inode *nfsi,
 	list_del_rcu(&delegation->super_list);
 	delegation->inode = NULL;
 	nfsi->delegation_state = 0;
-	rcu_assign_pointer(nfsi->delegation, NULL);
+	RCU_INIT_POINTER(nfsi->delegation, NULL);
 	spin_unlock(&delegation->lock);
 	return delegation;
 }
Signature · 1 line
1.9.1
#1 lore raw
Quoted 33 lines · Andreea-Cristina Bernat

The use of "rcu_assign_pointer()" is NULLing out the pointer. According to RCU_INIT_POINTER()'s block comment:
"1. This use of RCU_INIT_POINTER() is NULLing out the pointer"
it is better to use it instead of rcu_assign_pointer() because it has a smaller overhead.

The following Coccinelle semantic patch was used:
@@
@@

- rcu_assign_pointer
+ RCU_INIT_POINTER
(..., NULL)

Signed-off-by Andreea-Cristina Bernat <[email protected]>
---
 fs/nfs/delegation.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/nfs/delegation.c +1 -2
index 5d8ccec..349e63b 100644
@@ -249,7 +249,7 @@ nfs_detach_delegation_locked(struct nfs_inode *nfsi,
        list_del_rcu(&delegation->super_list);
        delegation->inode = NULL;
        nfsi->delegation_state = 0;
-       rcu_assign_pointer(nfsi->delegation, NULL);
+       RCU_INIT_POINTER(nfsi->delegation, NULL);
        spin_unlock(&delegation->lock);
        return delegation;
 }
--
Quoted 1 line

1.9.1

Sorry, but NACK.

We do need the memory barriers here, as the nfs_inode is globally visible, and other threads may be checking nfsi->delegation_state and/or delegation->inode in order to conclude whether or not there is still a delegation associated with this inode.

Cheers
Trond

Signature · 5 lines
Trond Myklebust

Linux NFS client maintainer, PrimaryData

[email protected]