DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v2] support free hugepages
@ 2014-10-29  5:47 linhaifeng
  2014-10-29  6:14 ` Qiu, Michael
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: linhaifeng @ 2014-10-29  5:47 UTC (permalink / raw)
  To: dev

rte_eal_hugepage_free() is used for unlink all hugepages.If you want to
free all hugepages you must make sure that you have stop to use it,and you
must call this function before exit process.

Signed-off-by: linhaifeng <haifeng.lin@huawei.com>
---
 .../lib/librte_eal/common/include/rte_memory.h     | 11 ++++++++
 .../lib/librte_eal/linuxapp/eal/eal_memory.c       | 31 ++++++++++++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/dpdk/dpdk-1.7.0/lib/librte_eal/common/include/rte_memory.h b/dpdk/dpdk-1.7.0/lib/librte_eal/common/include/rte_memory.h
index 4cf8ea9..f6ad95f 100644
--- a/dpdk/dpdk-1.7.0/lib/librte_eal/common/include/rte_memory.h
+++ b/dpdk/dpdk-1.7.0/lib/librte_eal/common/include/rte_memory.h
@@ -172,6 +172,17 @@ unsigned rte_memory_get_nchannel(void);
  */
 unsigned rte_memory_get_nrank(void);
 
+/**
+ * Unlink all hugepages which created by dpdk.
+ *
+ * @param void
+ *
+ * @return
+ *       0: successfully
+ *       negative: error
+ */
+int rte_eal_hugepage_free(void);
+
 #ifdef RTE_LIBRTE_XEN_DOM0
 /**
  * Return the physical address of elt, which is an element of the pool mp.
diff --git a/dpdk/dpdk-1.7.0/lib/librte_eal/linuxapp/eal/eal_memory.c b/dpdk/dpdk-1.7.0/lib/librte_eal/linuxapp/eal/eal_memory.c
index f2454f4..109207c 100644
--- a/dpdk/dpdk-1.7.0/lib/librte_eal/linuxapp/eal/eal_memory.c
+++ b/dpdk/dpdk-1.7.0/lib/librte_eal/linuxapp/eal/eal_memory.c
@@ -98,6 +98,13 @@
 #include "eal_filesystem.h"
 #include "eal_hugepages.h"
 
+struct hugepage_table {
+	struct hugepage_file *hugepg_tbl;
+	unsigned nr_hugefiles;
+};
+
+static struct hugepage_table g_hugepage_table;
+
 /**
  * @file
  * Huge page mapping under linux
@@ -1202,6 +1209,7 @@ rte_eal_hugepage_init(void)
 						(unsigned)
 							(used_hp[i].hugepage_sz / 0x100000),
 						j);
+				g_hugepage_table.nr_hugefiles += used_hp[i].num_pages[j];
 			}
 		}
 	}
@@ -1237,6 +1245,8 @@ rte_eal_hugepage_init(void)
 		goto fail;
 	}
 
+	g_hugepage_table.hugepg_tbl = hugepage;
+
 	/* free the temporary hugepage table */
 	free(tmp_hp);
 	tmp_hp = NULL;
@@ -1487,6 +1497,27 @@ error:
 	return -1;
 }
 
+int
+rte_eal_hugepage_free(void)
+{
+	struct hugepage_file *hugepg_tbl = g_hugepage_table.hugepg_tbl;
+	unsigned i;
+	unsigned nr_hugefiles = g_hugepage_table.nr_hugefiles;
+	int ret = 0;
+
+	for (i = 0; i < nr_hugefiles; i++) {
+		ret = unlink(hugepg_tbl[i].filepath);
+		if (ret != 0) {
+			RTE_LOG(ERR, EAL, "Failed to unlink %s", hugepg_tbl[i].filepath);
+			return ret;
+		}
+		hugepg_tbl[i].orig_va = NULL;
+	}
+
+	RTE_LOG(INFO, EAL, "unlink %u hugepage files\n", nr_hugefiles);
+	return ret;
+}
+
 static int
 rte_eal_memdevice_init(void)
 {
-- 
1.8.3.1

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

* Re: [dpdk-dev] [PATCH v2] support free hugepages
  2014-10-29  5:47 [dpdk-dev] [PATCH v2] support free hugepages linhaifeng
@ 2014-10-29  6:14 ` Qiu, Michael
  2014-10-29  6:40   ` Linhaifeng
  2014-10-30  3:17 ` Matthew Hall
  2014-10-30 15:32 ` Thomas Monjalon
  2 siblings, 1 reply; 7+ messages in thread
From: Qiu, Michael @ 2014-10-29  6:14 UTC (permalink / raw)
  To: linhaifeng, dev

在 10/29/2014 1:49 PM, linhaifeng 写道:
> rte_eal_hugepage_free() is used for unlink all hugepages.If you want to
> free all hugepages you must make sure that you have stop to use it,and you
> must call this function before exit process.
>
> Signed-off-by: linhaifeng <haifeng.lin@huawei.com>
> ---
>  .../lib/librte_eal/common/include/rte_memory.h     | 11 ++++++++
>  .../lib/librte_eal/linuxapp/eal/eal_memory.c       | 31 ++++++++++++++++++++++
>  2 files changed, 42 insertions(+)
>
> diff --git a/dpdk/dpdk-1.7.0/lib/librte_eal/common/include/rte_memory.h b/dpdk/dpdk-1.7.0/lib/librte_eal/common/include/rte_memory.h
> index 4cf8ea9..f6ad95f 100644
> --- a/dpdk/dpdk-1.7.0/lib/librte_eal/common/include/rte_memory.h
> +++ b/dpdk/dpdk-1.7.0/lib/librte_eal/common/include/rte_memory.h
> @@ -172,6 +172,17 @@ unsigned rte_memory_get_nchannel(void);
>   */
>  unsigned rte_memory_get_nrank(void);
>  
> +/**
> + * Unlink all hugepages which created by dpdk.
> + *
> + * @param void
> + *
> + * @return
> + *       0: successfully
> + *       negative: error
> + */
> +int rte_eal_hugepage_free(void);
> +
>  #ifdef RTE_LIBRTE_XEN_DOM0
>  /**
>   * Return the physical address of elt, which is an element of the pool mp.
> diff --git a/dpdk/dpdk-1.7.0/lib/librte_eal/linuxapp/eal/eal_memory.c b/dpdk/dpdk-1.7.0/lib/librte_eal/linuxapp/eal/eal_memory.c
> index f2454f4..109207c 100644
> --- a/dpdk/dpdk-1.7.0/lib/librte_eal/linuxapp/eal/eal_memory.c
> +++ b/dpdk/dpdk-1.7.0/lib/librte_eal/linuxapp/eal/eal_memory.c
> @@ -98,6 +98,13 @@
>  #include "eal_filesystem.h"
>  #include "eal_hugepages.h"
>  
> +struct hugepage_table {
> +	struct hugepage_file *hugepg_tbl;
> +	unsigned nr_hugefiles;
> +};
> +
> +static struct hugepage_table g_hugepage_table;
> +
>  /**
>   * @file
>   * Huge page mapping under linux
> @@ -1202,6 +1209,7 @@ rte_eal_hugepage_init(void)
>  						(unsigned)
>  							(used_hp[i].hugepage_sz / 0x100000),
>  						j);
> +				g_hugepage_table.nr_hugefiles += used_hp[i].num_pages[j];
>  			}
>  		}
>  	}
> @@ -1237,6 +1245,8 @@ rte_eal_hugepage_init(void)
>  		goto fail;
>  	}
>  
> +	g_hugepage_table.hugepg_tbl = hugepage;
> +
>  	/* free the temporary hugepage table */
>  	free(tmp_hp);
>  	tmp_hp = NULL;
> @@ -1487,6 +1497,27 @@ error:
>  	return -1;
>  }
>  
> +int
> +rte_eal_hugepage_free(void)
> +{
> +	struct hugepage_file *hugepg_tbl = g_hugepage_table.hugepg_tbl;
> +	unsigned i;
> +	unsigned nr_hugefiles = g_hugepage_table.nr_hugefiles;
> +	int ret = 0;
> +
> +	for (i = 0; i < nr_hugefiles; i++) {
> +		ret = unlink(hugepg_tbl[i].filepath);
> +		if (ret != 0) {
> +			RTE_LOG(ERR, EAL, "Failed to unlink %s", hugepg_tbl[i].filepath);
> +			return ret;
> +		}
> +		hugepg_tbl[i].orig_va = NULL;

BTW, is it better to first set hugepg_tbl[i].orig_vato NULL, then unlink
filepath?
It may be not a good idea to first remove then set to NULL.

Thanks,
Michael

> +	}
> +
> +	RTE_LOG(INFO, EAL, "unlink %u hugepage files\n", nr_hugefiles);
> +	return ret;
> +}
> +
>  static int
>  rte_eal_memdevice_init(void)
>  {


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

* Re: [dpdk-dev] [PATCH v2] support free hugepages
  2014-10-29  6:14 ` Qiu, Michael
@ 2014-10-29  6:40   ` Linhaifeng
  2014-10-29  8:04     ` Qiu, Michael
  0 siblings, 1 reply; 7+ messages in thread
From: Linhaifeng @ 2014-10-29  6:40 UTC (permalink / raw)
  To: Qiu, Michael, dev



On 2014/10/29 14:14, Qiu, Michael wrote:
> 在 10/29/2014 1:49 PM, linhaifeng 写道:
>> rte_eal_hugepage_free() is used for unlink all hugepages.If you want to
>> free all hugepages you must make sure that you have stop to use it,and you
>> must call this function before exit process.
>>
>> Signed-off-by: linhaifeng <haifeng.lin@huawei.com>
>> ---
>>  .../lib/librte_eal/common/include/rte_memory.h     | 11 ++++++++
>>  .../lib/librte_eal/linuxapp/eal/eal_memory.c       | 31 ++++++++++++++++++++++
>>  2 files changed, 42 insertions(+)
>>
>> diff --git a/dpdk/dpdk-1.7.0/lib/librte_eal/common/include/rte_memory.h b/dpdk/dpdk-1.7.0/lib/librte_eal/common/include/rte_memory.h
>> index 4cf8ea9..f6ad95f 100644
>> --- a/dpdk/dpdk-1.7.0/lib/librte_eal/common/include/rte_memory.h
>> +++ b/dpdk/dpdk-1.7.0/lib/librte_eal/common/include/rte_memory.h
>> @@ -172,6 +172,17 @@ unsigned rte_memory_get_nchannel(void);
>>   */
>>  unsigned rte_memory_get_nrank(void);
>>  
>> +/**
>> + * Unlink all hugepages which created by dpdk.
>> + *
>> + * @param void
>> + *
>> + * @return
>> + *       0: successfully
>> + *       negative: error
>> + */
>> +int rte_eal_hugepage_free(void);
>> +
>>  #ifdef RTE_LIBRTE_XEN_DOM0
>>  /**
>>   * Return the physical address of elt, which is an element of the pool mp.
>> diff --git a/dpdk/dpdk-1.7.0/lib/librte_eal/linuxapp/eal/eal_memory.c b/dpdk/dpdk-1.7.0/lib/librte_eal/linuxapp/eal/eal_memory.c
>> index f2454f4..109207c 100644
>> --- a/dpdk/dpdk-1.7.0/lib/librte_eal/linuxapp/eal/eal_memory.c
>> +++ b/dpdk/dpdk-1.7.0/lib/librte_eal/linuxapp/eal/eal_memory.c
>> @@ -98,6 +98,13 @@
>>  #include "eal_filesystem.h"
>>  #include "eal_hugepages.h"
>>  
>> +struct hugepage_table {
>> +	struct hugepage_file *hugepg_tbl;
>> +	unsigned nr_hugefiles;
>> +};
>> +
>> +static struct hugepage_table g_hugepage_table;
>> +
>>  /**
>>   * @file
>>   * Huge page mapping under linux
>> @@ -1202,6 +1209,7 @@ rte_eal_hugepage_init(void)
>>  						(unsigned)
>>  							(used_hp[i].hugepage_sz / 0x100000),
>>  						j);
>> +				g_hugepage_table.nr_hugefiles += used_hp[i].num_pages[j];
>>  			}
>>  		}
>>  	}
>> @@ -1237,6 +1245,8 @@ rte_eal_hugepage_init(void)
>>  		goto fail;
>>  	}
>>  
>> +	g_hugepage_table.hugepg_tbl = hugepage;
>> +
>>  	/* free the temporary hugepage table */
>>  	free(tmp_hp);
>>  	tmp_hp = NULL;
>> @@ -1487,6 +1497,27 @@ error:
>>  	return -1;
>>  }
>>  
>> +int
>> +rte_eal_hugepage_free(void)
>> +{
>> +	struct hugepage_file *hugepg_tbl = g_hugepage_table.hugepg_tbl;
>> +	unsigned i;
>> +	unsigned nr_hugefiles = g_hugepage_table.nr_hugefiles;
>> +	int ret = 0;
>> +
>> +	for (i = 0; i < nr_hugefiles; i++) {
>> +		ret = unlink(hugepg_tbl[i].filepath);
>> +		if (ret != 0) {
>> +			RTE_LOG(ERR, EAL, "Failed to unlink %s", hugepg_tbl[i].filepath);
>> +			return ret;
>> +		}
>> +		hugepg_tbl[i].orig_va = NULL;
> 
> BTW, is it better to first set hugepg_tbl[i].orig_vato NULL, then unlink
> filepath?
> It may be not a good idea to first remove then set to NULL.
> 
> Thanks,
> Michael
> 

If first set hugepg_tbl[i].orig_va to NULL,then failed to unlink you have to restore hugepg_tbl[i].orig_va.
So I first to unlink for less codes.

>> +	}
>> +
>> +	RTE_LOG(INFO, EAL, "unlink %u hugepage files\n", nr_hugefiles);
>> +	return ret;
>> +}
>> +
>>  static int
>>  rte_eal_memdevice_init(void)
>>  {
> 
> 
> .
> 

-- 
Regards,
Haifeng

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

* Re: [dpdk-dev] [PATCH v2] support free hugepages
  2014-10-29  6:40   ` Linhaifeng
@ 2014-10-29  8:04     ` Qiu, Michael
  2014-10-29  8:15       ` Linhaifeng
  0 siblings, 1 reply; 7+ messages in thread
From: Qiu, Michael @ 2014-10-29  8:04 UTC (permalink / raw)
  To: Linhaifeng, dev

10/29/2014 2:41 PM, Linhaifeng :
>
> On 2014/10/29 14:14, Qiu, Michael wrote:
>> 在 10/29/2014 1:49 PM, linhaifeng 写道:
>>> rte_eal_hugepage_free() is used for unlink all hugepages.If you want to
>>> free all hugepages you must make sure that you have stop to use it,and you
>>> must call this function before exit process.
>>>
>>> Signed-off-by: linhaifeng <haifeng.lin@huawei.com>
>>> ---
>>>  .../lib/librte_eal/common/include/rte_memory.h     | 11 ++++++++
>>>  .../lib/librte_eal/linuxapp/eal/eal_memory.c       | 31 ++++++++++++++++++++++
>>>  2 files changed, 42 insertions(+)
>>>
>>> diff --git a/dpdk/dpdk-1.7.0/lib/librte_eal/common/include/rte_memory.h b/dpdk/dpdk-1.7.0/lib/librte_eal/common/include/rte_memory.h
>>> index 4cf8ea9..f6ad95f 100644
>>> --- a/dpdk/dpdk-1.7.0/lib/librte_eal/common/include/rte_memory.h
>>> +++ b/dpdk/dpdk-1.7.0/lib/librte_eal/common/include/rte_memory.h
>>> @@ -172,6 +172,17 @@ unsigned rte_memory_get_nchannel(void);
>>>   */
>>>  unsigned rte_memory_get_nrank(void);
>>>  
>>> +/**
>>> + * Unlink all hugepages which created by dpdk.
>>> + *
>>> + * @param void
>>> + *
>>> + * @return
>>> + *       0: successfully
>>> + *       negative: error
>>> + */
>>> +int rte_eal_hugepage_free(void);
>>> +
>>>  #ifdef RTE_LIBRTE_XEN_DOM0
>>>  /**
>>>   * Return the physical address of elt, which is an element of the pool mp.
>>> diff --git a/dpdk/dpdk-1.7.0/lib/librte_eal/linuxapp/eal/eal_memory.c b/dpdk/dpdk-1.7.0/lib/librte_eal/linuxapp/eal/eal_memory.c
>>> index f2454f4..109207c 100644
>>> --- a/dpdk/dpdk-1.7.0/lib/librte_eal/linuxapp/eal/eal_memory.c
>>> +++ b/dpdk/dpdk-1.7.0/lib/librte_eal/linuxapp/eal/eal_memory.c
>>> @@ -98,6 +98,13 @@
>>>  #include "eal_filesystem.h"
>>>  #include "eal_hugepages.h"
>>>  
>>> +struct hugepage_table {
>>> +	struct hugepage_file *hugepg_tbl;
>>> +	unsigned nr_hugefiles;
>>> +};
>>> +
>>> +static struct hugepage_table g_hugepage_table;
>>> +
>>>  /**
>>>   * @file
>>>   * Huge page mapping under linux
>>> @@ -1202,6 +1209,7 @@ rte_eal_hugepage_init(void)
>>>  						(unsigned)
>>>  							(used_hp[i].hugepage_sz / 0x100000),
>>>  						j);
>>> +				g_hugepage_table.nr_hugefiles += used_hp[i].num_pages[j];
>>>  			}
>>>  		}
>>>  	}
>>> @@ -1237,6 +1245,8 @@ rte_eal_hugepage_init(void)
>>>  		goto fail;
>>>  	}
>>>  
>>> +	g_hugepage_table.hugepg_tbl = hugepage;
>>> +
>>>  	/* free the temporary hugepage table */
>>>  	free(tmp_hp);
>>>  	tmp_hp = NULL;
>>> @@ -1487,6 +1497,27 @@ error:
>>>  	return -1;
>>>  }
>>>  
>>> +int
>>> +rte_eal_hugepage_free(void)
>>> +{
>>> +	struct hugepage_file *hugepg_tbl = g_hugepage_table.hugepg_tbl;
>>> +	unsigned i;
>>> +	unsigned nr_hugefiles = g_hugepage_table.nr_hugefiles;
>>> +	int ret = 0;
>>> +
>>> +	for (i = 0; i < nr_hugefiles; i++) {
>>> +		ret = unlink(hugepg_tbl[i].filepath);
>>> +		if (ret != 0) {
>>> +			RTE_LOG(ERR, EAL, "Failed to unlink %s", hugepg_tbl[i].filepath);
>>> +			return ret;
>>> +		}
>>> +		hugepg_tbl[i].orig_va = NULL;
>> BTW, is it better to first set hugepg_tbl[i].orig_vato NULL, then unlink
>> filepath?
>> It may be not a good idea to first remove then set to NULL.
>>
>> Thanks,
>> Michael
>>
> If first set hugepg_tbl[i].orig_va to NULL,then failed to unlink you have to restore hugepg_tbl[i].orig_va.
> So I first to unlink for less codes.

But it may be a big issue(not safe) when you unlink the path before you
set the pointer to NULL,  another thing is , why you need to restore the
orig_va since the app will exit? Even you need to restore it just one or
two lines code, I think it is not a big deal.

Thanks,
Michael
>
>>> +	}
>>> +
>>> +	RTE_LOG(INFO, EAL, "unlink %u hugepage files\n", nr_hugefiles);
>>> +	return ret;
>>> +}
>>> +
>>>  static int
>>>  rte_eal_memdevice_init(void)
>>>  {
>>
>> .
>>


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

* Re: [dpdk-dev] [PATCH v2] support free hugepages
  2014-10-29  8:04     ` Qiu, Michael
@ 2014-10-29  8:15       ` Linhaifeng
  0 siblings, 0 replies; 7+ messages in thread
From: Linhaifeng @ 2014-10-29  8:15 UTC (permalink / raw)
  To: Qiu, Michael, dev



On 2014/10/29 16:04, Qiu, Michael wrote:
> 10/29/2014 2:41 PM, Linhaifeng :
>>
>> On 2014/10/29 14:14, Qiu, Michael wrote:
>>> 在 10/29/2014 1:49 PM, linhaifeng 写道:
>>>> rte_eal_hugepage_free() is used for unlink all hugepages.If you want to
>>>> free all hugepages you must make sure that you have stop to use it,and you
>>>> must call this function before exit process.
>>>>
>>>> Signed-off-by: linhaifeng <haifeng.lin@huawei.com>
>>>> ---
>>>>  .../lib/librte_eal/common/include/rte_memory.h     | 11 ++++++++
>>>>  .../lib/librte_eal/linuxapp/eal/eal_memory.c       | 31 ++++++++++++++++++++++
>>>>  2 files changed, 42 insertions(+)
>>>>
>>>> diff --git a/dpdk/dpdk-1.7.0/lib/librte_eal/common/include/rte_memory.h b/dpdk/dpdk-1.7.0/lib/librte_eal/common/include/rte_memory.h
>>>> index 4cf8ea9..f6ad95f 100644
>>>> --- a/dpdk/dpdk-1.7.0/lib/librte_eal/common/include/rte_memory.h
>>>> +++ b/dpdk/dpdk-1.7.0/lib/librte_eal/common/include/rte_memory.h
>>>> @@ -172,6 +172,17 @@ unsigned rte_memory_get_nchannel(void);
>>>>   */
>>>>  unsigned rte_memory_get_nrank(void);
>>>>  
>>>> +/**
>>>> + * Unlink all hugepages which created by dpdk.
>>>> + *
>>>> + * @param void
>>>> + *
>>>> + * @return
>>>> + *       0: successfully
>>>> + *       negative: error
>>>> + */
>>>> +int rte_eal_hugepage_free(void);
>>>> +
>>>>  #ifdef RTE_LIBRTE_XEN_DOM0
>>>>  /**
>>>>   * Return the physical address of elt, which is an element of the pool mp.
>>>> diff --git a/dpdk/dpdk-1.7.0/lib/librte_eal/linuxapp/eal/eal_memory.c b/dpdk/dpdk-1.7.0/lib/librte_eal/linuxapp/eal/eal_memory.c
>>>> index f2454f4..109207c 100644
>>>> --- a/dpdk/dpdk-1.7.0/lib/librte_eal/linuxapp/eal/eal_memory.c
>>>> +++ b/dpdk/dpdk-1.7.0/lib/librte_eal/linuxapp/eal/eal_memory.c
>>>> @@ -98,6 +98,13 @@
>>>>  #include "eal_filesystem.h"
>>>>  #include "eal_hugepages.h"
>>>>  
>>>> +struct hugepage_table {
>>>> +	struct hugepage_file *hugepg_tbl;
>>>> +	unsigned nr_hugefiles;
>>>> +};
>>>> +
>>>> +static struct hugepage_table g_hugepage_table;
>>>> +
>>>>  /**
>>>>   * @file
>>>>   * Huge page mapping under linux
>>>> @@ -1202,6 +1209,7 @@ rte_eal_hugepage_init(void)
>>>>  						(unsigned)
>>>>  							(used_hp[i].hugepage_sz / 0x100000),
>>>>  						j);
>>>> +				g_hugepage_table.nr_hugefiles += used_hp[i].num_pages[j];
>>>>  			}
>>>>  		}
>>>>  	}
>>>> @@ -1237,6 +1245,8 @@ rte_eal_hugepage_init(void)
>>>>  		goto fail;
>>>>  	}
>>>>  
>>>> +	g_hugepage_table.hugepg_tbl = hugepage;
>>>> +
>>>>  	/* free the temporary hugepage table */
>>>>  	free(tmp_hp);
>>>>  	tmp_hp = NULL;
>>>> @@ -1487,6 +1497,27 @@ error:
>>>>  	return -1;
>>>>  }
>>>>  
>>>> +int
>>>> +rte_eal_hugepage_free(void)
>>>> +{
>>>> +	struct hugepage_file *hugepg_tbl = g_hugepage_table.hugepg_tbl;
>>>> +	unsigned i;
>>>> +	unsigned nr_hugefiles = g_hugepage_table.nr_hugefiles;
>>>> +	int ret = 0;
>>>> +
>>>> +	for (i = 0; i < nr_hugefiles; i++) {
>>>> +		ret = unlink(hugepg_tbl[i].filepath);
>>>> +		if (ret != 0) {
>>>> +			RTE_LOG(ERR, EAL, "Failed to unlink %s", hugepg_tbl[i].filepath);
>>>> +			return ret;
>>>> +		}
>>>> +		hugepg_tbl[i].orig_va = NULL;
>>> BTW, is it better to first set hugepg_tbl[i].orig_vato NULL, then unlink
>>> filepath?
>>> It may be not a good idea to first remove then set to NULL.
>>>
>>> Thanks,
>>> Michael
>>>
>> If first set hugepg_tbl[i].orig_va to NULL,then failed to unlink you have to restore hugepg_tbl[i].orig_va.
>> So I first to unlink for less codes.
> 
> But it may be a big issue(not safe) when you unlink the path before you
> set the pointer to NULL,  another thing is , why you need to restore the
> orig_va since the app will exit? Even you need to restore it just one or
> two lines code, I think it is not a big deal.
> 
> Thanks,
> Michael

Thank you,Michael

Yes,it's not safe to unlink the path before set the pointer to NULL.I will fix it.

BTW.is need to return error when failed to unlink?May be not need?
>>
>>>> +	}
>>>> +
>>>> +	RTE_LOG(INFO, EAL, "unlink %u hugepage files\n", nr_hugefiles);
>>>> +	return ret;
>>>> +}
>>>> +
>>>>  static int
>>>>  rte_eal_memdevice_init(void)
>>>>  {
>>>
>>> .
>>>
> 
> 
> .
> 

-- 
Regards,
Haifeng

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

* Re: [dpdk-dev] [PATCH v2] support free hugepages
  2014-10-29  5:47 [dpdk-dev] [PATCH v2] support free hugepages linhaifeng
  2014-10-29  6:14 ` Qiu, Michael
@ 2014-10-30  3:17 ` Matthew Hall
  2014-10-30 15:32 ` Thomas Monjalon
  2 siblings, 0 replies; 7+ messages in thread
From: Matthew Hall @ 2014-10-30  3:17 UTC (permalink / raw)
  To: linhaifeng; +Cc: dev

On Wed, Oct 29, 2014 at 01:47:39PM +0800, linhaifeng wrote:
> +int
> +rte_eal_hugepage_free(void)
> +{
> +	struct hugepage_file *hugepg_tbl = g_hugepage_table.hugepg_tbl;
> +	unsigned i;
> +	unsigned nr_hugefiles = g_hugepage_table.nr_hugefiles;
> +	int ret = 0;
> +
> +	for (i = 0; i < nr_hugefiles; i++) {
> +		ret = unlink(hugepg_tbl[i].filepath);
> +		if (ret != 0) {
> +			RTE_LOG(ERR, EAL, "Failed to unlink %s", hugepg_tbl[i].filepath);
> +			return ret;

I would say, don't exit just because one couldn't be freed. Free everything 
you can but exit with an error so people can know what happened.

Thanks for the patch, good idea!

Matthew.

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

* Re: [dpdk-dev] [PATCH v2] support free hugepages
  2014-10-29  5:47 [dpdk-dev] [PATCH v2] support free hugepages linhaifeng
  2014-10-29  6:14 ` Qiu, Michael
  2014-10-30  3:17 ` Matthew Hall
@ 2014-10-30 15:32 ` Thomas Monjalon
  2 siblings, 0 replies; 7+ messages in thread
From: Thomas Monjalon @ 2014-10-30 15:32 UTC (permalink / raw)
  To: linhaifeng; +Cc: dev

2014-10-29 13:47, linhaifeng:
> rte_eal_hugepage_free() is used for unlink all hugepages.If you want to
> free all hugepages you must make sure that you have stop to use it,and you
> must call this function before exit process.
> 
> Signed-off-by: linhaifeng <haifeng.lin@huawei.com>

Thanks for raising the need.
There is a discussion in the thread of the first version of this patch.
Neil suggests an automatic clean-up with hugepage refcounting.
It seems to be a good idea.

So this patch won't be integrated as is.

-- 
Thomas

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

end of thread, other threads:[~2014-10-30 15:23 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-29  5:47 [dpdk-dev] [PATCH v2] support free hugepages linhaifeng
2014-10-29  6:14 ` Qiu, Michael
2014-10-29  6:40   ` Linhaifeng
2014-10-29  8:04     ` Qiu, Michael
2014-10-29  8:15       ` Linhaifeng
2014-10-30  3:17 ` Matthew Hall
2014-10-30 15:32 ` Thomas Monjalon

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