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

Re: [Xen-devel] [PATCH] xen/grant: Fix signed/unsigned comparisons issues



Hi Jan,

On 28/02/2020 08:41, Jan Beulich wrote:
On 27.02.2020 19:46, Andrew Cooper wrote:
Each function takes an unsigned count, and loops based on a signed i.  This
causes problems when between 2 and 4 billion operations are requested.

I can see problems, but not (as the title says) with comparison issues
(the signed i gets converted to unsigned for the purpose of the
comparison).

In practice, signed-ness issues aren't possible because count exceeding
INT_MAX is excluded earlier in do_grant_op(), but the code reads as if it is
buggy, and GCC obviously can't spot this either.

Bloat-o-meter reports:
   add/remove: 0/0 grow/shrink: 0/4 up/down: 0/-95 (-95)
   Function                                     old     new   delta
   do_grant_table_op                           7155    7140     -15
   gnttab_transfer                             2732    2716     -16
   gnttab_unmap_grant_ref                       771     739     -32
   gnttab_unmap_and_replace                     771     739     -32
   Total: Before=2996364, After=2996269, chg -0.00%

and inspection of gnttab_unmap_grant_ref() at least reveals one fewer local
variables on the stack.

This is a good thing for x86. However, having started to familiarize
myself a tiny bit with RISC-V, I'm no longer certain our present way
of preferring unsigned int for array indexing variable and alike is
actually a good thing without further abstraction. Nevertheless, this
isn't an immediate issue, so ...

Would you mind expanding a bit more about this comment here? How unsigned int is going to make things worse on RISC-V?

Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

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