| Message ID | 1476448852-30062-6-git-send-email-peterx@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Oct 14, 2016 at 08:40:43PM +0800, Peter Xu wrote: > Signed-off-by: Peter Xu <peterx@redhat.com> > --- > 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 > @@ -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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@redhat.com> > > --- > > 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@vger.kernel.org > 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Oct 20, 2016 at 02:30:26PM +0200, Andrew Jones wrote: > 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@redhat.com> > > > --- > > > 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. Moving over. > > 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. Yeah these looks more clean. Will take them. Thanks! -- peterx -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 @@ -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)) + #define __va(x) ((void *)((unsigned long) (x))) #define __pa(x) ((unsigned long) (x)) #define virt_to_pfn(kaddr) (__pa(kaddr) >> PAGE_SHIFT)
Signed-off-by: Peter Xu <peterx@redhat.com> --- lib/asm-generic/page.h | 5 +++++ 1 file changed, 5 insertions(+)