DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] net/virtio-user: fix hugepage files enumeration
@ 2018-04-26 15:34 Jianfeng Tan
  2018-04-27  9:31 ` Burakov, Anatoly
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Jianfeng Tan @ 2018-04-26 15:34 UTC (permalink / raw)
  To: dev; +Cc: maxime.coquelin, tiwei.bie, zhiyong.yang, Jianfeng Tan

After the commit 2a04139f66b4 ("eal: add single file segments option"),
one hugepage file could contain multiple hugepages which are further
mapped to different memory regions.

Original enumeration implementation cannot handle this situation.

This patch filters out the duplicated files; and adjust the size after
the enumeration.

Fixes: 6a84c37e3975 ("net/virtio-user: add vhost-user adapter layer")

Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
 .../howto/virtio_user_for_container_networking.rst |  3 ++-
 drivers/net/virtio/virtio_user/vhost_user.c        | 28 ++++++++++++++++++++--
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/doc/guides/howto/virtio_user_for_container_networking.rst b/doc/guides/howto/virtio_user_for_container_networking.rst
index aa68b53..476ce3a 100644
--- a/doc/guides/howto/virtio_user_for_container_networking.rst
+++ b/doc/guides/howto/virtio_user_for_container_networking.rst
@@ -109,7 +109,8 @@ We have below limitations in this solution:
  * Cannot work with --no-huge option. Currently, DPDK uses anonymous mapping
    under this option which cannot be reopened to share with vhost backend.
  * Cannot work when there are more than VHOST_MEMORY_MAX_NREGIONS(8) hugepages.
-   In another word, do not use 2MB hugepage so far.
+   If you have more regions (especially when 2MB hugepages are used), the option,
+   --single-file-segments, can help to reduce the number of shared files.
  * Applications should not use file name like HUGEFILE_FMT ("%smap_%d"). That
    will bring confusion when sharing hugepage files with backend by name.
  * Root privilege is a must. DPDK resolves physical addresses of hugepages
diff --git a/drivers/net/virtio/virtio_user/vhost_user.c b/drivers/net/virtio/virtio_user/vhost_user.c
index a6df97a..01201c9 100644
--- a/drivers/net/virtio/virtio_user/vhost_user.c
+++ b/drivers/net/virtio/virtio_user/vhost_user.c
@@ -138,12 +138,13 @@ struct hugepage_file_info {
 static int
 get_hugepage_file_info(struct hugepage_file_info huges[], int max)
 {
-	int idx;
+	int idx, k, exist;
 	FILE *f;
 	char buf[BUFSIZ], *tmp, *tail;
 	char *str_underline, *str_start;
 	int huge_index;
 	uint64_t v_start, v_end;
+	struct stat stats;
 
 	f = fopen("/proc/self/maps", "r");
 	if (!f) {
@@ -183,16 +184,39 @@ get_hugepage_file_info(struct hugepage_file_info huges[], int max)
 		if (sscanf(str_start, "map_%d", &huge_index) != 1)
 			continue;
 
+		/* skip duplicated file which is mapped to different regions */
+		for (k = 0, exist = -1; k < idx; ++k) {
+			if (!strcmp(huges[k].path, tmp)) {
+				exist = k;
+				break;
+			}
+		}
+		if (exist >= 0)
+			continue;
+
 		if (idx >= max) {
 			PMD_DRV_LOG(ERR, "Exceed maximum of %d", max);
 			goto error;
 		}
+
 		huges[idx].addr = v_start;
-		huges[idx].size = v_end - v_start;
+		huges[idx].size = v_end - v_start; /* To be corrected later */
 		snprintf(huges[idx].path, PATH_MAX, "%s", tmp);
 		idx++;
 	}
 
+	/* correct the size for files who have many regions */
+	for (k = 0; k < idx; ++k) {
+		if (stat(huges[k].path, &stats) < 0) {
+			PMD_DRV_LOG(ERR, "Failed to stat %s, %s\n",
+				    huges[k].path, strerror(errno));
+			continue;
+		}
+		huges[k].size = stats.st_size;
+		PMD_DRV_LOG(INFO, "file %s, size %"PRIx64"\n",
+			    huges[k].path, huges[k].size);
+	}
+
 	fclose(f);
 	return idx;
 
-- 
2.7.4

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

* Re: [dpdk-dev] [PATCH] net/virtio-user: fix hugepage files enumeration
  2018-04-26 15:34 [dpdk-dev] [PATCH] net/virtio-user: fix hugepage files enumeration Jianfeng Tan
@ 2018-04-27  9:31 ` Burakov, Anatoly
  2018-05-04  7:40   ` Maxime Coquelin
  2018-05-04 14:16 ` Maxime Coquelin
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 7+ messages in thread
From: Burakov, Anatoly @ 2018-04-27  9:31 UTC (permalink / raw)
  To: Jianfeng Tan, dev; +Cc: maxime.coquelin, tiwei.bie, zhiyong.yang

On 26-Apr-18 4:34 PM, Jianfeng Tan wrote:
> After the commit 2a04139f66b4 ("eal: add single file segments option"),
> one hugepage file could contain multiple hugepages which are further
> mapped to different memory regions.
> 
> Original enumeration implementation cannot handle this situation.
> 
> This patch filters out the duplicated files; and adjust the size after
> the enumeration.
> 
> Fixes: 6a84c37e3975 ("net/virtio-user: add vhost-user adapter layer")
> 
> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> ---
>   .../howto/virtio_user_for_container_networking.rst |  3 ++-
>   drivers/net/virtio/virtio_user/vhost_user.c        | 28 ++++++++++++++++++++--
>   2 files changed, 28 insertions(+), 3 deletions(-)
> 
> diff --git a/doc/guides/howto/virtio_user_for_container_networking.rst b/doc/guides/howto/virtio_user_for_container_networking.rst
> index aa68b53..476ce3a 100644
> --- a/doc/guides/howto/virtio_user_for_container_networking.rst
> +++ b/doc/guides/howto/virtio_user_for_container_networking.rst
> @@ -109,7 +109,8 @@ We have below limitations in this solution:
>    * Cannot work with --no-huge option. Currently, DPDK uses anonymous mapping
>      under this option which cannot be reopened to share with vhost backend.
>    * Cannot work when there are more than VHOST_MEMORY_MAX_NREGIONS(8) hugepages.
> -   In another word, do not use 2MB hugepage so far.
> +   If you have more regions (especially when 2MB hugepages are used), the option,
> +   --single-file-segments, can help to reduce the number of shared files.
>    * Applications should not use file name like HUGEFILE_FMT ("%smap_%d"). That
>      will bring confusion when sharing hugepage files with backend by name.
>    * Root privilege is a must. DPDK resolves physical addresses of hugepages
> diff --git a/drivers/net/virtio/virtio_user/vhost_user.c b/drivers/net/virtio/virtio_user/vhost_user.c
> index a6df97a..01201c9 100644
> --- a/drivers/net/virtio/virtio_user/vhost_user.c
> +++ b/drivers/net/virtio/virtio_user/vhost_user.c
> @@ -138,12 +138,13 @@ struct hugepage_file_info {
>   static int
>   get_hugepage_file_info(struct hugepage_file_info huges[], int max)
>   {
> -	int idx;
> +	int idx, k, exist;
>   	FILE *f;
>   	char buf[BUFSIZ], *tmp, *tail;
>   	char *str_underline, *str_start;
>   	int huge_index;
>   	uint64_t v_start, v_end;
> +	struct stat stats;
>   
>   	f = fopen("/proc/self/maps", "r");
>   	if (!f) {
> @@ -183,16 +184,39 @@ get_hugepage_file_info(struct hugepage_file_info huges[], int max)
>   		if (sscanf(str_start, "map_%d", &huge_index) != 1)
>   			continue;
>   
> +		/* skip duplicated file which is mapped to different regions */
> +		for (k = 0, exist = -1; k < idx; ++k) {
> +			if (!strcmp(huges[k].path, tmp)) {
> +				exist = k;
> +				break;
> +			}
> +		}
> +		if (exist >= 0)
> +			continue;
> +
>   		if (idx >= max) {
>   			PMD_DRV_LOG(ERR, "Exceed maximum of %d", max);
>   			goto error;
>   		}
> +
>   		huges[idx].addr = v_start;
> -		huges[idx].size = v_end - v_start;
> +		huges[idx].size = v_end - v_start; /* To be corrected later */
>   		snprintf(huges[idx].path, PATH_MAX, "%s", tmp);
>   		idx++;
>   	}
>   
> +	/* correct the size for files who have many regions */
> +	for (k = 0; k < idx; ++k) {
> +		if (stat(huges[k].path, &stats) < 0) {
> +			PMD_DRV_LOG(ERR, "Failed to stat %s, %s\n",
> +				    huges[k].path, strerror(errno));
> +			continue;
> +		}
> +		huges[k].size = stats.st_size;
> +		PMD_DRV_LOG(INFO, "file %s, size %"PRIx64"\n",
> +			    huges[k].path, huges[k].size);
> +	}
> +
>   	fclose(f);
>   	return idx;
>   
> 

That sounds like potentially a lot of strcmp()'s (quadratic?). Can't it 
be sped up somehow? Maybe use rte_hash for storing this data?

-- 
Thanks,
Anatoly

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

* Re: [dpdk-dev] [PATCH] net/virtio-user: fix hugepage files enumeration
  2018-04-27  9:31 ` Burakov, Anatoly
@ 2018-05-04  7:40   ` Maxime Coquelin
  2018-05-04  7:52     ` Burakov, Anatoly
  0 siblings, 1 reply; 7+ messages in thread
From: Maxime Coquelin @ 2018-05-04  7:40 UTC (permalink / raw)
  To: Burakov, Anatoly, Jianfeng Tan, dev; +Cc: tiwei.bie, zhiyong.yang

Hi Anatoly,

On 04/27/2018 11:31 AM, Burakov, Anatoly wrote:
> On 26-Apr-18 4:34 PM, Jianfeng Tan wrote:
>> After the commit 2a04139f66b4 ("eal: add single file segments option"),
>> one hugepage file could contain multiple hugepages which are further
>> mapped to different memory regions.
>>
>> Original enumeration implementation cannot handle this situation.
>>
>> This patch filters out the duplicated files; and adjust the size after
>> the enumeration.
>>
>> Fixes: 6a84c37e3975 ("net/virtio-user: add vhost-user adapter layer")
>>
>> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
>> ---
>>   .../howto/virtio_user_for_container_networking.rst |  3 ++-
>>   drivers/net/virtio/virtio_user/vhost_user.c        | 28 
>> ++++++++++++++++++++--
>>   2 files changed, 28 insertions(+), 3 deletions(-)
>>
>> diff --git a/doc/guides/howto/virtio_user_for_container_networking.rst 
>> b/doc/guides/howto/virtio_user_for_container_networking.rst
>> index aa68b53..476ce3a 100644
>> --- a/doc/guides/howto/virtio_user_for_container_networking.rst
>> +++ b/doc/guides/howto/virtio_user_for_container_networking.rst
>> @@ -109,7 +109,8 @@ We have below limitations in this solution:
>>    * Cannot work with --no-huge option. Currently, DPDK uses anonymous 
>> mapping
>>      under this option which cannot be reopened to share with vhost 
>> backend.
>>    * Cannot work when there are more than VHOST_MEMORY_MAX_NREGIONS(8) 
>> hugepages.
>> -   In another word, do not use 2MB hugepage so far.
>> +   If you have more regions (especially when 2MB hugepages are used), 
>> the option,
>> +   --single-file-segments, can help to reduce the number of shared 
>> files.
>>    * Applications should not use file name like HUGEFILE_FMT 
>> ("%smap_%d"). That
>>      will bring confusion when sharing hugepage files with backend by 
>> name.
>>    * Root privilege is a must. DPDK resolves physical addresses of 
>> hugepages
>> diff --git a/drivers/net/virtio/virtio_user/vhost_user.c 
>> b/drivers/net/virtio/virtio_user/vhost_user.c
>> index a6df97a..01201c9 100644
>> --- a/drivers/net/virtio/virtio_user/vhost_user.c
>> +++ b/drivers/net/virtio/virtio_user/vhost_user.c
>> @@ -138,12 +138,13 @@ struct hugepage_file_info {
>>   static int
>>   get_hugepage_file_info(struct hugepage_file_info huges[], int max)
>>   {
>> -    int idx;
>> +    int idx, k, exist;
>>       FILE *f;
>>       char buf[BUFSIZ], *tmp, *tail;
>>       char *str_underline, *str_start;
>>       int huge_index;
>>       uint64_t v_start, v_end;
>> +    struct stat stats;
>>       f = fopen("/proc/self/maps", "r");
>>       if (!f) {
>> @@ -183,16 +184,39 @@ get_hugepage_file_info(struct hugepage_file_info 
>> huges[], int max)
>>           if (sscanf(str_start, "map_%d", &huge_index) != 1)
>>               continue;
>> +        /* skip duplicated file which is mapped to different regions */
>> +        for (k = 0, exist = -1; k < idx; ++k) {
>> +            if (!strcmp(huges[k].path, tmp)) {
>> +                exist = k;
>> +                break;
>> +            }
>> +        }
>> +        if (exist >= 0)
>> +            continue;
>> +
>>           if (idx >= max) {
>>               PMD_DRV_LOG(ERR, "Exceed maximum of %d", max);
>>               goto error;
>>           }
>> +
>>           huges[idx].addr = v_start;
>> -        huges[idx].size = v_end - v_start;
>> +        huges[idx].size = v_end - v_start; /* To be corrected later */
>>           snprintf(huges[idx].path, PATH_MAX, "%s", tmp);
>>           idx++;
>>       }
>> +    /* correct the size for files who have many regions */
>> +    for (k = 0; k < idx; ++k) {
>> +        if (stat(huges[k].path, &stats) < 0) {
>> +            PMD_DRV_LOG(ERR, "Failed to stat %s, %s\n",
>> +                    huges[k].path, strerror(errno));
>> +            continue;
>> +        }
>> +        huges[k].size = stats.st_size;
>> +        PMD_DRV_LOG(INFO, "file %s, size %"PRIx64"\n",
>> +                huges[k].path, huges[k].size);
>> +    }
>> +
>>       fclose(f);
>>       return idx;
>>
> 
> That sounds like potentially a lot of strcmp()'s (quadratic?). Can't it 
> be sped up somehow? Maybe use rte_hash for storing this data?
> 

This patch is required to have virtio-user to work with 2MB pages.
While it may be improved later, I think we should pick it for v18.05.
Is it fine for you?

Thanks,
Maxime

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

* Re: [dpdk-dev] [PATCH] net/virtio-user: fix hugepage files enumeration
  2018-05-04  7:40   ` Maxime Coquelin
@ 2018-05-04  7:52     ` Burakov, Anatoly
  0 siblings, 0 replies; 7+ messages in thread
From: Burakov, Anatoly @ 2018-05-04  7:52 UTC (permalink / raw)
  To: Maxime Coquelin, Jianfeng Tan, dev; +Cc: tiwei.bie, zhiyong.yang

On 04-May-18 8:40 AM, Maxime Coquelin wrote:
> Hi Anatoly,
> 
> On 04/27/2018 11:31 AM, Burakov, Anatoly wrote:
>> On 26-Apr-18 4:34 PM, Jianfeng Tan wrote:
>>> After the commit 2a04139f66b4 ("eal: add single file segments option"),
>>> one hugepage file could contain multiple hugepages which are further
>>> mapped to different memory regions.
>>>
>>> Original enumeration implementation cannot handle this situation.
>>>
>>> This patch filters out the duplicated files; and adjust the size after
>>> the enumeration.
>>>
>>> Fixes: 6a84c37e3975 ("net/virtio-user: add vhost-user adapter layer")
>>>
>>> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
>>> ---
>>>   .../howto/virtio_user_for_container_networking.rst |  3 ++-
>>>   drivers/net/virtio/virtio_user/vhost_user.c        | 28 
>>> ++++++++++++++++++++--
>>>   2 files changed, 28 insertions(+), 3 deletions(-)
>>>
>>> diff --git 
>>> a/doc/guides/howto/virtio_user_for_container_networking.rst 
>>> b/doc/guides/howto/virtio_user_for_container_networking.rst
>>> index aa68b53..476ce3a 100644
>>> --- a/doc/guides/howto/virtio_user_for_container_networking.rst
>>> +++ b/doc/guides/howto/virtio_user_for_container_networking.rst
>>> @@ -109,7 +109,8 @@ We have below limitations in this solution:
>>>    * Cannot work with --no-huge option. Currently, DPDK uses 
>>> anonymous mapping
>>>      under this option which cannot be reopened to share with vhost 
>>> backend.
>>>    * Cannot work when there are more than 
>>> VHOST_MEMORY_MAX_NREGIONS(8) hugepages.
>>> -   In another word, do not use 2MB hugepage so far.
>>> +   If you have more regions (especially when 2MB hugepages are 
>>> used), the option,
>>> +   --single-file-segments, can help to reduce the number of shared 
>>> files.
>>>    * Applications should not use file name like HUGEFILE_FMT 
>>> ("%smap_%d"). That
>>>      will bring confusion when sharing hugepage files with backend by 
>>> name.
>>>    * Root privilege is a must. DPDK resolves physical addresses of 
>>> hugepages
>>> diff --git a/drivers/net/virtio/virtio_user/vhost_user.c 
>>> b/drivers/net/virtio/virtio_user/vhost_user.c
>>> index a6df97a..01201c9 100644
>>> --- a/drivers/net/virtio/virtio_user/vhost_user.c
>>> +++ b/drivers/net/virtio/virtio_user/vhost_user.c
>>> @@ -138,12 +138,13 @@ struct hugepage_file_info {
>>>   static int
>>>   get_hugepage_file_info(struct hugepage_file_info huges[], int max)
>>>   {
>>> -    int idx;
>>> +    int idx, k, exist;
>>>       FILE *f;
>>>       char buf[BUFSIZ], *tmp, *tail;
>>>       char *str_underline, *str_start;
>>>       int huge_index;
>>>       uint64_t v_start, v_end;
>>> +    struct stat stats;
>>>       f = fopen("/proc/self/maps", "r");
>>>       if (!f) {
>>> @@ -183,16 +184,39 @@ get_hugepage_file_info(struct 
>>> hugepage_file_info huges[], int max)
>>>           if (sscanf(str_start, "map_%d", &huge_index) != 1)
>>>               continue;
>>> +        /* skip duplicated file which is mapped to different regions */
>>> +        for (k = 0, exist = -1; k < idx; ++k) {
>>> +            if (!strcmp(huges[k].path, tmp)) {
>>> +                exist = k;
>>> +                break;
>>> +            }
>>> +        }
>>> +        if (exist >= 0)
>>> +            continue;
>>> +
>>>           if (idx >= max) {
>>>               PMD_DRV_LOG(ERR, "Exceed maximum of %d", max);
>>>               goto error;
>>>           }
>>> +
>>>           huges[idx].addr = v_start;
>>> -        huges[idx].size = v_end - v_start;
>>> +        huges[idx].size = v_end - v_start; /* To be corrected later */
>>>           snprintf(huges[idx].path, PATH_MAX, "%s", tmp);
>>>           idx++;
>>>       }
>>> +    /* correct the size for files who have many regions */
>>> +    for (k = 0; k < idx; ++k) {
>>> +        if (stat(huges[k].path, &stats) < 0) {
>>> +            PMD_DRV_LOG(ERR, "Failed to stat %s, %s\n",
>>> +                    huges[k].path, strerror(errno));
>>> +            continue;
>>> +        }
>>> +        huges[k].size = stats.st_size;
>>> +        PMD_DRV_LOG(INFO, "file %s, size %"PRIx64"\n",
>>> +                huges[k].path, huges[k].size);
>>> +    }
>>> +
>>>       fclose(f);
>>>       return idx;
>>>
>>
>> That sounds like potentially a lot of strcmp()'s (quadratic?). Can't 
>> it be sped up somehow? Maybe use rte_hash for storing this data?
>>
> 
> This patch is required to have virtio-user to work with 2MB pages.
> While it may be improved later, I think we should pick it for v18.05.
> Is it fine for you?

Looks fine to me otherwise.

> 
> Thanks,
> Maxime
> 


-- 
Thanks,
Anatoly

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

* Re: [dpdk-dev] [PATCH] net/virtio-user: fix hugepage files enumeration
  2018-04-26 15:34 [dpdk-dev] [PATCH] net/virtio-user: fix hugepage files enumeration Jianfeng Tan
  2018-04-27  9:31 ` Burakov, Anatoly
@ 2018-05-04 14:16 ` Maxime Coquelin
  2018-05-04 15:09 ` Maxime Coquelin
  2018-05-08 14:26 ` Ferruh Yigit
  3 siblings, 0 replies; 7+ messages in thread
From: Maxime Coquelin @ 2018-05-04 14:16 UTC (permalink / raw)
  To: Jianfeng Tan, dev; +Cc: tiwei.bie, zhiyong.yang



On 04/26/2018 05:34 PM, Jianfeng Tan wrote:
> After the commit 2a04139f66b4 ("eal: add single file segments option"),
> one hugepage file could contain multiple hugepages which are further
> mapped to different memory regions.
> 
> Original enumeration implementation cannot handle this situation.
> 
> This patch filters out the duplicated files; and adjust the size after
> the enumeration.
> 
> Fixes: 6a84c37e3975 ("net/virtio-user: add vhost-user adapter layer")
> 
> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> ---
>   .../howto/virtio_user_for_container_networking.rst |  3 ++-
>   drivers/net/virtio/virtio_user/vhost_user.c        | 28 ++++++++++++++++++++--
>   2 files changed, 28 insertions(+), 3 deletions(-)


Acked-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime

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

* Re: [dpdk-dev] [PATCH] net/virtio-user: fix hugepage files enumeration
  2018-04-26 15:34 [dpdk-dev] [PATCH] net/virtio-user: fix hugepage files enumeration Jianfeng Tan
  2018-04-27  9:31 ` Burakov, Anatoly
  2018-05-04 14:16 ` Maxime Coquelin
@ 2018-05-04 15:09 ` Maxime Coquelin
  2018-05-08 14:26 ` Ferruh Yigit
  3 siblings, 0 replies; 7+ messages in thread
From: Maxime Coquelin @ 2018-05-04 15:09 UTC (permalink / raw)
  To: Jianfeng Tan, dev; +Cc: tiwei.bie, zhiyong.yang



On 04/26/2018 05:34 PM, Jianfeng Tan wrote:
> After the commit 2a04139f66b4 ("eal: add single file segments option"),
> one hugepage file could contain multiple hugepages which are further
> mapped to different memory regions.
> 
> Original enumeration implementation cannot handle this situation.
> 
> This patch filters out the duplicated files; and adjust the size after
> the enumeration.
> 
> Fixes: 6a84c37e3975 ("net/virtio-user: add vhost-user adapter layer")
> 
> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> ---
>   .../howto/virtio_user_for_container_networking.rst |  3 ++-
>   drivers/net/virtio/virtio_user/vhost_user.c        | 28 ++++++++++++++++++++--
>   2 files changed, 28 insertions(+), 3 deletions(-)
> 

Applied to dpdk-next-virtio/master.

Thanks,
Maxime

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

* Re: [dpdk-dev] [PATCH] net/virtio-user: fix hugepage files enumeration
  2018-04-26 15:34 [dpdk-dev] [PATCH] net/virtio-user: fix hugepage files enumeration Jianfeng Tan
                   ` (2 preceding siblings ...)
  2018-05-04 15:09 ` Maxime Coquelin
@ 2018-05-08 14:26 ` Ferruh Yigit
  3 siblings, 0 replies; 7+ messages in thread
From: Ferruh Yigit @ 2018-05-08 14:26 UTC (permalink / raw)
  To: Jianfeng Tan, dev; +Cc: maxime.coquelin, tiwei.bie, zhiyong.yang

On 4/26/2018 4:34 PM, Jianfeng Tan wrote:
> After the commit 2a04139f66b4 ("eal: add single file segments option"),
> one hugepage file could contain multiple hugepages which are further
> mapped to different memory regions.
> 
> Original enumeration implementation cannot handle this situation.
> 
> This patch filters out the duplicated files; and adjust the size after
> the enumeration.
> 
> Fixes: 6a84c37e3975 ("net/virtio-user: add vhost-user adapter layer")
> 
> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> ---
>  .../howto/virtio_user_for_container_networking.rst |  3 ++-
>  drivers/net/virtio/virtio_user/vhost_user.c        | 28 ++++++++++++++++++++--
>  2 files changed, 28 insertions(+), 3 deletions(-)
> 
> diff --git a/doc/guides/howto/virtio_user_for_container_networking.rst b/doc/guides/howto/virtio_user_for_container_networking.rst
> index aa68b53..476ce3a 100644
> --- a/doc/guides/howto/virtio_user_for_container_networking.rst
> +++ b/doc/guides/howto/virtio_user_for_container_networking.rst
> @@ -109,7 +109,8 @@ We have below limitations in this solution:
>   * Cannot work with --no-huge option. Currently, DPDK uses anonymous mapping
>     under this option which cannot be reopened to share with vhost backend.
>   * Cannot work when there are more than VHOST_MEMORY_MAX_NREGIONS(8) hugepages.
> -   In another word, do not use 2MB hugepage so far.
> +   If you have more regions (especially when 2MB hugepages are used), the option,
> +   --single-file-segments, can help to reduce the number of shared files.
>   * Applications should not use file name like HUGEFILE_FMT ("%smap_%d"). That
>     will bring confusion when sharing hugepage files with backend by name.
>   * Root privilege is a must. DPDK resolves physical addresses of hugepages
> diff --git a/drivers/net/virtio/virtio_user/vhost_user.c b/drivers/net/virtio/virtio_user/vhost_user.c
> index a6df97a..01201c9 100644
> --- a/drivers/net/virtio/virtio_user/vhost_user.c
> +++ b/drivers/net/virtio/virtio_user/vhost_user.c
> @@ -138,12 +138,13 @@ struct hugepage_file_info {
>  static int
>  get_hugepage_file_info(struct hugepage_file_info huges[], int max)
>  {
> -	int idx;
> +	int idx, k, exist;
>  	FILE *f;
>  	char buf[BUFSIZ], *tmp, *tail;
>  	char *str_underline, *str_start;
>  	int huge_index;
>  	uint64_t v_start, v_end;
> +	struct stat stats;
>  
>  	f = fopen("/proc/self/maps", "r");
>  	if (!f) {
> @@ -183,16 +184,39 @@ get_hugepage_file_info(struct hugepage_file_info huges[], int max)
>  		if (sscanf(str_start, "map_%d", &huge_index) != 1)
>  			continue;
>  
> +		/* skip duplicated file which is mapped to different regions */
> +		for (k = 0, exist = -1; k < idx; ++k) {
> +			if (!strcmp(huges[k].path, tmp)) {
> +				exist = k;
> +				break;
> +			}
> +		}
> +		if (exist >= 0)
> +			continue;
> +
>  		if (idx >= max) {
>  			PMD_DRV_LOG(ERR, "Exceed maximum of %d", max);
>  			goto error;
>  		}
> +
>  		huges[idx].addr = v_start;
> -		huges[idx].size = v_end - v_start;
> +		huges[idx].size = v_end - v_start; /* To be corrected later */
>  		snprintf(huges[idx].path, PATH_MAX, "%s", tmp);
>  		idx++;
>  	}
>  
> +	/* correct the size for files who have many regions */
> +	for (k = 0; k < idx; ++k) {
> +		if (stat(huges[k].path, &stats) < 0) {
> +			PMD_DRV_LOG(ERR, "Failed to stat %s, %s\n",
> +				    huges[k].path, strerror(errno));
> +			continue;
> +		}
> +		huges[k].size = stats.st_size;
> +		PMD_DRV_LOG(INFO, "file %s, size %"PRIx64"\n",
> +			    huges[k].path, huges[k].size);

This cause a build error for 32bits [1], %z should be used for size_t, will fix
while applying.

[1]
.../drivers/net/virtio/virtio_user/vhost_user.c(216): error #181: argument of
type "size_t={unsigned int}" is incompatible with format "%llx", expecting
argument of type "unsigned long long"
                PMD_DRV_LOG(INFO, "file %s, size %"PRIx64"\n",

                ^

> +	}
> +
>  	fclose(f);
>  	return idx;
>  
> 

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

end of thread, other threads:[~2018-05-08 14:26 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-26 15:34 [dpdk-dev] [PATCH] net/virtio-user: fix hugepage files enumeration Jianfeng Tan
2018-04-27  9:31 ` Burakov, Anatoly
2018-05-04  7:40   ` Maxime Coquelin
2018-05-04  7:52     ` Burakov, Anatoly
2018-05-04 14:16 ` Maxime Coquelin
2018-05-04 15:09 ` Maxime Coquelin
2018-05-08 14:26 ` Ferruh Yigit

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).