On 10/02/2015 01:34 AM, Bart Van Assche wrote:
> 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:
>
Yes, you are right. I need to check queue_delayed_work() and issue
a scsi_device_put()/kref_put() if the item is already queued.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare@xxxxxxx +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
