DPDK patches and discussions
 help / color / mirror / Atom feed
* Re: [dpdk-dev] [PATCH v1 1/1] net/hinic: use mutex replace spin lock
@ 2019-07-09 12:14 Xuanziyang (William, Chip Application Design Logic and Hardware Development Dept IT_Products & Solutions)
  0 siblings, 0 replies; 3+ messages in thread
From: Xuanziyang (William, Chip Application Design Logic and Hardware Development Dept IT_Products & Solutions) @ 2019-07-09 12:14 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: dev, ferruh.yigit, Wangxiaoyun (Cloud,
	Network Chip Application Development Dept),
	Luoxianjun, Tanya Brokhman

> On Wed, 3 Jul 2019 23:35:42 +0800
> Ziyang Xuan <xuanziyang2@huawei.com> wrote:
> 
> >
> > +static inline int hinic_mutex_init(pthread_mutex_t *pthreadmutex,
> > +					const pthread_mutexattr_t *mattr) {
> > +	int err;
> > +
> > +	err = pthread_mutex_init(pthreadmutex, mattr);
> > +	if (unlikely(err))
> > +		PMD_DRV_LOG(ERR, "Fail to initialize mutex, error: %d", err);
> > +
> > +	return err;
> > +}
> > +
> > +static inline int hinic_mutex_destroy(pthread_mutex_t *pthreadmutex)
> > +{
> > +	int err;
> > +
> > +	err = pthread_mutex_destroy(pthreadmutex);
> > +	if (unlikely(err))
> > +		PMD_DRV_LOG(ERR, "Fail to destroy mutex, error: %d", err);
> > +
> > +	return err;
> > +}
> > +
> 
> I don't think the wrapper functions add much.
> pthread_mutex_init just sets internals of data structure and won't fail ever if
> mutexattr_t is NULL.
> 
> Just use pthread_mutex_init/pthread_mutex_destroy directly and ignore
> errors.

I think pthread_mutex_init/pthread_mutex_destroy may fail under some situations.
https://pubs.opengroup.org/onlinepubs/7908799/xsh/pthread_mutex_init.html

I want some logs when they failed to position problems conveniently and I would also
detect their failures. So I encapsulate the functions. And I think it is better.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [dpdk-dev] [PATCH v1 1/1] net/hinic: use mutex replace spin lock
  2019-07-03 15:35 Ziyang Xuan
@ 2019-07-03 15:37 ` Stephen Hemminger
  0 siblings, 0 replies; 3+ messages in thread
From: Stephen Hemminger @ 2019-07-03 15:37 UTC (permalink / raw)
  To: Ziyang Xuan
  Cc: dev, ferruh.yigit, cloud.wangxiaoyun, luoxianjun, tanya.brokhman

On Wed, 3 Jul 2019 23:35:42 +0800
Ziyang Xuan <xuanziyang2@huawei.com> wrote:

>  
> +static inline int hinic_mutex_init(pthread_mutex_t *pthreadmutex,
> +					const pthread_mutexattr_t *mattr)
> +{
> +	int err;
> +
> +	err = pthread_mutex_init(pthreadmutex, mattr);
> +	if (unlikely(err))
> +		PMD_DRV_LOG(ERR, "Fail to initialize mutex, error: %d", err);
> +
> +	return err;
> +}
> +
> +static inline int hinic_mutex_destroy(pthread_mutex_t *pthreadmutex)
> +{
> +	int err;
> +
> +	err = pthread_mutex_destroy(pthreadmutex);
> +	if (unlikely(err))
> +		PMD_DRV_LOG(ERR, "Fail to destroy mutex, error: %d", err);
> +
> +	return err;
> +}
> +

I don't think the wrapper functions add much. 
pthread_mutex_init just sets internals of data structure and won't fail ever
if mutexattr_t is NULL.

Just use pthread_mutex_init/pthread_mutex_destroy directly
and ignore errors.


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [dpdk-dev] [PATCH v1 1/1] net/hinic: use mutex replace spin lock
@ 2019-07-03 15:35 Ziyang Xuan
  2019-07-03 15:37 ` Stephen Hemminger
  0 siblings, 1 reply; 3+ messages in thread
From: Ziyang Xuan @ 2019-07-03 15:35 UTC (permalink / raw)
  To: dev
  Cc: ferruh.yigit, cloud.wangxiaoyun, luoxianjun, tanya.brokhman, Ziyang Xuan

Using spin lock to protect critical resources
of sending mgmt messages. This will make high
CPU usage when sending mgmt messages frequently.
We can use mutex to protect the critical resources
to reduce CPU usage while keep functioning properly.

Signed-off-by: Ziyang Xuan <xuanziyang2@huawei.com>
---
 drivers/net/hinic/Makefile              |  1 +
 drivers/net/hinic/base/hinic_compat.h   | 24 ++++++++++++++++++++++++
 drivers/net/hinic/base/hinic_pmd_mgmt.c | 17 +++++++++++------
 drivers/net/hinic/base/hinic_pmd_mgmt.h |  6 ++----
 4 files changed, 38 insertions(+), 10 deletions(-)

diff --git a/drivers/net/hinic/Makefile b/drivers/net/hinic/Makefile
index 123a626..42b4a78 100644
--- a/drivers/net/hinic/Makefile
+++ b/drivers/net/hinic/Makefile
@@ -20,6 +20,7 @@ endif
 LDLIBS += -lrte_eal -lrte_mbuf -lrte_mempool -lrte_ring
 LDLIBS += -lrte_ethdev -lrte_net -lrte_hash
 LDLIBS += -lrte_bus_pci
+LDLIBS += -lpthread
 
 EXPORT_MAP := rte_pmd_hinic_version.map
 
diff --git a/drivers/net/hinic/base/hinic_compat.h b/drivers/net/hinic/base/hinic_compat.h
index 48643c8..3670cfd 100644
--- a/drivers/net/hinic/base/hinic_compat.h
+++ b/drivers/net/hinic/base/hinic_compat.h
@@ -7,6 +7,7 @@
 
 #include <stdint.h>
 #include <sys/time.h>
+#include <pthread.h>
 #include <rte_common.h>
 #include <rte_byteorder.h>
 #include <rte_memzone.h>
@@ -253,4 +254,27 @@ static inline void hinic_be32_to_cpu(void *data, u32 len)
 	}
 }
 
+static inline int hinic_mutex_init(pthread_mutex_t *pthreadmutex,
+					const pthread_mutexattr_t *mattr)
+{
+	int err;
+
+	err = pthread_mutex_init(pthreadmutex, mattr);
+	if (unlikely(err))
+		PMD_DRV_LOG(ERR, "Fail to initialize mutex, error: %d", err);
+
+	return err;
+}
+
+static inline int hinic_mutex_destroy(pthread_mutex_t *pthreadmutex)
+{
+	int err;
+
+	err = pthread_mutex_destroy(pthreadmutex);
+	if (unlikely(err))
+		PMD_DRV_LOG(ERR, "Fail to destroy mutex, error: %d", err);
+
+	return err;
+}
+
 #endif /* _HINIC_COMPAT_H_ */
diff --git a/drivers/net/hinic/base/hinic_pmd_mgmt.c b/drivers/net/hinic/base/hinic_pmd_mgmt.c
index bc18765..6a659f8 100644
--- a/drivers/net/hinic/base/hinic_pmd_mgmt.c
+++ b/drivers/net/hinic/base/hinic_pmd_mgmt.c
@@ -342,8 +342,9 @@ static int hinic_pf_to_mgmt_init(struct hinic_hwdev *hwdev)
 	hwdev->pf_to_mgmt = pf_to_mgmt;
 	pf_to_mgmt->hwdev = hwdev;
 
-	spin_lock_init(&pf_to_mgmt->async_msg_lock);
-	spin_lock_init(&pf_to_mgmt->sync_msg_lock);
+	err = hinic_mutex_init(&pf_to_mgmt->sync_msg_lock, NULL);
+	if (err)
+		goto mutex_init_err;
 
 	err = alloc_msg_buf(pf_to_mgmt);
 	if (err) {
@@ -363,6 +364,9 @@ static int hinic_pf_to_mgmt_init(struct hinic_hwdev *hwdev)
 	free_msg_buf(pf_to_mgmt);
 
 alloc_msg_buf_err:
+	hinic_mutex_destroy(&pf_to_mgmt->sync_msg_lock);
+
+mutex_init_err:
 	kfree(pf_to_mgmt);
 
 	return err;
@@ -378,6 +382,7 @@ static void hinic_pf_to_mgmt_free(struct hinic_hwdev *hwdev)
 
 	hinic_api_cmd_free(pf_to_mgmt->cmd_chain);
 	free_msg_buf(pf_to_mgmt);
+	hinic_mutex_destroy(&pf_to_mgmt->sync_msg_lock);
 	kfree(pf_to_mgmt);
 }
 
@@ -391,7 +396,7 @@ static void hinic_pf_to_mgmt_free(struct hinic_hwdev *hwdev)
 	u32 timeo;
 	int err, i;
 
-	spin_lock(&pf_to_mgmt->sync_msg_lock);
+	pthread_mutex_lock(&pf_to_mgmt->sync_msg_lock);
 
 	SYNC_MSG_ID_INC(pf_to_mgmt);
 	recv_msg = &pf_to_mgmt->recv_resp_msg_from_mgmt;
@@ -450,7 +455,7 @@ static void hinic_pf_to_mgmt_free(struct hinic_hwdev *hwdev)
 unlock_sync_msg:
 	if (err && out_size)
 		*out_size = 0;
-	spin_unlock(&pf_to_mgmt->sync_msg_lock);
+	pthread_mutex_unlock(&pf_to_mgmt->sync_msg_lock);
 	return err;
 }
 
@@ -497,13 +502,13 @@ int hinic_msg_to_mgmt_no_ack(void *hwdev, enum hinic_mod_type mod, u8 cmd,
 		return err;
 	}
 
-	spin_lock(&pf_to_mgmt->sync_msg_lock);
+	pthread_mutex_lock(&pf_to_mgmt->sync_msg_lock);
 
 	err = send_msg_to_mgmt_sync(pf_to_mgmt, mod, cmd, buf_in, in_size,
 				    HINIC_MSG_NO_ACK, HINIC_MSG_DIRECT_SEND,
 				    MSG_NO_RESP);
 
-	spin_unlock(&pf_to_mgmt->sync_msg_lock);
+	pthread_mutex_unlock(&pf_to_mgmt->sync_msg_lock);
 
 	return err;
 }
diff --git a/drivers/net/hinic/base/hinic_pmd_mgmt.h b/drivers/net/hinic/base/hinic_pmd_mgmt.h
index 23951cb..7804708 100644
--- a/drivers/net/hinic/base/hinic_pmd_mgmt.h
+++ b/drivers/net/hinic/base/hinic_pmd_mgmt.h
@@ -81,10 +81,8 @@ enum comm_pf_to_mgmt_event_state {
 struct hinic_msg_pf_to_mgmt {
 	struct hinic_hwdev		*hwdev;
 
-	/* Async cmd can not be scheduling */
-	spinlock_t			async_msg_lock;
-	/* spinlock for sync message */
-	spinlock_t			sync_msg_lock;
+	/* mutex for sync message */
+	pthread_mutex_t			sync_msg_lock;
 
 	void				*async_msg_buf;
 	void				*sync_msg_buf;
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-07-09 12:14 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-09 12:14 [dpdk-dev] [PATCH v1 1/1] net/hinic: use mutex replace spin lock Xuanziyang (William, Chip Application Design Logic and Hardware Development Dept IT_Products & Solutions)
  -- strict thread matches above, loose matches on Subject: below --
2019-07-03 15:35 Ziyang Xuan
2019-07-03 15:37 ` Stephen Hemminger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).