Skip to content

Commit

Permalink
cifs: change iface_list from array to sorted linked list
Browse files Browse the repository at this point in the history
A server's published interface list can change over time, and needs
to be updated. We've storing iface_list as a simple array, which
makes it difficult to manipulate an existing list.

With this change, iface_list is modified into a linked list of
interfaces, which is kept sorted by speed.

Also added a reference counter for an iface entry, so that each
channel can maintain a backpointer to the iface and drop it
easily when needed.

Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
  • Loading branch information
sprasad-microsoft authored and Steve French committed Jun 23, 2022
1 parent 9de7499 commit aa45dad
Show file tree
Hide file tree
Showing 6 changed files with 201 additions and 129 deletions.
12 changes: 7 additions & 5 deletions fs/cifs/cifs_debug.c
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,8 @@ cifs_dump_iface(struct seq_file *m, struct cifs_server_iface *iface)
seq_printf(m, "\t\tIPv4: %pI4\n", &ipv4->sin_addr);
else if (iface->sockaddr.ss_family == AF_INET6)
seq_printf(m, "\t\tIPv6: %pI6\n", &ipv6->sin6_addr);
if (!iface->is_active)
seq_puts(m, "\t\t[for-cleanup]\n");
}

static int cifs_debug_files_proc_show(struct seq_file *m, void *v)
Expand Down Expand Up @@ -221,6 +223,7 @@ static int cifs_debug_data_proc_show(struct seq_file *m, void *v)
struct TCP_Server_Info *server;
struct cifs_ses *ses;
struct cifs_tcon *tcon;
struct cifs_server_iface *iface;
int c, i, j;

seq_puts(m,
Expand Down Expand Up @@ -456,11 +459,10 @@ static int cifs_debug_data_proc_show(struct seq_file *m, void *v)
if (ses->iface_count)
seq_printf(m, "\n\n\tServer interfaces: %zu",
ses->iface_count);
for (j = 0; j < ses->iface_count; j++) {
struct cifs_server_iface *iface;

iface = &ses->iface_list[j];
seq_printf(m, "\n\t%d)", j+1);
j = 0;
list_for_each_entry(iface, &ses->iface_list,
iface_head) {
seq_printf(m, "\n\t%d)", ++j);
cifs_dump_iface(m, iface);
if (is_ses_using_iface(ses, iface))
seq_puts(m, "\t\t[CONNECTED]\n");
Expand Down
54 changes: 53 additions & 1 deletion fs/cifs/cifsglob.h
Original file line number Diff line number Diff line change
Expand Up @@ -933,15 +933,67 @@ static inline void cifs_set_net_ns(struct TCP_Server_Info *srv, struct net *net)
#endif

struct cifs_server_iface {
struct list_head iface_head;
struct kref refcount;
size_t speed;
unsigned int rdma_capable : 1;
unsigned int rss_capable : 1;
unsigned int is_active : 1; /* unset if non existent */
struct sockaddr_storage sockaddr;
};

/* release iface when last ref is dropped */
static inline void
release_iface(struct kref *ref)
{
struct cifs_server_iface *iface = container_of(ref,
struct cifs_server_iface,
refcount);
list_del_init(&iface->iface_head);
kfree(iface);
}

/*
* compare two interfaces a and b
* return 0 if everything matches.
* return 1 if a has higher link speed, or rdma capable, or rss capable
* return -1 otherwise.
*/
static inline int
iface_cmp(struct cifs_server_iface *a, struct cifs_server_iface *b)
{
int cmp_ret = 0;

WARN_ON(!a || !b);
if (a->speed == b->speed) {
if (a->rdma_capable == b->rdma_capable) {
if (a->rss_capable == b->rss_capable) {
cmp_ret = memcmp(&a->sockaddr, &b->sockaddr,
sizeof(a->sockaddr));
if (!cmp_ret)
return 0;
else if (cmp_ret > 0)
return 1;
else
return -1;
} else if (a->rss_capable > b->rss_capable)
return 1;
else
return -1;
} else if (a->rdma_capable > b->rdma_capable)
return 1;
else
return -1;
} else if (a->speed > b->speed)
return 1;
else
return -1;
}

struct cifs_chan {
unsigned int in_reconnect : 1; /* if session setup in progress for this channel */
struct TCP_Server_Info *server;
struct cifs_server_iface *iface; /* interface in use */
__u8 signkey[SMB3_SIGN_KEY_SIZE];
};

Expand Down Expand Up @@ -993,7 +1045,7 @@ struct cifs_ses {
*/
spinlock_t iface_lock;
/* ========= begin: protected by iface_lock ======== */
struct cifs_server_iface *iface_list;
struct list_head iface_list;
size_t iface_count;
unsigned long iface_last_update; /* jiffies */
/* ========= end: protected by iface_lock ======== */
Expand Down
6 changes: 4 additions & 2 deletions fs/cifs/connect.c
Original file line number Diff line number Diff line change
Expand Up @@ -1894,9 +1894,11 @@ void cifs_put_smb_ses(struct cifs_ses *ses)
int i;

for (i = 1; i < chan_count; i++) {
spin_unlock(&ses->chan_lock);
if (ses->chans[i].iface) {
kref_put(&ses->chans[i].iface->refcount, release_iface);
ses->chans[i].iface = NULL;
}
cifs_put_tcp_session(ses->chans[i].server, 0);
spin_lock(&ses->chan_lock);
ses->chans[i].server = NULL;
}
}
Expand Down
9 changes: 8 additions & 1 deletion fs/cifs/misc.c
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ sesInfoAlloc(void)
INIT_LIST_HEAD(&ret_buf->tcon_list);
mutex_init(&ret_buf->session_mutex);
spin_lock_init(&ret_buf->iface_lock);
INIT_LIST_HEAD(&ret_buf->iface_list);
spin_lock_init(&ret_buf->chan_lock);
}
return ret_buf;
Expand All @@ -83,6 +84,8 @@ sesInfoAlloc(void)
void
sesInfoFree(struct cifs_ses *buf_to_free)
{
struct cifs_server_iface *iface = NULL, *niface = NULL;

if (buf_to_free == NULL) {
cifs_dbg(FYI, "Null buffer passed to sesInfoFree\n");
return;
Expand All @@ -96,7 +99,11 @@ sesInfoFree(struct cifs_ses *buf_to_free)
kfree(buf_to_free->user_name);
kfree(buf_to_free->domainName);
kfree_sensitive(buf_to_free->auth_key.response);
kfree(buf_to_free->iface_list);
spin_lock(&buf_to_free->iface_lock);
list_for_each_entry_safe(iface, niface, &buf_to_free->iface_list,
iface_head)
kref_put(&iface->refcount, release_iface);
spin_unlock(&buf_to_free->iface_lock);
kfree_sensitive(buf_to_free);
}

Expand Down
78 changes: 40 additions & 38 deletions fs/cifs/sess.c
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ bool is_ses_using_iface(struct cifs_ses *ses, struct cifs_server_iface *iface)

spin_lock(&ses->chan_lock);
for (i = 0; i < ses->chan_count; i++) {
if (is_server_using_iface(ses->chans[i].server, iface)) {
if (ses->chans[i].iface == iface) {
spin_unlock(&ses->chan_lock);
return true;
}
Expand Down Expand Up @@ -151,11 +151,9 @@ int cifs_try_adding_channels(struct cifs_sb_info *cifs_sb, struct cifs_ses *ses)
{
int old_chan_count, new_chan_count;
int left;
int i = 0;
int rc = 0;
int tries = 0;
struct cifs_server_iface *ifaces = NULL;
size_t iface_count;
struct cifs_server_iface *iface = NULL, *niface = NULL;

spin_lock(&ses->chan_lock);

Expand Down Expand Up @@ -184,33 +182,17 @@ int cifs_try_adding_channels(struct cifs_sb_info *cifs_sb, struct cifs_ses *ses)
}
spin_unlock(&ses->chan_lock);

/*
* Make a copy of the iface list at the time and use that
* instead so as to not hold the iface spinlock for opening
* channels
*/
spin_lock(&ses->iface_lock);
iface_count = ses->iface_count;
if (iface_count <= 0) {
spin_unlock(&ses->iface_lock);
cifs_dbg(VFS, "no iface list available to open channels\n");
return 0;
}
ifaces = kmemdup(ses->iface_list, iface_count*sizeof(*ifaces),
GFP_ATOMIC);
if (!ifaces) {
spin_unlock(&ses->iface_lock);
return 0;
}
spin_unlock(&ses->iface_lock);

/*
* Keep connecting to same, fastest, iface for all channels as
* long as its RSS. Try next fastest one if not RSS or channel
* creation fails.
*/
spin_lock(&ses->iface_lock);
iface = list_first_entry(&ses->iface_list, struct cifs_server_iface,
iface_head);
spin_unlock(&ses->iface_lock);

while (left > 0) {
struct cifs_server_iface *iface;

tries++;
if (tries > 3*ses->chan_max) {
Expand All @@ -219,27 +201,46 @@ int cifs_try_adding_channels(struct cifs_sb_info *cifs_sb, struct cifs_ses *ses)
break;
}

iface = &ifaces[i];
if (is_ses_using_iface(ses, iface) && !iface->rss_capable) {
i = (i+1) % iface_count;
continue;
spin_lock(&ses->iface_lock);
if (!ses->iface_count) {
spin_unlock(&ses->iface_lock);
break;
}

rc = cifs_ses_add_channel(cifs_sb, ses, iface);
if (rc) {
cifs_dbg(FYI, "failed to open extra channel on iface#%d rc=%d\n",
i, rc);
i = (i+1) % iface_count;
continue;
list_for_each_entry_safe_from(iface, niface, &ses->iface_list,
iface_head) {
/* skip ifaces that are unusable */
if (!iface->is_active ||
(is_ses_using_iface(ses, iface) &&
!iface->rss_capable)) {
continue;
}

/* take ref before unlock */
kref_get(&iface->refcount);

spin_unlock(&ses->iface_lock);
rc = cifs_ses_add_channel(cifs_sb, ses, iface);
spin_lock(&ses->iface_lock);

if (rc) {
cifs_dbg(VFS, "failed to open extra channel on iface:%pIS rc=%d\n",
&iface->sockaddr,
rc);
kref_put(&iface->refcount, release_iface);
continue;
}

cifs_dbg(FYI, "successfully opened new channel on iface:%pIS\n",
&iface->sockaddr);
break;
}
spin_unlock(&ses->iface_lock);

cifs_dbg(FYI, "successfully opened new channel on iface#%d\n",
i);
left--;
new_chan_count++;
}

kfree(ifaces);
return new_chan_count - old_chan_count;
}

Expand Down Expand Up @@ -355,6 +356,7 @@ cifs_ses_add_channel(struct cifs_sb_info *cifs_sb, struct cifs_ses *ses,
spin_unlock(&ses->chan_lock);
goto out;
}
chan->iface = iface;
ses->chan_count++;
atomic_set(&ses->chan_seq, 0);

Expand Down
Loading

0 comments on commit aa45dad

Please sign in to comment.