[PATCH 0/9] NFS: Clean up unnecessary functions

[PATCH 0/9] NFS: Clean up unnecessary functions

[Anna Schumaker]

I went searching for functions that simply pass their arguments to another
function without really adding anything useful.  I figure we can just call
the other function directly instead.

Let me know what you all think!
Anna

Anna Schumaker (9):
  NFS: Remove unused variable "pages_ptr"
  NFS: Rename nfs_readdir_free_pagearray() and nfs_readdir_large_page()
  SUNRPC: Drop double-underscores from rpc_cmp_addr{4|6}()
  NFS: Use RPC functions for matching sockaddrs
  NFS: Combine nfs_idmap_{init|quit}() and
    nfs_idmap_{init|quit}_keyring()
  NFS: Remove nfs4_match_stateid()
  NFS: Remove nfs41_server_notify_{target|highest}_slotid_update()
  NFS: Rename nfs_commit_unstable_pages() to nfs_write_inode()
  NFS: Remove nfs_release()

 fs/nfs/callback_proc.c      |  2 +-
 fs/nfs/client.c             | 78 ++-------------------------------------------
 fs/nfs/dir.c                | 20 ++++--------
 fs/nfs/file.c               |  3 +-
 fs/nfs/inode.c              |  8 +----
 fs/nfs/internal.h           |  4 ---
 fs/nfs/nfs4_fs.h            |  4 +--
 fs/nfs/nfs4client.c         |  5 +--
 fs/nfs/nfs4idmap.c          | 14 ++------
 fs/nfs/nfs4proc.c           | 10 ++----
 fs/nfs/nfs4state.c          | 12 +------
 fs/nfs/write.c              |  7 +---
 include/linux/sunrpc/addr.h | 12 +++----
 13 files changed, 27 insertions(+), 152 deletions(-)

-- 
2.4.5

[PATCH 1/9] NFS: Remove unused variable "pages_ptr"

[Anna Schumaker]

This variable is initialized to NULL and is never modified before being
passed to nfs_readdir_free_large_page().  But that's okay, because
nfs_readdir_free_large_page() only seems to exist as a way of calling
nfs_readdir_free_pagearray() without this parameter.  Let's simplify by
removing pages_ptr and nfs_readdir_free_pagearray().

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/dir.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 21457bb..ecb8ed5 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -590,16 +590,9 @@ void nfs_readdir_free_pagearray(struct page **pages, unsigned int npages)
 		put_page(pages[i]);
 }
 
-static
-void nfs_readdir_free_large_page(void *ptr, struct page **pages,
-		unsigned int npages)
-{
-	nfs_readdir_free_pagearray(pages, npages);
-}
-
 /*
  * nfs_readdir_large_page will allocate pages that must be freed with a call
- * to nfs_readdir_free_large_page
+ * to nfs_readdir_free_pagearray
  */
 static
 int nfs_readdir_large_page(struct page **pages, unsigned int npages)
@@ -623,7 +616,6 @@ static
 int nfs_readdir_xdr_to_array(nfs_readdir_descriptor_t *desc, struct page *page, struct inode *inode)
 {
 	struct page *pages[NFS_MAX_READDIR_PAGES];
-	void *pages_ptr = NULL;
 	struct nfs_entry entry;
 	struct file	*file = desc->file;
 	struct nfs_cache_array *array;
@@ -671,7 +663,7 @@ int nfs_readdir_xdr_to_array(nfs_readdir_descriptor_t *desc, struct page *page,
 		}
 	} while (array->eof_index < 0);
 
-	nfs_readdir_free_large_page(pages_ptr, pages, array_size);
+	nfs_readdir_free_pagearray(pages, array_size);
 out_release_array:
 	nfs_readdir_release_array(page);
 out_label_free:
-- 
2.4.5

Re: [PATCH 1/9] NFS: Remove unused variable "pages_ptr"

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 2/9] NFS: Rename nfs_readdir_free_pagearray() and nfs_readdir_large_page()

[Anna Schumaker]

nfs_readdir_xdr_to_array() uses both a cache array and an array of
pages, so I rename these functions to make it clearer how the code
works.  nfs_readdir_large_page() becomes nfs_readdir_alloc_pages()
because this function has absolutely nothing to do with setting up a
large page.  nfs_readdir_free_pagearray() becomes
nfs_readdir_free_pages() to stay consistent with the new alloc_pages()
function.

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/dir.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index ecb8ed5..c722212 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -583,7 +583,7 @@ out_nopages:
 }
 
 static
-void nfs_readdir_free_pagearray(struct page **pages, unsigned int npages)
+void nfs_readdir_free_pages(struct page **pages, unsigned int npages)
 {
 	unsigned int i;
 	for (i = 0; i < npages; i++)
@@ -595,7 +595,7 @@ void nfs_readdir_free_pagearray(struct page **pages, unsigned int npages)
  * to nfs_readdir_free_pagearray
  */
 static
-int nfs_readdir_large_page(struct page **pages, unsigned int npages)
+int nfs_readdir_alloc_pages(struct page **pages, unsigned int npages)
 {
 	unsigned int i;
 
@@ -608,7 +608,7 @@ int nfs_readdir_large_page(struct page **pages, unsigned int npages)
 	return 0;
 
 out_freepages:
-	nfs_readdir_free_pagearray(pages, i);
+	nfs_readdir_free_pages(pages, i);
 	return -ENOMEM;
 }
 
@@ -645,7 +645,7 @@ int nfs_readdir_xdr_to_array(nfs_readdir_descriptor_t *desc, struct page *page,
 	memset(array, 0, sizeof(struct nfs_cache_array));
 	array->eof_index = -1;
 
-	status = nfs_readdir_large_page(pages, array_size);
+	status = nfs_readdir_alloc_pages(pages, array_size);
 	if (status < 0)
 		goto out_release_array;
 	do {
@@ -663,7 +663,7 @@ int nfs_readdir_xdr_to_array(nfs_readdir_descriptor_t *desc, struct page *page,
 		}
 	} while (array->eof_index < 0);
 
-	nfs_readdir_free_pagearray(pages, array_size);
+	nfs_readdir_free_pages(pages, array_size);
 out_release_array:
 	nfs_readdir_release_array(page);
 out_label_free:
-- 
2.4.5

Re: [PATCH 2/9] NFS: Rename nfs_readdir_free_pagearray() and nfs_readdir_large_page()

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 3/9] SUNRPC: Drop double-underscores from rpc_cmp_addr{4|6}()

[Anna Schumaker]

I'm planning on using these functions inside the client, so remove the
underscores to make it feel like I'm using a public interface.

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 include/linux/sunrpc/addr.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/linux/sunrpc/addr.h b/include/linux/sunrpc/addr.h
index 07d8e53..772faef 100644
--- a/include/linux/sunrpc/addr.h
+++ b/include/linux/sunrpc/addr.h
@@ -46,8 +46,8 @@ static inline void rpc_set_port(struct sockaddr *sap,
 #define IPV6_SCOPE_DELIMITER		'%'
 #define IPV6_SCOPE_ID_LEN		sizeof("%nnnnnnnnnn")
 
-static inline bool __rpc_cmp_addr4(const struct sockaddr *sap1,
-				   const struct sockaddr *sap2)
+static inline bool rpc_cmp_addr4(const struct sockaddr *sap1,
+				 const struct sockaddr *sap2)
 {
 	const struct sockaddr_in *sin1 = (const struct sockaddr_in *)sap1;
 	const struct sockaddr_in *sin2 = (const struct sockaddr_in *)sap2;
@@ -67,8 +67,8 @@ static inline bool __rpc_copy_addr4(struct sockaddr *dst,
 }
 
 #if IS_ENABLED(CONFIG_IPV6)
-static inline bool __rpc_cmp_addr6(const struct sockaddr *sap1,
-				   const struct sockaddr *sap2)
+static inline bool rpc_cmp_addr6(const struct sockaddr *sap1,
+				 const struct sockaddr *sap2)
 {
 	const struct sockaddr_in6 *sin1 = (const struct sockaddr_in6 *)sap1;
 	const struct sockaddr_in6 *sin2 = (const struct sockaddr_in6 *)sap2;
@@ -122,9 +122,9 @@ static inline bool rpc_cmp_addr(const struct sockaddr *sap1,
 	if (sap1->sa_family == sap2->sa_family) {
 		switch (sap1->sa_family) {
 		case AF_INET:
-			return __rpc_cmp_addr4(sap1, sap2);
+			return rpc_cmp_addr4(sap1, sap2);
 		case AF_INET6:
-			return __rpc_cmp_addr6(sap1, sap2);
+			return rpc_cmp_addr6(sap1, sap2);
 		}
 	}
 	return false;
-- 
2.4.5

Re: [PATCH 3/9] SUNRPC: Drop double-underscores from rpc_cmp_addr{4|6}()

[Christoph Hellwig]

On Fri, Jul 10, 2015 at 04:58:11PM -0400, Anna Schumaker wrote:
> I'm planning on using these functions inside the client, so remove the
> underscores to make it feel like I'm using a public interface.
> 
> Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 4/9] NFS: Use RPC functions for matching sockaddrs

[Anna Schumaker]

They already exist and do the exact same thing.  Let's save ourselves
several lines of code!

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/client.c     | 78 +++--------------------------------------------------
 fs/nfs/internal.h   |  4 ---
 fs/nfs/nfs4client.c |  5 +---
 3 files changed, 4 insertions(+), 83 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index ecebb40..a8074ec 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -20,6 +20,7 @@
 #include <linux/stat.h>
 #include <linux/errno.h>
 #include <linux/unistd.h>
+#include <linux/sunrpc/addr.h>
 #include <linux/sunrpc/clnt.h>
 #include <linux/sunrpc/stats.h>
 #include <linux/sunrpc/metrics.h>
@@ -285,63 +286,13 @@ void nfs_put_client(struct nfs_client *clp)
 }
 EXPORT_SYMBOL_GPL(nfs_put_client);
 
-#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
-/*
- * Test if two ip6 socket addresses refer to the same socket by
- * comparing relevant fields. The padding bytes specifically, are not
- * compared. sin6_flowinfo is not compared because it only affects QoS
- * and sin6_scope_id is only compared if the address is "link local"
- * because "link local" addresses need only be unique to a specific
- * link. Conversely, ordinary unicast addresses might have different
- * sin6_scope_id.
- *
- * The caller should ensure both socket addresses are AF_INET6.
- */
-static int nfs_sockaddr_match_ipaddr6(const struct sockaddr *sa1,
-				      const struct sockaddr *sa2)
-{
-	const struct sockaddr_in6 *sin1 = (const struct sockaddr_in6 *)sa1;
-	const struct sockaddr_in6 *sin2 = (const struct sockaddr_in6 *)sa2;
-
-	if (!ipv6_addr_equal(&sin1->sin6_addr, &sin2->sin6_addr))
-		return 0;
-	else if (ipv6_addr_type(&sin1->sin6_addr) & IPV6_ADDR_LINKLOCAL)
-		return sin1->sin6_scope_id == sin2->sin6_scope_id;
-
-	return 1;
-}
-#else	/* !defined(CONFIG_IPV6) && !defined(CONFIG_IPV6_MODULE) */
-static int nfs_sockaddr_match_ipaddr6(const struct sockaddr *sa1,
-				      const struct sockaddr *sa2)
-{
-	return 0;
-}
-#endif
-
-/*
- * Test if two ip4 socket addresses refer to the same socket, by
- * comparing relevant fields. The padding bytes specifically, are
- * not compared.
- *
- * The caller should ensure both socket addresses are AF_INET.
- */
-static int nfs_sockaddr_match_ipaddr4(const struct sockaddr *sa1,
-				      const struct sockaddr *sa2)
-{
-	const struct sockaddr_in *sin1 = (const struct sockaddr_in *)sa1;
-	const struct sockaddr_in *sin2 = (const struct sockaddr_in *)sa2;
-
-	return sin1->sin_addr.s_addr == sin2->sin_addr.s_addr;
-}
-
 static int nfs_sockaddr_cmp_ip6(const struct sockaddr *sa1,
 				const struct sockaddr *sa2)
 {
 	const struct sockaddr_in6 *sin1 = (const struct sockaddr_in6 *)sa1;
 	const struct sockaddr_in6 *sin2 = (const struct sockaddr_in6 *)sa2;
 
-	return nfs_sockaddr_match_ipaddr6(sa1, sa2) &&
-		(sin1->sin6_port == sin2->sin6_port);
+	return rpc_cmp_addr6(sa1, sa2) && (sin1->sin6_port == sin2->sin6_port);
 }
 
 static int nfs_sockaddr_cmp_ip4(const struct sockaddr *sa1,
@@ -350,31 +301,8 @@ static int nfs_sockaddr_cmp_ip4(const struct sockaddr *sa1,
 	const struct sockaddr_in *sin1 = (const struct sockaddr_in *)sa1;
 	const struct sockaddr_in *sin2 = (const struct sockaddr_in *)sa2;
 
-	return nfs_sockaddr_match_ipaddr4(sa1, sa2) &&
-		(sin1->sin_port == sin2->sin_port);
-}
-
-#if defined(CONFIG_NFS_V4_1)
-/*
- * Test if two socket addresses represent the same actual socket,
- * by comparing (only) relevant fields, excluding the port number.
- */
-int nfs_sockaddr_match_ipaddr(const struct sockaddr *sa1,
-			      const struct sockaddr *sa2)
-{
-	if (sa1->sa_family != sa2->sa_family)
-		return 0;
-
-	switch (sa1->sa_family) {
-	case AF_INET:
-		return nfs_sockaddr_match_ipaddr4(sa1, sa2);
-	case AF_INET6:
-		return nfs_sockaddr_match_ipaddr6(sa1, sa2);
-	}
-	return 0;
+	return rpc_cmp_addr4(sa1, sa2) && (sin1->sin_port == sin2->sin_port);
 }
-EXPORT_SYMBOL_GPL(nfs_sockaddr_match_ipaddr);
-#endif /* CONFIG_NFS_V4_1 */
 
 /*
  * Test if two socket addresses represent the same actual socket,
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 9e6475b..b27e81f 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -219,10 +219,6 @@ static inline void nfs_fs_proc_exit(void)
 }
 #endif
 
-#ifdef CONFIG_NFS_V4_1
-int nfs_sockaddr_match_ipaddr(const struct sockaddr *, const struct sockaddr *);
-#endif
-
 /* callback_xdr.c */
 extern struct svc_version nfs4_callback_version1;
 extern struct svc_version nfs4_callback_version4;
diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
index 3aa6a9b..223bedd 100644
--- a/fs/nfs/nfs4client.c
+++ b/fs/nfs/nfs4client.c
@@ -729,10 +729,7 @@ static bool nfs4_cb_match_client(const struct sockaddr *addr,
 		return false;
 
 	/* Match only the IP address, not the port number */
-	if (!nfs_sockaddr_match_ipaddr(addr, clap))
-		return false;
-
-	return true;
+	return rpc_cmp_addr(addr, clap);
 }
 
 /*
-- 
2.4.5

Re: [PATCH 4/9] NFS: Use RPC functions for matching sockaddrs

[Christoph Hellwig]

>  static int nfs_sockaddr_cmp_ip6(const struct sockaddr *sa1,
>  				const struct sockaddr *sa2)
>  {
>  	const struct sockaddr_in6 *sin1 = (const struct sockaddr_in6 *)sa1;
>  	const struct sockaddr_in6 *sin2 = (const struct sockaddr_in6 *)sa2;
>  
> -	return nfs_sockaddr_match_ipaddr6(sa1, sa2) &&
> -		(sin1->sin6_port == sin2->sin6_port);
> +	return rpc_cmp_addr6(sa1, sa2) && (sin1->sin6_port == sin2->sin6_port);
>  }
>  
>  static int nfs_sockaddr_cmp_ip4(const struct sockaddr *sa1,
> @@ -350,31 +301,8 @@ static int nfs_sockaddr_cmp_ip4(const struct sockaddr *sa1,
>  	const struct sockaddr_in *sin1 = (const struct sockaddr_in *)sa1;
>  	const struct sockaddr_in *sin2 = (const struct sockaddr_in *)sa2;
>  
> -	return nfs_sockaddr_match_ipaddr4(sa1, sa2) &&
> -		(sin1->sin_port == sin2->sin_port);
> -}

I'd say kill nfs_sockaddr_cmp as well and use rpc_cmp_addr in
nfs_match_client.

Re: [PATCH 4/9] NFS: Use RPC functions for matching sockaddrs

[Anna Schumaker]

Hi Christoph,

On 07/13/2015 03:03 AM, Christoph Hellwig wrote:
>>  static int nfs_sockaddr_cmp_ip6(const struct sockaddr *sa1,
>>  				const struct sockaddr *sa2)
>>  {
>>  	const struct sockaddr_in6 *sin1 = (const struct sockaddr_in6 *)sa1;
>>  	const struct sockaddr_in6 *sin2 = (const struct sockaddr_in6 *)sa2;
>>  
>> -	return nfs_sockaddr_match_ipaddr6(sa1, sa2) &&
>> -		(sin1->sin6_port == sin2->sin6_port);
>> +	return rpc_cmp_addr6(sa1, sa2) && (sin1->sin6_port == sin2->sin6_port);
>>  }
>>  
>>  static int nfs_sockaddr_cmp_ip4(const struct sockaddr *sa1,
>> @@ -350,31 +301,8 @@ static int nfs_sockaddr_cmp_ip4(const struct sockaddr *sa1,
>>  	const struct sockaddr_in *sin1 = (const struct sockaddr_in *)sa1;
>>  	const struct sockaddr_in *sin2 = (const struct sockaddr_in *)sa2;
>>  
>> -	return nfs_sockaddr_match_ipaddr4(sa1, sa2) &&
>> -		(sin1->sin_port == sin2->sin_port);
>> -}
> 
> I'd say kill nfs_sockaddr_cmp as well and use rpc_cmp_addr in
> nfs_match_client.
> 

Good idea!  I'll add that in.

Thanks!
Anna

Re: [PATCH 4/9] NFS: Use RPC functions for matching sockaddrs

[Anna Schumaker]

On 07/13/2015 03:03 AM, Christoph Hellwig wrote:
>>  static int nfs_sockaddr_cmp_ip6(const struct sockaddr *sa1,
>>  				const struct sockaddr *sa2)
>>  {
>>  	const struct sockaddr_in6 *sin1 = (const struct sockaddr_in6 *)sa1;
>>  	const struct sockaddr_in6 *sin2 = (const struct sockaddr_in6 *)sa2;
>>  
>> -	return nfs_sockaddr_match_ipaddr6(sa1, sa2) &&
>> -		(sin1->sin6_port == sin2->sin6_port);
>> +	return rpc_cmp_addr6(sa1, sa2) && (sin1->sin6_port == sin2->sin6_port);
>>  }
>>  
>>  static int nfs_sockaddr_cmp_ip4(const struct sockaddr *sa1,
>> @@ -350,31 +301,8 @@ static int nfs_sockaddr_cmp_ip4(const struct sockaddr *sa1,
>>  	const struct sockaddr_in *sin1 = (const struct sockaddr_in *)sa1;
>>  	const struct sockaddr_in *sin2 = (const struct sockaddr_in *)sa2;
>>  
>> -	return nfs_sockaddr_match_ipaddr4(sa1, sa2) &&
>> -		(sin1->sin_port == sin2->sin_port);
>> -}
> 
> I'd say kill nfs_sockaddr_cmp as well and use rpc_cmp_addr in
> nfs_match_client.
> 

Okay, looking closer at the code now.  rpc_cmp_addr() explicitely doesn't check for port number, but nfs_sockaddr_cmp() does.  I could add port checking to rpc_cmp_addr(), but I don't know if it was left out intentionally when the code was written.

Trond?

Anna

Re: [PATCH 4/9] NFS: Use RPC functions for matching sockaddrs

[Chuck Lever]

On Jul 13, 2015, at 10:21 AM, Anna Schumaker <Anna.Schumaker@netapp.com> wrote:

> On 07/13/2015 03:03 AM, Christoph Hellwig wrote:
>>> static int nfs_sockaddr_cmp_ip6(const struct sockaddr *sa1,
>>> 				const struct sockaddr *sa2)
>>> {
>>> 	const struct sockaddr_in6 *sin1 = (const struct sockaddr_in6 *)sa1;
>>> 	const struct sockaddr_in6 *sin2 = (const struct sockaddr_in6 *)sa2;
>>> 
>>> -	return nfs_sockaddr_match_ipaddr6(sa1, sa2) &&
>>> -		(sin1->sin6_port == sin2->sin6_port);
>>> +	return rpc_cmp_addr6(sa1, sa2) && (sin1->sin6_port == sin2->sin6_port);
>>> }
>>> 
>>> static int nfs_sockaddr_cmp_ip4(const struct sockaddr *sa1,
>>> @@ -350,31 +301,8 @@ static int nfs_sockaddr_cmp_ip4(const struct sockaddr *sa1,
>>> 	const struct sockaddr_in *sin1 = (const struct sockaddr_in *)sa1;
>>> 	const struct sockaddr_in *sin2 = (const struct sockaddr_in *)sa2;
>>> 
>>> -	return nfs_sockaddr_match_ipaddr4(sa1, sa2) &&
>>> -		(sin1->sin_port == sin2->sin_port);
>>> -}
>> 
>> I'd say kill nfs_sockaddr_cmp as well and use rpc_cmp_addr in
>> nfs_match_client.
>> 
> 
> Okay, looking closer at the code now.  rpc_cmp_addr() explicitely doesn't check for port number, but nfs_sockaddr_cmp() does.  I could add port checking to rpc_cmp_addr(), but I don't know if it was left out intentionally when the code was written.

Consider adding rpc_cmp_addr_port() which does port checking too?


--
Chuck Lever

Re: [PATCH 4/9] NFS: Use RPC functions for matching sockaddrs

[Anna Schumaker]

On 07/13/2015 10:32 AM, Chuck Lever wrote:
> 
> On Jul 13, 2015, at 10:21 AM, Anna Schumaker <Anna.Schumaker@netapp.com> wrote:
> 
>> On 07/13/2015 03:03 AM, Christoph Hellwig wrote:
>>>> static int nfs_sockaddr_cmp_ip6(const struct sockaddr *sa1,
>>>> 				const struct sockaddr *sa2)
>>>> {
>>>> 	const struct sockaddr_in6 *sin1 = (const struct sockaddr_in6 *)sa1;
>>>> 	const struct sockaddr_in6 *sin2 = (const struct sockaddr_in6 *)sa2;
>>>>
>>>> -	return nfs_sockaddr_match_ipaddr6(sa1, sa2) &&
>>>> -		(sin1->sin6_port == sin2->sin6_port);
>>>> +	return rpc_cmp_addr6(sa1, sa2) && (sin1->sin6_port == sin2->sin6_port);
>>>> }
>>>>
>>>> static int nfs_sockaddr_cmp_ip4(const struct sockaddr *sa1,
>>>> @@ -350,31 +301,8 @@ static int nfs_sockaddr_cmp_ip4(const struct sockaddr *sa1,
>>>> 	const struct sockaddr_in *sin1 = (const struct sockaddr_in *)sa1;
>>>> 	const struct sockaddr_in *sin2 = (const struct sockaddr_in *)sa2;
>>>>
>>>> -	return nfs_sockaddr_match_ipaddr4(sa1, sa2) &&
>>>> -		(sin1->sin_port == sin2->sin_port);
>>>> -}
>>>
>>> I'd say kill nfs_sockaddr_cmp as well and use rpc_cmp_addr in
>>> nfs_match_client.
>>>
>>
>> Okay, looking closer at the code now.  rpc_cmp_addr() explicitely doesn't check for port number, but nfs_sockaddr_cmp() does.  I could add port checking to rpc_cmp_addr(), but I don't know if it was left out intentionally when the code was written.
> 
> Consider adding rpc_cmp_addr_port() which does port checking too?

I like this idea.  Thanks for the suggestion!

Anna

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

[PATCH 5/9] NFS: Combine nfs_idmap_{init|quit}() and nfs_idmap_{init|quit}_keyring()

[Anna Schumaker]

The idmap_init() and idmap_quit() functions only exist to call the
_keyring() version.  Let's just call the keyring() functions directly.

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/nfs4idmap.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/fs/nfs/nfs4idmap.c b/fs/nfs/nfs4idmap.c
index 535dfc6..2e49022 100644
--- a/fs/nfs/nfs4idmap.c
+++ b/fs/nfs/nfs4idmap.c
@@ -184,7 +184,7 @@ static struct key_type key_type_id_resolver = {
 	.read		= user_read,
 };
 
-static int nfs_idmap_init_keyring(void)
+int nfs_idmap_init(void)
 {
 	struct cred *cred;
 	struct key *keyring;
@@ -230,7 +230,7 @@ failed_put_cred:
 	return ret;
 }
 
-static void nfs_idmap_quit_keyring(void)
+void nfs_idmap_quit(void)
 {
 	key_revoke(id_resolver_cache->thread_keyring);
 	unregister_key_type(&key_type_id_resolver);
@@ -492,16 +492,6 @@ nfs_idmap_delete(struct nfs_client *clp)
 	kfree(idmap);
 }
 
-int nfs_idmap_init(void)
-{
-	return nfs_idmap_init_keyring();
-}
-
-void nfs_idmap_quit(void)
-{
-	nfs_idmap_quit_keyring();
-}
-
 static int nfs_idmap_prepare_message(char *desc, struct idmap *idmap,
 				     struct idmap_msg *im,
 				     struct rpc_pipe_msg *msg)
-- 
2.4.5

Re: [PATCH 5/9] NFS: Combine nfs_idmap_{init|quit}() and nfs_idmap_{init|quit}_keyring()

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 6/9] NFS: Remove nfs4_match_stateid()

[Anna Schumaker]

Let's just call nfs4_stateid_match() directly.

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/nfs4proc.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 6f228b5..b911fb0 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -8514,12 +8514,6 @@ static bool nfs41_match_stateid(const nfs4_stateid *s1,
 
 #endif /* CONFIG_NFS_V4_1 */
 
-static bool nfs4_match_stateid(const nfs4_stateid *s1,
-		const nfs4_stateid *s2)
-{
-	return nfs4_stateid_match(s1, s2);
-}
-
 
 static const struct nfs4_state_recovery_ops nfs40_reboot_recovery_ops = {
 	.owner_flag_bit = NFS_OWNER_RECLAIM_REBOOT,
@@ -8594,7 +8588,7 @@ static const struct nfs4_minor_version_ops nfs_v4_0_minor_ops = {
 		| NFS_CAP_POSIX_LOCK,
 	.init_client = nfs40_init_client,
 	.shutdown_client = nfs40_shutdown_client,
-	.match_stateid = nfs4_match_stateid,
+	.match_stateid = nfs4_stateid_match,
 	.find_root_sec = nfs4_find_root_sec,
 	.free_lock_state = nfs4_release_lockowner,
 	.alloc_seqid = nfs_alloc_seqid,
-- 
2.4.5

Re: [PATCH 6/9] NFS: Remove nfs4_match_stateid()

[Christoph Hellwig]

On Fri, Jul 10, 2015 at 04:58:14PM -0400, Anna Schumaker wrote:
> Let's just call nfs4_stateid_match() directly.
> 
> Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 7/9] NFS: Remove nfs41_server_notify_{target|highest}_slotid_update()

[Anna Schumaker]

All these functions do is call nfs41_ping_server() without adding
anything.  Let's remove them and give nfs41_ping_server() a better name
instead.

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/callback_proc.c |  2 +-
 fs/nfs/nfs4_fs.h       |  4 +---
 fs/nfs/nfs4proc.c      |  2 +-
 fs/nfs/nfs4state.c     | 12 +-----------
 4 files changed, 4 insertions(+), 16 deletions(-)

diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
index 29e3c1b..624bef7 100644
--- a/fs/nfs/callback_proc.c
+++ b/fs/nfs/callback_proc.c
@@ -554,7 +554,7 @@ __be32 nfs4_callback_recallslot(struct cb_recallslotargs *args, void *dummy,
 	status = htonl(NFS4_OK);
 
 	nfs41_set_target_slotid(fc_tbl, args->crsa_target_highest_slotid);
-	nfs41_server_notify_target_slotid_update(cps->clp);
+	nfs41_notify_server(cps->clp);
 out:
 	dprintk("%s: exit with status = %d\n", __func__, ntohl(status));
 	return status;
diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index ea3bee9..50cfc4c 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -405,9 +405,7 @@ int nfs40_discover_server_trunking(struct nfs_client *clp,
 int nfs41_discover_server_trunking(struct nfs_client *clp,
 			struct nfs_client **, struct rpc_cred *);
 extern void nfs4_schedule_session_recovery(struct nfs4_session *, int);
-extern void nfs41_server_notify_target_slotid_update(struct nfs_client *clp);
-extern void nfs41_server_notify_highest_slotid_update(struct nfs_client *clp);
-
+extern void nfs41_notify_server(struct nfs_client *);
 #else
 static inline void nfs4_schedule_session_recovery(struct nfs4_session *session, int err)
 {
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index b911fb0..b5a4f95 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -583,7 +583,7 @@ out_unlock:
 	spin_unlock(&tbl->slot_tbl_lock);
 	res->sr_slot = NULL;
 	if (send_new_highest_used_slotid)
-		nfs41_server_notify_highest_slotid_update(session->clp);
+		nfs41_notify_server(session->clp);
 }
 
 int nfs41_sequence_done(struct rpc_task *task, struct nfs4_sequence_res *res)
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 605840d..6f51282 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -2152,23 +2152,13 @@ void nfs4_schedule_session_recovery(struct nfs4_session *session, int err)
 }
 EXPORT_SYMBOL_GPL(nfs4_schedule_session_recovery);
 
-static void nfs41_ping_server(struct nfs_client *clp)
+void nfs41_notify_server(struct nfs_client *clp)
 {
 	/* Use CHECK_LEASE to ping the server with a SEQUENCE */
 	set_bit(NFS4CLNT_CHECK_LEASE, &clp->cl_state);
 	nfs4_schedule_state_manager(clp);
 }
 
-void nfs41_server_notify_target_slotid_update(struct nfs_client *clp)
-{
-	nfs41_ping_server(clp);
-}
-
-void nfs41_server_notify_highest_slotid_update(struct nfs_client *clp)
-{
-	nfs41_ping_server(clp);
-}
-
 static void nfs4_reset_all_state(struct nfs_client *clp)
 {
 	if (test_and_set_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state) == 0) {
-- 
2.4.5

Re: [PATCH 7/9] NFS: Remove nfs41_server_notify_{target|highest}_slotid_update()

[Christoph Hellwig]

On Fri, Jul 10, 2015 at 04:58:15PM -0400, Anna Schumaker wrote:
> All these functions do is call nfs41_ping_server() without adding
> anything.  Let's remove them and give nfs41_ping_server() a better name
> instead.
> 
> Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 8/9] NFS: Rename nfs_commit_unstable_pages() to nfs_write_inode()

[Anna Schumaker]

All nfs_write_inode() does is pass its arguments to
nfs_commit_unstable_pages().  Let's cut out the middle man and have
nfs_write_pages() do the work directly.

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/write.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/fs/nfs/write.c b/fs/nfs/write.c
index 359e9ad..c2dad4f 100644
--- a/fs/nfs/write.c
+++ b/fs/nfs/write.c
@@ -1789,7 +1789,7 @@ out_mark_dirty:
 	return res;
 }
 
-static int nfs_commit_unstable_pages(struct inode *inode, struct writeback_control *wbc)
+int nfs_write_inode(struct inode *inode, struct writeback_control *wbc)
 {
 	struct nfs_inode *nfsi = NFS_I(inode);
 	int flags = FLUSH_SYNC;
@@ -1824,11 +1824,6 @@ out_mark_dirty:
 	__mark_inode_dirty(inode, I_DIRTY_DATASYNC);
 	return ret;
 }
-
-int nfs_write_inode(struct inode *inode, struct writeback_control *wbc)
-{
-	return nfs_commit_unstable_pages(inode, wbc);
-}
 EXPORT_SYMBOL_GPL(nfs_write_inode);
 
 /*
-- 
2.4.5

Re: [PATCH 8/9] NFS: Rename nfs_commit_unstable_pages() to nfs_write_inode()

[Christoph Hellwig]

On Fri, Jul 10, 2015 at 04:58:16PM -0400, Anna Schumaker wrote:
> All nfs_write_inode() does is pass its arguments to
> nfs_commit_unstable_pages().  Let's cut out the middle man and have
> nfs_write_pages() do the work directly.
> 
> Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 9/9] NFS: Remove nfs_release()

[Anna Schumaker]

And call nfs_file_clear_open_context() directly.  This makes it obvious
that nfs_file_release() will always return 0.

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/file.c  | 3 ++-
 fs/nfs/inode.c | 8 +-------
 2 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/fs/nfs/file.c b/fs/nfs/file.c
index cc4fa1e..7538a85 100644
--- a/fs/nfs/file.c
+++ b/fs/nfs/file.c
@@ -82,7 +82,8 @@ nfs_file_release(struct inode *inode, struct file *filp)
 	dprintk("NFS: release(%pD2)\n", filp);
 
 	nfs_inc_stats(inode, NFSIOS_VFSRELEASE);
-	return nfs_release(inode, filp);
+	nfs_file_clear_open_context(filp);
+	return 0;
 }
 EXPORT_SYMBOL_GPL(nfs_file_release);
 
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index b77b328..a0d195f 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -887,7 +887,7 @@ struct nfs_open_context *nfs_find_open_context(struct inode *inode, struct rpc_c
 	return ctx;
 }
 
-static void nfs_file_clear_open_context(struct file *filp)
+void nfs_file_clear_open_context(struct file *filp)
 {
 	struct nfs_open_context *ctx = nfs_file_open_context(filp);
 
@@ -918,12 +918,6 @@ int nfs_open(struct inode *inode, struct file *filp)
 	return 0;
 }
 
-int nfs_release(struct inode *inode, struct file *filp)
-{
-	nfs_file_clear_open_context(filp);
-	return 0;
-}
-
 /*
  * This function is called whenever some part of NFS notices that
  * the cached attributes have to be refreshed.
-- 
2.4.5

Re: [PATCH 9/9] NFS: Remove nfs_release()

[Christoph Hellwig]

On Fri, Jul 10, 2015 at 04:58:17PM -0400, Anna Schumaker wrote:
> And call nfs_file_clear_open_context() directly.  This makes it obvious
> that nfs_file_release() will always return 0.

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>