On Sat, Jun 10, 2017 at 02:49:31PM -0700, Dan Williams wrote:
> Turn the macro into a static inline and rewrite the condition checks for
> better readability in preparation for adding another condition.
>
> Cc: Jan Kara <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Cc: Ross Zwisler <[email protected]>
> Cc: "Kirill A. Shutemov" <[email protected]>
> Signed-off-by: Dan Williams <[email protected]>
> ---
> include/linux/huge_mm.h | 35 ++++++++++++++++++++++++-----------
> 1 file changed, 24 insertions(+), 11 deletions(-)
>
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index a3762d49ba39..c4706e2c3358 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -85,14 +85,26 @@ extern struct kobj_attribute shmem_enabled_attr;
>
> extern bool is_vma_temporary_stack(struct vm_area_struct *vma);
>
> -#define transparent_hugepage_enabled(__vma) \
> - ((transparent_hugepage_flags & \
> - (1<<TRANSPARENT_HUGEPAGE_FLAG) || \
> - (transparent_hugepage_flags & \
> - (1<<TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG) && \
> - ((__vma)->vm_flags & VM_HUGEPAGE))) && \
> - !((__vma)->vm_flags & VM_NOHUGEPAGE) && \
> - !is_vma_temporary_stack(__vma))
> +extern unsigned long transparent_hugepage_flags;
> +
> +static inline bool transparent_hugepage_enabled(struct vm_area_struct *vma)
> +{
> + if (transparent_hugepage_flags & (1 << TRANSPARENT_HUGEPAGE_FLAG))
> + return true;
> +
> + if (transparent_hugepage_flags
> + & (1 << TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG))
> + /* check vma flags */;
> + else
> + return false;
> +
> + if ((vma->vm_flags & (VM_HUGEPAGE | VM_NOHUGEPAGE)) == VM_HUGEPAGE
> + && !is_vma_temporary_stack(vma))
> + return true;
> +
> + return false;
> +}
I don't think that these are equivalent. Here is the logic from the macro,
with whitespace added so things are more readable:
#define transparent_hugepage_enabled(__vma)
(
(
transparent_hugepage_flags & (1<<TRANSPARENT_HUGEPAGE_FLAG)
||
(
transparent_hugepage_flags & (1<<TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG)
&&
((__vma)->vm_flags & VM_HUGEPAGE)
)
)
&&
!((__vma)->vm_flags & VM_NOHUGEPAGE)
&&
!is_vma_temporary_stack(__vma)
)
So, if the VM_NOHUGEPAGE flag is set or if the vma is for a temporary stack,
we always bail. Also, we only care about the VM_HUGEPAGE flag in the presence
of TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG.
I think this static inline is logically equivalent (untested):
static inline bool transparent_hugepage_enabled(struct vm_area_struct *vma)
{
if ((vma->vm_flags & VM_NOHUGEPAGE) || is_vma_temporary_stack(vma))
return false;
if (transparent_hugepage_flags & (1 << TRANSPARENT_HUGEPAGE_FLAG))
return true;
if ((transparent_hugepage_flags &
(1 << TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG))
&& vma->vm_flags & VM_HUGEPAGE)
return true;
return false;
}
The ordering of the checks is different, but we're not using && or || to
short-circuit checks with side effects, so I think it is more readable and
should be fine.
_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm
