public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
* [bug report] firmware: arm_scmi: Add atomic mode support to virtio transport
@ 2022-10-21 10:30 Dan Carpenter
  2022-10-28 14:19 ` Cristian Marussi
  0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2022-10-21 10:30 UTC (permalink / raw)
  To: cristian.marussi; +Cc: linux-arm-kernel

Hello Cristian Marussi,

The patch 5a3b7185c47c: "firmware: arm_scmi: Add atomic mode support
to virtio transport" from Feb 17, 2022, leads to the following Smatch
static checker warning:

	drivers/firmware/arm_scmi/virtio.c:471 virtio_chan_setup()
	warn: 'vioch->deferred_tx_wq' from alloc_workqueue() not released on lines: 449,456,464.

drivers/firmware/arm_scmi/virtio.c
    419 static int virtio_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
    420                              bool tx)
    421 {
    422         struct scmi_vio_channel *vioch;
    423         int index = tx ? VIRTIO_SCMI_VQ_TX : VIRTIO_SCMI_VQ_RX;
    424         int i;
    425 
    426         if (!scmi_vdev)
    427                 return -EPROBE_DEFER;
    428 
    429         vioch = &((struct scmi_vio_channel *)scmi_vdev->priv)[index];
    430 
    431         /* Setup a deferred worker for polling. */
    432         if (tx && !vioch->deferred_tx_wq) {
    433                 vioch->deferred_tx_wq =
    434                         alloc_workqueue(dev_name(&scmi_vdev->dev),
    435                                         WQ_UNBOUND | WQ_FREEZABLE | WQ_SYSFS,
    436                                         0);

This workqueue is not freed on error paths.

    437                 if (!vioch->deferred_tx_wq)
    438                         return -ENOMEM;
    439 
    440                 INIT_WORK(&vioch->deferred_tx_work,
    441                           scmi_vio_deferred_tx_worker);
    442         }
    443 
    444         for (i = 0; i < vioch->max_msg; i++) {
    445                 struct scmi_vio_msg *msg;
    446 
    447                 msg = devm_kzalloc(cinfo->dev, sizeof(*msg), GFP_KERNEL);
    448                 if (!msg)
    449                         return -ENOMEM;

Here etc.

    450 
    451                 if (tx) {
    452                         msg->request = devm_kzalloc(cinfo->dev,
    453                                                     VIRTIO_SCMI_MAX_PDU_SIZE,
    454                                                     GFP_KERNEL);
    455                         if (!msg->request)
    456                                 return -ENOMEM;
    457                         spin_lock_init(&msg->poll_lock);
    458                         refcount_set(&msg->users, 1);
    459                 }
    460 
    461                 msg->input = devm_kzalloc(cinfo->dev, VIRTIO_SCMI_MAX_PDU_SIZE,
    462                                           GFP_KERNEL);
    463                 if (!msg->input)
    464                         return -ENOMEM;
    465 
    466                 scmi_finalize_message(vioch, msg);
    467         }
    468 
    469         scmi_vio_channel_ready(vioch, cinfo);
    470 
--> 471         return 0;
    472 }

regards,
dan carpenter

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [bug report] firmware: arm_scmi: Add atomic mode support to virtio transport
  2022-10-21 10:30 [bug report] firmware: arm_scmi: Add atomic mode support to virtio transport Dan Carpenter
@ 2022-10-28 14:19 ` Cristian Marussi
  0 siblings, 0 replies; 2+ messages in thread
From: Cristian Marussi @ 2022-10-28 14:19 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: linux-arm-kernel

On Fri, Oct 21, 2022 at 01:30:50PM +0300, Dan Carpenter wrote:
> Hello Cristian Marussi,

Hi Dan Carpenter,

thanks for the report.

> 
> The patch 5a3b7185c47c: "firmware: arm_scmi: Add atomic mode support
> to virtio transport" from Feb 17, 2022, leads to the following Smatch
> static checker warning:
> 
> 	drivers/firmware/arm_scmi/virtio.c:471 virtio_chan_setup()
> 	warn: 'vioch->deferred_tx_wq' from alloc_workqueue() not released on lines: 449,456,464.
> 
> drivers/firmware/arm_scmi/virtio.c
>     419 static int virtio_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
>     420                              bool tx)
>     421 {
>     422         struct scmi_vio_channel *vioch;
>     423         int index = tx ? VIRTIO_SCMI_VQ_TX : VIRTIO_SCMI_VQ_RX;
>     424         int i;
>     425 
>     426         if (!scmi_vdev)
>     427                 return -EPROBE_DEFER;
>     428 
>     429         vioch = &((struct scmi_vio_channel *)scmi_vdev->priv)[index];
>     430 
>     431         /* Setup a deferred worker for polling. */
>     432         if (tx && !vioch->deferred_tx_wq) {
>     433                 vioch->deferred_tx_wq =
>     434                         alloc_workqueue(dev_name(&scmi_vdev->dev),
>     435                                         WQ_UNBOUND | WQ_FREEZABLE | WQ_SYSFS,
>     436                                         0);
> 
> This workqueue is not freed on error paths.
> 
>     437                 if (!vioch->deferred_tx_wq)
>     438                         return -ENOMEM;
>     439 
>     440                 INIT_WORK(&vioch->deferred_tx_work,
>     441                           scmi_vio_deferred_tx_worker);
>     442         }
>     443 
>     444         for (i = 0; i < vioch->max_msg; i++) {
>     445                 struct scmi_vio_msg *msg;
>     446 
>     447                 msg = devm_kzalloc(cinfo->dev, sizeof(*msg), GFP_KERNEL);
>     448                 if (!msg)
>     449                         return -ENOMEM;
> 
> Here etc.
> 
>     450 
>     451                 if (tx) {
>     452                         msg->request = devm_kzalloc(cinfo->dev,
>     453                                                     VIRTIO_SCMI_MAX_PDU_SIZE,
>     454                                                     GFP_KERNEL);
>     455                         if (!msg->request)
>     456                                 return -ENOMEM;
>     457                         spin_lock_init(&msg->poll_lock);
>     458                         refcount_set(&msg->users, 1);
>     459                 }
>     460 
>     461                 msg->input = devm_kzalloc(cinfo->dev, VIRTIO_SCMI_MAX_PDU_SIZE,
>     462                                           GFP_KERNEL);
>     463                 if (!msg->input)
>     464                         return -ENOMEM;
>     465 
>     466                 scmi_finalize_message(vioch, msg);
>     467         }
>     468 
>     469         scmi_vio_channel_ready(vioch, cinfo);
>     470 
> --> 471         return 0;
>     472 }
> 

Posted a fix on this, using devres devm_ for that resource (wq) now.

https://lore.kernel.org/linux-arm-kernel/20221028140833.280091-6-cristian.marussi@arm.com/T/#u

Thanks,
Cristian

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-10-28 14:20 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-10-21 10:30 [bug report] firmware: arm_scmi: Add atomic mode support to virtio transport Dan Carpenter
2022-10-28 14:19 ` Cristian Marussi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox