|
| 1 | +cifs: deal with the channel loading lag while picking channels |
| 2 | + |
| 3 | +jira LE-4669 |
| 4 | +Rebuild_History Non-Buildable kernel-4.18.0-553.82.1.el8_10 |
| 5 | +commit-author Shyam Prasad N <sprasad@microsoft.com> |
| 6 | +commit 66d590b828b1fd9fa337047ae58fe1c4c6f43609 |
| 7 | +Empty-Commit: Cherry-Pick Conflicts during history rebuild. |
| 8 | +Will be included in final tarball splat. Ref for failed cherry-pick at: |
| 9 | +ciq/ciq_backports/kernel-4.18.0-553.82.1.el8_10/66d590b8.failed |
| 10 | + |
| 11 | +Our current approach to select a channel for sending requests is this: |
| 12 | +1. iterate all channels to find the min and max queue depth |
| 13 | +2. if min and max are not the same, pick the channel with min depth |
| 14 | +3. if min and max are same, round robin, as all channels are equally loaded |
| 15 | + |
| 16 | +The problem with this approach is that there's a lag between selecting |
| 17 | +a channel and sending the request (that increases the queue depth on the channel). |
| 18 | +While these numbers will eventually catch up, there could be a skew in the |
| 19 | +channel usage, depending on the application's I/O parallelism and the server's |
| 20 | +speed of handling requests. |
| 21 | + |
| 22 | +With sufficient parallelism, this lag can artificially increase the queue depth, |
| 23 | +thereby impacting the performance negatively. |
| 24 | + |
| 25 | +This change will change the step 1 above to start the iteration from the last |
| 26 | +selected channel. This is to reduce the skew in channel usage even in the presence |
| 27 | +of this lag. |
| 28 | + |
| 29 | +Fixes: ea90708d3cf3 ("cifs: use the least loaded channel for sending requests") |
| 30 | + Cc: <stable@vger.kernel.org> |
| 31 | + Signed-off-by: Shyam Prasad N <sprasad@microsoft.com> |
| 32 | + Signed-off-by: Steve French <stfrench@microsoft.com> |
| 33 | +(cherry picked from commit 66d590b828b1fd9fa337047ae58fe1c4c6f43609) |
| 34 | + Signed-off-by: Jonathan Maple <jmaple@ciq.com> |
| 35 | + |
| 36 | +# Conflicts: |
| 37 | +# fs/cifs/transport.c |
| 38 | +diff --cc fs/cifs/transport.c |
| 39 | +index 1bfe2ba50372,191783f553ce..000000000000 |
| 40 | +--- a/fs/cifs/transport.c |
| 41 | ++++ b/fs/cifs/transport.c |
| 42 | +@@@ -1043,21 -1016,43 +1043,59 @@@ cifs_cancelled_callback(struct mid_q_en |
| 43 | + struct TCP_Server_Info *cifs_pick_channel(struct cifs_ses *ses) |
| 44 | + { |
| 45 | + uint index = 0; |
| 46 | +++<<<<<<< HEAD:fs/cifs/transport.c |
| 47 | +++======= |
| 48 | ++ unsigned int min_in_flight = UINT_MAX, max_in_flight = 0; |
| 49 | ++ struct TCP_Server_Info *server = NULL; |
| 50 | ++ int i, start, cur; |
| 51 | +++>>>>>>> 66d590b828b1 (cifs: deal with the channel loading lag while picking channels):fs/smb/client/transport.c |
| 52 | + |
| 53 | + if (!ses) |
| 54 | + return NULL; |
| 55 | + |
| 56 | + spin_lock(&ses->chan_lock); |
| 57 | +++<<<<<<< HEAD:fs/cifs/transport.c |
| 58 | + + /* round robin */ |
| 59 | + +pick_another: |
| 60 | + + if (ses->chan_count > 1 && |
| 61 | + + !CIFS_ALL_CHANS_NEED_RECONNECT(ses)) { |
| 62 | + + index = (uint)atomic_inc_return(&ses->chan_seq); |
| 63 | + + index %= ses->chan_count; |
| 64 | + + |
| 65 | + + if (CIFS_CHAN_NEEDS_RECONNECT(ses, index)) |
| 66 | + + goto pick_another; |
| 67 | + + } |
| 68 | +++======= |
| 69 | ++ start = atomic_inc_return(&ses->chan_seq); |
| 70 | ++ for (i = 0; i < ses->chan_count; i++) { |
| 71 | ++ cur = (start + i) % ses->chan_count; |
| 72 | ++ server = ses->chans[cur].server; |
| 73 | ++ if (!server || server->terminate) |
| 74 | ++ continue; |
| 75 | ++ |
| 76 | ++ if (CIFS_CHAN_NEEDS_RECONNECT(ses, i)) |
| 77 | ++ continue; |
| 78 | ++ |
| 79 | ++ /* |
| 80 | ++ * strictly speaking, we should pick up req_lock to read |
| 81 | ++ * server->in_flight. But it shouldn't matter much here if we |
| 82 | ++ * race while reading this data. The worst that can happen is |
| 83 | ++ * that we could use a channel that's not least loaded. Avoiding |
| 84 | ++ * taking the lock could help reduce wait time, which is |
| 85 | ++ * important for this function |
| 86 | ++ */ |
| 87 | ++ if (server->in_flight < min_in_flight) { |
| 88 | ++ min_in_flight = server->in_flight; |
| 89 | ++ index = cur; |
| 90 | ++ } |
| 91 | ++ if (server->in_flight > max_in_flight) |
| 92 | ++ max_in_flight = server->in_flight; |
| 93 | ++ } |
| 94 | ++ |
| 95 | ++ /* if all channels are equally loaded, fall back to round-robin */ |
| 96 | ++ if (min_in_flight == max_in_flight) |
| 97 | ++ index = (uint)start % ses->chan_count; |
| 98 | +++>>>>>>> 66d590b828b1 (cifs: deal with the channel loading lag while picking channels):fs/smb/client/transport.c |
| 99 | + |
| 100 | + server = ses->chans[index].server; |
| 101 | + spin_unlock(&ses->chan_lock); |
| 102 | +* Unmerged path fs/cifs/transport.c |
0 commit comments