On 09/29/2015 03:48 AM, Hannes Reinecke wrote:
+static void alua_rtpg_work(struct work_struct *work)
+{
+       struct alua_port_group *pg =
+               container_of(work, struct alua_port_group, rtpg_work.work);
+       struct scsi_device *sdev;
+       LIST_HEAD(qdata_list);
+       int err = SCSI_DH_OK;
+       struct alua_queue_data *qdata, *tmp;
+       unsigned long flags;
+
+       spin_lock_irqsave(&pg->lock, flags);
+       sdev = pg->rtpg_sdev;
+       if (!sdev) {
+               WARN_ON(pg->flags & ALUA_PG_RUN_RTPG ||
+                       pg->flags & ALUA_PG_RUN_STPG);
+               spin_unlock_irqrestore(&pg->lock, flags);
+               return;
+       }
+       pg->flags |= ALUA_PG_RUNNING;
+       if (pg->flags & ALUA_PG_RUN_RTPG) {
+               spin_unlock_irqrestore(&pg->lock, flags);
+               err = alua_rtpg(sdev, pg);
+               spin_lock_irqsave(&pg->lock, flags);
+               if (err == SCSI_DH_RETRY) {
+                       pg->flags &= ~ALUA_PG_RUNNING;
+                       spin_unlock_irqrestore(&pg->lock, flags);
+                       queue_delayed_work(kaluad_wq, &pg->rtpg_work,
+                                          pg->interval * HZ);
+                       return;
+               }
+               pg->flags &= ~ALUA_PG_RUN_RTPG;
+               if (err != SCSI_DH_OK)
+                       pg->flags &= ~ALUA_PG_RUN_STPG;
+       }
+       if (pg->flags & ALUA_PG_RUN_STPG) {
+               spin_unlock_irqrestore(&pg->lock, flags);
+               err = alua_stpg(sdev, pg);
+               spin_lock_irqsave(&pg->lock, flags);
+               pg->flags &= ~ALUA_PG_RUN_STPG;
+               if (err == SCSI_DH_RETRY) {
+                       pg->flags |= ALUA_PG_RUN_RTPG;
+                       pg->interval = 0;
+                       pg->flags &= ~ALUA_PG_RUNNING;
+                       spin_unlock_irqrestore(&pg->lock, flags);
+                       queue_delayed_work(kaluad_wq, &pg->rtpg_work,
+                                          pg->interval * HZ);
+                       return;
+               }
+       }
+
+       list_splice_init(&pg->rtpg_list, &qdata_list);
+       pg->rtpg_sdev = NULL;
+       spin_unlock_irqrestore(&pg->lock, flags);
+
+       list_for_each_entry_safe(qdata, tmp, &qdata_list, entry) {
+               list_del(&qdata->entry);
+               if (qdata->callback_fn)
+                       qdata->callback_fn(qdata->callback_data, err);
+               kfree(qdata);
+       }
+       spin_lock_irqsave(&pg->lock, flags);
+       pg->flags &= ~ALUA_PG_RUNNING;
+       spin_unlock_irqrestore(&pg->lock, flags);
+       scsi_device_put(sdev);
+       kref_put(&pg->kref, release_port_group);
+}

With this patch series applied kmemleak reports several leaks that were not reported without this patch. Is scsi_device_put() + kref_put() always called before this function returns without requeueing the work item ? Shouldn't the return value of queue_delayed_work() be checked ? The leaks reported by kmemleak are:

unreferenced object 0xffff880423d31728 (size 128):
  comm "kworker/2:3", pid 3589, jiffies 4294946634 (age 501.720s)
  hex dump (first 32 bytes):
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<ffffffff814ed75e>] kmemleak_alloc+0x4e/0xb0
    [<ffffffff811a9128>] kmem_cache_alloc_trace+0xc8/0x210
    [<ffffffffa0269d78>] alua_rtpg_work+0x1b8/0xc10 [scsi_dh_alua]
    [<ffffffff8108d9a8>] process_one_work+0x1d8/0x610
    [<ffffffff8108def4>] worker_thread+0x114/0x460
    [<ffffffff810941e8>] kthread+0xf8/0x110
    [<ffffffff814f5c2f>] ret_from_fork+0x3f/0x70
    [<ffffffffffffffff>] 0xffffffffffffffff
unreferenced object 0xffff88042ba00150 (size 8):
  comm "srp_daemon", pid 608, jiffies 4294946454 (age 503.510s)
  hex dump (first 8 bytes):
    68 6f 73 74 31 35 00 a5                          host15..
  backtrace:
    [<ffffffff814ed75e>] kmemleak_alloc+0x4e/0xb0
    [<ffffffff811abd53>] __kmalloc_track_caller+0xe3/0x240
    [<ffffffff81295ce2>] kvasprintf+0x52/0x80
    [<ffffffff8128adbf>] kobject_set_name_vargs+0x1f/0x60
    [<ffffffff81382217>] dev_set_name+0x47/0x50
    [<ffffffffa000dcaa>] scsi_host_alloc+0x32a/0x4b0 [scsi_mod]
    [<ffffffffa0279524>] srp_create_target+0x54/0x1410 [ib_srp]
    [<ffffffff81381bf8>] dev_attr_store+0x18/0x30
    [<ffffffff812300f4>] sysfs_kf_write+0x44/0x60
    [<ffffffff8122f724>] kernfs_fop_write+0x144/0x190
    [<ffffffff811b8788>] __vfs_write+0x28/0xe0
    [<ffffffff811b8e19>] vfs_write+0xa9/0x190
    [<ffffffff811b9b19>] SyS_write+0x49/0xa0
    [<ffffffff814f5876>] entry_SYSCALL_64_fastpath+0x16/0x7a
    [<ffffffffffffffff>] 0xffffffffffffffff

Bart.

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to