[dpdk-stable] [PATCH v1 1/1] net/hinic: fix coredump when PMD used by fstack

Zhou Guoyang zhouguoyang at huawei.com
Mon Mar 29 13:05:24 CEST 2021


在 2021/3/23 1:17, Ferruh Yigit 写道:
> On 3/15/2021 8:14 AM, Guoyang Zhou wrote:
>> The fstack will use secondary process to access the memory of
>> eth_dev_ops , and it wants to get the info of dev, but hinic
>> driver does not initialized it when in secondary process.
>>
> 
> I guess the issue is not specific to the f-stack, perhaps can generalize the patch title.
> 
>> Fixes: 66f64dd6dc86 ("net/hinic: fix secondary process")
>> Cc: stable at dpdk.org
>> Signed-off-by: Guoyang Zhou <zhouguoyang at huawei.com>
>> ---
>>   drivers/net/hinic/base/hinic_compat.h | 25 ++++++++-----------------
>>   drivers/net/hinic/hinic_pmd_ethdev.c  |  5 +++++
>>   2 files changed, 13 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/net/hinic/base/hinic_compat.h b/drivers/net/hinic/base/hinic_compat.h
>> index 6dd210e..aea3320 100644
>> --- a/drivers/net/hinic/base/hinic_compat.h
>> +++ b/drivers/net/hinic/base/hinic_compat.h
>> @@ -171,6 +171,7 @@ static inline u32 readl(const volatile void *addr)
>>   #else
>>   #define CLOCK_TYPE CLOCK_MONOTONIC
>>   #endif
>> +#define HINIC_MUTEX_TIMEOUT  10
>>     static inline unsigned long clock_gettime_ms(void)
>>   {
>> @@ -225,24 +226,14 @@ static inline int hinic_mutex_destroy(pthread_mutex_t *pthreadmutex)
>>   static inline int hinic_mutex_lock(pthread_mutex_t *pthreadmutex)
>>   {
>>       int err;
>> +    struct timespec tout;
>>   -    err = pthread_mutex_lock(pthreadmutex);
>> -    if (!err) {
>> -        return err;
>> -    } else if (err == EOWNERDEAD) {
>> -        PMD_DRV_LOG(ERR, "Mutex lock failed. (ErrorNo=%d)", errno);
>> -#if defined(__GLIBC__)
>> -#if __GLIBC_PREREQ(2, 12)
>> -        (void)pthread_mutex_consistent(pthreadmutex);
>> -#else
>> -        (void)pthread_mutex_consistent_np(pthreadmutex);
>> -#endif
>> -#else
>> -        (void)pthread_mutex_consistent(pthreadmutex);
>> -#endif
>> -    } else {
>> -        PMD_DRV_LOG(ERR, "Mutex lock failed. (ErrorNo=%d)", errno);
>> -    }
>> +    (void)clock_gettime(CLOCK_TYPE, &tout);
>> +
>> +    tout.tv_sec += HINIC_MUTEX_TIMEOUT;
>> +    err = pthread_mutex_timedlock(pthreadmutex, &tout);
>> +    if (err)
>> +        PMD_DRV_LOG(ERR, "Mutex lock failed. (ErrorNo=%d)", err);
>>   
> 
> Is above change related to the secondary process fix?
> 

Yes, the hinic_mutex_lock function is related to the secondary process fix.
Becasue I found if do not fix it, when a lot of secondary processes calls
secondary ops (dev_infos_get), it will cause deadlock in low probability.

>>       return err;
>>   }
>> diff --git a/drivers/net/hinic/hinic_pmd_ethdev.c b/drivers/net/hinic/hinic_pmd_ethdev.c
>> index 1d6b710..057e7b1 100644
>> --- a/drivers/net/hinic/hinic_pmd_ethdev.c
>> +++ b/drivers/net/hinic/hinic_pmd_ethdev.c
>> @@ -3085,6 +3085,10 @@ static int hinic_dev_close(struct rte_eth_dev *dev)
>>       .filter_ctrl                   = hinic_dev_filter_ctrl,
>>   };
>>   +static const struct eth_dev_ops hinic_dev_sec_ops = {
>> +    .dev_infos_get                 = hinic_dev_infos_get,
>> +};
>> +
>>   static int hinic_func_init(struct rte_eth_dev *eth_dev)
>>   {
>>       struct rte_pci_device *pci_dev;
>> @@ -3099,6 +3103,7 @@ static int hinic_func_init(struct rte_eth_dev *eth_dev)
>>         /* EAL is SECONDARY and eth_dev is already created */
>>       if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
>> +        eth_dev->dev_ops = &hinic_dev_sec_ops;
> 
> Why not using existing dev_ops but creating a new one?
> 
>>           PMD_DRV_LOG(INFO, "Initialize %s in secondary process",
>>                   eth_dev->data->name);
>>  
> 
> .

Because many interfaces do not support calling from the secondary process due to
hardware reason, and from the current test situation, it does not be needed.

Regards,
Guoyang Zhou




More information about the stable mailing list