DPDK patches and discussions
 help / color / mirror / Atom feed
From: Suanming.Mou <mousuanming@huawei.com>
To: Thomas Monjalon <thomas@monjalon.net>
Cc: <dev@dpdk.org>, <vipin.varghese@intel.com>, <anatoly.burakov@intel.com>
Subject: Re: [dpdk-dev] [PATCH v8] app/pdump: add pudmp exits with primary support
Date: Wed, 8 May 2019 17:37:49 +0800	[thread overview]
Message-ID: <38d2212b-52ce-b4fd-0ad3-45bb21c389fb@huawei.com> (raw)
Message-ID: <20190508093749.-z1GMfrRYWR9RBxR22yE3goMsO396B6mjS-w1_JQmL8@z> (raw)
In-Reply-To: <1677555.97jmHZBzRH@xps>


On 2019/5/8 16:04, Thomas Monjalon wrote:
> Hi,
>
> I try to suggest some rewording below.

Thanks very much.

First,  let me explain what is the patch work for.

As we all know, the pdump tool works as the secondary process.

When the primary process exits, if the secondary process keeps running 
there, it will make the primary process can't start up again.

Since the ex-fbarry files are still attached by the secondary process 
pdump, the 'new' primary process can't get these files locked.

The patch is just to set up an alarm which runs every 0.5s periodically 
to monitor the primary process in the pdump.

Once the primary exits, so will the pdump.

>
> 03/05/2019 07:48, Suanming. Mou:
>> +/* Enough to set it to 500ms for exiting. */
>> +#define MONITOR_INTERVAL (500 * 1000)
> What is "enough"?

The 'enough' here means it will be fine to make the alarm to run 
periodically in every 500ms to check if the primary process exits.

(Yes, someone can also suggest, "Well , I think xxxms will be better.")

> What is "it"?

So, the "it" here means the MONITOR_INTERVAL...

> What is the relation between MONITOR_INTERVAL and exiting?

Once the primary exits, as the alarm runs every 500ms, the alarm will 
help the pdump to exit, the worst case will take 500ms for the pudmp to 
exit.

>
> [...]
>> +	/*
>> +	 * Don't worry about it is primary exit case. The alarm cancel
>> +	 * function will take care about that. Ignore the ENOENT case.
>> +	 */
> I don't understand the comment.
> Maybe you can explicit what is "it" and "that".
> It would be probably simpler if you just describe why you cancel
> this alarm.
> How is it related to ENOENT?

The 'it' and 'that' both means the pdump 'monitor_primary' recognises 
the primary process exited and set the 'quit_signal' case.

In that case, the 'monitor_primary' is not readd to the alarm. I add the 
comment in case someone want to say that "You are canceling a 
non-existent alarm."

It's OK to cancel a non-existent alarm. The 'rte_eal_alarm_cancel' just 
return 0 and set the ENOENT errno.

>
>> +	ret = rte_eal_alarm_cancel(monitor_primary, NULL);
>> +	if (ret < 0)
>> +		printf("Fail to disable monitor:%d\n", ret);
> [...]
>> -	/* create mempool, ring and vdevs info */
>> +	/* create mempool, ring, vdevs info and primary monitor */
> I don't see any value to this comment.
> You may just drop it.
That's OK.
>
>>   	create_mp_ring_vdev();
>>   	enable_pdump();
>> +	enable_primary_monitor();
>
>
>
And one more word, the 'app' in the git log  means 'application'.

Maybe it's better to change it to  'process' to make it describes more 
clearly.

Thanks again for the suggestions.

BR,

Mou



  parent reply	other threads:[~2019-05-08  9:38 UTC|newest]

Thread overview: 106+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-25 16:35 [dpdk-dev] [PATCH] app/pdump: exits once primary app exited Suanming.Mou
2019-04-25 15:51 ` Varghese, Vipin
2019-04-25 15:51   ` Varghese, Vipin
2019-04-26  7:11   ` Suanming.Mou
2019-04-26  7:11     ` Suanming.Mou
2019-04-26 10:54     ` Varghese, Vipin
2019-04-26 10:54       ` Varghese, Vipin
2019-04-26 10:56       ` Varghese, Vipin
2019-04-26 10:56         ` Varghese, Vipin
2019-04-26 12:08         ` Suanming.Mou
2019-04-26 12:08           ` Suanming.Mou
2019-04-26 13:46           ` Burakov, Anatoly
2019-04-26 13:46             ` Burakov, Anatoly
2019-04-26 14:32             ` Suanming.Mou
2019-04-26 14:32               ` Suanming.Mou
2019-04-26 14:39               ` Burakov, Anatoly
2019-04-26 14:39                 ` Burakov, Anatoly
2019-04-26 14:49                 ` Suanming.Mou
2019-04-26 14:49                   ` Suanming.Mou
2019-04-26 14:50                   ` Burakov, Anatoly
2019-04-26 14:50                     ` Burakov, Anatoly
2019-04-25 16:35 ` Suanming.Mou
2019-04-28  4:58 ` [dpdk-dev] [PATCH v2] app/pdump: add exit_with_primary option support Suanming.Mou
2019-04-28  4:58   ` Suanming.Mou
2019-04-28  5:07   ` [dpdk-dev] [PATCH v3] " Suanming.Mou
2019-04-28  5:07     ` Suanming.Mou
2019-04-30  3:39     ` [dpdk-dev] [PATCH v4] app/pdump: add pudmp exits with primary support Suanming.Mou
2019-04-30  2:33       ` Varghese, Vipin
2019-04-30  2:33         ` Varghese, Vipin
2019-04-30  3:43         ` Suanming.Mou
2019-04-30  3:43           ` Suanming.Mou
2019-04-30  5:03           ` Varghese, Vipin
2019-04-30  5:03             ` Varghese, Vipin
2019-04-30  9:34         ` Burakov, Anatoly
2019-04-30  9:34           ` Burakov, Anatoly
2019-04-30 10:37           ` Varghese, Vipin
2019-04-30 10:37             ` Varghese, Vipin
2019-04-30 16:38             ` Burakov, Anatoly
2019-04-30 16:38               ` Burakov, Anatoly
2019-04-30  3:39       ` Suanming.Mou
2019-04-30 11:35       ` [dpdk-dev] [PATCH v5] Make pdump exits with primary Suanming.Mou
2019-04-30 11:35         ` Suanming.Mou
2019-04-30 11:35         ` [dpdk-dev] [PATCH v5] app/pdump: add pudmp exits with primary support Suanming.Mou
2019-04-30  9:42           ` Burakov, Anatoly
2019-04-30  9:42             ` Burakov, Anatoly
2019-04-30 11:25             ` Suanming.Mou
2019-04-30 11:25               ` Suanming.Mou
2019-04-30 16:39               ` Burakov, Anatoly
2019-04-30 16:39                 ` Burakov, Anatoly
2019-05-02  3:07                 ` Suanming.Mou
2019-05-02  3:07                   ` Suanming.Mou
2019-04-30 11:35           ` Suanming.Mou
2019-04-30 12:44           ` Pattan, Reshma
2019-04-30 12:44             ` Pattan, Reshma
2019-05-02  5:20           ` [dpdk-dev] [PATCH v6] " Suanming.Mou
2019-05-02  5:20             ` Suanming.Mou
2019-05-02  8:04             ` Varghese, Vipin
2019-05-02  8:04               ` Varghese, Vipin
2019-05-02  8:32               ` Suanming.Mou
2019-05-02  8:32                 ` Suanming.Mou
2019-05-02  9:12                 ` Burakov, Anatoly
2019-05-02  9:12                   ` Burakov, Anatoly
2019-05-02  9:22                 ` Varghese, Vipin
2019-05-02  9:22                   ` Varghese, Vipin
2019-05-02  9:54             ` Pattan, Reshma
2019-05-02  9:54               ` Pattan, Reshma
2019-05-02 10:40               ` Suanming.Mou
2019-05-02 10:40                 ` Suanming.Mou
2019-05-02 12:35             ` [dpdk-dev] [PATCH v7] " Suanming.Mou
2019-05-02 11:03               ` Pattan, Reshma
2019-05-02 11:03                 ` Pattan, Reshma
2019-05-02 11:31               ` Burakov, Anatoly
2019-05-02 11:31                 ` Burakov, Anatoly
2019-05-02 12:35                 ` mousuanming
2019-05-02 12:35                   ` mousuanming
2019-05-02 12:35               ` Suanming.Mou
2019-05-03  5:48               ` [dpdk-dev] [PATCH v8] " Suanming.Mou
2019-05-03  5:48                 ` Suanming.Mou
2019-05-04 21:17                 ` Thomas Monjalon
2019-05-04 21:17                   ` Thomas Monjalon
2019-05-05  1:20                   ` Suanming.Mou
2019-05-05  1:20                     ` Suanming.Mou
2019-05-05  9:42                     ` Thomas Monjalon
2019-05-05  9:42                       ` Thomas Monjalon
2019-05-05 11:13                       ` Suanming.Mou
2019-05-05 11:13                         ` Suanming.Mou
2019-05-08  8:04                 ` Thomas Monjalon
2019-05-08  8:04                   ` Thomas Monjalon
2019-05-08  9:37                   ` Suanming.Mou [this message]
2019-05-08  9:37                     ` Suanming.Mou
2019-05-08 10:22                     ` Thomas Monjalon
2019-05-08 10:22                       ` Thomas Monjalon
2019-05-08 13:14                       ` Suanming.Mou
2019-05-08 13:14                         ` Suanming.Mou
2019-05-15  5:10                 ` [dpdk-dev] [PATCH v9] app/pdump: exit with primary process Suanming.Mou
2019-05-15  5:10                   ` Suanming.Mou
2019-06-20 12:32                   ` Pattan, Reshma
2019-06-23 22:30                     ` Thomas Monjalon
2019-07-10 14:04                       ` Suanming Mou
2019-07-10 22:28                         ` Thomas Monjalon
2019-04-29  9:14   ` [dpdk-dev] [PATCH v2] app/pdump: add exit_with_primary option support Burakov, Anatoly
2019-04-29  9:14     ` Burakov, Anatoly
2019-04-29  9:43     ` Suanming.Mou
2019-04-29  9:43       ` Suanming.Mou
2019-04-29 10:42       ` Burakov, Anatoly
2019-04-29 10:42         ` Burakov, Anatoly

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=38d2212b-52ce-b4fd-0ad3-45bb21c389fb@huawei.com \
    --to=mousuanming@huawei.com \
    --cc=anatoly.burakov@intel.com \
    --cc=dev@dpdk.org \
    --cc=thomas@monjalon.net \
    --cc=vipin.varghese@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).