From: mathieu.poirier@linaro.org (Mathieu Poirier)
To: linux-arm-kernel@lists.infradead.org
Subject: [bug report] coresight: tmc: allocating memory when needed
Date: Wed, 18 Apr 2018 10:03:59 -0600 [thread overview]
Message-ID: <CANLsYky5cC91hH6qi8-d9EOtU7KzoVyy9cp5Qi+XF2p6nb=ocw@mail.gmail.com> (raw)
In-Reply-To: <20180418124856.GA5771@mwanda>
Hi Dan,
On 18 April 2018 at 06:48, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> [ This bug is a couple years old now so presumably it can't be that
> serious but maybe it's still worth looking at? -dan ]
>
> Hello Mathieu Poirier,
>
> The patch de5461970b3e: "coresight: tmc: allocating memory when
> needed" from May 3, 2016, leads to the following static checker
> warning:
>
> drivers/hwtracing/coresight/coresight-tmc-etr.c:174 tmc_enable_etr_sink_sysfs()
> error: uninitialized symbol 'paddr'.
Yes, I can see how a static checker would complain. On the flip side
paddr can't be used uninitialised.
>
> drivers/hwtracing/coresight/coresight-tmc-etr.c
> 121 static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev)
> 122 {
> 123 int ret = 0;
> 124 bool used = false;
> 125 unsigned long flags;
> 126 void __iomem *vaddr = NULL;
> 127 dma_addr_t paddr;
> ^^^^^^^^^^^^^^^^
> 128 struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> 129
> 130
> 131 /*
> 132 * If we don't have a buffer release the lock and allocate memory.
> 133 * Otherwise keep the lock and move along.
> 134 */
> 135 spin_lock_irqsave(&drvdata->spinlock, flags);
> 136 if (!drvdata->vaddr) {
> ^^^^^^^^^^^^^^
> 137 spin_unlock_irqrestore(&drvdata->spinlock, flags);
> 138
> 139 /*
> 140 * Contiguous memory can't be allocated while a spinlock is
> 141 * held. As such allocate memory here and free it if a buffer
> 142 * has already been allocated (from a previous session).
> 143 */
> 144 vaddr = dma_alloc_coherent(drvdata->dev, drvdata->size,
> 145 &paddr, GFP_KERNEL);
> 146 if (!vaddr)
> 147 return -ENOMEM;
> 148
> 149 /* Let's try again */
> 150 spin_lock_irqsave(&drvdata->spinlock, flags);
> 151 }
> 152
> 153 if (drvdata->reading) {
> 154 ret = -EBUSY;
> 155 goto out;
> 156 }
> 157
> 158 /*
> 159 * In sysFS mode we can have multiple writers per sink. Since this
> 160 * sink is already enabled no memory is needed and the HW need not be
> 161 * touched.
> 162 */
> 163 if (drvdata->mode == CS_MODE_SYSFS)
> 164 goto out;
> 165
> 166 /*
> 167 * If drvdata::buf == NULL, use the memory allocated above.
> 168 * Otherwise a buffer still exists from a previous session, so
> 169 * simply use that.
> 170 */
> 171 if (drvdata->buf == NULL) {
> ^^^^^^^^^^^^^^^^^^^^
>
> Perhaps ->buf is only NULL when ->vaddr is NULL? It's surprising that
> we're not testing ->buf in both places.
You are correct, ->buf is only NULL when ->vaddr is. As such the test
should be the same at both places to avoid confusion. I'll send a
patch where ->vaddr is checked since ->buf is only for compatibility
with the reading function.
Thanks,
Mathieu
>
> 172 used = true;
> 173 drvdata->vaddr = vaddr;
> 174 drvdata->paddr = paddr;
> ^^^^^^^^^^^^^^^^^^^^^^^
>
> 175 drvdata->buf = drvdata->vaddr;
> 176 }
> 177
> 178 drvdata->mode = CS_MODE_SYSFS;
>
> regards,
> dan carpenter
prev parent reply other threads:[~2018-04-18 16:03 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-04-18 12:48 [bug report] coresight: tmc: allocating memory when needed Dan Carpenter
2018-04-18 16:03 ` Mathieu Poirier [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CANLsYky5cC91hH6qi8-d9EOtU7KzoVyy9cp5Qi+XF2p6nb=ocw@mail.gmail.com' \
--to=mathieu.poirier@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.