On Friday 09 October 2015 03:24 PM, Kirill A. Shutemov wrote:
> On Tue, Sep 22, 2015 at 04:04:53PM +0530, Vineet Gupta wrote:
>> - pgtable-generic.c: Fold individual #ifdef for each helper into a top
>> level #ifdef. Makes code more readable
> Makes sense.
>
>> - Per Andrew's suggestion removed the dummy implementations for !THP
>> in asm-generic/page-table.h to have build time failures vs. runtime.
> I'm not sure it's a good idea. This can lead to unnecessary #ifdefs where
> otherwise call to helper would be eliminated by compiler as dead code.
>
> What about dummy helpers with BUILD_BUG()?
Not really. With this patch, if arch doesn't define __HAVR_ARCH_xyz - we pick the
default implementation. What I changed is if arch defines the __HAVE but doesn't
define the function, then instead of pickign a stub with runtime or buildtime bug
on, we simply fail the build ?
Maybe I can add this to changelog to make it more explicit.
>
>> Signed-off-by: Vineet Gupta <vgupta@xxxxxxxxxxxx>
>> ---
>> include/asm-generic/pgtable.h | 49 ++++++++++++++++---------------------------
>> mm/pgtable-generic.c | 24 +++------------------
>> 2 files changed, 21 insertions(+), 52 deletions(-)
>>
>> diff --git a/include/asm-generic/pgtable.h b/include/asm-generic/pgtable.h
>> index 29c57b2cb344..2112f4147816 100644
>> --- a/include/asm-generic/pgtable.h
>> +++ b/include/asm-generic/pgtable.h
>> @@ -30,9 +30,12 @@ extern int ptep_set_access_flags(struct vm_area_struct *vma,
>> #endif
>>
>> #ifndef __HAVE_ARCH_PMDP_SET_ACCESS_FLAGS
>> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
>> extern int pmdp_set_access_flags(struct vm_area_struct *vma,
>> unsigned long address, pmd_t *pmdp,
>> pmd_t entry, int dirty);
>> +
>> +#endif /* CONFIG_TRANSPARENT_HUGEPAGE */
>> #endif
>>
>> #ifndef __HAVE_ARCH_PTEP_TEST_AND_CLEAR_YOUNG
>> @@ -64,14 +67,6 @@ static inline int pmdp_test_and_clear_young(struct vm_area_struct *vma,
>> set_pmd_at(vma->vm_mm, address, pmdp, pmd_mkold(pmd));
>> return r;
>> }
>> -#else /* CONFIG_TRANSPARENT_HUGEPAGE */
>> -static inline int pmdp_test_and_clear_young(struct vm_area_struct *vma,
>> - unsigned long address,
>> - pmd_t *pmdp)
>> -{
>> - BUG();
>> - return 0;
>> -}
>> #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
>> #endif
>>
>> @@ -81,8 +76,21 @@ int ptep_clear_flush_young(struct vm_area_struct *vma,
>> #endif
>>
>> #ifndef __HAVE_ARCH_PMDP_CLEAR_YOUNG_FLUSH
>> -int pmdp_clear_flush_young(struct vm_area_struct *vma,
>> - unsigned long address, pmd_t *pmdp);
>> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
>> +extern int pmdp_clear_flush_young(struct vm_area_struct *vma,
>> + unsigned long address, pmd_t *pmdp);
>> +#else
>> +/*
>> + * Despite relevant to THP only, this API is called from generic rmap code
>> + * under PageTransHuge(), hence needs a dummy implementation for !THP
>> + */
> Looks like a case I described above. BUILD_BUG_ON() should work fine here.
Indeed BUILD_BUG_ON is better here.
>
>> +static inline int pmdp_clear_flush_young(struct vm_area_struct *vma,
>> + unsigned long address, pmd_t *pmdp)
>> +{
>> + BUG();
>> + return 0;
>> +}
>> +#endif /* CONFIG_TRANSPARENT_HUGEPAGE */
>> #endif
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href