lkml.org 
[lkml]   [2020]   [Mar]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [GIT PULL 3/6] intel_th: msu: Make stopping the trace optional
On Tue, Mar 17, 2020 at 08:22:12AM +0200, Alexander Shishkin wrote:
> Some use cases prefer to keep collecting the trace data into the last
> available window while the other windows are being offloaded instead of
> stopping the trace. In this scenario, the window switch happens
> automatically when the next window becomes available again.
>
> Add an option to allow this and a sysfs attribute to enable it.
>
> Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> .../testing/sysfs-bus-intel_th-devices-msc | 8 ++++
> drivers/hwtracing/intel_th/msu.c | 37 ++++++++++++++++++-
> 2 files changed, 44 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-intel_th-devices-msc b/Documentation/ABI/testing/sysfs-bus-intel_th-devices-msc
> index 456cb62b384c..7fd2601c2831 100644
> --- a/Documentation/ABI/testing/sysfs-bus-intel_th-devices-msc
> +++ b/Documentation/ABI/testing/sysfs-bus-intel_th-devices-msc
> @@ -40,3 +40,11 @@ Description: (RW) Trigger window switch for the MSC's buffer, in
> triggering a window switch for the buffer. Returns an error in any
> other operating mode or attempts to write something other than "1".
>
> +What: /sys/bus/intel_th/devices/<intel_th_id>-msc<msc-id>/stop_on_full
> +Date: March 2020
> +KernelVersion: 5.7
> +Contact: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> +Description: (RW) Configure whether trace stops when the last available window
> + becomes full (1/y/Y) or wraps around and continues until the next
> + window becomes available again (0/n/N).
> +
> diff --git a/drivers/hwtracing/intel_th/msu.c b/drivers/hwtracing/intel_th/msu.c
> index 6e118b790d83..45916b48bcf0 100644
> --- a/drivers/hwtracing/intel_th/msu.c
> +++ b/drivers/hwtracing/intel_th/msu.c
> @@ -138,6 +138,7 @@ struct msc {
> struct list_head win_list;
> struct sg_table single_sgt;
> struct msc_window *cur_win;
> + struct msc_window *switch_on_unlock;
> unsigned long nr_pages;
> unsigned long single_sz;
> unsigned int single_wrap : 1;
> @@ -154,6 +155,8 @@ struct msc {
>
> struct list_head iter_list;
>
> + bool stop_on_full;
> +
> /* config */
> unsigned int enabled : 1,
> wrap : 1,
> @@ -1717,6 +1720,10 @@ void intel_th_msc_window_unlock(struct device *dev, struct sg_table *sgt)
> return;
>
> msc_win_set_lockout(win, WIN_LOCKED, WIN_READY);
> + if (msc->switch_on_unlock == win) {
> + msc->switch_on_unlock = NULL;
> + msc_win_switch(msc);
> + }
> }
> EXPORT_SYMBOL_GPL(intel_th_msc_window_unlock);
>
> @@ -1757,7 +1764,11 @@ static irqreturn_t intel_th_msc_interrupt(struct intel_th_device *thdev)
>
> /* next window: if READY, proceed, if LOCKED, stop the trace */
> if (msc_win_set_lockout(next_win, WIN_READY, WIN_INUSE)) {
> - schedule_work(&msc->work);
> + if (msc->stop_on_full)
> + schedule_work(&msc->work);
> + else
> + msc->switch_on_unlock = next_win;
> +
> return IRQ_HANDLED;
> }
>
> @@ -2050,11 +2061,35 @@ win_switch_store(struct device *dev, struct device_attribute *attr,
>
> static DEVICE_ATTR_WO(win_switch);
>
> +static ssize_t stop_on_full_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct msc *msc = dev_get_drvdata(dev);
> +
> + return scnprintf(buf, PAGE_SIZE, "%d\n", msc->stop_on_full);

No need for the scnprinf() crazyness for a single boolean value. Just
use sprintf() and keep it simple.

> +static ssize_t stop_on_full_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t size)
> +{
> + struct msc *msc = dev_get_drvdata(dev);
> + unsigned long val;
> + int ret;
> +
> + ret = kstrtobool(buf, &msc->stop_on_full);
> +
> + return ret ? ret : size;
> +}

Here's the problem, you don't use val.

And spell out the ? : crazyness:
if (ret)
return ret;
return size;

much simpler for people to read and understand.

thanks,

greg k-h

\
 
 \ /
  Last update: 2020-03-18 11:31    [W:0.121 / U:0.124 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site