Re: [PATCH 6/9] drm/vmwgfx: Implement an infrastructure for write-coherent resources

[Thomas Hellstrom]

On Mon, 2019-04-22 at 18:54 +0000, Deepak Singh Rawat wrote:
> Hi Thomas,
> 
> With minor comments below
> 
> Reviewed-by: Deepak Rawat <drawat@xxxxxxxxxx>
> 

Thanks for reviewing Deepak, Some comments below:

> On Fri, 2019-04-12 at 09:04 -0700, Thomas Hellstrom wrote:
> > This infrastructure will, for coherent resources, make sure that
> > from the user-space point of view, data written by the CPU is
> > immediately
> > automatically available to the GPU at resource validation time.
> > 
> > Signed-off-by: Thomas Hellstrom <thellstrom@xxxxxxxxxx>
> > ---
> >  drivers/gpu/drm/vmwgfx/Kconfig                |   1 +
> >  drivers/gpu/drm/vmwgfx/Makefile               |   2 +-
> >  drivers/gpu/drm/vmwgfx/vmwgfx_bo.c            |   5 +-
> >  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c           |   5 +
> >  drivers/gpu/drm/vmwgfx/vmwgfx_drv.h           |  26 +-
> >  drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c       |   1 -
> >  drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c    | 410
> > ++++++++++++++++++
> >  drivers/gpu/drm/vmwgfx/vmwgfx_resource.c      |  57 +++
> >  drivers/gpu/drm/vmwgfx/vmwgfx_resource_priv.h |  11 +
> >  drivers/gpu/drm/vmwgfx/vmwgfx_validation.c    |  74 ++++
> >  drivers/gpu/drm/vmwgfx/vmwgfx_validation.h    |  16 +-
> >  11 files changed, 588 insertions(+), 20 deletions(-)
> >  create mode 100644 drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c
> > 
> > diff --git a/drivers/gpu/drm/vmwgfx/Kconfig
> > b/drivers/gpu/drm/vmwgfx/Kconfig
> > index 6b28a326f8bb..d5fd81a521f6 100644
> > --- a/drivers/gpu/drm/vmwgfx/Kconfig
> > +++ b/drivers/gpu/drm/vmwgfx/Kconfig
> > @@ -8,6 +8,7 @@ config DRM_VMWGFX
> >  	select FB_CFB_IMAGEBLIT
> >  	select DRM_TTM
> >  	select FB
> > +	select AS_DIRTY_HELPERS
> >  	# Only needed for the transitional use of drm_crtc_init - can
> > be removed
> >  	# again once vmwgfx sets up the primary plane itself.
> >  	select DRM_KMS_HELPER
> > diff --git a/drivers/gpu/drm/vmwgfx/Makefile
> > b/drivers/gpu/drm/vmwgfx/Makefile
> > index 8841bd30e1e5..c877a21a0739 100644
> > --- a/drivers/gpu/drm/vmwgfx/Makefile
> > +++ b/drivers/gpu/drm/vmwgfx/Makefile
> > @@ -8,7 +8,7 @@ vmwgfx-y := vmwgfx_execbuf.o vmwgfx_gmr.o
> > vmwgfx_kms.o vmwgfx_drv.o \
> >  	    vmwgfx_cmdbuf_res.o vmwgfx_cmdbuf.o vmwgfx_stdu.o \
> >  	    vmwgfx_cotable.o vmwgfx_so.o vmwgfx_binding.o vmwgfx_msg.o
> > \
> >  	    vmwgfx_simple_resource.o vmwgfx_va.o vmwgfx_blit.o \
> > -	    vmwgfx_validation.o \
> > +	    vmwgfx_validation.o vmwgfx_page_dirty.o \
> >  	    ttm_object.o ttm_lock.o
> >  
> >  obj-$(CONFIG_DRM_VMWGFX) := vmwgfx.o
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> > b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> > index c0829d50eecc..90ca866640fe 100644
> > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> > @@ -463,6 +463,7 @@ void vmw_bo_bo_free(struct ttm_buffer_object
> > *bo)
> >  {
> >  	struct vmw_buffer_object *vmw_bo = vmw_buffer_object(bo);
> >  
> > +	WARN_ON(vmw_bo->dirty);
> >  	vmw_bo_unmap(vmw_bo);
> >  	kfree(vmw_bo);
> >  }
> > @@ -476,8 +477,10 @@ void vmw_bo_bo_free(struct ttm_buffer_object
> > *bo)
> >  static void vmw_user_bo_destroy(struct ttm_buffer_object *bo)
> >  {
> >  	struct vmw_user_buffer_object *vmw_user_bo =
> > vmw_user_buffer_object(bo);
> > +	struct vmw_buffer_object *vbo = &vmw_user_bo->vbo;
> >  
> > -	vmw_bo_unmap(&vmw_user_bo->vbo);
> > +	WARN_ON(vbo->dirty);
> 
> Is it possible for user-space to exploit this WARN? If yes then you
> might want to change the logic?
> 

Nope, if this WARN hits, then it's due to a bug. Don't want to use
BUG_ON() since it's non-fatal.

> > +	vmw_bo_unmap(vbo);
> >  	ttm_prime_object_kfree(vmw_user_bo, prime);
> >  }
> >  
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> > b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> > index 6165fe2c4504..74e94138877e 100644
> > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> > @@ -857,6 +857,11 @@ static int vmw_driver_load(struct drm_device
> > *dev, unsigned long chipset)
> >  		DRM_ERROR("Failed initializing TTM buffer object
> > driver.\n");
> >  		goto out_no_bdev;
> >  	}
> > +	dev_priv->vm_ops = *dev_priv->bdev.vm_ops;
> > +	dev_priv->vm_ops.fault = vmw_bo_vm_fault;
> > +	dev_priv->vm_ops.pfn_mkwrite = vmw_bo_vm_mkwrite;
> > +	dev_priv->vm_ops.page_mkwrite = vmw_bo_vm_mkwrite;
> > +	dev_priv->bdev.vm_ops = &dev_priv->vm_ops;
> >  
> >  	/*
> >  	 * Enable VRAM, but initially don't use it until SVGA is
> > enabled and
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> > b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> > index bd6919b90519..f05fce52fbb4 100644
> > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> > @@ -95,6 +95,7 @@ struct vmw_fpriv {
> >   * @dx_query_ctx: DX context if this buffer object is used as a DX
> > query MOB
> >   * @map: Kmap object for semi-persistent mappings
> >   * @res_prios: Eviction priority counts for attached resources
> > + * @dirty: structure for user-space dirty-tracking
> >   */
> >  struct vmw_buffer_object {
> >  	struct ttm_buffer_object base;
> > @@ -105,6 +106,7 @@ struct vmw_buffer_object {
> >  	/* Protected by reservation */
> >  	struct ttm_bo_kmap_obj map;
> >  	u32 res_prios[TTM_MAX_BO_PRIORITY];
> > +	struct vmw_bo_dirty *dirty;
> >  };
> >  
> >  /**
> > @@ -135,7 +137,8 @@ struct vmw_res_func;
> >   * @res_dirty: Resource contains data not yet in the backup
> > buffer.
> > Protected
> >   * by resource reserved.
> >   * @backup_dirty: Backup buffer contains data not yet in the HW
> > resource.
> > - * Protecte by resource reserved.
> > + * Protected by resource reserved.
> > + * @coherent: Emulate coherency by tracking vm accesses.
> >   * @backup: The backup buffer if any. Protected by resource
> > reserved.
> >   * @backup_offset: Offset into the backup buffer if any. Protected
> > by resource
> >   * reserved. Note that only a few resource types can have a
> > @backup_offset
> > @@ -152,14 +155,16 @@ struct vmw_res_func;
> >   * @hw_destroy: Callback to destroy the resource on the device, as
> > part of
> >   * resource destruction.
> >   */
> > +struct vmw_resource_dirty;
> >  struct vmw_resource {
> >  	struct kref kref;
> >  	struct vmw_private *dev_priv;
> >  	int id;
> >  	u32 used_prio;
> >  	unsigned long backup_size;
> > -	bool res_dirty;
> > -	bool backup_dirty;
> > +	u32 res_dirty : 1;
> > +	u32 backup_dirty : 1;
> 
> Is there a reason you changed res_dirty and backup_dirty from bool to
> u32. They are still areused as bool, right?

Got a comment when the WW mutex patches was reviewed, that bool should
be avoided in compund data types, so I'm trying to avoid them in new
code.

> 
> > +	u32 coherent : 1;
> >  	struct vmw_buffer_object *backup;
> >  	unsigned long backup_offset;
> >  	unsigned long pin_count;
> > @@ -167,6 +172,7 @@ struct vmw_resource {
> >  	struct list_head lru_head;
> >  	struct list_head mob_head;
> >  	struct list_head binding_head;
> > +	struct vmw_resource_dirty *dirty;
> >  	void (*res_free) (struct vmw_resource *res);
> >  	void (*hw_destroy) (struct vmw_resource *res);
> >  };
> > @@ -607,6 +613,9 @@ struct vmw_private {
> >  
> >  	/* Validation memory reservation */
> >  	struct vmw_validation_mem vvm;
> > +
> > +	/* VM operations */
> > +	struct vm_operations_struct vm_ops;
> >  };
> >  
> >  static inline struct vmw_surface *vmw_res_to_srf(struct
> > vmw_resource
> > *res)
> > @@ -723,6 +732,8 @@ extern void vmw_resource_evict_all(struct
> > vmw_private *dev_priv);
> >  extern void vmw_resource_unbind_list(struct vmw_buffer_object
> > *vbo);
> >  void vmw_resource_mob_attach(struct vmw_resource *res);
> >  void vmw_resource_mob_detach(struct vmw_resource *res);
> > +void vmw_resource_dirty_update(struct vmw_resource *res, pgoff_t
> > start,
> > +			       pgoff_t end);
> >  
> >  /**
> >   * vmw_resource_mob_attached - Whether a resource currently has a
> > mob attached
> > @@ -1411,6 +1422,15 @@ int vmw_host_log(const char *log);
> >  #define VMW_DEBUG_USER(fmt,
> > ...)                                              \
> >  	DRM_DEBUG_DRIVER(fmt, ##__VA_ARGS__)
> >  
> > +/* Resource dirtying - vmwgfx_page_dirty.c */
> > +void vmw_bo_dirty_scan(struct vmw_buffer_object *vbo);
> > +int vmw_bo_dirty_add(struct vmw_buffer_object *vbo);
> > +void vmw_bo_dirty_transfer_to_res(struct vmw_resource *res);
> > +void vmw_bo_dirty_clear_res(struct vmw_resource *res);
> > +void vmw_bo_dirty_release(struct vmw_buffer_object *vbo);
> > +vm_fault_t vmw_bo_vm_fault(struct vm_fault *vmf);
> > +vm_fault_t vmw_bo_vm_mkwrite(struct vm_fault *vmf);
> > +
> >  /**
> >   * Inline helper functions
> >   */
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
> > b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
> > index da3ac0bc2e14..7cb22119f516 100644
> > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
> > @@ -2483,7 +2483,6 @@ static int
> > vmw_cmd_dx_check_subresource(struct
> > vmw_private *dev_priv,
> >  		     offsetof(typeof(*cmd), sid));
> >  
> >  	cmd = container_of(header, typeof(*cmd), header);
> > -
> >  	return vmw_cmd_res_check(dev_priv, sw_context, vmw_res_surface,
> >  				 VMW_RES_DIRTY_NONE,
> > user_surface_converter,
> >  				 &cmd->sid, NULL);
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c
> > b/drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c
> > new file mode 100644
> > index 000000000000..87e4a73b1175
> > --- /dev/null
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c
> > @@ -0,0 +1,410 @@
> > +// SPDX-License-Identifier: GPL-2.0 OR MIT
> > +/*****************************************************************
> > **
> > *******
> > + *
> > + * Copyright 2019 VMware, Inc., Palo Alto, CA., USA
> > + *
> > + * Permission is hereby granted, free of charge, to any person
> > obtaining a
> > + * copy of this software and associated documentation files (the
> > + * "Software"), to deal in the Software without restriction,
> > including
> > + * without limitation the rights to use, copy, modify, merge,
> > publish,
> > + * distribute, sub license, and/or sell copies of the Software,
> > and
> > to
> > + * permit persons to whom the Software is furnished to do so,
> > subject to
> > + * the following conditions:
> > + *
> > + * The above copyright notice and this permission notice
> > (including
> > the
> > + * next paragraph) shall be included in all copies or substantial
> > portions
> > + * of the Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > EXPRESS OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO
> > EVENT SHALL
> > + * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE
> > FOR
> > ANY CLAIM,
> > + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
> > TORT OR
> > + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
> > SOFTWARE OR THE
> > + * USE OR OTHER DEALINGS IN THE SOFTWARE.
> > + *
> > +
> > *******************************************************************
> > **
> > *****/
> > +#include "vmwgfx_drv.h"
> > +
> > +/*
> > + * Different methods for tracking dirty:
> > + * VMW_BO_DIRTY_PAGETABLE - Scan the pagetable for hardware dirty
> > bits
> > + * VMW_BO_DIRTY_MKWRITE - Write-protect page table entries and
> > record write-
> > + * accesses in the VM mkwrite() callback
> > + */
> > +enum vmw_bo_dirty_method {
> > +	VMW_BO_DIRTY_PAGETABLE,
> > +	VMW_BO_DIRTY_MKWRITE,
> > +};
> > +
> > +/*
> > + * No dirtied pages at scan trigger a transition to the _MKWRITE
> > method,
> > + * similarly a certain percentage of dirty pages trigger a
> > transition to
> > + * the _PAGETABLE method. How many triggers should we wait for
> > before
> > + * changing method?
> > + */
> > +#define VMW_DIRTY_NUM_CHANGE_TRIGGERS 2
> > +
> > +/* Percentage to trigger a transition to the _PAGETABLE method */
> > +#define VMW_DIRTY_PERCENTAGE 10
> > +
> > +/**
> > + * struct vmw_bo_dirty - Dirty information for buffer objects
> > + * @start: First currently dirty bit
> > + * @end: Last currently dirty bit + 1
> > + * @method: The currently used dirty method
> > + * @change_count: Number of consecutive method change triggers
> > + * @ref_count: Reference count for this structure
> > + * @bitmap_size: The size of the bitmap in bits. Typically equal
> > to
> > the
> > + * nuber of pages in the bo.
> > + * @size: The accounting size for this struct.
> > + * @bitmap: A bitmap where each bit represents a page. A set bit
> > means a
> > + * dirty page.
> > + */
> > +struct vmw_bo_dirty {
> > +	unsigned long start;
> > +	unsigned long end;
> > +	enum vmw_bo_dirty_method method;
> > +	unsigned int change_count;
> > +	unsigned int ref_count;
> > +	unsigned long bitmap_size;
> > +	size_t size;
> > +	unsigned long bitmap[0];
> > +};
> > +
> > +/**
> > + * vmw_bo_dirty_scan_pagetable - Perform a pagetable scan for
> > dirty
> > bits
> > + * @vbo: The buffer object to scan
> > + *
> > + * Scans the pagetable for dirty bits. Clear those bits and modify
> > the
> > + * dirty structure with the results. This function may change the
> > + * dirty-tracking method.
> > + */
> > +static void vmw_bo_dirty_scan_pagetable(struct vmw_buffer_object
> > *vbo)
> > +{
> > +	struct vmw_bo_dirty *dirty = vbo->dirty;
> > +	pgoff_t offset = drm_vma_node_start(&vbo->base.vma_node);
> > +	struct address_space *mapping = vbo->base.bdev->dev_mapping;
> > +	pgoff_t num_marked;
> > +
> > +	num_marked = apply_as_clean(mapping,
> > +				    offset, dirty->bitmap_size,
> > +				    offset, &dirty->bitmap[0],
> > +				    &dirty->start, &dirty->end);
> > +	if (num_marked == 0)
> > +		dirty->change_count++;
> > +	else
> > +		dirty->change_count = 0;
> > +
> > +	if (dirty->change_count > VMW_DIRTY_NUM_CHANGE_TRIGGERS) {
> > +		dirty->change_count = 0;
> > +		dirty->method = VMW_BO_DIRTY_MKWRITE;
> > +		apply_as_wrprotect(mapping,
> > +				   offset, dirty->bitmap_size);
> > +		apply_as_clean(mapping,
> > +			       offset, dirty->bitmap_size,
> > +			       offset, &dirty->bitmap[0],
> > +			       &dirty->start, &dirty->end);
> > +	}
> > +}
> > +
> > +/**
> > + * vmw_bo_dirty_scan_mkwrite - Reset the mkwrite dirty-tracking
> > method
> > + * @vbo: The buffer object to scan
> > + *
> > + * Write-protect pages written to so that consecutive write
> > accesses
> > will
> > + * trigger a call to mkwrite.
> > + *
> > + * This function may change the dirty-tracking method.
> > + */
> > +static void vmw_bo_dirty_scan_mkwrite(struct vmw_buffer_object
> > *vbo)
> > +{
> > +	struct vmw_bo_dirty *dirty = vbo->dirty;
> > +	unsigned long offset = drm_vma_node_start(&vbo->base.vma_node);
> > +	struct address_space *mapping = vbo->base.bdev->dev_mapping;
> > +	pgoff_t num_marked;
> > +
> > +	if (dirty->end <= dirty->start)
> > +		return;
> > +
> > +	num_marked = apply_as_wrprotect(vbo->base.bdev->dev_mapping,
> > +					dirty->start + offset,
> > +					dirty->end - dirty->start);
> > +
> > +	if (100UL * num_marked / dirty->bitmap_size >
> > +	    VMW_DIRTY_PERCENTAGE) {
> > +		dirty->change_count++;
> > +	} else {
> > +		dirty->change_count = 0;
> > +	}
> > +
> > +	if (dirty->change_count > VMW_DIRTY_NUM_CHANGE_TRIGGERS) {
> > +		pgoff_t start = 0;
> > +		pgoff_t end = dirty->bitmap_size;
> > +
> > +		dirty->method = VMW_BO_DIRTY_PAGETABLE;
> > +		apply_as_clean(mapping, offset, end, offset, &dirty-
> > > bitmap[0],
> > +			       &start, &end);
> > +		bitmap_clear(&dirty->bitmap[0], 0, dirty->bitmap_size);
> > +		if (dirty->start < dirty->end)
> > +			bitmap_set(&dirty->bitmap[0], dirty->start,
> > +				   dirty->end - dirty->start);
> > +		dirty->change_count = 0;
> > +	}
> > +}
> > +
> > +
> > +/**
> > + * vmw_bo_dirty_scan - Scan for dirty pages and add them to the
> > dirty
> > + * tracking structure
> > + * @vbo: The buffer object to scan
> > + *
> > + * This function may change the dirty tracking method.
> > + */
> > +void vmw_bo_dirty_scan(struct vmw_buffer_object *vbo)
> > +{
> > +	struct vmw_bo_dirty *dirty = vbo->dirty;
> > +
> > +	if (dirty->method == VMW_BO_DIRTY_PAGETABLE)
> > +		vmw_bo_dirty_scan_pagetable(vbo);
> > +	else
> > +		vmw_bo_dirty_scan_mkwrite(vbo);
> > +}
> > +
> > +/**
> > + * vmw_bo_dirty_add - Add a dirty-tracking user to a buffer object
> > + * @vbo: The buffer object
> > + *
> > + * This function registers a dirty-tracking user to a buffer
> > object.
> > + * A user can be for example a resource or a vma in a special
> > user-
> > space
> > + * mapping.
> > + *
> > + * Return: Zero on success, -ENOMEM on memory allocation failure.
> > + */
> > +int vmw_bo_dirty_add(struct vmw_buffer_object *vbo)
> > +{
> > +	struct vmw_bo_dirty *dirty = vbo->dirty;
> > +	pgoff_t num_pages = vbo->base.num_pages;
> > +	size_t size, acc_size;
> > +	int ret;
> > +	static struct ttm_operation_ctx ctx = {
> > +		.interruptible = false,
> > +		.no_wait_gpu = false
> > +	};
> > +
> > +	if (dirty) {
> > +		dirty->ref_count++;
> > +		return 0;
> > +	}
> > +
> > +	size = sizeof(*dirty) + BITS_TO_LONGS(num_pages) *
> > sizeof(long);
> > +	acc_size = ttm_round_pot(size);
> > +	ret = ttm_mem_global_alloc(&ttm_mem_glob, acc_size, &ctx);
> > +	if (ret) {
> > +		VMW_DEBUG_USER("Out of graphics memory for buffer
> > object "
> > +			       "dirty tracker.\n");
> > +		return ret;
> > +	}
> > +	dirty = kvzalloc(size, GFP_KERNEL);
> > +	if (!dirty) {
> > +		ret = -ENOMEM;
> > +		goto out_no_dirty;
> > +	}
> > +
> > +	dirty->size = acc_size;
> > +	dirty->bitmap_size = num_pages;
> > +	dirty->start = dirty->bitmap_size;
> > +	dirty->end = 0;
> > +	dirty->ref_count = 1;
> > +	if (num_pages < PAGE_SIZE / sizeof(pte_t)) {
> > +		dirty->method = VMW_BO_DIRTY_PAGETABLE;
> > +	} else {
> > +		struct address_space *mapping = vbo->base.bdev-
> > > dev_mapping;
> > +		pgoff_t offset = drm_vma_node_start(&vbo-
> > > base.vma_node);
> > +
> > +		dirty->method = VMW_BO_DIRTY_MKWRITE;
> > +
> > +		/* Write-protect and then pick up already dirty bits */
> > +		apply_as_wrprotect(mapping, offset, num_pages);
> > +		apply_as_clean(mapping, offset, num_pages, offset,
> > +			       &dirty->bitmap[0], &dirty->start,
> > &dirty->end);
> > +	}
> > +
> > +	vbo->dirty = dirty;
> > +
> > +	return 0;
> > +
> > +out_no_dirty:
> > +	ttm_mem_global_free(&ttm_mem_glob, acc_size);
> > +	return ret;
> > +}
> > +
> > +/**
> > + * vmw_bo_dirty_release - Release a dirty-tracking user from a
> > buffer object
> > + * @vbo: The buffer object
> > + *
> > + * This function releases a dirty-tracking user from a buffer
> > object.
> > + * If the reference count reaches zero, then the dirty-tracking
> > object is
> > + * freed and the pointer to it cleared.
> > + *
> > + * Return: Zero on success, -ENOMEM on memory allocation failure.
> > + */
> > +void vmw_bo_dirty_release(struct vmw_buffer_object *vbo)
> > +{
> > +	struct vmw_bo_dirty *dirty = vbo->dirty;
> > +
> > +	if (dirty && --dirty->ref_count == 0) {
> > +		size_t acc_size = dirty->size;
> > +
> > +		kvfree(dirty);
> > +		ttm_mem_global_free(&ttm_mem_glob, acc_size);
> > +		vbo->dirty = NULL;
> > +	}
> > +}
> > +
> > +/**
> > + * vmw_bo_dirty_transfer_to_res - Pick up a resource's dirty
> > region
> > from
> > + * its backing mob.
> > + * @res: The resource
> > + *
> > + * This function will pick up all dirty ranges affecting the
> > resource from
> > + * it's backup mob, and call vmw_resource_dirty_update() once for
> > each
> > + * range. The transferred ranges will be cleared from the backing
> > mob's
> > + * dirty tracking.
> > + */
> > +void vmw_bo_dirty_transfer_to_res(struct vmw_resource *res)
> > +{
> > +	struct vmw_buffer_object *vbo = res->backup;
> > +	struct vmw_bo_dirty *dirty = vbo->dirty;
> > +	pgoff_t start, cur, end;
> > +	unsigned long res_start = res->backup_offset;
> > +	unsigned long res_end = res->backup_offset + res->backup_size;
> > +
> > +	WARN_ON_ONCE(res_start & ~PAGE_MASK);
> > +	res_start >>= PAGE_SHIFT;
> > +	res_end = DIV_ROUND_UP(res_end, PAGE_SIZE);
> > +
> > +	if (res_start >= dirty->end || res_end <= dirty->start)
> > +		return;
> > +
> > +	cur = max(res_start, dirty->start);
> > +	res_end = max(res_end, dirty->end);
> > +	while (cur < res_end) {
> > +		unsigned long num;
> > +
> > +		start = find_next_bit(&dirty->bitmap[0], res_end, cur);
> > +		if (start >= res_end)
> > +			break;
> > +
> > +		end = find_next_zero_bit(&dirty->bitmap[0], res_end,
> > start + 1);
> > +		cur = end + 1;
> > +		num = end - start;
> > +		bitmap_clear(&dirty->bitmap[0], start, num);
> > +		vmw_resource_dirty_update(res, start, end);
> > +	}
> > +
> > +	if (res_start <= dirty->start && res_end > dirty->start)
> > +		dirty->start = res_end;
> > +	if (res_start < dirty->end && res_end >= dirty->end)
> > +		dirty->end = res_start;
> > +}
> > +
> > +/**
> > + * vmw_bo_dirty_clear_res - Clear a resource's dirty region from
> > + * its backing mob.
> > + * @res: The resource
> > + *
> > + * This function will clar all dirty ranges affecting the
> > resource 
> 
> 

...

Will address the rest of the comments.

/Thomas