2022-02-18 00:41:20

by Yuanchu Xie

[permalink] [raw]
Subject: [PATCH 2/2] selftests/damon: make selftests executable

The damon selftests do not have the executable bit on. We fix that by
setting the x bits on the .sh files similar to other existing shell
selftests.

Fixes: 9ab3b0c8ef62 ("selftests/damon: split test cases")
Signed-off-by: Yuanchu Xie <[email protected]>
---
tools/testing/selftests/damon/debugfs_attrs.sh | 0
tools/testing/selftests/damon/debugfs_empty_targets.sh | 0
tools/testing/selftests/damon/debugfs_huge_count_read_write.sh | 0
tools/testing/selftests/damon/debugfs_schemes.sh | 0
tools/testing/selftests/damon/debugfs_target_ids.sh | 0
5 files changed, 0 insertions(+), 0 deletions(-)
mode change 100644 => 100755 tools/testing/selftests/damon/debugfs_attrs.sh
mode change 100644 => 100755 tools/testing/selftests/damon/debugfs_empty_targets.sh
mode change 100644 => 100755 tools/testing/selftests/damon/debugfs_huge_count_read_write.sh
mode change 100644 => 100755 tools/testing/selftests/damon/debugfs_schemes.sh
mode change 100644 => 100755 tools/testing/selftests/damon/debugfs_target_ids.sh

diff --git a/tools/testing/selftests/damon/debugfs_attrs.sh b/tools/testing/selftests/damon/debugfs_attrs.sh
old mode 100644
new mode 100755
diff --git a/tools/testing/selftests/damon/debugfs_empty_targets.sh b/tools/testing/selftests/damon/debugfs_empty_targets.sh
old mode 100644
new mode 100755
diff --git a/tools/testing/selftests/damon/debugfs_huge_count_read_write.sh b/tools/testing/selftests/damon/debugfs_huge_count_read_write.sh
old mode 100644
new mode 100755
diff --git a/tools/testing/selftests/damon/debugfs_schemes.sh b/tools/testing/selftests/damon/debugfs_schemes.sh
old mode 100644
new mode 100755
diff --git a/tools/testing/selftests/damon/debugfs_target_ids.sh b/tools/testing/selftests/damon/debugfs_target_ids.sh
old mode 100644
new mode 100755
--
2.35.1.265.g69c8d7142f-goog


2022-02-18 07:59:00

by SeongJae Park

[permalink] [raw]
Subject: Re: [PATCH 2/2] selftests/damon: make selftests executable

Hello Yuanchu,

Thank you for this patch!

On Fri, 18 Feb 2022 00:10:17 +0000 Yuanchu Xie <[email protected]> wrote:

> The damon selftests do not have the executable bit on. We fix that by
> setting the x bits on the .sh files similar to other existing shell
> selftests.
>
> Fixes: 9ab3b0c8ef62 ("selftests/damon: split test cases")
> Signed-off-by: Yuanchu Xie <[email protected]>

Reviewed-by: SeongJae Park <[email protected]>


Thanks,
SJ

> ---
> tools/testing/selftests/damon/debugfs_attrs.sh | 0
> tools/testing/selftests/damon/debugfs_empty_targets.sh | 0
> tools/testing/selftests/damon/debugfs_huge_count_read_write.sh | 0
> tools/testing/selftests/damon/debugfs_schemes.sh | 0
> tools/testing/selftests/damon/debugfs_target_ids.sh | 0
> 5 files changed, 0 insertions(+), 0 deletions(-)
> mode change 100644 => 100755 tools/testing/selftests/damon/debugfs_attrs.sh
> mode change 100644 => 100755 tools/testing/selftests/damon/debugfs_empty_targets.sh
> mode change 100644 => 100755 tools/testing/selftests/damon/debugfs_huge_count_read_write.sh
> mode change 100644 => 100755 tools/testing/selftests/damon/debugfs_schemes.sh
> mode change 100644 => 100755 tools/testing/selftests/damon/debugfs_target_ids.sh
>
> diff --git a/tools/testing/selftests/damon/debugfs_attrs.sh b/tools/testing/selftests/damon/debugfs_attrs.sh
> old mode 100644
> new mode 100755
> diff --git a/tools/testing/selftests/damon/debugfs_empty_targets.sh b/tools/testing/selftests/damon/debugfs_empty_targets.sh
> old mode 100644
> new mode 100755
> diff --git a/tools/testing/selftests/damon/debugfs_huge_count_read_write.sh b/tools/testing/selftests/damon/debugfs_huge_count_read_write.sh
> old mode 100644
> new mode 100755
> diff --git a/tools/testing/selftests/damon/debugfs_schemes.sh b/tools/testing/selftests/damon/debugfs_schemes.sh
> old mode 100644
> new mode 100755
> diff --git a/tools/testing/selftests/damon/debugfs_target_ids.sh b/tools/testing/selftests/damon/debugfs_target_ids.sh
> old mode 100644
> new mode 100755
> --
> 2.35.1.265.g69c8d7142f-goog

2022-02-18 08:49:53

by SeongJae Park

[permalink] [raw]
Subject: Re: [PATCH 2/2] selftests/damon: make selftests executable

On Fri, 18 Feb 2022 09:01:11 +0100 Greg KH <[email protected]> wrote:

> On Fri, Feb 18, 2022 at 07:52:54AM +0000, SeongJae Park wrote:
> > Hello Yuanchu,
> >
> > Thank you for this patch!
> >
> > On Fri, 18 Feb 2022 00:10:17 +0000 Yuanchu Xie <[email protected]> wrote:
> >
> > > The damon selftests do not have the executable bit on. We fix that by
> > > setting the x bits on the .sh files similar to other existing shell
> > > selftests.
> > >
> > > Fixes: 9ab3b0c8ef62 ("selftests/damon: split test cases")
> > > Signed-off-by: Yuanchu Xie <[email protected]>
> >
> > Reviewed-by: SeongJae Park <[email protected]>
>
> This type of change does not work outside of git, so why not just make
> the tool that calls these scripts not care about the executable bit like
> we do for other scripts?

Actually, we made kselftest receives scripts having no executable bit[1],
though it still prints warning. I guess Yuanchu wants to remove the warning?

To remove the warning, simply making kselftest (runner.sh) stop printing the
warning message might make more sense. Nevertheless, it's also true that
letting some scripts have executable bits while others not looks inconsistent
to me. That's why I left the warning message there. Should we remove the
warning from kselftest and remove executable bits from other selftest test
scripts? Or, let the inconsistency be? I have no real opinion here, so just
wanted to hear others' opinion if possible.

BTW, similar issue is also in Kconfig's 'shell' macro. For example,
'runst-version.sh' on -mm tree bothered me before[2], and now
'pahole-version.sh'. I might make change in 'scrips/Kconfig/preprocess.c'
later.

[1] https://lore.kernel.org/linux-kselftest/[email protected]/
[2] https://lore.kernel.org/all/[email protected]/


Thanks,
SJ

>
> thanks,
>
> greg k-h

2022-02-18 09:20:40

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 2/2] selftests/damon: make selftests executable

On Fri, Feb 18, 2022 at 07:52:54AM +0000, SeongJae Park wrote:
> Hello Yuanchu,
>
> Thank you for this patch!
>
> On Fri, 18 Feb 2022 00:10:17 +0000 Yuanchu Xie <[email protected]> wrote:
>
> > The damon selftests do not have the executable bit on. We fix that by
> > setting the x bits on the .sh files similar to other existing shell
> > selftests.
> >
> > Fixes: 9ab3b0c8ef62 ("selftests/damon: split test cases")
> > Signed-off-by: Yuanchu Xie <[email protected]>
>
> Reviewed-by: SeongJae Park <[email protected]>

This type of change does not work outside of git, so why not just make
the tool that calls these scripts not care about the executable bit like
we do for other scripts?

thanks,

greg k-h

2022-02-19 01:03:48

by Shuah Khan

[permalink] [raw]
Subject: Re: [PATCH 2/2] selftests/damon: make selftests executable

On 2/18/22 1:24 AM, SeongJae Park wrote:
> On Fri, 18 Feb 2022 09:01:11 +0100 Greg KH <[email protected]> wrote:
>
>> On Fri, Feb 18, 2022 at 07:52:54AM +0000, SeongJae Park wrote:
>>> Hello Yuanchu,
>>>
>>> Thank you for this patch!
>>>
>>> On Fri, 18 Feb 2022 00:10:17 +0000 Yuanchu Xie <[email protected]> wrote:
>>>
>>>> The damon selftests do not have the executable bit on. We fix that by
>>>> setting the x bits on the .sh files similar to other existing shell
>>>> selftests.
>>>>
>>>> Fixes: 9ab3b0c8ef62 ("selftests/damon: split test cases")
>>>> Signed-off-by: Yuanchu Xie <[email protected]>
>>>
>>> Reviewed-by: SeongJae Park <[email protected]>
>>
>> This type of change does not work outside of git, so why not just make
>> the tool that calls these scripts not care about the executable bit like
>> we do for other scripts?
>
> Actually, we made kselftest receives scripts having no executable bit[1],
> though it still prints warning. I guess Yuanchu wants to remove the warning?
>
> To remove the warning, simply making kselftest (runner.sh) stop printing the
> warning message might make more sense. Nevertheless, it's also true that
> letting some scripts have executable bits while others not looks inconsistent
> to me. That's why I left the warning message there. Should we remove the
> warning from kselftest and remove executable bits from other selftest test
> scripts? Or, let the inconsistency be? I have no real opinion here, so just
> wanted to hear others' opinion if possible.
>

I don't recall why we decided to add the check in runner.sh - let's keep them
consistent with the rest of the scripts. If we get rid of the check, we can
make the change then.

thanks,
-- Shuah

2022-04-19 05:32:36

by Yuanchu Xie

[permalink] [raw]
Subject: Re: [PATCH 2/2] selftests/damon: make selftests executable

> I don't recall why we decided to add the check in runner.sh - let's keep them
> consistent with the rest of the scripts. If we get rid of the check, we can
> make the change then.
>
> thanks,
> -- Shuah

It seems reasonable to add the x bit for these tests to be consistent with
the rest.

I also received an email from a [email protected]
telling me my patch series was included in shuah/linux-kselftest.git, but
that does not seem to be the case.
Is this a bug?

Sorry about the previous non-plain-text email. I never replied to anyone
before and didn't know what I was doing.

> Hello:
>
>
> This series was applied to shuah/linux-kselftest.git (next)
>
> by Jakub Kicinski <[email protected]>:
>
>
> On Fri, 18 Feb 2022 00:10:15 +0000 you wrote:
> > These patches fixes trivial errors with building
> > and running DAMON selftests.
> >
> > Yuanchu Xie (2):
> > selftests/damon: add damon to selftests root Makefile
> > selftests/damon: make selftests executable
> >
> > [...]
>
>
> Here is the summary with links:
> - [1/2] selftests/damon: add damon to selftests root Makefile
> (no matching commit)
> - [2/2] selftests/damon: make selftests executable
> https://git.kernel.org/shuah/linux-kselftest/c/1335648f0b6f
>
> You are awesome, thank you!
> --
> Deet-doot-dot, I am a bot.
> https://korg.docs.kernel.org/patchwork/pwbot.html