[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] credit2: make sure we pick a runnable unit from the runq if there is one
- To: Dario Faggioli <dfaggioli@xxxxxxxx>
- From: George Dunlap <George.Dunlap@xxxxxxxxxx>
- Date: Mon, 7 Jun 2021 12:10:24 +0000
- Accept-language: en-US
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
- Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=lty+9wvpX5M+uoA0u8g2Kgx9pOB3SNCum4VbbJTJl3E=; b=ELdNMEdOLBUfDC+cJOpO5kNLGqcfU8/8yaroV7D4no3zDnjEzwod9IUbh1b2TQRcnn6FW4Zs4Nicm4szOFlnbSiezMKShMTBMtDtHeBvDVP7vUUP5FXo0CBBVWrbpkphaHB5PxvdhR18UnW+PC2bAx5n3wqVbTXGFdfSclvwSOU6EI/bgurtcnVg0cMN2KxQuMDxU0iMv5CU1FNfNP3g9SCjHZC0hHWbauTUVL2O9RwJeP4CAa1Bb1tNrorj7NeLQMd8BQer/lQ+mUVNFLfVm+xf/IhoADNFh4eahScERodbUXPfkHo83BI5NyMx7/8+OthoXEjwOuElqsx45AlgaQ==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=gnuRjpSHYO7iVRdfPmwLHtSTHUHAxUEcuWjiHRnEiySSvLE8ZELrnxCgOro1B35MgW3m/VjwcyKKJ7nta76ULsSc6FIIj5SuB8f+iUeDpl8NigtIZZd3LZQSTdm75x9CGUqoxZGJ6Ap/nPU+oaeXql1ljjY1HDdtew2uMY5L3JJf3RBJCH5a+pZIOWw9KTG0Y0Srjv9hA65MvRG7GXmhGIf19gxPyAqyL/2gxAlvOWyurRgIE8997rQVVEbd00chRy5WrofmkxUJJgZox6R0tZWJKszxBC87K/etI81hk0lo/TwprHT6EGR8iaWq3lOaxTbvyx2UxJE2ywnN/Uhr1g==
- Authentication-results: esa1.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
- Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Michał Leszczyński <michal.leszczynski@xxxxxxx>, "Dion Kant" <g.w.kant@xxxxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>
- Delivery-date: Mon, 07 Jun 2021 12:10:44 +0000
- Ironport-hdrordr: A9a23:OuCCI6q0I0ZEopa6o8zrB3gaV5obeYIsimQD101hICG9Ffb1qy nOppsmPHrP4wr5N0tPpTntAsi9qBHnhP1ICPgqXYtKNTOO0AHEEGgF1/qG/9SKIVydygcy79 YHT4FOTPH2EFhmnYLbzWCDYq8dKQC8gcSVbDHlvhBQcT0=
- Ironport-sdr: Tn4tcp6KGSrHmthdplQFfZ8Jrd64hNGWDx2gM814EtlZ8bM6FH1RXGGWyAZq6zsfUoEL/eiCGZ VuojP5O9YSUtJLEmIZG529FESpycmqZ3g7vTe/OZ4Qta61Gm+hlVQwk9DgpUXsNZo6URKkL5MZ RqBQzYJhw4NhhLUcy/+gEC/Rv6jJlSN9BRXtCKQ6pAm45z8glgXpibBV7t+bb46/DALVbaN/KL CYjtjVvyg70QVi4PRBV0givcviF+tpP/6mN/aFPKzQIhj/rmfYwNMBcb9TjgRxvnb+N96Y4HG6 h1Y=
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
- Thread-index: AQHXU9QMSaNdW1us1k+tzXl6c+sgY6sIhIeA
- Thread-topic: [PATCH] credit2: make sure we pick a runnable unit from the runq if there is one
> On May 28, 2021, at 4:12 PM, Dario Faggioli <dfaggioli@xxxxxxxx> wrote:
>
> A !runnable unit (temporarily) present in the runq may cause us to
> stop scanning the runq itself too early. Of course, we don't run any
> non-runnable vCPUs, but we end the scan and we fallback to picking
> the idle unit. In other word, this prevent us to find there and pick
> the actual unit that we're meant to start running (which might be
> further ahead in the runq).
>
> Depending on the vCPU pinning configuration, this may lead to such
> unit to be stuck in the runq for long time, causing malfunctioning
> inside the guest.
>
> Fix this by checking runnable/non-runnable status up-front, in the runq
> scanning function.
>
> Reported-by: Michał Leszczyński <michal.leszczynski@xxxxxxx>
> Reported-by: Dion Kant <g.w.kant@xxxxxxxxxx>
> Signed-off-by: Dario Faggioli <dfaggioli@xxxxxxxx>
Thanks for tracking this down, Dario!
Reviewed-by: George Dunlap <george.dunlap@xxxxxxxxxx>
Just one comment:
> @@ -3496,8 +3500,7 @@ runq_candidate(struct csched2_runqueue_data *rqd,
> * some budget, then choose it.
> */
> if ( (yield || svc->credit > snext->credit) &&
> - (!has_cap(svc) || unit_grab_budget(svc)) &&
> - unit_runnable_state(svc->unit) )
> + (!has_cap(svc) || unit_grab_budget(svc)) )
> snext = svc;
By the same logic, shouldn’t we also move the `(!has_cap() …)` clause into a
separate `if(x) continue` clause? There may be runnable units further down the
queue which aren’t capped / haven’t exhausted their budget yet.
-George
|
