Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 16 Nov 2022 16:25:33 +0800
From:   Aaron Lu <aaron.lu@intel.com>
To:     Vincent Guittot <vincent.guittot@linaro.org>
CC:     Joel Fernandes <joel@joelfernandes.org>, <mingo@redhat.com>,
        <peterz@infradead.org>, <juri.lelli@redhat.com>,
        <dietmar.eggemann@arm.com>, <rostedt@goodmis.org>,
        <bsegall@google.com>, <mgorman@suse.de>, <bristot@redhat.com>,
        <vschneid@redhat.com>, <linux-kernel@vger.kernel.org>,
        <parth@linux.ibm.com>, <qyousef@layalina.io>,
        <chris.hyser@oracle.com>, <patrick.bellasi@matbug.net>,
        <David.Laight@aculab.com>, <pjt@google.com>, <pavel@ucw.cz>,
        <tj@kernel.org>, <qperret@google.com>,
        <tim.c.chen@linux.intel.com>, <joshdon@google.com>, <timj@gnu.org>,
        <kprateek.nayak@amd.com>, <yu.c.chen@intel.com>,
        <youssefesmat@chromium.org>, <riel@redhat.com>
Subject: Re: [PATCH v8 1/9] sched/fair: fix unfairness at wakeup
Message-ID: <Y3SefaraLvX2hPYj@ziqianlu-desk2>
References: <20221110175009.18458-1-vincent.guittot@linaro.org>
 <20221110175009.18458-2-vincent.guittot@linaro.org>
 <Y3Gwr2p5BcofuZ8e@google.com>
 <CAKfTPtD2A3QaMON_jkhaobY0Wy+LpnAnfhtuaozK_OkmSvuf8Q@mail.gmail.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <CAKfTPtD2A3QaMON_jkhaobY0Wy+LpnAnfhtuaozK_OkmSvuf8Q@mail.gmail.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?bimFnC8BjtrTkkjXCpzfUvttEu/IT/CNJxx2W8HDs5bkhv1AsozujqFSAJX8?=
 =?us-ascii?Q?DWDHuhK6LV6EyNyZ6VTE3fNTp0TyJ7Jj9CFQFj4gqaLEwK5+avLTbcYJpbyc?=
 =?us-ascii?Q?9OBHmmIUDGDz76CLiklh6Seokufdj0+jqmFHLxKIuPnGuS0FJikMnsGJ8v1p?=
 =?us-ascii?Q?in2FQ51bOsUM6j1/RsQn7RTHgAZWifGdb3Kz68wO+c7uNin5UyWKcBK1U+31?=
 =?us-ascii?Q?wSx9bzIFg7tiW83fCw8rM/wlMVsIHpBlih/49yXtGKC5R02u2gGSG4eVH/t7?=
 =?us-ascii?Q?I6VJqRcYeyPtm2Oedjm1wYhGUgulx3XwdKfTsN8g5foHeU6V7qwtN5+cppY/?=
 =?us-ascii?Q?6EU2mFk9kon6bBxNG49KzGmrcO6NMImIEuAAPTamA0N5mcbBTTdIvFY/D23q?=
 =?us-ascii?Q?BBlSUgtR6RhQMCzdsJRXxdzSA99k6PKpN8xwfA1BOA+phKewr7QUFXLoyQHh?=
 =?us-ascii?Q?eTQ87QKhzxDsY+7E1HB+z0pMRg/vjXpDO5E1s2ulAPvM4es/bNg24U7WrOCZ?=
 =?us-ascii?Q?2qfEl7Q9Ze1hmMEpqajv0Veby1LJ9fFQ6YjGICxbWn9J8AVElSrVfTuD/HUr?=
 =?us-ascii?Q?aY+vJBrpHorgaNxReWTMxcCRuvAYecekCnIPo14tIA3w5hz0aLfpWCQ6lidr?=
 =?us-ascii?Q?NfybuuBTZ5oC0wX8bQ67tLr8qsfKdK2ob/+YSlz0/8xVzUD6odblaHfjIBNC?=
 =?us-ascii?Q?Lx9OyXF4xssNrZLYBS90Qm8fW2tIwAMuoxwC8qQw+OhT5IU3D3MLI8wpaqgI?=
 =?us-ascii?Q?8db+R/B/NJ9Z68Q2Lj9NxvPnSjWqye3mAwF/MlO4PPP9VBQcnZskdkMAEYEe?=
 =?us-ascii?Q?EnGUY4pnqVQubtG/+js45CUJLHCGmOO2kSFn6gmMUHRoDTP05xzqR4aPOtVN?=
 =?us-ascii?Q?rS61Cy+XnAeWAHNvVDP3t4VZri4iOOAI91YiQcixWQA/YoXNcR24kKOsgegi?=
 =?us-ascii?Q?iXM86U2NC+d472vAvg2U9TI/ZsxCODVGPE3FVQcWNCL3nX7FzXpyY09QTSwp?=
 =?us-ascii?Q?unfRJSv2BzlsuB2CdZPql6Pi5Q0RJBRIaY4JHAAymy0+zb1n5xQNgNyZY+/s?=
 =?us-ascii?Q?gqzNUGbgkKroiX54MM/9EohbEXTZvd0FQuBcV8AOg3RJFANNpD/SVaSEAhHh?=
 =?us-ascii?Q?xFrsqARZ0AJWCHF4w6lTSgKrV/9RQOITaTBcDQAC+bKR3pr/bC+HVYHXCR5X?=
 =?us-ascii?Q?g3K55jpDJDCT5M083A+TQIlWQOkSSA8I163B7I0jnmKyP6bmejQ0BY0A9nDN?=
 =?us-ascii?Q?X2IPjh3H3IRTqUJX+t8RHkSRKGC/nLrtnNa/rSD1JATd9hx5zWP/37LMBC+a?=
 =?us-ascii?Q?yItCLUv8LI8VmEX1IIbfyYeCOYCtxZrUFpncJ4mAJ+wPv680xechZSlM5brO?=
 =?us-ascii?Q?OLlK99XHSu3qQIl7/2Kl2Y8a/i/QLwwZ1QkyvDZdzA8aQYdjpkiW1y000R/l?=
 =?us-ascii?Q?AJrxU5LS4xxXIqZ+PrSigUz1H+Lw2K0sx6b3C9P2gjCnv4Pl13Nhmxlx4fQy?=
 =?us-ascii?Q?YsRIE6IP9ATjTiOVgPUji/rmT2kZfXgjuT8uSUR4UOatx44ZE2OhYhBlRjQ8?=
 =?us-ascii?Q?DszKES3ipl8BYfJ4BYJ2SNIW3XyAZIzf6sHayuET?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 343e5341-1706-4139-6df2-08dac7ac2cb6
X-MS-Exchange-CrossTenant-AuthSource: BL0PR11MB3060.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Nov 2022 08:25:53.0426
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: ageGMT7U5QLtYdLeU1gwSb9Jv3F9MlXQxBMj87235Kp0o1thpSzNBfqGvjRHdimLsh3ySQ5hDuiFwQV7+Ot3NQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR11MB7245
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        SPF_HELO_NONE,SPF_NONE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Mon, Nov 14, 2022 at 12:05:18PM +0100, Vincent Guittot wrote:
> On Mon, 14 Nov 2022 at 04:06, Joel Fernandes <joel@joelfernandes.org> wrote:
> >
> > Hi Vincent,
> >
> > On Thu, Nov 10, 2022 at 06:50:01PM +0100, Vincent Guittot wrote:

... ...

> > > +static inline unsigned long  get_latency_max(void)
> > > +{
> > > +     unsigned long thresh = get_sched_latency(false);
> > > +
> > > +     thresh -= sysctl_sched_min_granularity;
> >
> > Could you clarify, why are you subtracting sched_min_granularity here? Could
> > you add some comments here to make it clear?
> 
> If the waking task failed to preempt current it could to wait up to
> sysctl_sched_min_granularity before preempting it during next tick.

check_preempt_tick() compares vdiff/delta between the leftmost se and
curr against curr's ideal_runtime, it doesn't use thresh here or the
adjusted wakeup_gran, so I don't see why reducing thresh here can help
se to preempt curr during next tick if it failed to preempt curr in its
wakeup path.

I can see reducing thresh here with whatever value can help the waking
se to preempt curr in wakeup_preempt_entity() though, because most
likely the waking se's vruntime is cfs_rq->min_vruntime -
sysctl_sched_latency/2 and curr->vruntime is near cfs_rq->min_vruntime
so vdiff is about sysctl_sched_latency/2, which is the same value as
get_sched_latency(false) and when thresh is reduced some bit, then vdiff
in wakeup_preempt_entity() will be larger than gran and make it possible
to preempt.

So I'm confused by your comment or I might misread the code.