From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=66.187.233.73; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id AF5B8210C1ED5 for ; Wed, 25 Jul 2018 08:18:14 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 3F8C3814F0B8; Wed, 25 Jul 2018 15:18:13 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-122-38.rdu2.redhat.com [10.10.122.38]) by smtp.corp.redhat.com (Postfix) with ESMTP id 40CF92166BA3; Wed, 25 Jul 2018 15:18:12 +0000 (UTC) To: "Dong, Eric" , "edk2-devel@lists.01.org" Cc: "Ni, Ruiyu" References: <20180725075020.240-1-eric.dong@intel.com> <20180725075020.240-3-eric.dong@intel.com> <78bc8b2c-1579-5f6f-1d9c-eedc0899f6dc@redhat.com> From: Laszlo Ersek Message-ID: <51dc2a09-17cb-2942-d578-74f25d18de65@redhat.com> Date: Wed, 25 Jul 2018 17:18:11 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.78 on 10.11.54.6 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Wed, 25 Jul 2018 15:18:13 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Wed, 25 Jul 2018 15:18:13 +0000 (UTC) for IP:'10.11.54.6' DOMAIN:'int-mx06.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'lersek@redhat.com' RCPT:'' Subject: Re: [Patch v3 2/3] UefiCpuPkg/MpInitLib: Remove StartCount and volatile definition. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 25 Jul 2018 15:18:14 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 07/25/18 14:09, Dong, Eric wrote: > Hi Laszlo, > > >> -----Original Message----- >> From: Laszlo Ersek [mailto:lersek@redhat.com] >> Sent: Wednesday, July 25, 2018 7:47 PM >> To: Dong, Eric ; edk2-devel@lists.01.org >> Cc: Ni, Ruiyu >> Subject: Re: [edk2] [Patch v3 2/3] UefiCpuPkg/MpInitLib: Remove StartCount >> and volatile definition. >> >> Hi Eric, >> >> On 07/25/18 09:50, Eric Dong wrote: >>> The StartCount is duplicated with RunningCount, replace it with >>> RunningCount. Also the volatile for RunningCount is not needed. >> >> after staring at this patch for a long time, I think it is correct. >> However, I suggest that we improve the commit message, because this patch >> actually does three things: >> >> (1) It removes "volatile" from RunningCount. >> >> [This is OK, because only the BSP modifies it.] >> >> (2) [This is the tricky part.] >> >> When we detect a timeout in CheckAllAPs(), and collect the list of failed CPUs, >> the size of the list is derived from the following difference, before the patch: >> >> StartCount - FinishedCount >> >> where "StartCount" is set by the BSP at startup, and FinishedCount is >> incremented by the APs themselves. >> > > I think FinishedCount used here is a typo. What the logic from the code context here is want to get the failed Aps and return them. So it should use RunningCount here, right? Also the FinishedCount may be bigger than StartCount if many Aps has been disabled. Right? I agree FinishedCount looks questionable / out of place in the original code. Thanks Laszlo