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
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 <[email protected]>
---
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
They already exist and do the exact same thing. Let's save ourselves
several lines of code!
Signed-off-by: Anna Schumaker <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
Let's just call nfs4_stateid_match() directly.
Signed-off-by: Anna Schumaker <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
Looks good,
Reviewed-by: Christoph Hellwig <[email protected]>
Looks good,
Reviewed-by: Christoph Hellwig <[email protected]>
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 <[email protected]>
Looks good,
Reviewed-by: Christoph Hellwig <[email protected]>
> 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.
Looks good,
Reviewed-by: Christoph Hellwig <[email protected]>
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 <[email protected]>
Looks good,
Reviewed-by: Christoph Hellwig <[email protected]>
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 <[email protected]>
Looks good,
Reviewed-by: Christoph Hellwig <[email protected]>
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 <[email protected]>
Looks good,
Reviewed-by: Christoph Hellwig <[email protected]>
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 <[email protected]>
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
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
On Jul 13, 2015, at 10:21 AM, Anna Schumaker <[email protected]> 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
On 07/13/2015 10:32 AM, Chuck Lever wrote:
>
> On Jul 13, 2015, at 10:21 AM, Anna Schumaker <[email protected]> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>