From patchwork Fri Sep 30 08:18:23 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Xu YiPing X-Patchwork-Id: 77165 Delivered-To: patch@linaro.org Received: by 10.140.106.72 with SMTP id d66csp141789qgf; Fri, 30 Sep 2016 01:18:53 -0700 (PDT) X-Received: by 10.66.230.168 with SMTP id sz8mr9687002pac.13.1475223533514; Fri, 30 Sep 2016 01:18:53 -0700 (PDT) Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id xr8si19025203pab.19.2016.09.30.01.18.53; Fri, 30 Sep 2016 01:18:53 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752047AbcI3ISv (ORCPT + 27 others); Fri, 30 Sep 2016 04:18:51 -0400 Received: from szxga03-in.huawei.com ([119.145.14.66]:56922 "EHLO szxga03-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750818AbcI3ISr (ORCPT ); Fri, 30 Sep 2016 04:18:47 -0400 Received: from 172.24.1.136 (EHLO szxeml430-hub.china.huawei.com) ([172.24.1.136]) by szxrg03-dlp.huawei.com (MOS 4.4.3-GA FastPath queued) with ESMTP id CIQ33033; Fri, 30 Sep 2016 16:18:36 +0800 (CST) Received: from vm167-7.huawei.com (10.177.167.7) by szxeml430-hub.china.huawei.com (10.82.67.185) with Microsoft SMTP Server id 14.3.235.1; Fri, 30 Sep 2016 16:18:24 +0800 From: Xu YiPing To: , , , , , , , CC: , , Subject: [patch] staging: ion: use two separate locks for heaps and clients in ion_device Date: Fri, 30 Sep 2016 16:18:23 +0800 Message-ID: <1475223503-65906-1-git-send-email-xuyiping@hisilicon.com> X-Mailer: git-send-email 1.9.1 MIME-Version: 1.0 X-Originating-IP: [10.177.167.7] X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 --- drivers/staging/android/ion/ion.c | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) -- 2.7.4 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;