Re: [RFC v5 PATCH 8/9] hugetlbfs: add hugetlbfs_fallocate()

[Mike Kravetz]

On 07/23/2015 11:25 PM, Michal Hocko wrote:
> I hope this is the current version of the pathc - I somehow got lost in
> last submissions where the discussion happens in v4 thread. This version
> seems to have the same issue:

Yes, Michal this issue exists in the version put into mmotm and was
noticed by kbuild test robot and Stephen in linux-next build.

Your patch below is the most obvious.  Thanks!  However, is this
the preferred method of handling this type of issue?  Is it
preferred to create wrappers for the code which handles numa
policy?  Then there could be two versions of the wrapper:  one
if CONFIG_NUMA is defined and one (a no-op) if not.  I am happy
with either, but am a relative newbie in this area so am looking
for a little guidance.

--
Mike Kravetz

> ---
>  From 04c37a979c5ce8cd39d3243e4e2c12905e4f1e6e Mon Sep 17 00:00:00 2001
> From: Michal Hocko <mhocko@xxxxxxxx>
> Date: Fri, 24 Jul 2015 08:14:32 +0200
> Subject: [PATCH] mmotm:
>   hugetlbfs-new-huge_add_to_page_cache-helper-routine-fix
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> hugetlbfs_fallocate relies on CONFIG_NUMA and fails to compile otherwise.
> This just makes the code compile but it seems it begs for a better solution.
>
> fs/hugetlbfs/inode.c: In function âhugetlbfs_fallocateâ:
> fs/hugetlbfs/inode.c:578:13: error: âstruct vm_area_structâ has no member named âvm_policyâ
>     pseudo_vma.vm_policy =
>               ^
> fs/hugetlbfs/inode.c:579:4: error: implicit declaration of function âmpol_shared_policy_lookupâ [-Werror=implicit-function-declaration]
>      mpol_shared_policy_lookup(&HUGETLBFS_I(inode)->policy,
>      ^
> fs/hugetlbfs/inode.c:595:28: error: âstruct vm_area_structâ has no member named âvm_policyâ
>      mpol_cond_put(pseudo_vma.vm_policy);
>                              ^
> fs/hugetlbfs/inode.c:601:27: error: âstruct vm_area_structâ has no member named âvm_policyâ
>     mpol_cond_put(pseudo_vma.vm_policy);
>
> Signed-off-by: Michal Hocko <mhocko@xxxxxxxx>
> ---
>   fs/hugetlbfs/inode.c | 6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
> index d977cae89d29..dfca09218d77 100644
> --- a/fs/hugetlbfs/inode.c
> +++ b/fs/hugetlbfs/inode.c
> @@ -575,9 +575,11 @@ static long hugetlbfs_fallocate(struct file *file, int mode, loff_t offset,
>   		}
>
>   		/* Get policy based on index */
> +#ifdef CONFIG_NUMA
>   		pseudo_vma.vm_policy =
>   			mpol_shared_policy_lookup(&HUGETLBFS_I(inode)->policy,
>   							index);
> +#endif
>
>   		/* addr is the offset within the file (zero based) */
>   		addr = index * hpage_size;
> @@ -592,13 +594,17 @@ static long hugetlbfs_fallocate(struct file *file, int mode, loff_t offset,
>   		if (page) {
>   			put_page(page);
>   			mutex_unlock(&hugetlb_fault_mutex_table[hash]);
> +#ifdef CONFIG_NUMA
>   			mpol_cond_put(pseudo_vma.vm_policy);
> +#endif
>   			continue;
>   		}
>
>   		/* Allocate page and add to page cache */
>   		page = alloc_huge_page(&pseudo_vma, addr, avoid_reserve);
> +#ifdef CONFIG_NUMA
>   		mpol_cond_put(pseudo_vma.vm_policy);
> +#endif
>   		if (IS_ERR(page)) {
>   			mutex_unlock(&hugetlb_fault_mutex_table[hash]);
>   			error = PTR_ERR(page);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/