On 05/11/2013 06:23 PM, Kirill A. Shutemov wrote:
> From: "Kirill A. Shutemov" <kirill.shutemov@xxxxxxxxxxxxxxx>
>
> Since we're going to have huge pages backed by files,
> wait_split_huge_page() has to serialize not only over anon_vma_lock,
> but over i_mmap_mutex too.
...
> -#define wait_split_huge_page(__anon_vma, __pmd) \
> +#define wait_split_huge_page(__vma, __pmd) \
> do { \
> pmd_t *____pmd = (__pmd); \
> - anon_vma_lock_write(__anon_vma); \
> - anon_vma_unlock_write(__anon_vma); \
> + struct address_space *__mapping = \
> + vma->vm_file->f_mapping; \
> + struct anon_vma *__anon_vma = (__vma)->anon_vma; \
> + if (__mapping) \
> + mutex_lock(&__mapping->i_mmap_mutex); \
> + if (__anon_vma) { \
> + anon_vma_lock_write(__anon_vma); \
> + anon_vma_unlock_write(__anon_vma); \
> + } \
> + if (__mapping) \
> + mutex_unlock(&__mapping->i_mmap_mutex); \
> BUG_ON(pmd_trans_splitting(*____pmd) || \
> pmd_trans_huge(*____pmd)); \
> } while (0)
Kirill, I asked about this patch in the previous series, and you wrote
some very nice, detailed answers to my stupid questions. But, you
didn't add any comments or update the patch description. So, if a
reviewer or anybody looking at the changelog in the future has my same
stupid questions, they're unlikely to find the very nice description
that you wrote up.
I'd highly suggest that you go back through the comments you've received
before and make sure that you both answered the questions, *and* made
sure to cover those questions either in the code or in the patch
descriptions.
Could you also describe the lengths to which you've gone to try and keep
this macro from growing in to any larger of an abomination. Is it truly
_impossible_ to turn this in to a normal function? Or will it simply be
a larger amount of work that you can do right now? What would it take?
--
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=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>