2023-07-13 14:14:50

by Ryan Roberts

[permalink] [raw]
Subject: [PATCH v1 2/9] selftests/mm: Give scripts execute permission

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 <[email protected]>
---
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
--
2.25.1



2023-07-13 15:02:00

by David Hildenbrand

[permalink] [raw]
Subject: Re: [PATCH v1 2/9] selftests/mm: Give scripts execute permission

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 <[email protected]>
> ---
> 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 <[email protected]>
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


Reviewed-by: David Hildenbrand <[email protected]>

--
Cheers,

David / dhildenb


2023-07-13 18:08:02

by SeongJae Park

[permalink] [raw]
Subject: Re: [PATCH v1 2/9] selftests/mm: Give scripts execute permission

On Thu, 13 Jul 2023 16:39:33 +0200 David Hildenbrand <[email protected]> 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 <[email protected]>
> > ---
> > 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 <[email protected]>
> 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.

[1] https://lore.kernel.org/all/[email protected]/
[2] https://lore.kernel.org/mm-commits/[email protected]/
[3] https://lore.kernel.org/all/[email protected]/
[4] https://lore.kernel.org/stable/2023042743-cheesy-parasitic-206d@gregkh/


Thanks,
SJ

>
>
> Reviewed-by: David Hildenbrand <[email protected]>
>
> --
> Cheers,
>
> David / dhildenb
>
>
>

2023-07-14 10:40:49

by Ryan Roberts

[permalink] [raw]
Subject: Re: [PATCH v1 2/9] selftests/mm: Give scripts execute permission

On 13/07/2023 18:32, SeongJae Park wrote:
> On Thu, 13 Jul 2023 16:39:33 +0200 David Hildenbrand <[email protected]> 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 <[email protected]>
>>> ---
>>> 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 <[email protected]>
>> 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? 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.


>
> [1] https://lore.kernel.org/all/[email protected]/
> [2] https://lore.kernel.org/mm-commits/[email protected]/
> [3] https://lore.kernel.org/all/[email protected]/
> [4] https://lore.kernel.org/stable/2023042743-cheesy-parasitic-206d@gregkh/
>
>
> Thanks,
> SJ
>
>>
>>
>> Reviewed-by: David Hildenbrand <[email protected]>
>>
>> --
>> Cheers,
>>
>> David / dhildenb
>>
>>
>>


2023-07-14 16:18:40

by SeongJae Park

[permalink] [raw]
Subject: Re: [PATCH v1 2/9] selftests/mm: Give scripts execute permission

On Fri, 14 Jul 2023 10:44:14 +0100 Ryan Roberts <[email protected]> wrote:

> On 13/07/2023 18:32, SeongJae Park wrote:
> > On Thu, 13 Jul 2023 16:39:33 +0200 David Hildenbrand <[email protected]> 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 <[email protected]>
> >>> ---
> >>> 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 <[email protected]>
> >> 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/[email protected]/


Thanks,
SJ

>
>
> >
> > [1] https://lore.kernel.org/all/[email protected]/
> > [2] https://lore.kernel.org/mm-commits/[email protected]/
> > [3] https://lore.kernel.org/all/[email protected]/
> > [4] https://lore.kernel.org/stable/2023042743-cheesy-parasitic-206d@gregkh/
> >
> >
> > Thanks,
> > SJ
> >
> >>
> >>
> >> Reviewed-by: David Hildenbrand <[email protected]>
> >>
> >> --
> >> Cheers,
> >>
> >> David / dhildenb
> >>
> >>
> >>
>
>

2023-07-14 16:26:32

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v1 2/9] selftests/mm: Give scripts execute permission

On Fri, Jul 14, 2023 at 04:00:58PM +0000, SeongJae Park wrote:
> On Fri, 14 Jul 2023 10:44:14 +0100 Ryan Roberts <[email protected]> wrote:

> > 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].

I think just from a usability point of view we want to end up with
things people are expected to execute actually executable.


Attachments:
(No filename) (573.00 B)
signature.asc (499.00 B)
Download all attachments

2023-07-14 16:56:55

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH v1 2/9] selftests/mm: Give scripts execute permission

On Fri, 14 Jul 2023 10:44:14 +0100 Ryan Roberts <[email protected]> wrote:

> 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? 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).

Such patches need delicate handling :(

I queued this as a standalone thing, for 6.5-rcx.

2023-07-14 17:11:55

by Ryan Roberts

[permalink] [raw]
Subject: Re: [PATCH v1 2/9] selftests/mm: Give scripts execute permission

On 14/07/2023 17:26, Andrew Morton wrote:
> On Fri, 14 Jul 2023 10:44:14 +0100 Ryan Roberts <[email protected]> wrote:
>
>> 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? 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).
>
> Such patches need delicate handling :(
>
> I queued this as a standalone thing, for 6.5-rcx.

That's great - thanks Andrew! Do I'll drop this patch for my v2 of the series
(hopefully Monday).