From: Jerin Jacob <jerinjacobk@gmail.com> To: "Burakov, Anatoly" <anatoly.burakov@intel.com> Cc: Harman Kalra <hkalra@marvell.com>, dpdk-dev <dev@dpdk.org>, David Hunt <david.hunt@intel.com>, Liang Ma <liang.j.ma@intel.com>, Reshma Pattan <reshma.pattan@intel.com> Subject: Re: [dpdk-dev] [EXT] Re: [PATCH 3/3] l3fwd-power: add interrupt-only mode Date: Mon, 15 Jun 2020 17:13:54 +0530 Message-ID: <CALBAE1OE6DGu7Gnh-ydfnnuOMZODznJKHbkz=aUzh5vKk7GTbA@mail.gmail.com> (raw) In-Reply-To: <0dff9790-c57f-7912-8ae3-7ee67f846d3d@intel.com> On Mon, Jun 15, 2020 at 5:02 PM Burakov, Anatoly <anatoly.burakov@intel.com> wrote: > > On 02-Jun-20 1:16 PM, Harman Kalra wrote: > > On Tue, Jun 02, 2020 at 03:53:07PM +0530, Harman Kalra wrote: > >> On Mon, Jun 01, 2020 at 01:50:26PM +0100, Burakov, Anatoly wrote: > >>> On 30-May-20 11:02 AM, Harman Kalra wrote: > >>>> On Fri, May 29, 2020 at 03:19:45PM +0100, Burakov, Anatoly wrote: > >>>>> External Email > >>>>> > >>>>> ---------------------------------------------------------------------- > >>>>> On 29-May-20 2:19 PM, Harman Kalra wrote: > >>>>> > >>>>>>> if (ret < 0) > >>>>>>> rte_exit(EXIT_FAILURE, "Invalid L3FWD parameters\n"); > >>>>>>> - if (app_mode != APP_MODE_TELEMETRY && init_power_library()) > >>>>>>> + if (app_mode == APP_MODE_DEFAULT) > >>>>>>> + app_mode = APP_MODE_LEGACY; > >>>>>>> + > >>>>>>> + /* only legacy and empty poll mode rely on power library */ > >>>>>>> + if ((app_mode == APP_MODE_LEGACY || app_mode == APP_MODE_EMPTY_POLL) && > >>>>>>> + init_power_library()) > >>>>>>> rte_exit(EXIT_FAILURE, "init_power_library failed\n"); > >>>>>> Hi, > >>>>>> > >>>>>> Rather than just exiting from here can we have a else condition to > >>>>>> automatically enter into the "interrupt only" mode. > >>>>>> Please correct me if I am missing something. > >>>>> > >>>>> Hi, > >>>>> > >>>>> Thanks for your review. I don't think silently proceeding is a good idea. If > >>>>> the user wants interrupt-only mode, they should request it. Silently falling > >>>>> back to interrupt-only mode will create an illusion of successful > >>>>> initialization and set the wrong expectation for how the application will > >>>>> behave. > >>>>> > >>>> > >>>> Hi, > >>>> > >>>> Thanks for the explanation which even I also believe is logically perfect. > >>>> > >>>> But since l3fwd-power is an old application and has many users around > >>>> which are currently using this app in interrupt only mode without giving > >>>> an extra argument. But suddenly they will start getting failure messages with > >>>> the new patchset. > >>>> > >>>> My only intent with else condition was backward compatibility. > >>>> Or may be we can have more descriptive failure message, something like > >>>> "init_power_library failed, check manual for other modes". > >>>> > >>>> Thanks > >>>> Harman > >>>> > >>>> > >>> > >>> I think we can compormise on an informative log message suggesting to use > >>> interrupt mode. I'm not keen on reverting to previous buggy behavior :) > >>> > >> Hi > >> > >> I am not insisting to revert to previous behavior, I am just trying to > >> highlight some probable issues that many users might face as its an old > >> application. > >> Since many arm based soc might not be supporting frequency scaling, can > >> we add the following check as soon as the application starts, probe the > >> platform if it supports frequency scaling, if not automatically set the > >> mode to interrupt mode, something like: > >> if (access("/sys/devices/system/cpu/cpu0/cpufreq/scaling_governor", > >> F_OK)) > >> app_mode = APP_MODE_INTERRUPT; > > > > Sorry, no direct check in application but we can introduce a new API in > > power library: > > bool rte_is_freq_scaling() { > > return access("/sys/devices/system/cpu/cpu0/cpufreq/scaling_governor", > > F_OK); > > } > > > > and in the application we can use "rte_is_freq_scaling()" at the start. > > > > What you're suggesting here is effectively what you have already > suggested: silently fall back to interrupt-only mode if power lib init > failed. I already outlined why i don't think it's a good approach. Is probing "/sys/devices/system/cpu/cpu0/cpufreq/scaling_governor" file presence, detects the power lib availability . Right? Not the failure. Right? IMO, it make sense to have following case: # first check, Is the system is capable of power lib if so use power lib # if the system is not capable of using power lib use interrupt mode. I think, there is difference between system capable of using power lib vs power lib not available or power lib failure. > > >> > >> > >> Thanks > >> Harman > >> > >>>>> -- > >>>>> Thanks, > >>>>> Anatoly > >>> > >>> > >>> -- > >>> Thanks, > >>> Anatoly > > > -- > Thanks, > Anatoly
next prev parent reply other threads:[~2020-06-15 11:44 UTC|newest] Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-05-28 9:13 [dpdk-dev] [PATCH 0/3] Add interrupt-only mode to l3fwd-power Anatoly Burakov 2020-05-28 9:13 ` [dpdk-dev] [PATCH 1/3] l3fwd-power: disable interrupts by default Anatoly Burakov 2020-05-28 9:13 ` [dpdk-dev] [PATCH 2/3] l3fwd-power: only allow supported power library envs Anatoly Burakov 2020-05-28 9:13 ` [dpdk-dev] [PATCH 3/3] l3fwd-power: add interrupt-only mode Anatoly Burakov 2020-05-29 13:19 ` Harman Kalra 2020-05-29 14:19 ` Burakov, Anatoly 2020-05-30 10:02 ` [dpdk-dev] [EXT] " Harman Kalra 2020-06-01 12:50 ` Burakov, Anatoly 2020-06-02 10:23 ` Harman Kalra 2020-06-02 12:16 ` Harman Kalra 2020-06-15 11:31 ` Burakov, Anatoly 2020-06-15 11:43 ` Jerin Jacob [this message] 2020-06-15 15:05 ` Burakov, Anatoly 2020-06-15 15:21 ` Jerin Jacob 2020-06-15 15:45 ` Burakov, Anatoly 2020-06-15 16:29 ` Jerin Jacob 2020-06-16 9:31 ` Burakov, Anatoly 2020-06-16 17:09 ` Jerin Jacob 2020-06-08 1:24 ` [dpdk-dev] [PATCH 0/3] Add interrupt-only mode to l3fwd-power Wang, Yinan 2020-06-18 17:18 ` [dpdk-dev] [PATCH v2 0/7] " Anatoly Burakov 2020-06-19 10:53 ` [dpdk-dev] [PATCH v3 " Anatoly Burakov 2020-07-11 11:35 ` Thomas Monjalon 2020-06-19 10:53 ` [dpdk-dev] [PATCH v3 1/7] l3fwd-power: disable interrupts by default Anatoly Burakov 2020-06-19 10:53 ` [dpdk-dev] [PATCH v3 2/7] l3fwd-power: only allow supported power library envs Anatoly Burakov 2020-06-19 10:53 ` [dpdk-dev] [PATCH v3 3/7] l3fwd-power: code style and flow fixes Anatoly Burakov 2020-06-19 10:53 ` [dpdk-dev] [PATCH v3 4/7] l3fwd-power: add support for requesting legacy mode Anatoly Burakov 2020-06-19 10:53 ` [dpdk-dev] [PATCH v3 5/7] l3fwd-power: add interrupt-only mode Anatoly Burakov 2020-06-19 10:53 ` [dpdk-dev] [PATCH v3 6/7] power: add API to probe support for a specific env Anatoly Burakov 2020-06-19 10:53 ` [dpdk-dev] [PATCH v3 7/7] l3fwd-power: add auto-selection of default mode Anatoly Burakov 2020-07-11 10:07 ` Thomas Monjalon 2020-06-18 17:18 ` [dpdk-dev] [PATCH v2 1/7] l3fwd-power: disable interrupts by default Anatoly Burakov 2020-06-18 17:18 ` [dpdk-dev] [PATCH v2 2/7] l3fwd-power: only allow supported power library envs Anatoly Burakov 2020-06-18 17:18 ` [dpdk-dev] [PATCH v2 3/7] l3fwd-power: code style and flow fixes Anatoly Burakov 2020-06-18 17:18 ` [dpdk-dev] [PATCH v2 4/7] l3fwd-power: add support for requesting legacy mode Anatoly Burakov 2020-06-18 17:18 ` [dpdk-dev] [PATCH v2 5/7] l3fwd-power: add interrupt-only mode Anatoly Burakov 2020-06-18 17:18 ` [dpdk-dev] [PATCH v2 6/7] power: add API to probe support for a specific env Anatoly Burakov 2020-06-18 17:18 ` [dpdk-dev] [PATCH v2 7/7] l3fwd-power: add auto-selection of default mode Anatoly Burakov 2020-06-19 7:37 ` [dpdk-dev] [EXT] " Harman Kalra 2020-06-19 9:56 ` 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='CALBAE1OE6DGu7Gnh-ydfnnuOMZODznJKHbkz=aUzh5vKk7GTbA@mail.gmail.com' \ --to=jerinjacobk@gmail.com \ --cc=anatoly.burakov@intel.com \ --cc=david.hunt@intel.com \ --cc=dev@dpdk.org \ --cc=hkalra@marvell.com \ --cc=liang.j.ma@intel.com \ --cc=reshma.pattan@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
DPDK patches and discussions This inbox may be cloned and mirrored by anyone: git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \ dev@dpdk.org public-inbox-index dev Example config snippet for mirrors. Newsgroup available over NNTP: nntp://inbox.dpdk.org/inbox.dpdk.dev AGPL code for this site: git clone https://public-inbox.org/public-inbox.git