[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 04/16] tmem: cleanup: rm unneeded parameters from put path



On Wed, Nov 20, 2013 at 04:46:13PM +0800, Bob Liu wrote:
> tmem only takes a page as unit, so parameters tmem_offset, pfn_offset and len
> are meanless, this patch remove those parameters from do_tmem_put() related 
> path
      ^^^^^^^^                   ^-s


Not exactly right. They were used by TMEM_WRITE and TMEM_READ, but your
other patch "tmem: cleanup: rm unused tmem_op" removed them.

Looking a bit on Google I see this thread:

http://marc.info/?l=xen-devel&m=136975816209598


which implies that the Windows GPL driver is using it? Did you
check its code? (See http://xenbits.xen.org/ext/win-pvdrivers/)

If it is not in the code, can you also CC James Harper on it so
he knows not use those ops?

> 
> Signed-off-by: Bob Liu <bob.liu@xxxxxxxxxx>
> ---
>  xen/common/tmem.c          |   32 +++++++++++++++-----------------
>  xen/common/tmem_xen.c      |   27 +++++----------------------
>  xen/include/xen/tmem_xen.h |    3 +--
>  3 files changed, 21 insertions(+), 41 deletions(-)
> 
> diff --git a/xen/common/tmem.c b/xen/common/tmem.c
> index c272930..e2e69bf 100644
> --- a/xen/common/tmem.c
> +++ b/xen/common/tmem.c
> @@ -1421,7 +1421,6 @@ out:
>  }
>  
>  static int do_tmem_dup_put(struct tmem_page_descriptor *pgp, xen_pfn_t cmfn,
> -       pagesize_t tmem_offset, pagesize_t pfn_offset, pagesize_t len,
>         tmem_cli_va_param_t clibuf)
>  {
>      struct tmem_pool *pool;
> @@ -1442,7 +1441,7 @@ static int do_tmem_dup_put(struct tmem_page_descriptor 
> *pgp, xen_pfn_t cmfn,
>      if ( client->live_migrating )
>          goto failed_dup; /* no dups allowed when migrating */
>      /* can we successfully manipulate pgp to change out the data? */
> -    if ( len != 0 && client->compress && pgp->size != 0 )
> +    if ( client->compress && pgp->size != 0 )
>      {
>          ret = do_tmem_put_compress(pgp, cmfn, clibuf);
>          if ( ret == 1 )
> @@ -1461,9 +1460,7 @@ copy_uncompressed:
>      if ( ( pgp->pfp = tmem_page_alloc(pool) ) == NULL )
>          goto failed_dup;
>      pgp->size = 0;
> -    /* tmem_copy_from_client properly handles len==0 and offsets != 0 */
> -    ret = tmem_copy_from_client(pgp->pfp, cmfn, tmem_offset, pfn_offset, len,
> -                               tmem_cli_buf_null);
> +    ret = tmem_copy_from_client(pgp->pfp, cmfn, tmem_cli_buf_null);
>      if ( ret < 0 )
>          goto bad_copy;
>      if ( tmem_dedup_enabled() && !is_persistent(pool) )
> @@ -1509,11 +1506,9 @@ cleanup:
>      return ret;
>  }
>  
> -
>  static int do_tmem_put(struct tmem_pool *pool,
>                struct oid *oidp, uint32_t index,
> -              xen_pfn_t cmfn, pagesize_t tmem_offset,
> -              pagesize_t pfn_offset, pagesize_t len, tmem_cli_va_param_t 
> clibuf)
> +              xen_pfn_t cmfn, tmem_cli_va_param_t clibuf)
>  {
>      struct tmem_object_root *obj = NULL, *objfound = NULL, *objnew = NULL;
>      struct tmem_page_descriptor *pgp = NULL, *pgpdel = NULL;
> @@ -1527,8 +1522,7 @@ static int do_tmem_put(struct tmem_pool *pool,
>      {
>          ASSERT_SPINLOCK(&objfound->obj_spinlock);
>          if ((pgp = pgp_lookup_in_obj(objfound, index)) != NULL)
> -            return do_tmem_dup_put(pgp, cmfn, tmem_offset, pfn_offset, len,
> -                                   clibuf);
> +            return do_tmem_dup_put(pgp, cmfn, clibuf);
>      }
>  
>      /* no puts allowed into a frozen pool (except dup puts) */
> @@ -1560,7 +1554,7 @@ static int do_tmem_put(struct tmem_pool *pool,
>      pgp->index = index;
>      pgp->size = 0;
>  
> -    if ( len != 0 && client->compress )
> +    if ( client->compress )
>      {
>          ASSERT(pgp->pfp == NULL);
>          ret = do_tmem_put_compress(pgp, cmfn, clibuf);
> @@ -1586,9 +1580,7 @@ copy_uncompressed:
>          ret = -ENOMEM;
>          goto delete_and_free;
>      }
> -    /* tmem_copy_from_client properly handles len==0 (TMEM_NEW_PAGE) */
> -    ret = tmem_copy_from_client(pgp->pfp, cmfn, tmem_offset, pfn_offset, len,
> -                               clibuf);
> +    ret = tmem_copy_from_client(pgp->pfp, cmfn, clibuf);
>      if ( ret < 0 )
>          goto bad_copy;
>      if ( tmem_dedup_enabled() && !is_persistent(pool) )
> @@ -2440,10 +2432,17 @@ static int tmemc_restore_put_page(int cli_id, 
> uint32_t pool_id, struct oid *oidp
>      struct client *client = tmem_client_from_cli_id(cli_id);
>      struct tmem_pool *pool = (client == NULL || pool_id >= 
> MAX_POOLS_PER_DOMAIN)
>                     ? NULL : client->pools[pool_id];
> +    unsigned int pagesize;
>  
>      if ( pool == NULL )
>          return -1;
> -    return do_tmem_put(pool, oidp, index, 0, 0, 0, bufsize, buf);
> +    pagesize = 1 << (pool->pageshift + 12);

Ahem! 12? #define please.

> +    if (bufsize != pagesize) {
> +        tmem_client_err("tmem: %s bufsize %d not page aligend(%d)\n", 
> __func__,
> +                bufsize, pagesize);
> +     return -1;
> +    }
> +    return do_tmem_put(pool, oidp, index, 0, buf);
>  }
>  
>  static int tmemc_restore_flush_page(int cli_id, uint32_t pool_id, struct oid 
> *oidp,
> @@ -2648,8 +2647,7 @@ EXPORT long do_tmem_op(tmem_cli_op_t uops)
>          break;
>      case TMEM_PUT_PAGE:
>          tmem_ensure_avail_pages();
> -        rc = do_tmem_put(pool, oidp, op.u.gen.index, op.u.gen.cmfn, 0, 0,
> -                         PAGE_SIZE, tmem_cli_buf_null);
> +        rc = do_tmem_put(pool, oidp, op.u.gen.index, op.u.gen.cmfn, 
> tmem_cli_buf_null);
>          if (rc == 1) succ_put = 1;
>          else non_succ_put = 1;
>          break;
> diff --git a/xen/common/tmem_xen.c b/xen/common/tmem_xen.c
> index af67703..165c7cf 100644
> --- a/xen/common/tmem_xen.c
> +++ b/xen/common/tmem_xen.c
> @@ -100,25 +100,16 @@ static inline void cli_put_page(void *cli_va, struct 
> page_info *cli_pfp,
>  #endif
>  
>  EXPORT int tmem_copy_from_client(struct page_info *pfp,
> -    xen_pfn_t cmfn, pagesize_t tmem_offset,
> -    pagesize_t pfn_offset, pagesize_t len, tmem_cli_va_param_t clibuf)
> +    xen_pfn_t cmfn, tmem_cli_va_param_t clibuf)
>  {
>      unsigned long tmem_mfn, cli_mfn = 0;
>      char *tmem_va, *cli_va = NULL;
>      struct page_info *cli_pfp = NULL;
>      int rc = 1;
>  
> -    if ( tmem_offset > PAGE_SIZE || pfn_offset > PAGE_SIZE || len > 
> PAGE_SIZE )
> -        return -EINVAL;
>      ASSERT(pfp != NULL);
>      tmem_mfn = page_to_mfn(pfp);
>      tmem_va = map_domain_page(tmem_mfn);
> -    if ( tmem_offset == 0 && pfn_offset == 0 && len == 0 )
> -    {
> -        memset(tmem_va, 0, PAGE_SIZE);
> -        unmap_domain_page(tmem_va);
> -        return 1;
> -    }
>      if ( guest_handle_is_null(clibuf) )
>      {
>          cli_va = cli_get_page(cmfn, &cli_mfn, &cli_pfp, 0);
> @@ -129,21 +120,13 @@ EXPORT int tmem_copy_from_client(struct page_info *pfp,
>          }
>      }
>      smp_mb();
> -    if ( len == PAGE_SIZE && !tmem_offset && !pfn_offset && cli_va )
> -        memcpy(tmem_va, cli_va, PAGE_SIZE);
> -    else if ( (tmem_offset+len <= PAGE_SIZE) &&
> -              (pfn_offset+len <= PAGE_SIZE) )
> +    if ( cli_va )
>      {
> -        if ( cli_va )
> -            memcpy(tmem_va + tmem_offset, cli_va + pfn_offset, len);
> -        else if ( copy_from_guest_offset(tmem_va + tmem_offset, clibuf,
> -                                         pfn_offset, len) )
> -            rc = -EFAULT;
> +        memcpy(tmem_va, cli_va, PAGE_SIZE);
> +        cli_put_page(cli_va, cli_pfp, cli_mfn, 0);
>      }
> -    else if ( len )
> +    else
>          rc = -EINVAL;
> -    if ( cli_va )
> -        cli_put_page(cli_va, cli_pfp, cli_mfn, 0);
>      unmap_domain_page(tmem_va);
>      return rc;
>  }
> diff --git a/xen/include/xen/tmem_xen.h b/xen/include/xen/tmem_xen.h
> index 4e4c227..25ce268 100644
> --- a/xen/include/xen/tmem_xen.h
> +++ b/xen/include/xen/tmem_xen.h
> @@ -385,8 +385,7 @@ int tmem_decompress_to_client(xen_pfn_t, void *, size_t,
>  int tmem_compress_from_client(xen_pfn_t, void **, size_t *,
>                            tmem_cli_va_param_t);
>  
> -int tmem_copy_from_client(struct page_info *, xen_pfn_t, pagesize_t 
> tmem_offset,
> -    pagesize_t pfn_offset, pagesize_t len, tmem_cli_va_param_t);
> +int tmem_copy_from_client(struct page_info *, xen_pfn_t, 
> tmem_cli_va_param_t);
>  
>  int tmem_copy_to_client(xen_pfn_t, struct page_info *, pagesize_t 
> tmem_offset,
>      pagesize_t pfn_offset, pagesize_t len, tmem_cli_va_param_t);
> -- 
> 1.7.10.4
> 

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.