| Message ID | 20220111192150.379274-2-elder@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: ipa: fix two replenish bugs | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net |
| netdev/fixes_present | success | Fixes tag present in non-next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Series has a cover letter |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/cc_maintainers | success | CCed 4 of 4 maintainers |
| netdev/build_clang | success | Errors and warnings before: 0 this patch: 0 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/verify_fixes | success | Fixes tag looks correct |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 19 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
On Tue, Jan 11, 2022 at 01:21:49PM -0600, Alex Elder wrote: > In ipa_endpoint_replenish(), if an error occurs when attempting to > replenish a receive buffer, we just quit and try again later. In > that case we increment the backlog count to reflect that the attempt > was unsuccessful. Then, if the add_one flag was true we increment > the backlog again. > > This second increment is not included in the backlog local variable > though, and its value determines whether delayed work should be > scheduled. This is a bug. > > Fix this by determining whether 1 or 2 should be added to the > backlog before adding it in a atomic_add_return() call. > > Fixes: 84f9bd12d46db ("soc: qcom: ipa: IPA endpoints") > Signed-off-by: Alex Elder <elder@linaro.org> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
diff --git a/drivers/net/ipa/ipa_endpoint.c b/drivers/net/ipa/ipa_endpoint.c index 49d9a077d0375..8b055885cf3cf 100644 --- a/drivers/net/ipa/ipa_endpoint.c +++ b/drivers/net/ipa/ipa_endpoint.c @@ -1080,6 +1080,7 @@ static void ipa_endpoint_replenish(struct ipa_endpoint *endpoint, bool add_one) { struct gsi *gsi; u32 backlog; + int delta; if (!endpoint->replenish_enabled) { if (add_one) @@ -1097,10 +1098,8 @@ static void ipa_endpoint_replenish(struct ipa_endpoint *endpoint, bool add_one) try_again_later: /* The last one didn't succeed, so fix the backlog */ - backlog = atomic_inc_return(&endpoint->replenish_backlog); - - if (add_one) - atomic_inc(&endpoint->replenish_backlog); + delta = add_one ? 2 : 1; + backlog = atomic_add_return(delta, &endpoint->replenish_backlog); /* Whenever a receive buffer transaction completes we'll try to * replenish again. It's unlikely, but if we fail to supply even
In ipa_endpoint_replenish(), if an error occurs when attempting to replenish a receive buffer, we just quit and try again later. In that case we increment the backlog count to reflect that the attempt was unsuccessful. Then, if the add_one flag was true we increment the backlog again. This second increment is not included in the backlog local variable though, and its value determines whether delayed work should be scheduled. This is a bug. Fix this by determining whether 1 or 2 should be added to the backlog before adding it in a atomic_add_return() call. Fixes: 84f9bd12d46db ("soc: qcom: ipa: IPA endpoints") Signed-off-by: Alex Elder <elder@linaro.org> --- drivers/net/ipa/ipa_endpoint.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)