[09/14] eal: close mem config on cleanup
diff mbox series

Message ID 20200104013341.19809-10-stephen@networkplumber.org
State Superseded, archived
Delegated to: David Marchand
Headers show
Series
  • cleanup resources on shutdown
Related show

Checks

Context Check Description
ci/Intel-compilation fail Compilation issues
ci/checkpatch success coding style OK

Commit Message

Stephen Hemminger Jan. 4, 2020, 1:33 a.m. UTC
Resolves file descriptor left open after rte_eal_cleanup.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_eal/linux/eal/eal.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Burakov, Anatoly April 27, 2020, 12:12 p.m. UTC | #1
On 04-Jan-20 1:33 AM, Stephen Hemminger wrote:
> Resolves file descriptor left open after rte_eal_cleanup.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>   lib/librte_eal/linux/eal/eal.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
> index 9ad81378f23c..e5c2a24322e9 100644
> --- a/lib/librte_eal/linux/eal/eal.c
> +++ b/lib/librte_eal/linux/eal/eal.c
> @@ -1346,6 +1346,12 @@ rte_eal_cleanup(void)
>   	rte_mp_channel_cleanup();
>   	eal_cleanup_config(&internal_config);
>   	rte_eal_log_cleanup();
> +
> +	if (mem_cfg_fd != -1) {
> +		close(mem_cfg_fd);
> +		mem_cfg_fd = -1;
> +	}
> +
>   	return 0;
>   }
>   
> 

For the patch,

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

However i think it's incomplete, as there are also memory-backing 
fbarrays that are still mapped. Also, secondary processes have their own 
shadow copies of the master page table located in the mem config, so 
those should be destroyed on cleanup too.
Stephen Hemminger April 27, 2020, 5 p.m. UTC | #2
On Mon, 27 Apr 2020 13:12:32 +0100
"Burakov, Anatoly" <anatoly.burakov@intel.com> wrote:

> On 04-Jan-20 1:33 AM, Stephen Hemminger wrote:
> > Resolves file descriptor left open after rte_eal_cleanup.
> > 
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
> >   lib/librte_eal/linux/eal/eal.c | 6 ++++++
> >   1 file changed, 6 insertions(+)
> > 
> > diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
> > index 9ad81378f23c..e5c2a24322e9 100644
> > --- a/lib/librte_eal/linux/eal/eal.c
> > +++ b/lib/librte_eal/linux/eal/eal.c
> > @@ -1346,6 +1346,12 @@ rte_eal_cleanup(void)
> >   	rte_mp_channel_cleanup();
> >   	eal_cleanup_config(&internal_config);
> >   	rte_eal_log_cleanup();
> > +
> > +	if (mem_cfg_fd != -1) {
> > +		close(mem_cfg_fd);
> > +		mem_cfg_fd = -1;
> > +	}
> > +
> >   	return 0;
> >   }
> >   
> >   
> 
> For the patch,
> 
> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
> 
> However i think it's incomplete, as there are also memory-backing 
> fbarrays that are still mapped. Also, secondary processes have their own 
> shadow copies of the master page table located in the mem config, so 
> those should be destroyed on cleanup too.
> 

This patch set was targeting things in stages. It is not complete, some of the
cleanups would be hard to do. Just getting the obvious things first.
Burakov, Anatoly April 28, 2020, 9:20 a.m. UTC | #3
On 27-Apr-20 6:00 PM, Stephen Hemminger wrote:
> On Mon, 27 Apr 2020 13:12:32 +0100
> "Burakov, Anatoly" <anatoly.burakov@intel.com> wrote:
> 
>> On 04-Jan-20 1:33 AM, Stephen Hemminger wrote:
>>> Resolves file descriptor left open after rte_eal_cleanup.
>>>
>>> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>>> ---
>>>    lib/librte_eal/linux/eal/eal.c | 6 ++++++
>>>    1 file changed, 6 insertions(+)
>>>
>>> diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
>>> index 9ad81378f23c..e5c2a24322e9 100644
>>> --- a/lib/librte_eal/linux/eal/eal.c
>>> +++ b/lib/librte_eal/linux/eal/eal.c
>>> @@ -1346,6 +1346,12 @@ rte_eal_cleanup(void)
>>>    	rte_mp_channel_cleanup();
>>>    	eal_cleanup_config(&internal_config);
>>>    	rte_eal_log_cleanup();
>>> +
>>> +	if (mem_cfg_fd != -1) {
>>> +		close(mem_cfg_fd);
>>> +		mem_cfg_fd = -1;
>>> +	}
>>> +
>>>    	return 0;
>>>    }
>>>    
>>>    
>>
>> For the patch,
>>
>> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
>>
>> However i think it's incomplete, as there are also memory-backing
>> fbarrays that are still mapped. Also, secondary processes have their own
>> shadow copies of the master page table located in the mem config, so
>> those should be destroyed on cleanup too.
>>
> 
> This patch set was targeting things in stages. It is not complete, some of the
> cleanups would be hard to do. Just getting the obvious things first.
> 

Fair enough. You still got my ack :)

Patch
diff mbox series

diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
index 9ad81378f23c..e5c2a24322e9 100644
--- a/lib/librte_eal/linux/eal/eal.c
+++ b/lib/librte_eal/linux/eal/eal.c
@@ -1346,6 +1346,12 @@  rte_eal_cleanup(void)
 	rte_mp_channel_cleanup();
 	eal_cleanup_config(&internal_config);
 	rte_eal_log_cleanup();
+
+	if (mem_cfg_fd != -1) {
+		close(mem_cfg_fd);
+		mem_cfg_fd = -1;
+	}
+
 	return 0;
 }