On Fri, 14 Jul 2023 10:44:14 +0100 Ryan Roberts ryan.roberts@arm.com wrote:
On 13/07/2023 18:32, SeongJae Park wrote:
On Thu, 13 Jul 2023 16:39:33 +0200 David Hildenbrand david@redhat.com wrote:
On 13.07.23 15:54, Ryan Roberts wrote:
When run under run_vmtests.sh, test scripts were failing to run with "permission denied" due to the scripts not being executable.
It is also annoying not to be able to directly invoke run_vmtests.sh, which is solved by giving also it the execute permission.
Signed-off-by: Ryan Roberts ryan.roberts@arm.com
tools/testing/selftests/mm/charge_reserved_hugetlb.sh | 0 tools/testing/selftests/mm/check_config.sh | 0 tools/testing/selftests/mm/hugetlb_reparenting_test.sh | 0 tools/testing/selftests/mm/run_vmtests.sh | 0 tools/testing/selftests/mm/test_hmm.sh | 0 tools/testing/selftests/mm/test_vmalloc.sh | 0 tools/testing/selftests/mm/va_high_addr_switch.sh | 0 tools/testing/selftests/mm/write_hugetlb_memory.sh | 0 8 files changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 tools/testing/selftests/mm/charge_reserved_hugetlb.sh mode change 100644 => 100755 tools/testing/selftests/mm/check_config.sh mode change 100644 => 100755 tools/testing/selftests/mm/hugetlb_reparenting_test.sh mode change 100644 => 100755 tools/testing/selftests/mm/run_vmtests.sh mode change 100644 => 100755 tools/testing/selftests/mm/test_hmm.sh mode change 100644 => 100755 tools/testing/selftests/mm/test_vmalloc.sh mode change 100644 => 100755 tools/testing/selftests/mm/va_high_addr_switch.sh mode change 100644 => 100755 tools/testing/selftests/mm/write_hugetlb_memory.sh
diff --git a/tools/testing/selftests/mm/charge_reserved_hugetlb.sh b/tools/testing/selftests/mm/charge_reserved_hugetlb.sh old mode 100644 new mode 100755 diff --git a/tools/testing/selftests/mm/check_config.sh b/tools/testing/selftests/mm/check_config.sh old mode 100644 new mode 100755 diff --git a/tools/testing/selftests/mm/hugetlb_reparenting_test.sh b/tools/testing/selftests/mm/hugetlb_reparenting_test.sh old mode 100644 new mode 100755 diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh old mode 100644 new mode 100755 diff --git a/tools/testing/selftests/mm/test_hmm.sh b/tools/testing/selftests/mm/test_hmm.sh old mode 100644 new mode 100755 diff --git a/tools/testing/selftests/mm/test_vmalloc.sh b/tools/testing/selftests/mm/test_vmalloc.sh old mode 100644 new mode 100755 diff --git a/tools/testing/selftests/mm/va_high_addr_switch.sh b/tools/testing/selftests/mm/va_high_addr_switch.sh old mode 100644 new mode 100755 diff --git a/tools/testing/selftests/mm/write_hugetlb_memory.sh b/tools/testing/selftests/mm/write_hugetlb_memory.sh old mode 100644 new mode 100755
Sounds reasonable to me.
Probably due to:
commit baa489fabd01596d5426d6e112b34ba5fb59ab82 Author: SeongJae Park sj@kernel.org Date: Tue Jan 3 18:07:53 2023 +0000
selftests/vm: rename selftests/vm to selftests/mm Rename selftets/vm to selftests/mm for being more consistent with the code, documentation, and tools directories, and won't be confused with virtual machines.and indeed, it contains
diff --git a/tools/testing/selftests/vm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh old mode 100755 new mode 100644 similarity index 100% rename from tools/testing/selftests/vm/run_vmtests.sh rename to tools/testing/selftests/mm/run_vmtests.sh
Thank you for tracking this and kindly Cc-ing me! I'd like to clarify a little bit more, though. The permission change has made by the commit as you found. Nevertheless, the submitted version[1] of the patch didn't change the permission. I guess the change was made while managing it via some file permission unsupported patches management tool.
I had a similar issue with DAMON selftest and sent a patch restoring the permission. Greg suggested me to update the framework instead, to support such management tool[2], so I made it[3]. It recently also merged into 5.15.y for DAMON selftests[4].
I have no strong opinion about whether we need to keep the permission or it's good to have no execute permission since kselftest framework supports it. I just wanted to clarify the events I've shown. Please correct me if I missed or wrong something. Cc-ing Greg, since he might have an opinion.
Thanks for the detailed explanation. Are you effectively saying this patch will turn into a no-op once its been munged through the various patch management tools?
Depending on what tool maintainers that will pick this patch is using in what way, I guess.
That's disappointing because it's a pain to have to invoke everything though bash explicitly. Many other scripts manage to have the correct execute permission set (see everything in ./scripts for example).
Personally I'd rather keep this patch and try rather than proactively do a work around.
I don't have a strong opinion here, as mentioned before. That said, I feel it would be good to have a clear agreement or explanation about that, since I got similar situation before[1].
[1] https://lore.kernel.org/damon/20230221175612.131555-1-sj@kernel.org/
Thanks, SJ
[1] https://lore.kernel.org/all/20230103180754.129637-5-sj@kernel.org/ [2] https://lore.kernel.org/mm-commits/YRJisBs9AunccCD4@kroah.com/ [3] https://lore.kernel.org/all/20210810164534.25902-1-sj38.park@gmail.com/ [4] https://lore.kernel.org/stable/2023042743-cheesy-parasitic-206d@gregkh/
Thanks, SJ
Reviewed-by: David Hildenbrand david@redhat.com
-- Cheers,
David / dhildenb