Re: [dpdk-dev] [PATCH v2] support free hugepages

["Qiu, Michael" <michael.qiu@intel.com>]

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