fbarray: fix attach deadlock

Message ID 20190329050951.153202-1-dariusz.stojaczyk@intel.com (mailing list archive)
State Superseded, archived
Headers
Series fbarray: fix attach deadlock |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Stojaczyk, Dariusz March 29, 2019, 5:09 a.m. UTC
  rte_fbarray_attach() currently locks its internal
spinlock, but never releases it. Secondary processes
won't even start if there is more than one fbarray
to be attached to - the second rte_fbarray_attach()
would be just stuck.

Fix it by releasing the lock at the end of
rte_fbarray_attach(). I believe this was the original
intention.

Fixes: 5b61c62cfd76 ("fbarray: add internal tailq for mapped areas")
Cc: anatoly.burakov@intel.com
Cc: thomas@monjalon.net

Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
---
 lib/librte_eal/common/eal_common_fbarray.c | 2 ++
 1 file changed, 2 insertions(+)
  

Comments

Gavin Hu March 29, 2019, 8:53 a.m. UTC | #1
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Darek Stojaczyk
> Sent: Friday, March 29, 2019 1:10 PM
> To: dev@dpdk.org
> Cc: james.r.harris@intel.com; changpeng.liu@intel.com; Darek Stojaczyk
> <dariusz.stojaczyk@intel.com>; anatoly.burakov@intel.com;
> thomas@monjalon.net
> Subject: [dpdk-dev] [PATCH] fbarray: fix attach deadlock
>
> rte_fbarray_attach() currently locks its internal
> spinlock, but never releases it. Secondary processes
> won't even start if there is more than one fbarray
> to be attached to - the second rte_fbarray_attach()
> would be just stuck.
>
> Fix it by releasing the lock at the end of
> rte_fbarray_attach(). I believe this was the original
> intention.
>
> Fixes: 5b61c62cfd76 ("fbarray: add internal tailq for mapped areas")
> Cc: anatoly.burakov@intel.com
> Cc: thomas@monjalon.net
>
> Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
> ---
>  lib/librte_eal/common/eal_common_fbarray.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/lib/librte_eal/common/eal_common_fbarray.c
> b/lib/librte_eal/common/eal_common_fbarray.c
> index 0e7366e5e..5ca8d6f0e 100644
> --- a/lib/librte_eal/common/eal_common_fbarray.c
> +++ b/lib/librte_eal/common/eal_common_fbarray.c
> @@ -906,6 +906,7 @@ rte_fbarray_attach(struct rte_fbarray *arr)
>
>  /* we're done */
>
> +rte_spinlock_unlock(&mem_area_lock);
>  return 0;
>  fail:
>  if (data)
> @@ -913,6 +914,7 @@ rte_fbarray_attach(struct rte_fbarray *arr)
>  if (fd >= 0)
>  close(fd);
>  free(ma);
> +rte_spinlock_unlock(&mem_area_lock);
>  return -1;
>  }

This is an obvious issue, good catch!
Reviewed-by: Gavin Hu <gavin.hu@arm.com>

> --
> 2.17.1

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
  

Patch

diff --git a/lib/librte_eal/common/eal_common_fbarray.c b/lib/librte_eal/common/eal_common_fbarray.c
index 0e7366e5e..5ca8d6f0e 100644
--- a/lib/librte_eal/common/eal_common_fbarray.c
+++ b/lib/librte_eal/common/eal_common_fbarray.c
@@ -906,6 +906,7 @@  rte_fbarray_attach(struct rte_fbarray *arr)
 
 	/* we're done */
 
+	rte_spinlock_unlock(&mem_area_lock);
 	return 0;
 fail:
 	if (data)
@@ -913,6 +914,7 @@  rte_fbarray_attach(struct rte_fbarray *arr)
 	if (fd >= 0)
 		close(fd);
 	free(ma);
+	rte_spinlock_unlock(&mem_area_lock);
 	return -1;
 }