[patch] staging: ion: use two separate locks for heaps and clients in ion_device

[patch] staging: ion: use two separate locks for heaps and clients in ion_device

[Xu YiPing]

ion_alloc may get into slow path to get free page,
the call stack:

__alloc_pages_slowpath
ion_page_pool_alloc_pages
alloc_buffer_page
ion_system_heap_allocate
ion_buffer_create  <-- hold ion_device->lock
ion_alloc

after that, kernel invokes low-memory killer to kill some apps in
order to free memory in android system. However, sometimes, the
killing is blocked,
the app's call stack:

rwsem_down_write_failed
down_write
ion_client_destroy
ion_release
fput
do_signal

the killing is blocked because ion_device->lock is held by ion_alloc.

ion_alloc hold the lock for accessing the heaps list,
ion_destroy_client hold the lock for accessing the clients list.

so, we can use two separate locks for heaps and clients, to avoid the
unnecessary race.

Signed-off-by: Xu YiPing <xuyiping@hisilicon.com>
---
 drivers/staging/android/ion/ion.c | 37 ++++++++++++++++++++-----------------
 1 file changed, 20 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index a2cf93b..331ad0f 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -46,7 +46,8 @@
  * @dev:		the actual misc device
  * @buffers:		an rb tree of all the existing buffers
  * @buffer_lock:	lock protecting the tree of buffers
- * @lock:		rwsem protecting the tree of heaps and clients
+ * @client_lock:	rwsem protecting the tree of clients
+ * @heap_lock:		rwsem protecting the tree of heaps
  * @heaps:		list of all the heaps in the system
  * @user_clients:	list of all the clients created from userspace
  */
@@ -54,7 +55,8 @@ struct ion_device {
 	struct miscdevice dev;
 	struct rb_root buffers;
 	struct mutex buffer_lock;
-	struct rw_semaphore lock;
+	struct rw_semaphore client_lock;
+	struct rw_semaphore heap_lock;
 	struct plist_head heaps;
 	long (*custom_ioctl)(struct ion_client *client, unsigned int cmd,
 			     unsigned long arg);
@@ -146,7 +148,7 @@ static inline void ion_buffer_page_clean(struct page **page)
 	*page = (struct page *)((unsigned long)(*page) & ~(1UL));
 }

-/* this function should only be called while dev->lock is held */
+/* this function should only be called while dev->heap_lock is held */
 static void ion_buffer_add(struct ion_device *dev,
 			   struct ion_buffer *buffer)
 {
@@ -172,7 +174,7 @@ static void ion_buffer_add(struct ion_device *dev,
 	rb_insert_color(&buffer->node, &dev->buffers);
 }

-/* this function should only be called while dev->lock is held */
+/* this function should only be called while dev->heap_lock is held */
 static struct ion_buffer *ion_buffer_create(struct ion_heap *heap,
 				     struct ion_device *dev,
 				     unsigned long len,
@@ -511,7 +513,7 @@ struct ion_handle *ion_alloc(struct ion_client *client, size_t len,
 	if (!len)
 		return ERR_PTR(-EINVAL);

-	down_read(&dev->lock);
+	down_read(&dev->heap_lock);
 	plist_for_each_entry(heap, &dev->heaps, node) {
 		/* if the caller didn't specify this heap id */
 		if (!((1 << heap->id) & heap_id_mask))
@@ -520,7 +522,7 @@ struct ion_handle *ion_alloc(struct ion_client *client, size_t len,
 		if (!IS_ERR(buffer))
 			break;
 	}
-	up_read(&dev->lock);
+	up_read(&dev->heap_lock);

 	if (buffer == NULL)
 		return ERR_PTR(-ENODEV);
@@ -713,7 +715,7 @@ static int is_client_alive(struct ion_client *client)
 	node = ion_root_client->rb_node;
 	dev = container_of(ion_root_client, struct ion_device, clients);

-	down_read(&dev->lock);
+	down_read(&dev->client_lock);
 	while (node) {
 		tmp = rb_entry(node, struct ion_client, node);
 		if (client < tmp) {
@@ -721,12 +723,12 @@ static int is_client_alive(struct ion_client *client)
 		} else if (client > tmp) {
 			node = node->rb_right;
 		} else {
-			up_read(&dev->lock);
+			up_read(&dev->client_lock);
 			return 1;
 		}
 	}

-	up_read(&dev->lock);
+	up_read(&dev->client_lock);
 	return 0;
 }

@@ -841,12 +843,12 @@ struct ion_client *ion_client_create(struct ion_device *dev,
 	if (!client->name)
 		goto err_free_client;

-	down_write(&dev->lock);
+	down_write(&dev->client_lock);
 	client->display_serial = ion_get_client_serial(&dev->clients, name);
 	client->display_name = kasprintf(
 		GFP_KERNEL, "%s-%d", name, client->display_serial);
 	if (!client->display_name) {
-		up_write(&dev->lock);
+		up_write(&dev->client_lock);
 		goto err_free_client_name;
 	}
 	p = &dev->clients.rb_node;
@@ -873,7 +875,7 @@ struct ion_client *ion_client_create(struct ion_device *dev,
 			path, client->display_name);
 	}

-	up_write(&dev->lock);
+	up_write(&dev->client_lock);

 	return client;

@@ -903,12 +905,12 @@ void ion_client_destroy(struct ion_client *client)

 	idr_destroy(&client->idr);

-	down_write(&dev->lock);
+	down_write(&dev->client_lock);
 	if (client->task)
 		put_task_struct(client->task);
 	rb_erase(&client->node, &dev->clients);
 	debugfs_remove_recursive(client->debug_root);
-	up_write(&dev->lock);
+	up_write(&dev->client_lock);

 	kfree(client->display_name);
 	kfree(client->name);
@@ -1603,7 +1605,7 @@ void ion_device_add_heap(struct ion_device *dev, struct ion_heap *heap)
 		ion_heap_init_shrinker(heap);

 	heap->dev = dev;
-	down_write(&dev->lock);
+	down_write(&dev->heap_lock);
 	/*
 	 * use negative heap->id to reverse the priority -- when traversing
 	 * the list later attempt higher id numbers first
@@ -1638,7 +1640,7 @@ void ion_device_add_heap(struct ion_device *dev, struct ion_heap *heap)
 		}
 	}

-	up_write(&dev->lock);
+	up_write(&dev->heap_lock);
 }
 EXPORT_SYMBOL(ion_device_add_heap);

@@ -1685,7 +1687,8 @@ debugfs_done:
 	idev->custom_ioctl = custom_ioctl;
 	idev->buffers = RB_ROOT;
 	mutex_init(&idev->buffer_lock);
-	init_rwsem(&idev->lock);
+	init_rwsem(&idev->client_lock);
+	init_rwsem(&idev->heap_lock);
 	plist_head_init(&idev->heaps);
 	idev->clients = RB_ROOT;
 	ion_root_client = &idev->clients;
--
2.7.4

Re: [patch] staging: ion: use two separate locks for heaps and clients in ion_device

[Laura Abbott]

On 09/30/2016 01:18 AM, Xu YiPing wrote:
> ion_alloc may get into slow path to get free page,
> the call stack:
>
> __alloc_pages_slowpath
> ion_page_pool_alloc_pages
> alloc_buffer_page
> ion_system_heap_allocate
> ion_buffer_create  <-- hold ion_device->lock
> ion_alloc
>
> after that, kernel invokes low-memory killer to kill some apps in
> order to free memory in android system. However, sometimes, the
> killing is blocked,
> the app's call stack:
>
> rwsem_down_write_failed
> down_write
> ion_client_destroy
> ion_release
> fput
> do_signal
>
> the killing is blocked because ion_device->lock is held by ion_alloc.
>
> ion_alloc hold the lock for accessing the heaps list,
> ion_destroy_client hold the lock for accessing the clients list.
>
> so, we can use two separate locks for heaps and clients, to avoid the
> unnecessary race.
>

I've reviewed this and it looks okay at first pass but I don't want it
applied just yet. Ion locking is a bit of a mess and has been added
once piece at a time. It needs a fundamental review. There will be Ion
discussions at plumbers at the end of October. Let's come back to this
after plumbers.

> Signed-off-by: Xu YiPing <xuyiping@hisilicon.com>
> ---
>  drivers/staging/android/ion/ion.c | 37 ++++++++++++++++++++-----------------
>  1 file changed, 20 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
> index a2cf93b..331ad0f 100644
> --- a/drivers/staging/android/ion/ion.c
> +++ b/drivers/staging/android/ion/ion.c
> @@ -46,7 +46,8 @@
>   * @dev:		the actual misc device
>   * @buffers:		an rb tree of all the existing buffers
>   * @buffer_lock:	lock protecting the tree of buffers
> - * @lock:		rwsem protecting the tree of heaps and clients
> + * @client_lock:	rwsem protecting the tree of clients
> + * @heap_lock:		rwsem protecting the tree of heaps
>   * @heaps:		list of all the heaps in the system
>   * @user_clients:	list of all the clients created from userspace
>   */
> @@ -54,7 +55,8 @@ struct ion_device {
>  	struct miscdevice dev;
>  	struct rb_root buffers;
>  	struct mutex buffer_lock;
> -	struct rw_semaphore lock;
> +	struct rw_semaphore client_lock;
> +	struct rw_semaphore heap_lock;
>  	struct plist_head heaps;
>  	long (*custom_ioctl)(struct ion_client *client, unsigned int cmd,
>  			     unsigned long arg);
> @@ -146,7 +148,7 @@ static inline void ion_buffer_page_clean(struct page **page)
>  	*page = (struct page *)((unsigned long)(*page) & ~(1UL));
>  }
>
> -/* this function should only be called while dev->lock is held */
> +/* this function should only be called while dev->heap_lock is held */
>  static void ion_buffer_add(struct ion_device *dev,
>  			   struct ion_buffer *buffer)
>  {
> @@ -172,7 +174,7 @@ static void ion_buffer_add(struct ion_device *dev,
>  	rb_insert_color(&buffer->node, &dev->buffers);
>  }
>
> -/* this function should only be called while dev->lock is held */
> +/* this function should only be called while dev->heap_lock is held */
>  static struct ion_buffer *ion_buffer_create(struct ion_heap *heap,
>  				     struct ion_device *dev,
>  				     unsigned long len,
> @@ -511,7 +513,7 @@ struct ion_handle *ion_alloc(struct ion_client *client, size_t len,
>  	if (!len)
>  		return ERR_PTR(-EINVAL);
>
> -	down_read(&dev->lock);
> +	down_read(&dev->heap_lock);
>  	plist_for_each_entry(heap, &dev->heaps, node) {
>  		/* if the caller didn't specify this heap id */
>  		if (!((1 << heap->id) & heap_id_mask))
> @@ -520,7 +522,7 @@ struct ion_handle *ion_alloc(struct ion_client *client, size_t len,
>  		if (!IS_ERR(buffer))
>  			break;
>  	}
> -	up_read(&dev->lock);
> +	up_read(&dev->heap_lock);
>
>  	if (buffer == NULL)
>  		return ERR_PTR(-ENODEV);
> @@ -713,7 +715,7 @@ static int is_client_alive(struct ion_client *client)
>  	node = ion_root_client->rb_node;
>  	dev = container_of(ion_root_client, struct ion_device, clients);
>
> -	down_read(&dev->lock);
> +	down_read(&dev->client_lock);
>  	while (node) {
>  		tmp = rb_entry(node, struct ion_client, node);
>  		if (client < tmp) {
> @@ -721,12 +723,12 @@ static int is_client_alive(struct ion_client *client)
>  		} else if (client > tmp) {
>  			node = node->rb_right;
>  		} else {
> -			up_read(&dev->lock);
> +			up_read(&dev->client_lock);
>  			return 1;
>  		}
>  	}
>
> -	up_read(&dev->lock);
> +	up_read(&dev->client_lock);
>  	return 0;
>  }
>
> @@ -841,12 +843,12 @@ struct ion_client *ion_client_create(struct ion_device *dev,
>  	if (!client->name)
>  		goto err_free_client;
>
> -	down_write(&dev->lock);
> +	down_write(&dev->client_lock);
>  	client->display_serial = ion_get_client_serial(&dev->clients, name);
>  	client->display_name = kasprintf(
>  		GFP_KERNEL, "%s-%d", name, client->display_serial);
>  	if (!client->display_name) {
> -		up_write(&dev->lock);
> +		up_write(&dev->client_lock);
>  		goto err_free_client_name;
>  	}
>  	p = &dev->clients.rb_node;
> @@ -873,7 +875,7 @@ struct ion_client *ion_client_create(struct ion_device *dev,
>  			path, client->display_name);
>  	}
>
> -	up_write(&dev->lock);
> +	up_write(&dev->client_lock);
>
>  	return client;
>
> @@ -903,12 +905,12 @@ void ion_client_destroy(struct ion_client *client)
>
>  	idr_destroy(&client->idr);
>
> -	down_write(&dev->lock);
> +	down_write(&dev->client_lock);
>  	if (client->task)
>  		put_task_struct(client->task);
>  	rb_erase(&client->node, &dev->clients);
>  	debugfs_remove_recursive(client->debug_root);
> -	up_write(&dev->lock);
> +	up_write(&dev->client_lock);
>
>  	kfree(client->display_name);
>  	kfree(client->name);
> @@ -1603,7 +1605,7 @@ void ion_device_add_heap(struct ion_device *dev, struct ion_heap *heap)
>  		ion_heap_init_shrinker(heap);
>
>  	heap->dev = dev;
> -	down_write(&dev->lock);
> +	down_write(&dev->heap_lock);
>  	/*
>  	 * use negative heap->id to reverse the priority -- when traversing
>  	 * the list later attempt higher id numbers first
> @@ -1638,7 +1640,7 @@ void ion_device_add_heap(struct ion_device *dev, struct ion_heap *heap)
>  		}
>  	}
>
> -	up_write(&dev->lock);
> +	up_write(&dev->heap_lock);
>  }
>  EXPORT_SYMBOL(ion_device_add_heap);
>
> @@ -1685,7 +1687,8 @@ debugfs_done:
>  	idev->custom_ioctl = custom_ioctl;
>  	idev->buffers = RB_ROOT;
>  	mutex_init(&idev->buffer_lock);
> -	init_rwsem(&idev->lock);
> +	init_rwsem(&idev->client_lock);
> +	init_rwsem(&idev->heap_lock);
>  	plist_head_init(&idev->heaps);
>  	idev->clients = RB_ROOT;
>  	ion_root_client = &idev->clients;
> --
> 2.7.4
>

Re: [patch] staging: ion: use two separate locks for heaps and clients in ion_device

[YiPing Xu]

On 2016/10/5 2:02, Laura Abbott wrote:
> On 09/30/2016 01:18 AM, Xu YiPing wrote:
>> ion_alloc may get into slow path to get free page,
>> the call stack:
>>
>> __alloc_pages_slowpath
>> ion_page_pool_alloc_pages
>> alloc_buffer_page
>> ion_system_heap_allocate
>> ion_buffer_create  <-- hold ion_device->lock
>> ion_alloc
>>
>> after that, kernel invokes low-memory killer to kill some apps in
>> order to free memory in android system. However, sometimes, the
>> killing is blocked,
>> the app's call stack:
>>
>> rwsem_down_write_failed
>> down_write
>> ion_client_destroy
>> ion_release
>> fput
>> do_signal
>>
>> the killing is blocked because ion_device->lock is held by ion_alloc.
>>
>> ion_alloc hold the lock for accessing the heaps list,
>> ion_destroy_client hold the lock for accessing the clients list.
>>
>> so, we can use two separate locks for heaps and clients, to avoid the
>> unnecessary race.
>>
>
> I've reviewed this and it looks okay at first pass but I don't want it
> applied just yet. Ion locking is a bit of a mess and has been added

yes, and now "debugfs_mutex" and "ion_root_client" is redundant, after 
commit 49d200deaa680501f19a247b1fffb29301e51d2b and 
9fd4dcece43a53e5a9e65a973df5693702ee6401.

> once piece at a time. It needs a fundamental review. There will be Ion
> discussions at plumbers at the end of October. Let's come back to this
> after plumbers.
>
>> Signed-off-by: Xu YiPing <xuyiping@hisilicon.com>
>> ---
>>  drivers/staging/android/ion/ion.c | 37
>> ++++++++++++++++++++-----------------
>>  1 file changed, 20 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/staging/android/ion/ion.c
>> b/drivers/staging/android/ion/ion.c
>> index a2cf93b..331ad0f 100644
>> --- a/drivers/staging/android/ion/ion.c
>> +++ b/drivers/staging/android/ion/ion.c
>> @@ -46,7 +46,8 @@
>>   * @dev:        the actual misc device
>>   * @buffers:        an rb tree of all the existing buffers
>>   * @buffer_lock:    lock protecting the tree of buffers
>> - * @lock:        rwsem protecting the tree of heaps and clients
>> + * @client_lock:    rwsem protecting the tree of clients
>> + * @heap_lock:        rwsem protecting the tree of heaps
>>   * @heaps:        list of all the heaps in the system
>>   * @user_clients:    list of all the clients created from userspace
>>   */
>> @@ -54,7 +55,8 @@ struct ion_device {
>>      struct miscdevice dev;
>>      struct rb_root buffers;
>>      struct mutex buffer_lock;
>> -    struct rw_semaphore lock;
>> +    struct rw_semaphore client_lock;
>> +    struct rw_semaphore heap_lock;
>>      struct plist_head heaps;
>>      long (*custom_ioctl)(struct ion_client *client, unsigned int cmd,
>>                   unsigned long arg);
>> @@ -146,7 +148,7 @@ static inline void ion_buffer_page_clean(struct
>> page **page)
>>      *page = (struct page *)((unsigned long)(*page) & ~(1UL));
>>  }
>>
>> -/* this function should only be called while dev->lock is held */
>> +/* this function should only be called while dev->heap_lock is held */
>>  static void ion_buffer_add(struct ion_device *dev,
>>                 struct ion_buffer *buffer)
>>  {
>> @@ -172,7 +174,7 @@ static void ion_buffer_add(struct ion_device *dev,
>>      rb_insert_color(&buffer->node, &dev->buffers);
>>  }
>>
>> -/* this function should only be called while dev->lock is held */
>> +/* this function should only be called while dev->heap_lock is held */
>>  static struct ion_buffer *ion_buffer_create(struct ion_heap *heap,
>>                       struct ion_device *dev,
>>                       unsigned long len,
>> @@ -511,7 +513,7 @@ struct ion_handle *ion_alloc(struct ion_client
>> *client, size_t len,
>>      if (!len)
>>          return ERR_PTR(-EINVAL);
>>
>> -    down_read(&dev->lock);
>> +    down_read(&dev->heap_lock);
>>      plist_for_each_entry(heap, &dev->heaps, node) {
>>          /* if the caller didn't specify this heap id */
>>          if (!((1 << heap->id) & heap_id_mask))
>> @@ -520,7 +522,7 @@ struct ion_handle *ion_alloc(struct ion_client
>> *client, size_t len,
>>          if (!IS_ERR(buffer))
>>              break;
>>      }
>> -    up_read(&dev->lock);
>> +    up_read(&dev->heap_lock);
>>
>>      if (buffer == NULL)
>>          return ERR_PTR(-ENODEV);
>> @@ -713,7 +715,7 @@ static int is_client_alive(struct ion_client *client)
>>      node = ion_root_client->rb_node;
>>      dev = container_of(ion_root_client, struct ion_device, clients);
>>
>> -    down_read(&dev->lock);
>> +    down_read(&dev->client_lock);
>>      while (node) {
>>          tmp = rb_entry(node, struct ion_client, node);
>>          if (client < tmp) {
>> @@ -721,12 +723,12 @@ static int is_client_alive(struct ion_client
>> *client)
>>          } else if (client > tmp) {
>>              node = node->rb_right;
>>          } else {
>> -            up_read(&dev->lock);
>> +            up_read(&dev->client_lock);
>>              return 1;
>>          }
>>      }
>>
>> -    up_read(&dev->lock);
>> +    up_read(&dev->client_lock);
>>      return 0;
>>  }
>>
>> @@ -841,12 +843,12 @@ struct ion_client *ion_client_create(struct
>> ion_device *dev,
>>      if (!client->name)
>>          goto err_free_client;
>>
>> -    down_write(&dev->lock);
>> +    down_write(&dev->client_lock);
>>      client->display_serial = ion_get_client_serial(&dev->clients, name);
>>      client->display_name = kasprintf(
>>          GFP_KERNEL, "%s-%d", name, client->display_serial);
>>      if (!client->display_name) {
>> -        up_write(&dev->lock);
>> +        up_write(&dev->client_lock);
>>          goto err_free_client_name;
>>      }
>>      p = &dev->clients.rb_node;
>> @@ -873,7 +875,7 @@ struct ion_client *ion_client_create(struct
>> ion_device *dev,
>>              path, client->display_name);
>>      }
>>
>> -    up_write(&dev->lock);
>> +    up_write(&dev->client_lock);
>>
>>      return client;
>>
>> @@ -903,12 +905,12 @@ void ion_client_destroy(struct ion_client *client)
>>
>>      idr_destroy(&client->idr);
>>
>> -    down_write(&dev->lock);
>> +    down_write(&dev->client_lock);
>>      if (client->task)
>>          put_task_struct(client->task);
>>      rb_erase(&client->node, &dev->clients);
>>      debugfs_remove_recursive(client->debug_root);
>> -    up_write(&dev->lock);
>> +    up_write(&dev->client_lock);
>>
>>      kfree(client->display_name);
>>      kfree(client->name);
>> @@ -1603,7 +1605,7 @@ void ion_device_add_heap(struct ion_device *dev,
>> struct ion_heap *heap)
>>          ion_heap_init_shrinker(heap);
>>
>>      heap->dev = dev;
>> -    down_write(&dev->lock);
>> +    down_write(&dev->heap_lock);
>>      /*
>>       * use negative heap->id to reverse the priority -- when traversing
>>       * the list later attempt higher id numbers first
>> @@ -1638,7 +1640,7 @@ void ion_device_add_heap(struct ion_device *dev,
>> struct ion_heap *heap)
>>          }
>>      }
>>
>> -    up_write(&dev->lock);
>> +    up_write(&dev->heap_lock);
>>  }
>>  EXPORT_SYMBOL(ion_device_add_heap);
>>
>> @@ -1685,7 +1687,8 @@ debugfs_done:
>>      idev->custom_ioctl = custom_ioctl;
>>      idev->buffers = RB_ROOT;
>>      mutex_init(&idev->buffer_lock);
>> -    init_rwsem(&idev->lock);
>> +    init_rwsem(&idev->client_lock);
>> +    init_rwsem(&idev->heap_lock);
>>      plist_head_init(&idev->heaps);
>>      idev->clients = RB_ROOT;
>>      ion_root_client = &idev->clients;
>> --
>> 2.7.4
>>
>
>
> .
>

Re: [patch] staging: ion: use two separate locks for heaps and clients in ion_device

[Greg KH]

On Fri, Oct 07, 2016 at 04:27:49PM +0800, YiPing Xu wrote:
> 
> 
> On 2016/10/5 2:02, Laura Abbott wrote:
> > On 09/30/2016 01:18 AM, Xu YiPing wrote:
> > > ion_alloc may get into slow path to get free page,
> > > the call stack:
> > > 
> > > __alloc_pages_slowpath
> > > ion_page_pool_alloc_pages
> > > alloc_buffer_page
> > > ion_system_heap_allocate
> > > ion_buffer_create  <-- hold ion_device->lock
> > > ion_alloc
> > > 
> > > after that, kernel invokes low-memory killer to kill some apps in
> > > order to free memory in android system. However, sometimes, the
> > > killing is blocked,
> > > the app's call stack:
> > > 
> > > rwsem_down_write_failed
> > > down_write
> > > ion_client_destroy
> > > ion_release
> > > fput
> > > do_signal
> > > 
> > > the killing is blocked because ion_device->lock is held by ion_alloc.
> > > 
> > > ion_alloc hold the lock for accessing the heaps list,
> > > ion_destroy_client hold the lock for accessing the clients list.
> > > 
> > > so, we can use two separate locks for heaps and clients, to avoid the
> > > unnecessary race.
> > > 
> > 
> > I've reviewed this and it looks okay at first pass but I don't want it
> > applied just yet. Ion locking is a bit of a mess and has been added
> 
> yes, and now "debugfs_mutex" and "ion_root_client" is redundant, after
> commit 49d200deaa680501f19a247b1fffb29301e51d2b and
> 9fd4dcece43a53e5a9e65a973df5693702ee6401.

Ok, now dropping this patch from my queue.

thanks,

greg k-h