From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 31505C4363A for ; Mon, 26 Oct 2020 10:43:40 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 9CFB020809 for ; Mon, 26 Oct 2020 10:43:39 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=umich.edu header.i=@umich.edu header.b="Pwy0BP9o" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 9CFB020809 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=umich.edu Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Received: from list by lists.xenproject.org with outflank-mailman.12208.31966 (Exim 4.92) (envelope-from ) id 1kWzyC-00073O-4E; Mon, 26 Oct 2020 10:43:20 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 12208.31966; Mon, 26 Oct 2020 10:43:20 +0000 X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1kWzyC-00073H-1H; Mon, 26 Oct 2020 10:43:20 +0000 Received: by outflank-mailman (input) for mailman id 12208; Mon, 26 Oct 2020 10:43:18 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1kWzy9-00073C-Uz for xen-devel@lists.xenproject.org; Mon, 26 Oct 2020 10:43:18 +0000 Received: from mail-ed1-x542.google.com (unknown [2a00:1450:4864:20::542]) by us1-rack-iad1.inumbo.com (Halon) with ESMTPS id 812f991b-620c-43dc-bbc8-4f0b037e5333; Mon, 26 Oct 2020 10:43:16 +0000 (UTC) Received: by mail-ed1-x542.google.com with SMTP id t20so8693990edr.11 for ; Mon, 26 Oct 2020 03:43:16 -0700 (PDT) Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1kWzy9-00073C-Uz for xen-devel@lists.xenproject.org; Mon, 26 Oct 2020 10:43:18 +0000 X-Inumbo-ID: 812f991b-620c-43dc-bbc8-4f0b037e5333 Received: from mail-ed1-x542.google.com (unknown [2a00:1450:4864:20::542]) by us1-rack-iad1.inumbo.com (Halon) with ESMTPS id 812f991b-620c-43dc-bbc8-4f0b037e5333; Mon, 26 Oct 2020 10:43:16 +0000 (UTC) Received: by mail-ed1-x542.google.com with SMTP id t20so8693990edr.11 for ; Mon, 26 Oct 2020 03:43:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=umich.edu; s=google-2016-06-03; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=GcXznKRACrGiaPsHMISkyH4VxsBYAdOZpQikx+ceUek=; b=Pwy0BP9osbbaeoprrRW3gjZE/Z2I3hXZ5nRJ3hCzCyOhQnwj7KfaFUHKOjHgs0HcMW xb+w6NNW8QDlAjOlVlT1AOclqe8dy4BvmH3jvxMln9PNSsbqi32pewQjACAtK3nsKxFO W1YMPxoo58Q5UXcCwAXvyiJCKcQC+vezk73eYQvzCvx2AvIKDNEoqpkB9muZeKWG1GkW bQmo38K5m11UwTV21xUFWZi3k76PbrJ67EeyPGDWPXlDO3Z7UHcmYD3dA4oe+joAfolF haupZsDw5Yg17X+qesV/yKhXIzipWnxiT2lMbFGspRLDxR+5y31/+dIIJUA5wwRIKazN oOWg== 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=GcXznKRACrGiaPsHMISkyH4VxsBYAdOZpQikx+ceUek=; b=OQR3qsDsn5Ty76LsYof0HEElmSgJ0fqq0TnlNck1Sr/p3+7b0aIuCDZL9TVpJltMOl Rx8uBJRo0IfTnKVLrba1nQ+OkfV9i804lg/5xjyQMfXAmyogWc9UORFkFeBpoCt1zkKl 1oimNE+MxqqPfY8S/nAn8FXaufiy88RS9RfJHvMPCU+W6R3aGQakJ+0UTCwK7Mt3ci2e LMgis1EUwHfhiaXQ57OkKKKo6WapdMzWbfyiBP9URquslx2E3QI2qi7xvAduZ0cKGP27 mCcbHPI8ZJJCyU5AzQ7TOYIyKifDuoUDFhapUDdE6b+MrA6MB3Ite5NlJ2vbHBWzjsob tznA== X-Gm-Message-State: AOAM530I1W/bkfNOkABYoZgCoNWc2lKeCl+wbhH0LcnABn1xBIOwniZR VxmKuYMbntrErgshLfMUDTrJoxou8YG6COze6UY= X-Google-Smtp-Source: ABdhPJxKx1FFgSO/qBC+JY1UHW/GhXxKoGrdoN/L4YFYVS6ASeReHimJLQay6YoAZIAWgkXTF8vFFb4ZE2+ujQhXrrg= X-Received: by 2002:a50:8125:: with SMTP id 34mr15683908edc.39.1603708996142; Mon, 26 Oct 2020 03:43:16 -0700 (PDT) MIME-Version: 1.0 References: <158524252335.30595.3422322089286433323.stgit@Palanthas> In-Reply-To: <158524252335.30595.3422322089286433323.stgit@Palanthas> From: George Dunlap Date: Mon, 26 Oct 2020 10:43:04 +0000 Message-ID: Subject: Re: [Xen-devel] [PATCH] xen: credit2: document that min_rqd is valid and ok to use To: Dario Faggioli Cc: xen-devel , =?UTF-8?B?SsO8cmdlbkdyb8Of?= , Jan Beulich , George Dunlap Content-Type: multipart/alternative; boundary="00000000000009ce5705b290995f" --00000000000009ce5705b290995f Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, Mar 26, 2020 at 5:09 PM Dario Faggioli wrote: > > Code is a bit involved, and it is not easy to tell that min_rqd, inside > csched2_res_pick() is actually pointing to a runqueue, when it is > dereferenced. > > Add a comment and an ASSERT() for that. > > Suggested-by: Jan Beulich > Signed-off-by: Dario Faggioli > --- > Cc: J=C3=BCrgen Gro=C3=9F > --- > xen/common/sched/credit2.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/xen/common/sched/credit2.c b/xen/common/sched/credit2.c > index c7241944a8..9da51e624b 100644 > --- a/xen/common/sched/credit2.c > +++ b/xen/common/sched/credit2.c > @@ -2387,6 +2387,13 @@ csched2_res_pick(const struct scheduler *ops, const struct sched_unit *unit) > goto out_up; > } > > + /* > + * If we're here, min_rqd must be valid. In fact, either we picked a > + * runqueue in the "list_for_each" (as min_avgload is initialized to > + * MAX_LOAD) or we just did that (in the "else" branch) above. > + */ Sorry it's taken so long to get back to you on this. The problem with this is that there are actually *three* alternate clauses above: 1. (has_soft && min_s_rqd) 2. min_rqd 3. It's obvious that if we hit #2 or #3, that min_rqd will be set. But it's not immediately obvious why the condition in #1 guarantees that min_rqd will be set. Is it because if we get to the point in the above loop where min_s_rqd is set, then min_rqd will always be set if it hasn't been set already? Or to put it a different way -- the only way for min_rqd *not* to be set is if it always bailed before min_s_rqd was set? -George --00000000000009ce5705b290995f Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


On Thu, Mar 26, 2020 at 5:09 PM Dario Faggiol= i <dfaggioli@suse.com> wrot= e:
>
> Code is a bit involved, and it is not easy to tell that = min_rqd, inside
> csched2_res_pick() is actually pointing to a runque= ue, when it is
> dereferenced.
>
> Add a comment and an A= SSERT() for that.
>
> Suggested-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: = Dario Faggioli <dfaggioli@suse.com= >
> ---
> Cc: J=C3=BCrgen Gro=C3=9F <jgross@suse.com>
> ---
> =C2=A0xen/co= mmon/sched/credit2.c | =C2=A0 =C2=A07 +++++++
> =C2=A01 file changed,= 7 insertions(+)
>
> diff --git a/xen/common/sched/credit2.c b/= xen/common/sched/credit2.c
> index c7241944a8..9da51e624b 100644
&= gt; --- a/xen/common/sched/credit2.c
> +++ b/xen/common/sched/credit2= .c
> @@ -2387,6 +2387,13 @@ csched2_res_pick(const struct scheduler *= ops, const struct sched_unit *unit)
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0goto out_up;
> =C2=A0 =C2=A0 =C2=A0}
>
> + =C2=A0 =C2= =A0/*
> + =C2=A0 =C2=A0 * If we're here, min_rqd must be valid. I= n fact, either we picked a
> + =C2=A0 =C2=A0 * runqueue in the "= list_for_each" (as min_avgload is initialized to
> + =C2=A0 =C2= =A0 * MAX_LOAD) or we just did that (in the "else" branch) above.=
> + =C2=A0 =C2=A0 */


Sorry it's taken so long to= get back to you on this.

The problem with this is= that there are actually *three* alternate clauses above:

1. (has_soft && min_s_rqd)
2. min_rqd
3= . <none of the above>

It's obvious that = if we hit #2 or #3, that min_rqd will be set.=C2=A0 But it's not immedi= ately obvious why the condition in #1 guarantees that min_rqd will be set.<= /div>

Is it because if we get to the point in the above = loop where min_s_rqd is set, then min_rqd will always be set if it hasn'= ;t been set already?=C2=A0 Or to put it a different way -- the only way for= min_rqd *not* to be set is if it always bailed before min_s_rqd was set?

=C2=A0-George
--00000000000009ce5705b290995f--