[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Ping: [PATCH] x86: replace a few do_div() uses


  • To: Jan Beulich <jbeulich@xxxxxxxx>, Andrew Cooper <amc96@xxxxxxxx>
  • From: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>
  • Date: Fri, 18 Feb 2022 13:22:58 +0000
  • Accept-language: en-GB, 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-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=ArgSZLkCL7gUMLOr3Uj2rcxDXVSxdO5cj6dGuKr9+oA=; b=L9LxzMeuMcZVmmY0/ttiruOVJxiET2EIiU1JjA2IvkhP5S2FvXr7rHIqSxw78j1rG5+eW6AwXaF2ABVkbz5S7QrbBHDWQlKA2LRvPHQqDKoIXZ1bTDthG+2pwyW1BWZKTknw76KeLNMAa6mDmZ65qIEW+PrqNVxSM8OZSeYXyV5dGmCUN+4NnBAUvkH5OJJdD3g9DqQBLHA6tZ2knlK0HhpRug8s6etZ5tDV8OnvhzrLC3i1XUWZxXw/tEnVOFj2LFYKe0K/7FIXbTcU/XtnVWE7IMe91oVrOWKCnF78HYC64MLgBAr3kNiAvbw3waByDPRh4EsCxNLDJ6tzokANJw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=BOljm7JtnDn9OBfgWtza9Md80vRZPemG13QihsAeslDSZfPbFfk4jWauof5+9wpn7BaG3vXmVzQdZBJ2X8qYTZhn1AyRGg1U1jxaM2J3veDJ2ATJPYlKjwC4oc/bDHX7GIw8KXfg/5oltaugzsO9jQJpBOZt4+oPeD/QTEU+IXfpMDUTiRIuFEZ7mdcPpjyPRPIBEix4aTO/bECl3KAVAIHEWxpFkNERW+YkVqimDaBPvQJ5PtIoTmZX5snhuKu7CG/9+6XDRGoOlLbotei/kEkQ3wiVz4/bkhQJppeRD/N1MgGmH5z2LUq4a0aZNp7rlT3KNm88XOm7p0I/uffQ9A==
  • Authentication-results: esa4.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: Wei Liu <wl@xxxxxxx>, Roger Pau Monne <roger.pau@xxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Fri, 18 Feb 2022 13:23:10 +0000
  • Ironport-data: A9a23:QcfR2qIkbC37QXRyFE+R85UlxSXFcZb7ZxGr2PjKsXjdYENS3jRUm zEfXD2GMv6Jamqncop0bdy18E5X7JDRm4c1HABlqX01Q3x08seUXt7xwmUcns+xwm8vaGo9s q3yv/GZdJhcokf0/0vrav67xZVF/fngqoDUUYYoAQgsA180IMsdoUg7wbRh2Nc42YHR7z6l4 rseneWOYDdJ5BYsWo4kw/rrRMRH5amaVJsw5zTSVNgT1LPsvyB94KE3fMldG0DQUIhMdtNWc s6YpF2PEsE1yD92Yj+tuu6TnkTn2dc+NyDW4pZdc/DKbhSvOkXee0v0XRYRQR4/ttmHozx+4 O5B6qeJT19yAqDdlb8hdUFUOSRSE7ITrdcrIVDn2SCS50jPcn+qyPRyFkAme4Yf/46bA0kXq 6ZecmpUKEne2aTmm9pXScE17ignBODtMJkSpTdLyjbBAOx9aZvCX7/L9ZlT2zJYasVmQ6iBO ZJBNmUHgBLoOgB2JmceLMMCt6SrrUDGLTx3hmnNuv9ii4TU5FMoi+W8WDbPQfSVQe1Fk0Deo XjJl0zpDxdfONGBxD6t9nO3mvSJjS79QJgVFrCz6rhtmlL77m4ZBQASVFC7ieKkkUP4UNVaQ 2Qd/yF/84Ap7kelCN/wQ3WFTGWs50BGHYAKSqtjtV/LmvG8Dxul6nYsdTIeU+Ug7JQKSC0nj WOvx9/DABBBr+jAIZ6CzYu8oTS3MCkTCGYNYy4YUAcIi+XeTJEPYgHnFYg6TvPs5jHhMXSpm m3R8nBi71kGpZNTj82GEUb7byVAT3QjZio8/U3pU22s9WuVj6b1NtXzuTA3ARutRbt1r2VtX lBYyqByD8hUVPlhcRBhps1UTdlFAN7fbVXhbaZHRcVJythU0yfLkXpsyD9/Plx1Fc0PZCXkZ kTe0SsIusMOZSH2Nf4rPdvrYyjP8UQGPY20PhwzRoATCqWdiSfdpH0+DaJu9zuFfLcQfVEXZ s7ALJfE4YcyAqV71jumL9rxIpdwrh3SMVj7HMihpzz+iOL2TCfMFd8tbQvfBshkvfjsiFiEr L5i2z6ilkw3vBvWOXKMr+b+7DkicBAGOHwBg5YJLrXaelI+QgnMyZb5mNscRmCspIwM/s/g9 XChQE5Ijl35gHzMMwKRbX5/LrjoWP5CQbgTZ0TA4X7AN6AfXLuS
  • Ironport-hdrordr: A9a23:xV2Gnq3HyL9l2CO89qZdUQqjBRZyeYIsimQD101hICG9Lfb2qy n+ppgmPEHP5Qr5AEtQ5OxpOMG7MBbhHQYc2/heAV7QZnibhILOFvAi0WKC+UyuJ8SazIBgPM hbAtFD4bHLfDtHZIPBkXOF+rUbsZm6GcKT9J/jJh5WJGkAAcAB0+46MHfhLqQffngdOXNTLu v52iMznUvHRZ1hVLXdOpBqZZmgm/T70LbdJTIWDR8u7weDyRmy7qThLhSe1hACFxtS3LYL6w H+4k/Ez5Tml8v+5g7X1mfV4ZgTssDm0MF/CMuFjdVQAinwizyveJ9qV9S5zXIISaCUmRMXee v30lAd1vdImjXsl6aO0ELQMjzboXITArnZuAelaDXY0JfErXkBerV8bMpiA2XkAgwbzYxBOe twrhKkX9A8N2KwoA3to9fPTB1kjUyyvD4rlvMSlWVWVc8EZKZWtpF3xjIeLH4sJlOz1GkcKp gkMCgc3ocjTXqKK3TC+mV/yt2lWXo+Wh+AX0gZo8SQlzxbhmpwwUcUzNEW2i5ozuNwd7BUo+ Dfdqh4nrBHScEbKap7GecaWMOyTmjAWwjFPm6eKUnuUKsHJ3XOoZjq56hd3pDmRLUYiJ8p3J jRWlJRsmA/P0roFM2VxZVOtgvARW2sNA6dg/22J6IJzIEUaICbQxFreWpe5PdI+c9vcfEzc8 zDTa5rPw==
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHYB5LijlOnVfNOdkqZn9v3W4GnqqxfHNSAgAABroCAOhh0AIAATy4A
  • Thread-topic: Ping: [PATCH] x86: replace a few do_div() uses

On 18/02/2022 08:39, Jan Beulich wrote:
> On 12.01.2022 10:28, Jan Beulich wrote:
>> On 12.01.2022 10:22, Andrew Cooper wrote:
>>> On 12/01/2022 09:00, Jan Beulich wrote:
>>>> When the macro's "return value" is not used, the macro use can be
>>>> replaced by a simply division, avoiding some obfuscation.
>>>>
>>>> According to my observations, no change to generated code.
>>>>
>>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>> I like this change in principle, but see below.
>>>
>>> do_div() needs to be deleted, because it's far too easy screw up.  At a
>>> bare minimum, it should be replaced with a static inline that takes it's
>>> first parameter by pointer, because then at least every callsite reads
>>> correctly in terms of the C language.
>> That ought to be a 2nd step, requiring agreement with Arm folks (and
>> adjustments to their code).
>>
>>>> --- a/xen/arch/x86/time.c
>>>> +++ b/xen/arch/x86/time.c
>>>> @@ -610,8 +610,7 @@ static uint64_t xen_timer_cpu_frequency(
>>>>      struct vcpu_time_info *info = &this_cpu(vcpu_info)->time;
>>>>      uint64_t freq;
>>>>  
>>>> -    freq = 1000000000ULL << 32;
>>>> -    do_div(freq, info->tsc_to_system_mul);
>>>> +    freq = (1000000000ULL << 32) / info->tsc_to_system_mul;
>>>>      if ( info->tsc_shift < 0 )
>>>>          freq <<= -info->tsc_shift;
>>> do_div()'s output is consumed here.  I don't think this hunk is safe to
>>> convert.
>> If by "output" you mean its "return value", then it clearly isn't
>> consumed. And I continue to think that I did express correctly the
>> effect do_div() did have on "freq".
> I think I did address both points (the earlier one was actually more a
> remark imo anyway, not a request to change anything right in this patch),
> so may I please ask for an ack (or a response clarifying what I'm not
> understanding in what you have said)?

No - you're right.  My mistake.

Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.