diff mbox series

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

Message ID	20200104013341.19809-10-stephen@networkplumber.org (mailing list archive)
State	Superseded, archived
Delegated to:	David Marchand
Headers	From: Stephen Hemminger <stephen@networkplumber.org> To: dev@dpdk.org Cc: Stephen Hemminger <stephen@networkplumber.org> Date: Fri, 3 Jan 2020 17:33:36 -0800 Message-Id: <20200104013341.19809-10-stephen@networkplumber.org> In-Reply-To: <20200104013341.19809-1-stephen@networkplumber.org> References: <20200104013341.19809-1-stephen@networkplumber.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [dpdk-dev] [PATCH 09/14] eal: close mem config on cleanup Precedence: list Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org>
Series	cleanup resources on shutdown \| [00/14] cleanup resources on shutdown [01/14] eal: log: close on cleanup [02/14] eal: log: free dynamic state on cleanup [03/14] eal: alarm: close timerfd on eal cleanup [04/14] eal: cleanup threads [05/14] eal: intr: cleanup resources [06/14] eal: mp: end the multiprocess thread during cleanup [07/14] eal: interrupts close epoll fd on shutdown [08/14] eal: vfio: cleanup the mp sync handle [09/14] eal: close mem config on cleanup [10/14] tap: close netlink socket on device close [11/14] eal: cleanup plugins data [12/14] ethdev: raise priority of old driver warning [13/14] eal: hotplug: cleanup multiprocess resources [14/14] eal: malloc: cleanup mp resources

Checks

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

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

diff mbox series

Patch

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;
 }