On Mon, Jan 16, 2017 at 01:23:29AM -0600, Suravee Suthikulpanit wrote: > From: Suravee Suthikulpanit <[email protected]> > > * Clean up various bitwise operations in perf_iommu_enable_event ()
It is a function. > * Make use macros BIT(x) "Make use"? > This should not affect logic and functionality. > > Cc: Peter Zijlstra <[email protected]> > Cc: Borislav Petkov <[email protected]> > Signed-off-by: Suravee Suthikulpanit <[email protected]> > --- > arch/x86/events/amd/iommu.c | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/arch/x86/events/amd/iommu.c b/arch/x86/events/amd/iommu.c > index 44638d0..1aa25d8 100644 > --- a/arch/x86/events/amd/iommu.c > +++ b/arch/x86/events/amd/iommu.c > @@ -164,11 +164,11 @@ static int get_next_avail_iommu_bnk_cntr(struct > perf_amd_iommu *perf_iommu) > for (bank = 0, shift = 0; bank < max_banks; bank++) { > for (cntr = 0; cntr < max_cntrs; cntr++) { > shift = bank + (bank*3) + cntr; > - if (perf_iommu->cntr_assign_mask & (1ULL<<shift)) { > + if (perf_iommu->cntr_assign_mask & BIT(shift)) { BIT_ULL() otherwise you're introducing a bug. > continue; > } else { > - perf_iommu->cntr_assign_mask |= (1ULL<<shift); > - retval = ((u16)((u16)bank<<8) | (u8)(cntr)); > + perf_iommu->cntr_assign_mask |= BIT(shift); Ditto. > + retval = ((u16)((u16)bank << 8) | (u8)(cntr)); That's some ugly casting. Why? (u8)(cntr) is supposed to do what exactly? Cut off to 255 if max_cntrs grows bigger? Can that even happen? Same for bank? Then you're casting to u16 even though retval is int. Ah, because it can be negative too. How about this instead? retval = ((bank & 0xff) << 8) | (cntr & 0xff); This way it is really obvious what you're trying to do. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.
