Re: [PATCH v7 02/61] radix tree: Use bottom four bits of gfp_t for flags

From: Jeff Layton
Date: Sat Mar 03 2018 - 07:44:47 EST


On Mon, 2018-02-19 at 11:44 -0800, Matthew Wilcox wrote:
> From: Matthew Wilcox <mawilcox@xxxxxxxxxxxxx>
>
> None of these four bits may be used for slab allocations, so we can
> use them for flags as long as we mask them off before passing them
> to the slab allocator. Move the IDR flag from the top bits to the
> bottom bits.
>
> Signed-off-by: Matthew Wilcox <mawilcox@xxxxxxxxxxxxx>
> ---
> include/linux/idr.h | 3 ++-
> include/linux/radix-tree.h | 7 ++++---
> lib/radix-tree.c | 3 ++-
> tools/testing/radix-tree/linux/gfp.h | 1 +
> 4 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/idr.h b/include/linux/idr.h
> index 7d6a6313f0ab..913c335054f0 100644
> --- a/include/linux/idr.h
> +++ b/include/linux/idr.h
> @@ -29,7 +29,8 @@ struct idr {
> #define IDR_FREE 0
>
> /* Set the IDR flag and the IDR_FREE tag */
> -#define IDR_RT_MARKER ((__force gfp_t)(3 << __GFP_BITS_SHIFT))
> +#define IDR_RT_MARKER (ROOT_IS_IDR | (__force gfp_t) \
> + (1 << (ROOT_TAG_SHIFT + IDR_FREE)))
>
> #define IDR_INIT_BASE(base) { \
> .idr_rt = RADIX_TREE_INIT(IDR_RT_MARKER), \
> diff --git a/include/linux/radix-tree.h b/include/linux/radix-tree.h
> index fc55ff31eca7..6c4e2e716dac 100644
> --- a/include/linux/radix-tree.h
> +++ b/include/linux/radix-tree.h
> @@ -104,9 +104,10 @@ struct radix_tree_node {
> unsigned long tags[RADIX_TREE_MAX_TAGS][RADIX_TREE_TAG_LONGS];
> };
>
> -/* The top bits of gfp_mask are used to store the root tags and the IDR flag */
> -#define ROOT_IS_IDR ((__force gfp_t)(1 << __GFP_BITS_SHIFT))
> -#define ROOT_TAG_SHIFT (__GFP_BITS_SHIFT + 1)
> +/* The IDR tag is stored in the low bits of the GFP flags */
> +#define ROOT_IS_IDR ((__force gfp_t)4)
> +/* The top bits of gfp_mask are used to store the root tags */
> +#define ROOT_TAG_SHIFT (__GFP_BITS_SHIFT)
>
> struct radix_tree_root {
> gfp_t gfp_mask;
> diff --git a/lib/radix-tree.c b/lib/radix-tree.c
> index 0a7ae3288a24..66732e2f9606 100644
> --- a/lib/radix-tree.c
> +++ b/lib/radix-tree.c
> @@ -146,7 +146,7 @@ static unsigned int radix_tree_descend(const struct radix_tree_node *parent,
>
> static inline gfp_t root_gfp_mask(const struct radix_tree_root *root)
> {
> - return root->gfp_mask & __GFP_BITS_MASK;
> + return root->gfp_mask & ((__GFP_BITS_MASK >> 4) << 4);

Maybe phrase this in terms of a constant like GFP_ZONEMASK here? Would
this be more appropriate?

root->gfp_mask & (__GFP_BITS_MASK & ~GFP_ZONEMASK);

> }
> static inline void tag_set(struct radix_tree_node *node, unsigned int tag,
> @@ -2285,6 +2285,7 @@ void __init radix_tree_init(void)
> int ret;
>
> BUILD_BUG_ON(RADIX_TREE_MAX_TAGS + __GFP_BITS_SHIFT > 32);
> + BUILD_BUG_ON(GFP_ZONEMASK != (__force gfp_t)15);
> radix_tree_node_cachep = kmem_cache_create("radix_tree_node",
> sizeof(struct radix_tree_node), 0,
> SLAB_PANIC | SLAB_RECLAIM_ACCOUNT,
> diff --git a/tools/testing/radix-tree/linux/gfp.h b/tools/testing/radix-tree/linux/gfp.h
> index e9fff59dfd8a..a72007d9818b 100644
> --- a/tools/testing/radix-tree/linux/gfp.h
> +++ b/tools/testing/radix-tree/linux/gfp.h
> @@ -18,6 +18,7 @@
>
> #define __GFP_RECLAIM (__GFP_DIRECT_RECLAIM|__GFP_KSWAPD_RECLAIM)
>
> +#define GFP_ZONEMASK 0x0fu
> #define GFP_ATOMIC (__GFP_HIGH|__GFP_ATOMIC|__GFP_KSWAPD_RECLAIM)
> #define GFP_KERNEL (__GFP_RECLAIM | __GFP_IO | __GFP_FS)
> #define GFP_NOWAIT (__GFP_KSWAPD_RECLAIM)

--
Jeff Layton <jlayton@xxxxxxxxxx>