Re: [Xen-devel] [PATCH v1 03/12] tmem: Wrap tmem dedup code with CONFIG_TMEM_DEDUP

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 28.09.16 at 11:42, <konrad.wilk@xxxxxxxxxx> wrote:
> This config is not defined anywhere but it makes it way
> easier to figure out what code to deal with.

And the functionality thus removed is
- dead code,
- broken,
- anything else?

> --- a/xen/common/tmem.c
> +++ b/xen/common/tmem.c
> @@ -71,10 +71,14 @@ struct tmem_page_descriptor {
>      pagesize_t size; /* 0 == PAGE_SIZE (pfp), -1 == data invalid,
>                      else compressed data (cdata). */
>      uint32_t index;
> +#ifdef CONFIG_TMEM_DEDUP
>      /* Must hold pcd_tree_rwlocks[firstbyte] to use pcd pointer/siblings. */
>      uint16_t firstbyte; /* NON_SHAREABLE->pfp  otherwise->pcd. */
> +#endif
>      bool_t eviction_attempted;  /* CHANGE TO lifetimes? (settable). */
> +#ifdef CONFIG_TMEM_DEDUP
>      struct list_head pcd_siblings;
> +#endif

Perhaps worth moving the former field down, so you end up with
just one #ifdef?

> @@ -1085,6 +1101,8 @@ static struct client *client_create(domid_t cli_id)
>  
>      client->cli_id = cli_id;
>      client->compress = tmem_compression_enabled();
> +#ifdef CONFIG_TMEM_DEDUP
> +#endif

???

> @@ -2362,29 +2397,41 @@ unsigned long tmem_freeable_pages(void)
>  /* Called at hypervisor startup. */
>  static int __init init_tmem(void)
>  {
> -    int i;
>      if ( !tmem_enabled() )
>          return 0;
>  
> +#ifdef CONFIG_TMEM_DEDUP
>      if ( tmem_dedup_enabled() )
> +    {
> +        unsigned int i;
> +
>          for (i = 0; i < 256; i++ )
>          {
>              pcd_tree_roots[i] = RB_ROOT;
>              rwlock_init(&pcd_tree_rwlocks[i]);
>          }
> -
> +    }
> +#endif
>      if ( !tmem_mempool_init() )
>          return 0;
>  
>      if ( tmem_init() )
>      {
>          printk("tmem: initialized comp=%d dedup=%d tze=%d\n",
> -            tmem_compression_enabled(), tmem_dedup_enabled(), 
> tmem_tze_enabled());
> +               tmem_compression_enabled(),
> +#ifdef CONFIG_TMEM_DEDUP
> +               tmem_dedup_enabled(),
> +#else
> +               0,
> +#endif

Why can't tmem_dedup_enabled() simply return constant zero when
CONFIG_TMEM_DEDUP is undefined?

> --- a/xen/common/tmem_xen.c
> +++ b/xen/common/tmem_xen.c
> @@ -20,8 +20,10 @@ boolean_param("tmem", opt_tmem);
>  bool_t __read_mostly opt_tmem_compress = 0;
>  boolean_param("tmem_compress", opt_tmem_compress);
>  
> +#ifdef CONFIG_TMEM_DEDUP
>  bool_t __read_mostly opt_tmem_dedup = 0;
>  boolean_param("tmem_dedup", opt_tmem_dedup);
> +#endif

This option's documentation entry would seem to need some
adjustment then too, albeit it's not really clear what would be best
here.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel