Hi Borislav,
On 6/20/2019 6:44 AM, Borislav Petkov wrote:
> On Wed, Jun 19, 2019 at 01:27:16PM -0700, Reinette Chatre wrote:
>> @@ -2494,26 +2498,19 @@ static int mkdir_mondata_all(struct kernfs_node
>> *parent_kn,
>> */
>> static void cbm_ensure_valid(u32 *_val, struct rdt_resource *r)
>> {
>> - /*
>> - * Convert the u32 _val to an unsigned long required by all the bit
>> - * operations within this function. No more than 32 bits of this
>> - * converted value can be accessed because all bit operations are
>> - * additionally provided with cbm_len that is initialized during
>> - * hardware enumeration using five bits from the EAX register and
>> - * thus never can exceed 32 bits.
>> - */
>> - unsigned long *val = (unsigned long *)_val;
>> + unsigned long val = *_val;
>> unsigned int cbm_len = r->cache.cbm_len;
>> unsigned long first_bit, zero_bit;
>
> Please sort function local variables declaration in a reverse christmas
> tree order:
>
> <type A> longest_variable_name;
> <type B> shorter_var_name;
> <type C> even_shorter;
> <type D> i;
>
>> - if (*val == 0)
>> + if (val == 0)
>
> if (!val)
>
>> return;
>>
>> - first_bit = find_first_bit(val, cbm_len);
>> - zero_bit = find_next_zero_bit(val, cbm_len, first_bit);
>> + first_bit = find_first_bit(&val, cbm_len);
>> + zero_bit = find_next_zero_bit(&val, cbm_len, first_bit);
>>
>> /* Clear any remaining bits to ensure contiguous region */
>> - bitmap_clear(val, zero_bit, cbm_len - zero_bit);
>> + bitmap_clear(&val, zero_bit, cbm_len - zero_bit);
>> + *_val = (u32)val;
>
> ... and also, that function should simply return the u32 value instead
> of using @_val as an input and output var.
>
> But that should be a separate cleanup patch anyway.
Thank you very much. I will provide that separate cleanup patch.
Reinette
