From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 5A58CA0093; Mon, 15 Jun 2020 13:44:15 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id D2C2E2B94; Mon, 15 Jun 2020 13:44:13 +0200 (CEST) Received: from mail-io1-f68.google.com (mail-io1-f68.google.com [209.85.166.68]) by dpdk.org (Postfix) with ESMTP id DED9725B3 for ; Mon, 15 Jun 2020 13:44:11 +0200 (CEST) Received: by mail-io1-f68.google.com with SMTP id x189so8359136iof.9 for ; Mon, 15 Jun 2020 04:44:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=btHvZZUhOKez9rsuJJV9TS7OFSx79ckk3Exeex8pWSI=; b=ogbR3OmEr3Vv/2GHIkQw5czRLalaqHX1jpJtxylatV8d3YJf6vIINZTl3wka2+DJvv jU9mapu9P/xTuLv09uklDPOqGqrwPg24a2ODPmX1jiMyC7O2qfTq8DK/tWgfGXJOpeHl pwnUxCEe+ozbaVUut3XTSiBfPlyL0tQ4BJl3g/ShzAky9Eq1ZURJAnLCdvfQKO6QlCfa nnWQci2XR0iLb3QXuMunmf/inEygVas3iYCFQy+dnqLwzKXoYY9KXognyZ1yk31NPBfw NSp0EEaUiRrQPMfY+NBFJEzw+py6rR5g+1mSmu4hsoiLom2f+uhbTsBrTOapAPSFgHaN EZ4Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=btHvZZUhOKez9rsuJJV9TS7OFSx79ckk3Exeex8pWSI=; b=N5MOMcWXQ2L+GyaCS8659x6lSwTKk3VK4PkggTPz6R9v23iwMa+0pDlCt6HQh+ccgK YUhqNl1sV+aW/fS2NVwcpSe+/L7p9RxAxTN2szpI652Mt+CBiovqeBaBOcM0M3K2fdHE aNFv9F/s4zJqMyV3B67udI7BN8IDIsg+t+Id9dbELM9yQD9jZh440EN1Odywb6ueyDZ2 Lfzk2SKh331WwdkNGldwjgf5rhFGmoXff8mty7bNy27Q21bw6opJ2khyiuwCMdrJxNNc U7sKcup0ka9oKKFL46HopPQQYMTYTSYh0OpA3sMhvsU51d+dZMsbeymLiUOoG7Ry7le+ o7iw== X-Gm-Message-State: AOAM530i4FrZULGgVrf/eTieIlf7QQRhov5583GMla5Et1dTdAWb1Ft1 +5nYDjr2FKgNccN23c/Tg20r8KuwjSOf35Z+/A4= X-Google-Smtp-Source: ABdhPJwjlhUGXs47EfTtyCe5OHxGCf3kDmHwRaT7sfpm7RSP4TwmmZ87eWjH5PvsKInHy+HY7GoA5fMrs+fSi+zXimY= X-Received: by 2002:a05:6602:2001:: with SMTP id y1mr26158461iod.94.1592221451078; Mon, 15 Jun 2020 04:44:11 -0700 (PDT) MIME-Version: 1.0 References: <20200529131955.GA83122@outlook.office365.com> <660267ae-00a0-bb55-fbc3-9ac1473957ea@intel.com> <20200530100228.GA24648@outlook.office365.com> <20200602102259.GA54715@outlook.office365.com> <20200602121654.GA106816@outlook.office365.com> <0dff9790-c57f-7912-8ae3-7ee67f846d3d@intel.com> In-Reply-To: <0dff9790-c57f-7912-8ae3-7ee67f846d3d@intel.com> From: Jerin Jacob Date: Mon, 15 Jun 2020 17:13:54 +0530 Message-ID: To: "Burakov, Anatoly" Cc: Harman Kalra , dpdk-dev , David Hunt , Liang Ma , Reshma Pattan Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [EXT] Re: [PATCH 3/3] l3fwd-power: add interrupt-only mode X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Mon, Jun 15, 2020 at 5:02 PM Burakov, Anatoly 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