Current coding style prefers octal permissions values. This patch
changes symbolic permissions to octal values.
Signed-off-by: Yang Shunyong <[email protected]>
---
drivers/dma/dmatest.c | 30 ++++++++++++++----------------
1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
index ec5f9d2..4cdccc3 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -24,60 +24,58 @@
#include <linux/wait.h>
static unsigned int test_buf_size = 16384;
-module_param(test_buf_size, uint, S_IRUGO | S_IWUSR);
+module_param(test_buf_size, uint, 0644);
MODULE_PARM_DESC(test_buf_size, "Size of the memcpy test buffer");
static char test_channel[20];
-module_param_string(channel, test_channel, sizeof(test_channel),
- S_IRUGO | S_IWUSR);
+module_param_string(channel, test_channel, sizeof(test_channel), 0644);
MODULE_PARM_DESC(channel, "Bus ID of the channel to test (default: any)");
static char test_device[32];
-module_param_string(device, test_device, sizeof(test_device),
- S_IRUGO | S_IWUSR);
+module_param_string(device, test_device, sizeof(test_device), 0644);
MODULE_PARM_DESC(device, "Bus ID of the DMA Engine to test (default: any)");
static unsigned int threads_per_chan = 1;
-module_param(threads_per_chan, uint, S_IRUGO | S_IWUSR);
+module_param(threads_per_chan, uint, 0644);
MODULE_PARM_DESC(threads_per_chan,
"Number of threads to start per channel (default: 1)");
static unsigned int max_channels;
-module_param(max_channels, uint, S_IRUGO | S_IWUSR);
+module_param(max_channels, uint, 0644);
MODULE_PARM_DESC(max_channels,
"Maximum number of channels to use (default: all)");
static unsigned int iterations;
-module_param(iterations, uint, S_IRUGO | S_IWUSR);
+module_param(iterations, uint, 0644);
MODULE_PARM_DESC(iterations,
"Iterations before stopping test (default: infinite)");
static unsigned int dmatest;
-module_param(dmatest, uint, S_IRUGO | S_IWUSR);
+module_param(dmatest, uint, 0644);
MODULE_PARM_DESC(dmatest,
"dmatest 0-memcpy 1-memset (default: 0)");
static unsigned int xor_sources = 3;
-module_param(xor_sources, uint, S_IRUGO | S_IWUSR);
+module_param(xor_sources, uint, 0644);
MODULE_PARM_DESC(xor_sources,
"Number of xor source buffers (default: 3)");
static unsigned int pq_sources = 3;
-module_param(pq_sources, uint, S_IRUGO | S_IWUSR);
+module_param(pq_sources, uint, 0644);
MODULE_PARM_DESC(pq_sources,
"Number of p+q source buffers (default: 3)");
static int timeout = 3000;
-module_param(timeout, uint, S_IRUGO | S_IWUSR);
+module_param(timeout, uint, 0644);
MODULE_PARM_DESC(timeout, "Transfer Timeout in msec (default: 3000), "
"Pass -1 for infinite timeout");
static bool noverify;
-module_param(noverify, bool, S_IRUGO | S_IWUSR);
+module_param(noverify, bool, 0644);
MODULE_PARM_DESC(noverify, "Disable random data setup and verification");
static bool verbose;
-module_param(verbose, bool, S_IRUGO | S_IWUSR);
+module_param(verbose, bool, 0644);
MODULE_PARM_DESC(verbose, "Enable \"success\" result messages (default: off)");
/**
@@ -131,7 +129,7 @@ struct dmatest_params {
.get = dmatest_run_get,
};
static bool dmatest_run;
-module_param_cb(run, &run_ops, &dmatest_run, S_IRUGO | S_IWUSR);
+module_param_cb(run, &run_ops, &dmatest_run, 0644);
MODULE_PARM_DESC(run, "Run the test (default: false)");
/* Maximum amount of mismatched bytes in buffer to print */
@@ -216,7 +214,7 @@ static int dmatest_wait_get(char *val, const struct kernel_param *kp)
.get = dmatest_wait_get,
.set = param_set_bool,
};
-module_param_cb(wait, &wait_ops, &wait, S_IRUGO);
+module_param_cb(wait, &wait_ops, &wait, 0444);
MODULE_PARM_DESC(wait, "Wait for tests to complete (default: false)");
static bool dmatest_match_channel(struct dmatest_params *params,
--
1.8.3.1
Existing option noverify disables both random src/dst address offset
setup and data verification. Sometimes, we need to control random
src/dst address setup and verification separately, such as disabling
random to make sure that test covers addresses in all interleaving
banks in one run. This patch adds option norandom to disable random
offset setup. Option noverify has been changed to disable data
verification only.
Signed-off-by: Yang Shunyong <[email protected]>
---
drivers/dma/dmatest.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
index 4cdccc3..cc3ab97 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -72,7 +72,11 @@
static bool noverify;
module_param(noverify, bool, 0644);
-MODULE_PARM_DESC(noverify, "Disable random data setup and verification");
+MODULE_PARM_DESC(noverify, "Disable data verification (default: verify)");
+
+static bool norandom;
+module_param(norandom, bool, 0644);
+MODULE_PARM_DESC(norandom, "Disable random offset setup (default: random)");
static bool verbose;
module_param(verbose, bool, 0644);
@@ -101,6 +105,7 @@ struct dmatest_params {
unsigned int pq_sources;
int timeout;
bool noverify;
+ bool norandom;
};
/**
@@ -573,7 +578,7 @@ static int dmatest_func(void *data)
break;
}
- if (params->noverify)
+ if (params->norandom)
len = params->buf_size;
else
len = dmatest_random() % params->buf_size + 1;
@@ -584,17 +589,19 @@ static int dmatest_func(void *data)
total_len += len;
- if (params->noverify) {
+ if (params->norandom) {
src_off = 0;
dst_off = 0;
} else {
- start = ktime_get();
src_off = dmatest_random() % (params->buf_size - len + 1);
dst_off = dmatest_random() % (params->buf_size - len + 1);
src_off = (src_off >> align) << align;
dst_off = (dst_off >> align) << align;
+ }
+ if (!params->noverify) {
+ start = ktime_get();
dmatest_init_srcs(thread->srcs, src_off, len,
params->buf_size, is_memset);
dmatest_init_dsts(thread->dsts, dst_off, len,
@@ -973,6 +980,7 @@ static void run_threaded_test(struct dmatest_info *info)
params->pq_sources = pq_sources;
params->timeout = timeout;
params->noverify = noverify;
+ params->norandom = norandom;
request_channels(info, DMA_MEMCPY);
request_channels(info, DMA_MEMSET);
--
1.8.3.1
On Mon, Jan 22, 2018 at 06:44:40PM +0800, Yang Shunyong wrote:
> Current coding style prefers octal permissions values. This patch
> changes symbolic permissions to octal values.
Is this preference documented anywhere?
--
~Vinod
Hi, Vinod
On Mon, 2018-01-29 at 10:18 +0530, Vinod Koul wrote:
> On Mon, Jan 22, 2018 at 06:44:40PM +0800, Yang Shunyong wrote:
> >
> > Current coding style prefers octal permissions values. This patch
> > changes symbolic permissions to octal values.
> Is this preference documented anywhere?
>
When using symbolic permissions like "S_IRUGO | S_IWUSR". The
checkpatch.pl will output some warnings to suggest to use octal values.
I quote following lines from checkpatch.pl,
# check for uses of S_<PERMS> that could be octal for readability
...
if (WARN("SYMBOLIC_PERMS",
?????????????????????????????????"Symbolic permissions '$oval' are not
preferred. Consider using octal permissions '$octal'.\n" . $herecurr)
&&
????????????????????????????$fix) {
????????????????????????????????$fixed[$fixlinenr] =~ s/$val/$octal/;
????????????????????????}
Thanks.
Shunyong
On Mon, Jan 29, 2018 at 06:26:40AM +0000, Yang, Shunyong wrote:
> Hi, Vinod
>
> On Mon, 2018-01-29 at 10:18 +0530, Vinod Koul wrote:
> > On Mon, Jan 22, 2018 at 06:44:40PM +0800, Yang Shunyong wrote:
> > >
> > > Current coding style prefers octal permissions values. This patch
> > > changes symbolic permissions to octal values.
> > Is this preference documented anywhere?
> >
>
> When using symbolic permissions like "S_IRUGO | S_IWUSR". The
> checkpatch.pl will output some warnings to suggest to use octal values.
> I quote following lines from checkpatch.pl,
>
> # check for uses of S_<PERMS> that could be octal for readability
> ...
> if (WARN("SYMBOLIC_PERMS",
> ?????????????????????????????????"Symbolic permissions '$oval' are not
> preferred. Consider using octal permissions '$octal'.\n" . $herecurr)
> &&
It is a warning and a preference. And I don't prefer changing code for no
benefit. Sorry but I am not considering this change
--
~Vinod
On Mon, Jan 22, 2018 at 06:44:41PM +0800, Yang Shunyong wrote:
> Existing option noverify disables both random src/dst address offset
> setup and data verification. Sometimes, we need to control random
> src/dst address setup and verification separately, such as disabling
> random to make sure that test covers addresses in all interleaving
> banks in one run. This patch adds option norandom to disable random
> offset setup. Option noverify has been changed to disable data
> verification only.
This doesn't apply for me due to dependency on previous patch. Please rebase
and resend.
--
~Vinod
Hi, Vinod
On Wed, 2018-01-31 at 13:04 +0530, Vinod Koul wrote:
> On Mon, Jan 22, 2018 at 06:44:41PM +0800, Yang Shunyong wrote:
> >
> > Existing option noverify disables both random src/dst address
> > offset
> > setup and data verification. Sometimes, we need to control random
> > src/dst address setup and verification separately, such as
> > disabling
> > random to make sure that test covers addresses in all interleaving
> > banks in one run. This patch adds option norandom to disable random
> > offset setup. Option noverify has been changed to disable data
> > verification only.
> This doesn't apply for me due to dependency on previous patch. Please
> rebase
> and resend.
>
Thanks. I have finished the rebase. I will send out v2 patch when I
finish test.
Thanks.
Shunyong.
On Mon, Jan 29, 2018 at 10:18 AM, Vinod Koul <[email protected]> wrote:
> On Mon, Jan 22, 2018 at 06:44:40PM +0800, Yang Shunyong wrote:
>> Current coding style prefers octal permissions values. This patch
>> changes symbolic permissions to octal values.
>
> Is this preference documented anywhere?
This is where it started.
lkml.kernel.org/r/CA+55aFzu59sXo7qA8CoCo+tSJgEkW7fWDjkt_tsTzPWZ7gxj1A@mail.gmail.com
On Wed, Jan 31, 2018 at 03:31:48PM +0530, Viresh Kumar wrote:
> On Mon, Jan 29, 2018 at 10:18 AM, Vinod Koul <[email protected]> wrote:
> > On Mon, Jan 22, 2018 at 06:44:40PM +0800, Yang Shunyong wrote:
> >> Current coding style prefers octal permissions values. This patch
> >> changes symbolic permissions to octal values.
> >
> > Is this preference documented anywhere?
>
> This is where it started.
>
> lkml.kernel.org/r/CA+55aFzu59sXo7qA8CoCo+tSJgEkW7fWDjkt_tsTzPWZ7gxj1A@mail.gmail.com
Right but lets no go changing to octals everywhere, new code sure :)
I don't see plans to remove representations entirely from kernel yet.
--
~Vinod
On Thu, Feb 1, 2018 at 9:15 AM, Vinod Koul <[email protected]> wrote:
> Right but lets no go changing to octals everywhere, new code sure :)
> I don't see plans to remove representations entirely from kernel yet.
Sure. I was just pointing out to the discussion where the *preferred* thing
happened.
On Thu, 2018-02-01 at 09:16 +0530, Viresh Kumar wrote:
> On Thu, Feb 1, 2018 at 9:15 AM, Vinod Koul <[email protected]> wrote:
> > Right but lets no go changing to octals everywhere, new code sure :)
> > I don't see plans to remove representations entirely from kernel yet.
>
> Sure. I was just pointing out to the discussion where the *preferred* thing
> happened.
Not really true. Linus prefers actual octal
and suggested treewide conversions.
https://lwn.net/Articles/696229/