[PATCH 20/21] NFSv4/pnfs: Key the data-server cache by its address set
Benjamin Coddington <ben.coddington-F/[email protected]>
| Newsgroups | gmane.linux.nfs |
|---|---|
| Message-ID | <80584abd9bb30509483e42ad12ba75543c9d5d1f.1786653063.git.bcodding@hammerspace.com> |
Populating the data-server cache was quadratic: every GETDEVICEINFO decode scanned the whole per-net cache under one spinlock, with an O(n*m) address-set comparison per entry. A striping mount with hundreds to ~1000 distinct data servers pays that in a burst at first access and again on notification-driven re-resolution. Hash each DS to a bucket keyed on its address set: per-address jhash over exactly the fields same_sockaddr() compares, combined by addition so multipath ordering cannot change the bucket, and the existing comparator as the in-bucket tiebreaker. Lookup and insert now touch one bucket; teardown is unchanged (hlist_del_init needs no bucket). The comparator also tightens from subset to set-equality to agree with the order-independent key. The subset test was itself a latent bug: two data servers where one's address set contained the other's were wrongly merged into whichever was cached first. A flexfiles or files-layout device decodes a deterministic multipath list per GETDEVICEINFO, so equal sets are what servers actually hand out. Assisted-by: Claude:claude-fable-5 Signed-off-by: Benjamin Coddington <bcodding-F/[email protected]> --- fs/nfs/pnfs_nfs.c | 73 +++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 64 insertions(+), 9 deletions(-) diff --git a/fs/nfs/pnfs_nfs.c b/fs/nfs/pnfs_nfs.c index e15e0059c56b..a94f4a2933cd 100644 --- a/fs/nfs/pnfs_nfs.c +++ b/fs/nfs/pnfs_nfs.c @@ -15,6 +15,8 @@ #include "nfs4session.h" #include "internal.h" +#include <linux/hash.h> +#include <linux/jhash.h> #include "pnfs.h" #include "netns.h" #include "nfs4trace.h" @@ -577,8 +579,10 @@ same_sockaddr(struct sockaddr *addr1, struct sockaddr *addr2) } /* - * Checks if 'dsaddrs1' contains a subset of 'dsaddrs2'. If it does, - * declare a match. + * Checks if 'dsaddrs1' and 'dsaddrs2' contain the same set of + * addresses. If they do, declare a match. Equal element counts + * plus a subset test give set equality (neither side can hold + * duplicates: each list decodes one device's multipath addresses). */ static bool _same_data_server_addrs_locked(const struct list_head *dsaddrs1, @@ -588,6 +592,10 @@ _same_data_server_addrs_locked(const struct list_head *dsaddrs1, struct sockaddr *sa1, *sa2; bool match = false; + if (list_count_nodes((struct list_head *)dsaddrs1) != + list_count_nodes((struct list_head *)dsaddrs2)) + return false; + list_for_each_entry(da1, dsaddrs1, da_node) { sa1 = (struct sockaddr *)&da1->da_addr; match = false; @@ -603,6 +611,53 @@ _same_data_server_addrs_locked(const struct list_head *dsaddrs1, return match; } +/* + * Hash exactly the fields same_sockaddr() compares: family, address + * bytes and port. The v6 scope id is deliberately left out -- it + * only discriminates link-local addresses, and hashing it would be + * harmless but pointless: unequal link-local addresses that share a + * bucket are still separated by the comparator. + */ +static u32 +nfs4_ds_addr_hash(const struct sockaddr *sa) +{ + u32 h = sa->sa_family; + + switch (sa->sa_family) { + case AF_INET: { + const struct sockaddr_in *a = (const struct sockaddr_in *)sa; + + h = jhash(&a->sin_addr.s_addr, sizeof(a->sin_addr.s_addr), h); + h = jhash(&a->sin_port, sizeof(a->sin_port), h); + break; + } + case AF_INET6: { + const struct sockaddr_in6 *a = (const struct sockaddr_in6 *)sa; + + h = jhash(&a->sin6_addr, sizeof(a->sin6_addr), h); + h = jhash(&a->sin6_port, sizeof(a->sin6_port), h); + break; + } + } + return h; +} + +/* + * Bucket index for a DS's address set. Per-address hashes combine + * by addition so the multipath list order cannot change the bucket, + * matching the order-independent set comparison above. + */ +static u32 +nfs4_ds_addrs_hash(const struct list_head *dsaddrs) +{ + const struct nfs4_pnfs_ds_addr *da; + u32 h = 0; + + list_for_each_entry(da, dsaddrs, da_node) + h += nfs4_ds_addr_hash((const struct sockaddr *)&da->da_addr); + return hash_32(h, NFS4_DS_CACHE_HASH_BITS); +} + /* * Lookup DS by addresses. nfs4_data_server_lock is held */ @@ -610,13 +665,11 @@ static struct nfs4_pnfs_ds * _data_server_lookup_locked(const struct nfs_net *nn, const struct list_head *dsaddrs) { struct nfs4_pnfs_ds *ds; + u32 bucket = nfs4_ds_addrs_hash(dsaddrs); - for (int i = 0; i < NFS4_DS_CACHE_HASH_SIZE; i++) - hlist_for_each_entry(ds, &nn->nfs4_data_server_cache[i], - ds_node) - if (_same_data_server_addrs_locked(&ds->ds_addrs, - dsaddrs)) - return ds; + hlist_for_each_entry(ds, &nn->nfs4_data_server_cache[bucket], ds_node) + if (_same_data_server_addrs_locked(&ds->ds_addrs, dsaddrs)) + return ds; return NULL; } @@ -751,7 +804,9 @@ nfs4_pnfs_ds_add(const struct net *net, struct list_head *dsaddrs, gfp_t gfp_fla INIT_HLIST_NODE(&ds->ds_node); ds->ds_net = net; ds->ds_clp = NULL; - hlist_add_head(&ds->ds_node, &nn->nfs4_data_server_cache[0]); + hlist_add_head(&ds->ds_node, + &nn->nfs4_data_server_cache[ + nfs4_ds_addrs_hash(&ds->ds_addrs)]); dprintk("%s add new data server %s\n", __func__, ds->ds_remotestr); } else { -- 2.53.0