github-mirror/linux
1
0
Fork 0
mirror of https://github.com/torvalds/linux.git synced 2026-11-07 06:17:02 +01:00
Code Issues Actions Packages Projects Releases Wiki Activity Graph

NFSv4/pnfs: Key the data-server cache by its address set and version

Browse Source 
Populating the data-server cache was quadratic: every GETDEVICEINFO
decode scanned the whole per-net cache under one spinlock.  At the
anticipated scale of 1024 data servers a striping mount pays that in a
burst at first access, and again on notification-driven re-resolution.

Hash each DS to a bucket keyed on the whole cache key -- per-address
jhash over exactly the fields same_sockaddr() compares, combined by
addition so multipath ordering cannot change the bucket, then the NFS
version folded in -- with the existing comparator as the in-bucket
tiebreaker.  Lookup and insert now touch one bucket; teardown is
unchanged (hlist_del_init needs no bucket).

Keying on the whole set requires the comparator to test set equality,
so it is tightened from the subset test it did before -- a subset
match would hash to a different bucket and simply never be found.
That test was also wrong in a way worth naming.  It answered "is
dsaddrs1 a subset of dsaddrs2", and the caller passes the cached list
first, so a cached data server whose address set was contained in an
incoming one was returned for that incoming set.  The aliasing was
therefore one-directional: cache {A,B} first and an incoming {A} did
not match, but cache {A} first and an incoming {A,B} did.

The consequence was mild, which is why it went unnoticed: every
address on one device's multipath list names the same data server, so
a merged entry's addresses are all paths that server also advertised.
The effect is lost path diversity and a truncated ds_remotestr (and
the netaddr flexfiles reports in layoutstats), not I/O sent to the
wrong server.

Assisted-by: Claude:claude-fable-5
Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
Signed-off-by: Anna Schumaker <anna.schumaker@hammerspace.com>
This commit is contained in:
Benjamin Coddington 2026-09-15 08:22:25 -04:00 committed by Anna Schumaker
parent 630295dfc6
commit bcb2579c9d
1 changed files with 58 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

68
fs/nfs/pnfs_nfs.c
View File

@ -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,8 @@ 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' hold the same set of addresses.
* If they do, declare a match.
*/
static bool
_same_data_server_addrs_locked(const struct list_head *dsaddrs1,
@ -588,6 +590,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 +609,47 @@ _same_data_server_addrs_locked(const struct list_head *dsaddrs1,
return match;
}
/* Hash family, address bytes, and port - as same_sockaddr() */
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 cache key. 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_cache_hash(const struct list_head *dsaddrs, u32 version)
{
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(jhash_1word(version, h), NFS4_DS_CACHE_HASH_BITS);
}
/*
* Lookup DS by addresses and NFS version. nfs4_data_server_lock is held
*/
@ -611,14 +658,12 @@ _data_server_lookup_locked(const struct nfs_net *nn,
const struct list_head *dsaddrs, u32 version)
{
struct nfs4_pnfs_ds *ds;
u32 bucket = nfs4_ds_cache_hash(dsaddrs, version);
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 (ds->ds_version == version &&
_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 (ds->ds_version == version &&
_same_data_server_addrs_locked(&ds->ds_addrs, dsaddrs))
return ds;
return NULL;
}
@ -735,6 +780,7 @@ nfs4_pnfs_ds_add(const struct net *net, struct list_head *dsaddrs, u32 version,
{
struct nfs_net *nn = net_generic(net, nfs_net_id);
struct nfs4_pnfs_ds *tmp_ds, *ds = NULL;
struct hlist_head *bucket;
char *remotestr;
if (list_empty(dsaddrs)) {
@ -748,6 +794,8 @@ nfs4_pnfs_ds_add(const struct net *net, struct list_head *dsaddrs, u32 version,
/* this is only used for debugging, so it's ok if its NULL */
remotestr = nfs4_pnfs_remotestr(dsaddrs, gfp_flags);
/* @dsaddrs is empty after the splice below. */
bucket = &nn->nfs4_data_server_cache[nfs4_ds_cache_hash(dsaddrs, version)];
spin_lock(&nn->nfs4_data_server_lock);
tmp_ds = _data_server_lookup_locked(nn, dsaddrs, version);
@ -760,7 +808,7 @@ nfs4_pnfs_ds_add(const struct net *net, struct list_head *dsaddrs, u32 version,
ds->ds_net = net;
ds->ds_clp = NULL;
ds->ds_version = version;
hlist_add_head(&ds->ds_node, &nn->nfs4_data_server_cache[0]);
hlist_add_head(&ds->ds_node, bucket);
dprintk("%s add new data server %s\n", __func__,
ds->ds_remotestr);
} else {