| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <Pine.LNX.4.64.1305242353210.7187@axis700.grange>
Date: Fri, 24 May 2013 23:55:24 +0200 (CEST)
From: Guennadi Liakhovetski <g.liakhovetski@....de>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
cc: Will Deacon <will.deacon@....com>,
Vinod Koul <vinod.koul@...el.com>,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
viresh.kumar@...aro.org, djbw@...com
Subject: Re: [PATCH] dmatest: do not allow to interrupt ongoing tests
Hi Andy
On Thu, 23 May 2013, Andy Shevchenko wrote:
> When user interrupts ongoing transfers the dmatest may end up with console
> lockup, oops, or data mismatch. This patch prevents user to abort any ongoing
> test.
Personally I would be against such a change. What about interrupting the
test with rmmod? Is it still possible after this your patch or not? If not
- this doesn't seem like a good idea to me. Why don't we just fix those
bugs, that you're describing?
Thanks
Guennadi
>
> Documentation is updated accordingly.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> Reported-by: Will Deacon <will.deacon@....com>
> Tested-by: Will Deacon <will.deacon@....com>
> ---
> Documentation/dmatest.txt | 6 +++---
> drivers/dma/dmatest.c | 45 +++++++++++++++++++++++----------------------
> 2 files changed, 26 insertions(+), 25 deletions(-)
>
> diff --git a/Documentation/dmatest.txt b/Documentation/dmatest.txt
> index 279ac0a..132a094 100644
> --- a/Documentation/dmatest.txt
> +++ b/Documentation/dmatest.txt
> @@ -34,7 +34,7 @@ command:
> After a while you will start to get messages about current status or error like
> in the original code.
>
> -Note that running a new test will stop any in progress test.
> +Note that running a new test will not stop any in progress test.
>
> The following command should return actual state of the test.
> % cat /sys/kernel/debug/dmatest/run
> @@ -52,8 +52,8 @@ To wait for test done the user may perform a busy loop that checks the state.
>
> The module parameters that is supplied to the kernel command line will be used
> for the first performed test. After user gets a control, the test could be
> -interrupted or re-run with same or different parameters. For the details see
> -the above section "Part 2 - When dmatest is built as a module..."
> +re-run with the same or different parameters. For the details see the above
> +section "Part 2 - When dmatest is built as a module..."
>
> In both cases the module parameters are used as initial values for the test case.
> You always could check them at run-time by running
> diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
> index d8ce4ec..e88ded2 100644
> --- a/drivers/dma/dmatest.c
> +++ b/drivers/dma/dmatest.c
> @@ -716,8 +716,7 @@ static int dmatest_func(void *data)
> }
> dma_async_issue_pending(chan);
>
> - wait_event_freezable_timeout(done_wait,
> - done.done || kthread_should_stop(),
> + wait_event_freezable_timeout(done_wait, done.done,
> msecs_to_jiffies(params->timeout));
>
> status = dma_async_is_tx_complete(chan, cookie, NULL, NULL);
> @@ -997,7 +996,6 @@ static void stop_threaded_test(struct dmatest_info *info)
> static int __restart_threaded_test(struct dmatest_info *info, bool run)
> {
> struct dmatest_params *params = &info->params;
> - int ret;
>
> /* Stop any running test first */
> __stop_threaded_test(info);
> @@ -1012,13 +1010,23 @@ static int __restart_threaded_test(struct dmatest_info *info, bool run)
> memcpy(params, &info->dbgfs_params, sizeof(*params));
>
> /* Run test with new parameters */
> - ret = __run_threaded_test(info);
> - if (ret) {
> - __stop_threaded_test(info);
> - pr_err("dmatest: Can't run test\n");
> + return __run_threaded_test(info);
> +}
> +
> +static bool __is_threaded_test_run(struct dmatest_info *info)
> +{
> + struct dmatest_chan *dtc;
> +
> + list_for_each_entry(dtc, &info->channels, node) {
> + struct dmatest_thread *thread;
> +
> + list_for_each_entry(thread, &dtc->threads, node) {
> + if (!thread->done)
> + return true;
> + }
> }
>
> - return ret;
> + return false;
> }
>
> static ssize_t dtf_write_string(void *to, size_t available, loff_t *ppos,
> @@ -1091,22 +1099,10 @@ static ssize_t dtf_read_run(struct file *file, char __user *user_buf,
> {
> struct dmatest_info *info = file->private_data;
> char buf[3];
> - struct dmatest_chan *dtc;
> - bool alive = false;
>
> mutex_lock(&info->lock);
> - list_for_each_entry(dtc, &info->channels, node) {
> - struct dmatest_thread *thread;
> -
> - list_for_each_entry(thread, &dtc->threads, node) {
> - if (!thread->done) {
> - alive = true;
> - break;
> - }
> - }
> - }
>
> - if (alive) {
> + if (__is_threaded_test_run(info)) {
> buf[0] = 'Y';
> } else {
> __stop_threaded_test(info);
> @@ -1132,7 +1128,12 @@ static ssize_t dtf_write_run(struct file *file, const char __user *user_buf,
>
> if (strtobool(buf, &bv) == 0) {
> mutex_lock(&info->lock);
> - ret = __restart_threaded_test(info, bv);
> +
> + if (__is_threaded_test_run(info))
> + ret = -EBUSY;
> + else
> + ret = __restart_threaded_test(info, bv);
> +
> mutex_unlock(&info->lock);
> }
>
> --
> 1.8.2.rc0.22.gb3600c3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists