[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJ9a7Vhzoynt69+O1zya8LM3dOPhar9YxQXqHygbeKQC8CfHtg@mail.gmail.com>
Date: Thu, 19 Dec 2024 12:24:18 +0000
From: Mike Leach <mike.leach@...aro.org>
To: Yeoreum Yun <yeoreum.yun@....com>
Cc: suzuki.poulose@....com, james.clark@...aro.org,
alexander.shishkin@...ux.intel.com, bigeasy@...utronix.de,
clrkwllms@...nel.org, rostedt@...dmis.org, coresight@...ts.linaro.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-rt-devel@...ts.linux.dev
Subject: Re: [PATCH v3 5/9] coresight-etb10: change etb_drvdata spinlock's
type to raw_spinlock_t
On Mon, 16 Dec 2024 at 11:50, Yeoreum Yun <yeoreum.yun@....com> wrote:
>
> In coresight-etb10 drivers, etb_drvdata->spinlock can be held
> during __schedule() by perf_event_task_sched_out()/in().
>
> Since etb_drvdata->spinlock type is spinlock_t and
> perf_event_task_sched_out()/in() is called after acquiring rq_lock,
> which is raw_spinlock_t (an unsleepable lock),
> this poses an issue in PREEMPT_RT kernel where spinlock_t is sleepable.
>
> To address this, change type etb_drvdata->spinlock in coresight-etb10 drivers,
> which can be called by perf_event_task_sched_out()/in(),
> from spinlock_t to raw_spinlock_t.
>
> Signed-off-by: Yeoreum Yun <yeoreum.yun@....com>
> ---
> drivers/hwtracing/coresight/coresight-etb10.c | 26 +++++++++----------
> 1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c
> index aea9ac9c4bd0..7948597d483d 100644
> --- a/drivers/hwtracing/coresight/coresight-etb10.c
> +++ b/drivers/hwtracing/coresight/coresight-etb10.c
> @@ -84,7 +84,7 @@ struct etb_drvdata {
> struct clk *atclk;
> struct coresight_device *csdev;
> struct miscdevice miscdev;
> - spinlock_t spinlock;
> + raw_spinlock_t spinlock;
> local_t reading;
> pid_t pid;
> u8 *buf;
> @@ -145,7 +145,7 @@ static int etb_enable_sysfs(struct coresight_device *csdev)
> unsigned long flags;
> struct etb_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>
> - spin_lock_irqsave(&drvdata->spinlock, flags);
> + raw_spin_lock_irqsave(&drvdata->spinlock, flags);
>
> /* Don't messup with perf sessions. */
> if (coresight_get_mode(csdev) == CS_MODE_PERF) {
> @@ -163,7 +163,7 @@ static int etb_enable_sysfs(struct coresight_device *csdev)
>
> csdev->refcnt++;
> out:
> - spin_unlock_irqrestore(&drvdata->spinlock, flags);
> + raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
> return ret;
> }
>
> @@ -176,7 +176,7 @@ static int etb_enable_perf(struct coresight_device *csdev, void *data)
> struct perf_output_handle *handle = data;
> struct cs_buffers *buf = etm_perf_sink_config(handle);
>
> - spin_lock_irqsave(&drvdata->spinlock, flags);
> + raw_spin_lock_irqsave(&drvdata->spinlock, flags);
>
> /* No need to continue if the component is already in used by sysFS. */
> if (coresight_get_mode(drvdata->csdev) == CS_MODE_SYSFS) {
> @@ -219,7 +219,7 @@ static int etb_enable_perf(struct coresight_device *csdev, void *data)
> }
>
> out:
> - spin_unlock_irqrestore(&drvdata->spinlock, flags);
> + raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
> return ret;
> }
>
> @@ -352,11 +352,11 @@ static int etb_disable(struct coresight_device *csdev)
> struct etb_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> unsigned long flags;
>
> - spin_lock_irqsave(&drvdata->spinlock, flags);
> + raw_spin_lock_irqsave(&drvdata->spinlock, flags);
>
> csdev->refcnt--;
> if (csdev->refcnt) {
> - spin_unlock_irqrestore(&drvdata->spinlock, flags);
> + raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
> return -EBUSY;
> }
>
> @@ -366,7 +366,7 @@ static int etb_disable(struct coresight_device *csdev)
> /* Dissociate from monitored process. */
> drvdata->pid = -1;
> coresight_set_mode(csdev, CS_MODE_DISABLED);
> - spin_unlock_irqrestore(&drvdata->spinlock, flags);
> + raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
>
> dev_dbg(&csdev->dev, "ETB disabled\n");
> return 0;
> @@ -443,7 +443,7 @@ static unsigned long etb_update_buffer(struct coresight_device *csdev,
>
> capacity = drvdata->buffer_depth * ETB_FRAME_SIZE_WORDS;
>
> - spin_lock_irqsave(&drvdata->spinlock, flags);
> + raw_spin_lock_irqsave(&drvdata->spinlock, flags);
>
> /* Don't do anything if another tracer is using this sink */
> if (csdev->refcnt != 1)
> @@ -566,7 +566,7 @@ static unsigned long etb_update_buffer(struct coresight_device *csdev,
> __etb_enable_hw(drvdata);
> CS_LOCK(drvdata->base);
> out:
> - spin_unlock_irqrestore(&drvdata->spinlock, flags);
> + raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
>
> return to_read;
> }
> @@ -587,13 +587,13 @@ static void etb_dump(struct etb_drvdata *drvdata)
> {
> unsigned long flags;
>
> - spin_lock_irqsave(&drvdata->spinlock, flags);
> + raw_spin_lock_irqsave(&drvdata->spinlock, flags);
> if (coresight_get_mode(drvdata->csdev) == CS_MODE_SYSFS) {
> __etb_disable_hw(drvdata);
> etb_dump_hw(drvdata);
> __etb_enable_hw(drvdata);
> }
> - spin_unlock_irqrestore(&drvdata->spinlock, flags);
> + raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
>
> dev_dbg(&drvdata->csdev->dev, "ETB dumped\n");
> }
> @@ -746,7 +746,7 @@ static int etb_probe(struct amba_device *adev, const struct amba_id *id)
> drvdata->base = base;
> desc.access = CSDEV_ACCESS_IOMEM(base);
>
> - spin_lock_init(&drvdata->spinlock);
> + raw_spin_lock_init(&drvdata->spinlock);
>
> drvdata->buffer_depth = etb_get_buffer_depth(drvdata);
>
> --
> LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}
>
Reviewed-by: Mike Leach <mike.leach@...aro.org>
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
Powered by blists - more mailing lists