]> www.infradead.org Git - users/jedix/linux-maple.git/commitdiff
sparc64: Make memory allocations ATOMIC to fix lockdep warnings
authorBabu Moger <babu.moger@oracle.com>
Tue, 19 Jan 2016 19:16:43 +0000 (11:16 -0800)
committerAllen Pais <allen.pais@oracle.com>
Thu, 21 Jan 2016 17:47:46 +0000 (23:17 +0530)
Orabug: 22392548

Memory allocations are done holding spin_unlock_irqrestore with
non atomic flags. This was caught by lockdep as  below.

Make these allocations ATOMIC as these functions are always
called while holding the lock.

------------[ cut here ]------------
WARNING: at kernel/lockdep.c:2649 lockdep_trace_alloc+0xc0/0xe8()
Modules linked in:
Call Trace:
 [000000000047a304] warn_slowpath_common+0x4c/0x6c
 [000000000047a340] warn_slowpath_null+0x1c/0x2c
 [00000000004b0c70] lockdep_trace_alloc+0xc0/0xe8
 [0000000000554b64] kmem_cache_alloc_trace+0x18/0x1a8
 [00000000004540c8] ds_add_service_provider+0x58/0x120
 [0000000000454204] ds_add_builtin_services+0x74/0xac
 [000000000045450c] ds_probe+0x2d0/0x448
 [0000000000450d54] vio_device_probe+0xb0/0xd4
 [0000000000736e08] driver_probe_device+0x13c/0x234
 [0000000000736f60] __driver_attach+0x60/0x8c
 [0000000000736440] bus_for_each_dev+0x4c/0x9c
 [0000000000736b18] driver_attach+0x1c/0x30
 [0000000000735c58] bus_add_driver+0xcc/0x260
 [00000000007373d4] driver_register+0xc0/0x170
 [0000000000451414] vio_register_driver+0x18/0x40
 [0000000000cd2db4] ds_init+0x140/0x170
---[ end trace 139ce121c98e96c9 ]---

Also fixed the deadlock scenario showed by lockdep.

=======================================================
[ INFO: possible circular locking dependency detected ]
2.6.39-uek2-bm #35
-------------------------------------------------------
ldoms-ds/526 is trying to acquire lock:
 (&(&ds->ds_lock)->rlock){-.-...}, at:
[<0000000000455000>] ds_callout_thread+0x148/0x594

but task is already holding lock:
 (ds_data_lock){......}, at:
[<0000000000454fe4>] ds_callout_thread+0x12c/0x594

which lock already depends on the new lock.

the existing dependency chain (in reverse order) is:

-> #1 (ds_data_lock){......}:
       [<00000000008fc1f0>] _raw_spin_lock_irqsave+0x38/0x78
       [<0000000000454538>] ds_probe+0x2fc/0x448
       [<0000000000450d54>] vio_device_probe+0xb0/0xd4
       [<0000000000736e08>] driver_probe_device+0x13c/0x234
       [<0000000000736f60>] __driver_attach+0x60/0x8c
       [<0000000000736440>] bus_for_each_dev+0x4c/0x9c
       [<0000000000736b18>] driver_attach+0x1c/0x30
       [<0000000000735c58>] bus_add_driver+0xcc/0x260
       [<00000000007373d4>] driver_register+0xc0/0x170
       [<0000000000451414>] vio_register_driver+0x18/0x40
       [<0000000000cd2db4>] ds_init+0x140/0x170
       [<0000000000426e30>] do_one_initcall+0x70/0x150
       [<0000000000cca218>] kernel_init+0x100/0x190
       [<000000000042b82c>] kernel_thread+0x38/0x50
       [<00000000008e8474>] rest_init+0x20/0xc8

-> #0 (&(&ds->ds_lock)->rlock){-.-...}:
       [<00000000004b4ac8>] lock_acquire+0xa4/0xbc
       [<00000000008fc1f0>] _raw_spin_lock_irqsave+0x38/0x78
       [<0000000000455000>] ds_callout_thread+0x148/0x594
       [<000000000049ce5c>] kthread+0x64/0x78
       [<000000000042b82c>] kernel_thread+0x38/0x50
       [<000000000049cf84>] kthreadd+0x114/0x168

other info that might help us debug this:

 Possible unsafe locking scenario:

       CPU0                    CPU1
       ----                    ----
  lock(ds_data_lock);
                               lock(&(&ds->ds_lock)->rlock);
                               lock(ds_data_lock);
  lock(&(&ds->ds_lock)->rlock);

 *** DEADLOCK ***

1 lock held by ldoms-ds/526:
 #0:  (ds_data_lock){......}, at:
[<0000000000454fe4>] ds_callout_thread+0x12c/0x594

stack backtrace:
Call Trace:
 [00000000004b1f54] print_circular_bug+0x2b4/0x2c4
 [00000000004b3d4c] __lock_acquire+0x1428/0x1c08
 [00000000004b4ac8] lock_acquire+0xa4/0xbc
 [00000000008fc1f0] _raw_spin_lock_irqsave+0x38/0x78
 [0000000000455000] ds_callout_thread+0x148/0x594
 [000000000049ce5c] kthread+0x64/0x78
 [000000000042b82c] kernel_thread+0x38/0x50
 [000000000049cf84] kthreadd+0x114/0x168

Signed-off-by: Babu Moger <babu.moger@oracle.com>
Signed-off-by: Aaron Young <aaron.young@oracle.com>
(cherry picked from commit a065c92b422de2d2c21f81bc6c189bac07f57e8b)

arch/sparc/kernel/ds.c

index 9ed18a51dac221be058934dd46fd9eb9fd26ec08..70844bbf7bc8f56f3e893b8a593b2b2c27ba31ce 100644 (file)
@@ -1487,7 +1487,7 @@ int ds_cap_send(ds_svc_hdl_t hdl, void *buf, size_t buflen)
 
        /* build the data packet containing the data */
        msglen = sizeof(struct ds_data_req) + buflen;
-       hdr = kzalloc(msglen, GFP_KERNEL);
+       hdr = kzalloc(msglen, GFP_ATOMIC);
        if (hdr == NULL) {
                pr_err("ds-%llu: %s: failed to alloc mem for data msg.\n",
                    ds->id, __func__);
@@ -2179,11 +2179,11 @@ static struct ds_service_info *ds_add_service_provider(struct ds_dev *ds,
 
        dprintk("entered.\n");
 
-       svc_info = kzalloc(sizeof(struct ds_service_info), GFP_KERNEL);
+       svc_info = kzalloc(sizeof(struct ds_service_info), GFP_ATOMIC);
        if (unlikely(svc_info == NULL))
                return NULL;
 
-       svc_info->id = kmemdup(id, (strlen(id) + 1), GFP_KERNEL);
+       svc_info->id = kmemdup(id, (strlen(id) + 1), GFP_ATOMIC);
        svc_info->vers = vers;
        svc_info->ops = *ops;
        svc_info->is_client = false;
@@ -2230,11 +2230,11 @@ static struct ds_service_info *ds_add_service_client(struct ds_dev *ds,
 
        dprintk("entered.\n");
 
-       svc_info = kzalloc(sizeof(struct ds_service_info), GFP_KERNEL);
+       svc_info = kzalloc(sizeof(struct ds_service_info), GFP_ATOMIC);
        if (unlikely(svc_info == NULL))
                return NULL;
 
-       svc_info->id = kmemdup(id, (strlen(id) + 1), GFP_KERNEL);
+       svc_info->id = kmemdup(id, (strlen(id) + 1), GFP_ATOMIC);
        svc_info->vers = vers;
        svc_info->ops = *ops;
        svc_info->is_client = true;
@@ -4119,7 +4119,7 @@ static int ds_set_pri(void)
 
        /* build the data packet containing the data */
        msglen = sizeof(struct ds_data_req) + buflen;
-       hdr = kzalloc(msglen, GFP_KERNEL);
+       hdr = kzalloc(msglen, GFP_ATOMIC);
        if (hdr == NULL) {
                pr_err("%s: failed to alloc mem for PRI data msg.\n",
                    __func__);
@@ -4379,12 +4379,6 @@ static int ds_probe(struct vio_dev *vdev, const struct vio_device_id *id)
                ds_add_builtin_services(ds, ds_sp_builtin_template,
                    ARRAY_SIZE(ds_sp_builtin_template));
 
-       /* add the ds_dev to the global ds_data device list */
-       spin_lock_irqsave(&ds_data_lock, flags);
-       list_add_tail(&ds->list, &ds_data.ds_dev_list);
-       ds_data.num_ds_dev_list++;
-       spin_unlock_irqrestore(&ds_data_lock, flags);
-
        /*
         * begin the process of registering services.
         * Note - we do this here to allow loopback services
@@ -4397,6 +4391,12 @@ static int ds_probe(struct vio_dev *vdev, const struct vio_device_id *id)
 
        UNLOCK_DS_DEV(ds, ds_flags)
 
+       /* add the ds_dev to the global ds_data device list */
+       spin_lock_irqsave(&ds_data_lock, flags);
+       list_add_tail(&ds->list, &ds_data.ds_dev_list);
+       ds_data.num_ds_dev_list++;
+       spin_unlock_irqrestore(&ds_data_lock, flags);
+
        return rv;
 
 out_free_ldc: