Re: [kvm-unit-tests PATCH 05/14] page: add page alignment checker

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

 



On Thu, Oct 20, 2016 at 02:23:14PM +0200, Andrew Jones wrote:
> On Fri, Oct 14, 2016 at 08:40:43PM +0800, Peter Xu wrote:
> > Signed-off-by: Peter Xu <peterx@xxxxxxxxxx>
> > ---
> >  lib/asm-generic/page.h | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/lib/asm-generic/page.h b/lib/asm-generic/page.h
> > index 7b8a08b..cbdc8f6 100644
> > --- a/lib/asm-generic/page.h
> > +++ b/lib/asm-generic/page.h

Another comment. IS_ALIGNED shouldn't be here, it should be
in lib/libcflat.h under the definition of ALIGN there.

drew

> > @@ -19,6 +19,11 @@
> >  
> >  #define PAGE_ALIGN(addr)	ALIGN(addr, PAGE_SIZE)
> >  
> > +#define IS_ALIGNED(x, a)	(((x) & ((typeof(x))(a) - 1)) == 0)
> > +#define PAGE_ALIGNED_4K(x)	IS_ALIGNED((x), (0x1000))
> > +#define PAGE_ALIGNED_2M(x)	IS_ALIGNED((x), (0x200000))
> > +#define PAGE_ALIGNED_1G(x)	IS_ALIGNED((x), (0x40000000))
> 
> I like IS_ALIGNED, but not the others. The others are strangely
> named because it's half asking if an address is page aligned and
> half asking if it's aligned to a given boundary. Why not just
> ask the boundary, like ALIGNED_2M(x)? Anyway, my preference would
> be to replace them with SZ_4K, SZ_2M, SZ_1G definitions. Users would
> then do
> 
>  IS_ALIGNED(addr, SZ_2M)
> 
> or whatever, which isn't much more typing.
> 
> Thanks,
> drew
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux