diff mbox series

[v6,1/2] selftests: Rename sigaltstack to generic signal

Message ID 20240822121415.3589190-2-dev.jain@arm.com (mailing list archive)
State Accepted
Commit 11f0c8e0468a8bc625164f68dd5ff2a9436658db
Headers show
Series Add test to distinguish between thread's signal mask and ucontext_t | expand

Commit Message

Dev Jain Aug. 22, 2024, 12:14 p.m. UTC
Rename sigaltstack to generic signal directory, to allow adding more
signal tests in the future.

Signed-off-by: Dev Jain <dev.jain@arm.com>
Reviewed-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/Makefile                                | 2 +-
 tools/testing/selftests/{sigaltstack => signal}/.gitignore      | 0
 tools/testing/selftests/{sigaltstack => signal}/Makefile        | 0
 .../selftests/{sigaltstack => signal}/current_stack_pointer.h   | 0
 tools/testing/selftests/{sigaltstack => signal}/sas.c           | 0
 5 files changed, 1 insertion(+), 1 deletion(-)
 rename tools/testing/selftests/{sigaltstack => signal}/.gitignore (100%)
 rename tools/testing/selftests/{sigaltstack => signal}/Makefile (100%)
 rename tools/testing/selftests/{sigaltstack => signal}/current_stack_pointer.h (100%)
 rename tools/testing/selftests/{sigaltstack => signal}/sas.c (100%)

Comments

Shuah Khan Aug. 27, 2024, 11:44 a.m. UTC | #1
On 8/22/24 06:14, Dev Jain wrote:
> Rename sigaltstack to generic signal directory, to allow adding more
> signal tests in the future.

Sorry - I think I mentioned I don't like this test renamed. Why are you sending
this rename still included in the patch series?

thanks,
-- Shuah
Dev Jain Aug. 27, 2024, 11:46 a.m. UTC | #2
On 8/27/24 17:14, Shuah Khan wrote:
> On 8/22/24 06:14, Dev Jain wrote:
>> Rename sigaltstack to generic signal directory, to allow adding more
>> signal tests in the future.
>
> Sorry - I think I mentioned I don't like this test renamed. Why are 
> you sending
> this rename still included in the patch series?

I am not renaming the test, just the directory. The directory name
is changed to signal, and I have retained the name of the test -
sas.c.
>
> thanks,
> -- Shuah
Dev Jain Aug. 30, 2024, 4:29 p.m. UTC | #3
On 8/27/24 17:16, Dev Jain wrote:
>
> On 8/27/24 17:14, Shuah Khan wrote:
>> On 8/22/24 06:14, Dev Jain wrote:
>>> Rename sigaltstack to generic signal directory, to allow adding more
>>> signal tests in the future.
>>
>> Sorry - I think I mentioned I don't like this test renamed. Why are 
>> you sending
>> this rename still included in the patch series?
>
> I am not renaming the test, just the directory. The directory name
> is changed to signal, and I have retained the name of the test -
> sas.c.

Gentle ping: I guess there was a misunderstanding; in v5, I was
also changing the name of the test, to which you objected, and
I agreed. But, we need to change the name of the directory since
the new test has no relation to the current directory name,
"sigaltstack". The patch description explains that the directory
should be generically named.

>
>>
>> thanks,
>> -- Shuah
Shuah Khan Sept. 3, 2024, 9:44 p.m. UTC | #4
On 8/30/24 10:29, Dev Jain wrote:
> 
> On 8/27/24 17:16, Dev Jain wrote:
>>
>> On 8/27/24 17:14, Shuah Khan wrote:
>>> On 8/22/24 06:14, Dev Jain wrote:
>>>> Rename sigaltstack to generic signal directory, to allow adding more
>>>> signal tests in the future.
>>>
>>> Sorry - I think I mentioned I don't like this test renamed. Why are you sending
>>> this rename still included in the patch series?
>>
>> I am not renaming the test, just the directory. The directory name
>> is changed to signal, and I have retained the name of the test -
>> sas.c.
> 
> Gentle ping: I guess there was a misunderstanding; in v5, I was
> also changing the name of the test, to which you objected, and
> I agreed. But, we need to change the name of the directory since
> the new test has no relation to the current directory name,
> "sigaltstack". The patch description explains that the directory
> should be generically named.
> 

Right. You are no longer changing the test name. You are still
changing the directory name. The problem I mentioned stays the
same. Any fixes to the existing tests in this directory can no
longer auto applied to stables releases.

Other than the desire to rename the directory to generic, what
other value does this change bring?

thanks,
-- Shuah
Dev Jain Sept. 4, 2024, 4:52 a.m. UTC | #5
On 9/4/24 03:14, Shuah Khan wrote:
> On 8/30/24 10:29, Dev Jain wrote:
>>
>> On 8/27/24 17:16, Dev Jain wrote:
>>>
>>> On 8/27/24 17:14, Shuah Khan wrote:
>>>> On 8/22/24 06:14, Dev Jain wrote:
>>>>> Rename sigaltstack to generic signal directory, to allow adding more
>>>>> signal tests in the future.
>>>>
>>>> Sorry - I think I mentioned I don't like this test renamed. Why are 
>>>> you sending
>>>> this rename still included in the patch series?
>>>
>>> I am not renaming the test, just the directory. The directory name
>>> is changed to signal, and I have retained the name of the test -
>>> sas.c.
>>
>> Gentle ping: I guess there was a misunderstanding; in v5, I was
>> also changing the name of the test, to which you objected, and
>> I agreed. But, we need to change the name of the directory since
>> the new test has no relation to the current directory name,
>> "sigaltstack". The patch description explains that the directory
>> should be generically named.
>>
>
> Right. You are no longer changing the test name. You are still
> changing the directory name. The problem I mentioned stays the
> same. Any fixes to the existing tests in this directory can no
> longer auto applied to stables releases.

I understand your point, but commit baa489fabd01 (selftests/vm: rename
selftests/vm to selftests/mm) is also present. That was a lot bigger change;
sigaltstack contains just one test currently, whose fixes possibly would 
have
to be backported, so I guess it should not be that much of a big problem?

>
> Other than the desire to rename the directory to generic, what
> other value does this change bring?

Do you have an alternative suggestion as to where I should put my new 
test then;
I do not see what is the value of creating another directory to just include
my test. This will unnecessarily clutter the selftests/ directory with
directories containing single tests. And, putting this in "sigaltstack" 
is just
wrong since this test has no relation with sigaltstack.

>
Shuah Khan Sept. 4, 2024, 5:05 p.m. UTC | #6
On 9/3/24 22:52, Dev Jain wrote:
> 
> On 9/4/24 03:14, Shuah Khan wrote:
>> On 8/30/24 10:29, Dev Jain wrote:
>>>
>>> On 8/27/24 17:16, Dev Jain wrote:
>>>>
>>>> On 8/27/24 17:14, Shuah Khan wrote:
>>>>> On 8/22/24 06:14, Dev Jain wrote:
>>>>>> Rename sigaltstack to generic signal directory, to allow adding more
>>>>>> signal tests in the future.
>>>>>
>>>>> Sorry - I think I mentioned I don't like this test renamed. Why are you sending
>>>>> this rename still included in the patch series?
>>>>
>>>> I am not renaming the test, just the directory. The directory name
>>>> is changed to signal, and I have retained the name of the test -
>>>> sas.c.
>>>
>>> Gentle ping: I guess there was a misunderstanding; in v5, I was
>>> also changing the name of the test, to which you objected, and
>>> I agreed. But, we need to change the name of the directory since
>>> the new test has no relation to the current directory name,
>>> "sigaltstack". The patch description explains that the directory
>>> should be generically named.
>>>
>>
>> Right. You are no longer changing the test name. You are still
>> changing the directory name. The problem I mentioned stays the
>> same. Any fixes to the existing tests in this directory can no
>> longer auto applied to stables releases.
> 
> I understand your point, but commit baa489fabd01 (selftests/vm: rename
> selftests/vm to selftests/mm) is also present. That was a lot bigger change;
> sigaltstack contains just one test currently, whose fixes possibly would have
> to be backported, so I guess it should not be that much of a big problem?
> 
>>

So who does the backports whenevenr something changes? You are adding
work where as the automated process would just work without this
change. It doesn't matter if there is another test that changed
the name.

>> Other than the desire to rename the directory to generic, what
>> other value does this change bring?
> 
> Do you have an alternative suggestion as to where I should put my new test then;
> I do not see what is the value of creating another directory to just include
> my test. This will unnecessarily clutter the selftests/ directory with
> directories containing single tests. And, putting this in "sigaltstack" is just
> wrong since this test has no relation with sigaltstack.
> 

If this new test has no relation to sigaltstack, then why are you changing
and renaming the sigaltstack directory? Adding a new directory is much better
than going down a path that is more confusing and adding backport overhead.

Two options:
-- Add a new directory or add a note and keep it under sigaltstack
-- Do you foresee this new growing?

thanks,
-- Shuah
Dev Jain Sept. 5, 2024, 5:56 a.m. UTC | #7
On 9/4/24 22:35, Shuah Khan wrote:
> On 9/3/24 22:52, Dev Jain wrote:
>>
>> On 9/4/24 03:14, Shuah Khan wrote:
>>> On 8/30/24 10:29, Dev Jain wrote:
>>>>
>>>> On 8/27/24 17:16, Dev Jain wrote:
>>>>>
>>>>> On 8/27/24 17:14, Shuah Khan wrote:
>>>>>> On 8/22/24 06:14, Dev Jain wrote:
>>>>>>> Rename sigaltstack to generic signal directory, to allow adding 
>>>>>>> more
>>>>>>> signal tests in the future.
>>>>>>
>>>>>> Sorry - I think I mentioned I don't like this test renamed. Why 
>>>>>> are you sending
>>>>>> this rename still included in the patch series?
>>>>>
>>>>> I am not renaming the test, just the directory. The directory name
>>>>> is changed to signal, and I have retained the name of the test -
>>>>> sas.c.
>>>>
>>>> Gentle ping: I guess there was a misunderstanding; in v5, I was
>>>> also changing the name of the test, to which you objected, and
>>>> I agreed. But, we need to change the name of the directory since
>>>> the new test has no relation to the current directory name,
>>>> "sigaltstack". The patch description explains that the directory
>>>> should be generically named.
>>>>
>>>
>>> Right. You are no longer changing the test name. You are still
>>> changing the directory name. The problem I mentioned stays the
>>> same. Any fixes to the existing tests in this directory can no
>>> longer auto applied to stables releases.
>>
>> I understand your point, but commit baa489fabd01 (selftests/vm: rename
>> selftests/vm to selftests/mm) is also present. That was a lot bigger 
>> change;
>> sigaltstack contains just one test currently, whose fixes possibly 
>> would have
>> to be backported, so I guess it should not be that much of a big 
>> problem?
>>
>>>
>
> So who does the backports whenevenr something changes? You are adding
> work where as the automated process would just work without this
> change. It doesn't matter if there is another test that changed
> the name.
>
>>> Other than the desire to rename the directory to generic, what
>>> other value does this change bring?
>>
>> Do you have an alternative suggestion as to where I should put my new 
>> test then;
>> I do not see what is the value of creating another directory to just 
>> include
>> my test. This will unnecessarily clutter the selftests/ directory with
>> directories containing single tests. And, putting this in 
>> "sigaltstack" is just
>> wrong since this test has no relation with sigaltstack.
>>
>
> If this new test has no relation to sigaltstack, then why are you 
> changing
> and renaming the sigaltstack directory?

Because the functionality I am testing is of signals, and signals are a 
superset
of sigaltstack. Still, I can think of a compromise, if semantically you 
want to
consider the new test as not testing signals, but a specific syscall 
"sigaction"
and its interaction with blocking of signals, how about naming the new 
directory "sigaction"?
> Adding a new directory is much better
> than going down a path that is more confusing and adding backport 
> overhead.
>
> Two options:
> -- Add a new directory or add a note and keep it under sigaltstack
> -- Do you foresee this new growing?
>
> thanks,
> -- Shuah
>
>
Mark Brown Sept. 5, 2024, 11:25 a.m. UTC | #8
On Thu, Sep 05, 2024 at 11:26:02AM +0530, Dev Jain wrote:
> On 9/4/24 22:35, Shuah Khan wrote:

> > So who does the backports whenevenr something changes? You are adding
> > work where as the automated process would just work without this
> > change. It doesn't matter if there is another test that changed
> > the name.

I thought git was supposed to have some ability to try to cope with
renames, though heuristic based?  It does seem to work sometimes.  TBH
I'm also not sure how frequent an issue backporting fixes to this one
test is going to be, it's had a couple of minor fixes for warnings in
the past few years and the last substantial update was in 2021.

> > 
> > If this new test has no relation to sigaltstack, then why are you
> > changing
> > and renaming the sigaltstack directory?

> Because the functionality I am testing is of signals, and signals are a
> superset
> of sigaltstack. Still, I can think of a compromise, if semantically you want

I do tend to agree here, it seems neater to merge things and from the
point of view of running the tests in CI it's nice to not have too many
tiny suites, they create runtime overhead.

> to
> consider the new test as not testing signals, but a specific syscall
> "sigaction"
> and its interaction with blocking of signals, how about naming the new
> directory "sigaction"?

That's not going to scale amazingly if we test any other aspects of
signals...  I'd just call it "signal" and if it's not possible to get
the merge done just leave the sigaltstack suite as it is.
Shuah Khan Sept. 6, 2024, 7:59 p.m. UTC | #9
On 9/4/24 23:56, Dev Jain wrote:
> 
> On 9/4/24 22:35, Shuah Khan wrote:
>> On 9/3/24 22:52, Dev Jain wrote:
>>>
>>> On 9/4/24 03:14, Shuah Khan wrote:
>>>> On 8/30/24 10:29, Dev Jain wrote:
>>>>>
>>>>> On 8/27/24 17:16, Dev Jain wrote:
>>>>>>
>>>>>> On 8/27/24 17:14, Shuah Khan wrote:
>>>>>>> On 8/22/24 06:14, Dev Jain wrote:
>>>>>>>> Rename sigaltstack to generic signal directory, to allow adding more
>>>>>>>> signal tests in the future.
>>>>>>>
>>>>>>> Sorry - I think I mentioned I don't like this test renamed. Why are you sending
>>>>>>> this rename still included in the patch series?
>>>>>>
>>>>>> I am not renaming the test, just the directory. The directory name
>>>>>> is changed to signal, and I have retained the name of the test -
>>>>>> sas.c.
>>>>>
>>>>> Gentle ping: I guess there was a misunderstanding; in v5, I was
>>>>> also changing the name of the test, to which you objected, and
>>>>> I agreed. But, we need to change the name of the directory since
>>>>> the new test has no relation to the current directory name,
>>>>> "sigaltstack". The patch description explains that the directory
>>>>> should be generically named.
>>>>>
>>>>
>>>> Right. You are no longer changing the test name. You are still
>>>> changing the directory name. The problem I mentioned stays the
>>>> same. Any fixes to the existing tests in this directory can no
>>>> longer auto applied to stables releases.
>>>
>>> I understand your point, but commit baa489fabd01 (selftests/vm: rename
>>> selftests/vm to selftests/mm) is also present. That was a lot bigger change;
>>> sigaltstack contains just one test currently, whose fixes possibly would have
>>> to be backported, so I guess it should not be that much of a big problem?
>>>
>>>>
>>
>> So who does the backports whenevenr something changes? You are adding
>> work where as the automated process would just work without this
>> change. It doesn't matter if there is another test that changed
>> the name.
>>
>>>> Other than the desire to rename the directory to generic, what
>>>> other value does this change bring?
>>>
>>> Do you have an alternative suggestion as to where I should put my new test then;
>>> I do not see what is the value of creating another directory to just include
>>> my test. This will unnecessarily clutter the selftests/ directory with
>>> directories containing single tests. And, putting this in "sigaltstack" is just
>>> wrong since this test has no relation with sigaltstack.
>>>
>>
>> If this new test has no relation to sigaltstack, then why are you changing
>> and renaming the sigaltstack directory?
> 
> Because the functionality I am testing is of signals, and signals are a superset
> of sigaltstack. Still, I can think of a compromise, if semantically you want to
> consider the new test as not testing signals, but a specific syscall "sigaction"
> and its interaction with blocking of signals, how about naming the new directory "sigaction"?
>> Adding a new directory is much better
>> than going down a path that is more confusing and adding backport overhead.
>>

Okay - they are related except that you view signalstack as a subset
of signals. I saw Mark's response as well saying sigaction isn't
a good name for this.

Rename usually wipe out git history as well based on what have seen
in the past.

My main concern is backports. Considering sigstack hasn't changed
2021 (as Mark's email), let's rename it.

I am reluctantly agreeing to the rename as it seems to make sense
in this case.

thanks,
-- Shuah
Dev Jain Sept. 9, 2024, 5:16 a.m. UTC | #10
On 9/7/24 01:29, Shuah Khan wrote:
> On 9/4/24 23:56, Dev Jain wrote:
>>
>> On 9/4/24 22:35, Shuah Khan wrote:
>>> On 9/3/24 22:52, Dev Jain wrote:
>>>>
>>>> On 9/4/24 03:14, Shuah Khan wrote:
>>>>> On 8/30/24 10:29, Dev Jain wrote:
>>>>>>
>>>>>> On 8/27/24 17:16, Dev Jain wrote:
>>>>>>>
>>>>>>> On 8/27/24 17:14, Shuah Khan wrote:
>>>>>>>> On 8/22/24 06:14, Dev Jain wrote:
>>>>>>>>> Rename sigaltstack to generic signal directory, to allow 
>>>>>>>>> adding more
>>>>>>>>> signal tests in the future.
>>>>>>>>
>>>>>>>> Sorry - I think I mentioned I don't like this test renamed. Why 
>>>>>>>> are you sending
>>>>>>>> this rename still included in the patch series?
>>>>>>>
>>>>>>> I am not renaming the test, just the directory. The directory name
>>>>>>> is changed to signal, and I have retained the name of the test -
>>>>>>> sas.c.
>>>>>>
>>>>>> Gentle ping: I guess there was a misunderstanding; in v5, I was
>>>>>> also changing the name of the test, to which you objected, and
>>>>>> I agreed. But, we need to change the name of the directory since
>>>>>> the new test has no relation to the current directory name,
>>>>>> "sigaltstack". The patch description explains that the directory
>>>>>> should be generically named.
>>>>>>
>>>>>
>>>>> Right. You are no longer changing the test name. You are still
>>>>> changing the directory name. The problem I mentioned stays the
>>>>> same. Any fixes to the existing tests in this directory can no
>>>>> longer auto applied to stables releases.
>>>>
>>>> I understand your point, but commit baa489fabd01 (selftests/vm: rename
>>>> selftests/vm to selftests/mm) is also present. That was a lot 
>>>> bigger change;
>>>> sigaltstack contains just one test currently, whose fixes possibly 
>>>> would have
>>>> to be backported, so I guess it should not be that much of a big 
>>>> problem?
>>>>
>>>>>
>>>
>>> So who does the backports whenevenr something changes? You are adding
>>> work where as the automated process would just work without this
>>> change. It doesn't matter if there is another test that changed
>>> the name.
>>>
>>>>> Other than the desire to rename the directory to generic, what
>>>>> other value does this change bring?
>>>>
>>>> Do you have an alternative suggestion as to where I should put my 
>>>> new test then;
>>>> I do not see what is the value of creating another directory to 
>>>> just include
>>>> my test. This will unnecessarily clutter the selftests/ directory with
>>>> directories containing single tests. And, putting this in 
>>>> "sigaltstack" is just
>>>> wrong since this test has no relation with sigaltstack.
>>>>
>>>
>>> If this new test has no relation to sigaltstack, then why are you 
>>> changing
>>> and renaming the sigaltstack directory?
>>
>> Because the functionality I am testing is of signals, and signals are 
>> a superset
>> of sigaltstack. Still, I can think of a compromise, if semantically 
>> you want to
>> consider the new test as not testing signals, but a specific syscall 
>> "sigaction"
>> and its interaction with blocking of signals, how about naming the 
>> new directory "sigaction"?
>>> Adding a new directory is much better
>>> than going down a path that is more confusing and adding backport 
>>> overhead.
>>>
>
> Okay - they are related except that you view signalstack as a subset
> of signals. I saw Mark's response as well saying sigaction isn't
> a good name for this.
>
> Rename usually wipe out git history as well based on what have seen
> in the past.
>
> My main concern is backports. Considering sigstack hasn't changed
> 2021 (as Mark's email), let's rename it.
>
> I am reluctantly agreeing to the rename as it seems to make sense
> in this case.

Thanks! I guess there is no update required from my side, and you can
pull this series?
>
> thanks,
> -- Shuah
>
Shuah Khan Sept. 9, 2024, 5:54 p.m. UTC | #11
On 9/8/24 23:16, Dev Jain wrote:
> 
> On 9/7/24 01:29, Shuah Khan wrote:
>> On 9/4/24 23:56, Dev Jain wrote:
>>>
>>> On 9/4/24 22:35, Shuah Khan wrote:
>>>> On 9/3/24 22:52, Dev Jain wrote:
>>>>>
>>>>> On 9/4/24 03:14, Shuah Khan wrote:
>>>>>> On 8/30/24 10:29, Dev Jain wrote:
>>>>>>>
>>>>>>> On 8/27/24 17:16, Dev Jain wrote:
>>>>>>>>
>>>>>>>> On 8/27/24 17:14, Shuah Khan wrote:
>>>>>>>>> On 8/22/24 06:14, Dev Jain wrote:
>>>>>>>>>> Rename sigaltstack to generic signal directory, to allow adding more
>>>>>>>>>> signal tests in the future.
>>>>>>>>>
>>>>>>>>> Sorry - I think I mentioned I don't like this test renamed. Why are you sending
>>>>>>>>> this rename still included in the patch series?
>>>>>>>>
>>>>>>>> I am not renaming the test, just the directory. The directory name
>>>>>>>> is changed to signal, and I have retained the name of the test -
>>>>>>>> sas.c.
>>>>>>>
>>>>>>> Gentle ping: I guess there was a misunderstanding; in v5, I was
>>>>>>> also changing the name of the test, to which you objected, and
>>>>>>> I agreed. But, we need to change the name of the directory since
>>>>>>> the new test has no relation to the current directory name,
>>>>>>> "sigaltstack". The patch description explains that the directory
>>>>>>> should be generically named.
>>>>>>>
>>>>>>
>>>>>> Right. You are no longer changing the test name. You are still
>>>>>> changing the directory name. The problem I mentioned stays the
>>>>>> same. Any fixes to the existing tests in this directory can no
>>>>>> longer auto applied to stables releases.
>>>>>
>>>>> I understand your point, but commit baa489fabd01 (selftests/vm: rename
>>>>> selftests/vm to selftests/mm) is also present. That was a lot bigger change;
>>>>> sigaltstack contains just one test currently, whose fixes possibly would have
>>>>> to be backported, so I guess it should not be that much of a big problem?
>>>>>
>>>>>>
>>>>
>>>> So who does the backports whenevenr something changes? You are adding
>>>> work where as the automated process would just work without this
>>>> change. It doesn't matter if there is another test that changed
>>>> the name.
>>>>
>>>>>> Other than the desire to rename the directory to generic, what
>>>>>> other value does this change bring?
>>>>>
>>>>> Do you have an alternative suggestion as to where I should put my new test then;
>>>>> I do not see what is the value of creating another directory to just include
>>>>> my test. This will unnecessarily clutter the selftests/ directory with
>>>>> directories containing single tests. And, putting this in "sigaltstack" is just
>>>>> wrong since this test has no relation with sigaltstack.
>>>>>
>>>>
>>>> If this new test has no relation to sigaltstack, then why are you changing
>>>> and renaming the sigaltstack directory?
>>>
>>> Because the functionality I am testing is of signals, and signals are a superset
>>> of sigaltstack. Still, I can think of a compromise, if semantically you want to
>>> consider the new test as not testing signals, but a specific syscall "sigaction"
>>> and its interaction with blocking of signals, how about naming the new directory "sigaction"?
>>>> Adding a new directory is much better
>>>> than going down a path that is more confusing and adding backport overhead.
>>>>
>>
>> Okay - they are related except that you view signalstack as a subset
>> of signals. I saw Mark's response as well saying sigaction isn't
>> a good name for this.
>>
>> Rename usually wipe out git history as well based on what have seen
>> in the past.
>>
>> My main concern is backports. Considering sigstack hasn't changed
>> 2021 (as Mark's email), let's rename it.
>>
>> I am reluctantly agreeing to the rename as it seems to make sense
>> in this case.
> 
> Thanks! I guess there is no update required from my side, and you can
> pull this series?
>>

I can pull this with x86v maintainer ack.

Or to go through x86 tree:

Acked-by: Shuah Khan <skhan@linuxfoundation.org>

thanks,
-- Shuah
Dev Jain Sept. 16, 2024, 3:58 a.m. UTC | #12
On 9/9/24 23:24, Shuah Khan wrote:
> On 9/8/24 23:16, Dev Jain wrote:
>>
>> On 9/7/24 01:29, Shuah Khan wrote:
>>> On 9/4/24 23:56, Dev Jain wrote:
>>>>
>>>> On 9/4/24 22:35, Shuah Khan wrote:
>>>>> On 9/3/24 22:52, Dev Jain wrote:
>>>>>>
>>>>>> On 9/4/24 03:14, Shuah Khan wrote:
>>>>>>> On 8/30/24 10:29, Dev Jain wrote:
>>>>>>>>
>>>>>>>> On 8/27/24 17:16, Dev Jain wrote:
>>>>>>>>>
>>>>>>>>> On 8/27/24 17:14, Shuah Khan wrote:
>>>>>>>>>> On 8/22/24 06:14, Dev Jain wrote:
>>>>>>>>>>> Rename sigaltstack to generic signal directory, to allow 
>>>>>>>>>>> adding more
>>>>>>>>>>> signal tests in the future.
>>>>>>>>>>
>>>>>>>>>> Sorry - I think I mentioned I don't like this test renamed. 
>>>>>>>>>> Why are you sending
>>>>>>>>>> this rename still included in the patch series?
>>>>>>>>>
>>>>>>>>> I am not renaming the test, just the directory. The directory 
>>>>>>>>> name
>>>>>>>>> is changed to signal, and I have retained the name of the test -
>>>>>>>>> sas.c.
>>>>>>>>
>>>>>>>> Gentle ping: I guess there was a misunderstanding; in v5, I was
>>>>>>>> also changing the name of the test, to which you objected, and
>>>>>>>> I agreed. But, we need to change the name of the directory since
>>>>>>>> the new test has no relation to the current directory name,
>>>>>>>> "sigaltstack". The patch description explains that the directory
>>>>>>>> should be generically named.
>>>>>>>>
>>>>>>>
>>>>>>> Right. You are no longer changing the test name. You are still
>>>>>>> changing the directory name. The problem I mentioned stays the
>>>>>>> same. Any fixes to the existing tests in this directory can no
>>>>>>> longer auto applied to stables releases.
>>>>>>
>>>>>> I understand your point, but commit baa489fabd01 (selftests/vm: 
>>>>>> rename
>>>>>> selftests/vm to selftests/mm) is also present. That was a lot 
>>>>>> bigger change;
>>>>>> sigaltstack contains just one test currently, whose fixes 
>>>>>> possibly would have
>>>>>> to be backported, so I guess it should not be that much of a big 
>>>>>> problem?
>>>>>>
>>>>>>>
>>>>>
>>>>> So who does the backports whenevenr something changes? You are adding
>>>>> work where as the automated process would just work without this
>>>>> change. It doesn't matter if there is another test that changed
>>>>> the name.
>>>>>
>>>>>>> Other than the desire to rename the directory to generic, what
>>>>>>> other value does this change bring?
>>>>>>
>>>>>> Do you have an alternative suggestion as to where I should put my 
>>>>>> new test then;
>>>>>> I do not see what is the value of creating another directory to 
>>>>>> just include
>>>>>> my test. This will unnecessarily clutter the selftests/ directory 
>>>>>> with
>>>>>> directories containing single tests. And, putting this in 
>>>>>> "sigaltstack" is just
>>>>>> wrong since this test has no relation with sigaltstack.
>>>>>>
>>>>>
>>>>> If this new test has no relation to sigaltstack, then why are you 
>>>>> changing
>>>>> and renaming the sigaltstack directory?
>>>>
>>>> Because the functionality I am testing is of signals, and signals 
>>>> are a superset
>>>> of sigaltstack. Still, I can think of a compromise, if semantically 
>>>> you want to
>>>> consider the new test as not testing signals, but a specific 
>>>> syscall "sigaction"
>>>> and its interaction with blocking of signals, how about naming the 
>>>> new directory "sigaction"?
>>>>> Adding a new directory is much better
>>>>> than going down a path that is more confusing and adding backport 
>>>>> overhead.
>>>>>
>>>
>>> Okay - they are related except that you view signalstack as a subset
>>> of signals. I saw Mark's response as well saying sigaction isn't
>>> a good name for this.
>>>
>>> Rename usually wipe out git history as well based on what have seen
>>> in the past.
>>>
>>> My main concern is backports. Considering sigstack hasn't changed
>>> 2021 (as Mark's email), let's rename it.
>>>
>>> I am reluctantly agreeing to the rename as it seems to make sense
>>> in this case.
>>
>> Thanks! I guess there is no update required from my side, and you can
>> pull this series?
>>>
>
> I can pull this with x86v maintainer ack.
>
> Or to go through x86 tree:
>
> Acked-by: Shuah Khan <skhan@linuxfoundation.org>
>
>
Gentle ping, adding all x86 maintainers and the x86 list, in case they 
missed.
Dev Jain Oct. 7, 2024, 4:37 a.m. UTC | #13
On 9/16/24 09:28, Dev Jain wrote:
>
> On 9/9/24 23:24, Shuah Khan wrote:
>> On 9/8/24 23:16, Dev Jain wrote:
>>>
>>> On 9/7/24 01:29, Shuah Khan wrote:
>>>> On 9/4/24 23:56, Dev Jain wrote:
>>>>>
>>>>> On 9/4/24 22:35, Shuah Khan wrote:
>>>>>> On 9/3/24 22:52, Dev Jain wrote:
>>>>>>>
>>>>>>> On 9/4/24 03:14, Shuah Khan wrote:
>>>>>>>> On 8/30/24 10:29, Dev Jain wrote:
>>>>>>>>>
>>>>>>>>> On 8/27/24 17:16, Dev Jain wrote:
>>>>>>>>>>
>>>>>>>>>> On 8/27/24 17:14, Shuah Khan wrote:
>>>>>>>>>>> On 8/22/24 06:14, Dev Jain wrote:
>>>>>>>>>>>> Rename sigaltstack to generic signal directory, to allow 
>>>>>>>>>>>> adding more
>>>>>>>>>>>> signal tests in the future.
>>>>>>>>>>>
>>>>>>>>>>> Sorry - I think I mentioned I don't like this test renamed. 
>>>>>>>>>>> Why are you sending
>>>>>>>>>>> this rename still included in the patch series?
>>>>>>>>>>
>>>>>>>>>> I am not renaming the test, just the directory. The directory 
>>>>>>>>>> name
>>>>>>>>>> is changed to signal, and I have retained the name of the test -
>>>>>>>>>> sas.c.
>>>>>>>>>
>>>>>>>>> Gentle ping: I guess there was a misunderstanding; in v5, I was
>>>>>>>>> also changing the name of the test, to which you objected, and
>>>>>>>>> I agreed. But, we need to change the name of the directory since
>>>>>>>>> the new test has no relation to the current directory name,
>>>>>>>>> "sigaltstack". The patch description explains that the directory
>>>>>>>>> should be generically named.
>>>>>>>>>
>>>>>>>>
>>>>>>>> Right. You are no longer changing the test name. You are still
>>>>>>>> changing the directory name. The problem I mentioned stays the
>>>>>>>> same. Any fixes to the existing tests in this directory can no
>>>>>>>> longer auto applied to stables releases.
>>>>>>>
>>>>>>> I understand your point, but commit baa489fabd01 (selftests/vm: 
>>>>>>> rename
>>>>>>> selftests/vm to selftests/mm) is also present. That was a lot 
>>>>>>> bigger change;
>>>>>>> sigaltstack contains just one test currently, whose fixes 
>>>>>>> possibly would have
>>>>>>> to be backported, so I guess it should not be that much of a big 
>>>>>>> problem?
>>>>>>>
>>>>>>>>
>>>>>>
>>>>>> So who does the backports whenevenr something changes? You are 
>>>>>> adding
>>>>>> work where as the automated process would just work without this
>>>>>> change. It doesn't matter if there is another test that changed
>>>>>> the name.
>>>>>>
>>>>>>>> Other than the desire to rename the directory to generic, what
>>>>>>>> other value does this change bring?
>>>>>>>
>>>>>>> Do you have an alternative suggestion as to where I should put 
>>>>>>> my new test then;
>>>>>>> I do not see what is the value of creating another directory to 
>>>>>>> just include
>>>>>>> my test. This will unnecessarily clutter the selftests/ 
>>>>>>> directory with
>>>>>>> directories containing single tests. And, putting this in 
>>>>>>> "sigaltstack" is just
>>>>>>> wrong since this test has no relation with sigaltstack.
>>>>>>>
>>>>>>
>>>>>> If this new test has no relation to sigaltstack, then why are you 
>>>>>> changing
>>>>>> and renaming the sigaltstack directory?
>>>>>
>>>>> Because the functionality I am testing is of signals, and signals 
>>>>> are a superset
>>>>> of sigaltstack. Still, I can think of a compromise, if 
>>>>> semantically you want to
>>>>> consider the new test as not testing signals, but a specific 
>>>>> syscall "sigaction"
>>>>> and its interaction with blocking of signals, how about naming the 
>>>>> new directory "sigaction"?
>>>>>> Adding a new directory is much better
>>>>>> than going down a path that is more confusing and adding backport 
>>>>>> overhead.
>>>>>>
>>>>
>>>> Okay - they are related except that you view signalstack as a subset
>>>> of signals. I saw Mark's response as well saying sigaction isn't
>>>> a good name for this.
>>>>
>>>> Rename usually wipe out git history as well based on what have seen
>>>> in the past.
>>>>
>>>> My main concern is backports. Considering sigstack hasn't changed
>>>> 2021 (as Mark's email), let's rename it.
>>>>
>>>> I am reluctantly agreeing to the rename as it seems to make sense
>>>> in this case.
>>>
>>> Thanks! I guess there is no update required from my side, and you can
>>> pull this series?
>>>>
>>
>> I can pull this with x86v maintainer ack.
>>
>> Or to go through x86 tree:
>>
>> Acked-by: Shuah Khan <skhan@linuxfoundation.org>
>>
>>
> Gentle ping, adding all x86 maintainers and the x86 list, in case they 
> missed.

Gentle ping
Mark Brown Oct. 7, 2024, 2:45 p.m. UTC | #14
On Mon, Oct 07, 2024 at 10:07:24AM +0530, Dev Jain wrote:
> On 9/16/24 09:28, Dev Jain wrote:

> > Gentle ping, adding all x86 maintainers and the x86 list, in case they
> > missed.

> Gentle ping

Given that this was posted prior to the merge window you should probably
resend it at this point.
diff mbox series

Patch

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index bc8fe9e8f7f2..edbe30fb3304 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -87,7 +87,7 @@  TARGETS += rtc
 TARGETS += rust
 TARGETS += seccomp
 TARGETS += sgx
-TARGETS += sigaltstack
+TARGETS += signal
 TARGETS += size
 TARGETS += sparc64
 TARGETS += splice
diff --git a/tools/testing/selftests/sigaltstack/.gitignore b/tools/testing/selftests/signal/.gitignore
similarity index 100%
rename from tools/testing/selftests/sigaltstack/.gitignore
rename to tools/testing/selftests/signal/.gitignore
diff --git a/tools/testing/selftests/sigaltstack/Makefile b/tools/testing/selftests/signal/Makefile
similarity index 100%
rename from tools/testing/selftests/sigaltstack/Makefile
rename to tools/testing/selftests/signal/Makefile
diff --git a/tools/testing/selftests/sigaltstack/current_stack_pointer.h b/tools/testing/selftests/signal/current_stack_pointer.h
similarity index 100%
rename from tools/testing/selftests/sigaltstack/current_stack_pointer.h
rename to tools/testing/selftests/signal/current_stack_pointer.h
diff --git a/tools/testing/selftests/sigaltstack/sas.c b/tools/testing/selftests/signal/sas.c
similarity index 100%
rename from tools/testing/selftests/sigaltstack/sas.c
rename to tools/testing/selftests/signal/sas.c