| Message ID | 20220128185449.64936-1-imbrenda@linux.ibm.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | s390x: smp: avoid hardcoded CPU addresses | expand |
On 1/28/22 19:54, Claudio Imbrenda wrote: > On s390x there are no guarantees about the CPU addresses, except that > they shall be unique. This means that in some environments, it's > possible that there is no match between the CPU address and its > position (index) in the list of available CPUs returned by the system. While I support this patch set I've yet to find an environment where this gave me headaches. > > This series fixes a small bug in the SMP initialization code, adds a > guarantee that the boot CPU will always have index 0, and introduces > some functions to allow tests to use CPU indexes instead of using > hardcoded CPU addresses. This will allow the tests to run successfully > in more environments (e.g. z/VM, LPAR). I'm wondering if we should do it the other way round and make the smp_* functions take a idx instead of a cpu addr. The only instance where this gets a bit ugly is the sigp calls which we would also need to convert. > Some existing tests are adapted to take advantage of the new > functionalities. > > Claudio Imbrenda (5): > lib: s390x: smp: add functions to work with CPU indexes > lib: s390x: smp: guarantee that boot CPU has index 0 > s390x: smp: avoid hardcoded CPU addresses > s390x: firq: avoid hardcoded CPU addresses > s390x: skrf: avoid hardcoded CPU addresses > > lib/s390x/smp.h | 2 ++ > lib/s390x/smp.c | 28 ++++++++++++----- > s390x/firq.c | 17 +++++----- > s390x/skrf.c | 8 +++-- > s390x/smp.c | 83 ++++++++++++++++++++++++++----------------------- > 5 files changed, 79 insertions(+), 59 deletions(-) >
On Thu, 3 Feb 2022 09:45:56 +0100 Janosch Frank <frankja@linux.ibm.com> wrote: > On 1/28/22 19:54, Claudio Imbrenda wrote: > > On s390x there are no guarantees about the CPU addresses, except that > > they shall be unique. This means that in some environments, it's > > possible that there is no match between the CPU address and its > > position (index) in the list of available CPUs returned by the system. > > While I support this patch set I've yet to find an environment where > this gave me headaches. > > > > > This series fixes a small bug in the SMP initialization code, adds a > > guarantee that the boot CPU will always have index 0, and introduces > > some functions to allow tests to use CPU indexes instead of using > > hardcoded CPU addresses. This will allow the tests to run successfully > > in more environments (e.g. z/VM, LPAR). > > I'm wondering if we should do it the other way round and make the smp_* > functions take a idx instead of a cpu addr. The only instance where this > gets a bit ugly is the sigp calls which we would also need to convert. yes, in fact this is something I was already planning to do :) for sigp, we can either convert, or add a wrapper with idx. > > > Some existing tests are adapted to take advantage of the new > > functionalities. > > > > Claudio Imbrenda (5): > > lib: s390x: smp: add functions to work with CPU indexes > > lib: s390x: smp: guarantee that boot CPU has index 0 > > s390x: smp: avoid hardcoded CPU addresses > > s390x: firq: avoid hardcoded CPU addresses > > s390x: skrf: avoid hardcoded CPU addresses > > > > lib/s390x/smp.h | 2 ++ > > lib/s390x/smp.c | 28 ++++++++++++----- > > s390x/firq.c | 17 +++++----- > > s390x/skrf.c | 8 +++-- > > s390x/smp.c | 83 ++++++++++++++++++++++++++----------------------- > > 5 files changed, 79 insertions(+), 59 deletions(-) > > >
On 2/3/22 14:37, Claudio Imbrenda wrote: > On Thu, 3 Feb 2022 09:45:56 +0100 > Janosch Frank <frankja@linux.ibm.com> wrote: > >> On 1/28/22 19:54, Claudio Imbrenda wrote: >>> On s390x there are no guarantees about the CPU addresses, except that >>> they shall be unique. This means that in some environments, it's >>> possible that there is no match between the CPU address and its >>> position (index) in the list of available CPUs returned by the system. >> >> While I support this patch set I've yet to find an environment where >> this gave me headaches. >> >>> >>> This series fixes a small bug in the SMP initialization code, adds a >>> guarantee that the boot CPU will always have index 0, and introduces >>> some functions to allow tests to use CPU indexes instead of using >>> hardcoded CPU addresses. This will allow the tests to run successfully >>> in more environments (e.g. z/VM, LPAR). >> >> I'm wondering if we should do it the other way round and make the smp_* >> functions take a idx instead of a cpu addr. The only instance where this >> gets a bit ugly is the sigp calls which we would also need to convert. > > yes, in fact this is something I was already planning to do :) Do you want to add that in a v2 to reduce the additional churn? > > for sigp, we can either convert, or add a wrapper with idx. How about adding a wrapper to smp.c? smp_cpu_sigp() smp_cpu_sigp_retry() That would fall in line with the naming of the smp functions and it's clear that we refer to a specific cpu from the smp lib. We can then leave the sigp.h functions as is so Nico can use them for the invalid addr tests. > >> >>> Some existing tests are adapted to take advantage of the new >>> functionalities. >>> >>> Claudio Imbrenda (5): >>> lib: s390x: smp: add functions to work with CPU indexes >>> lib: s390x: smp: guarantee that boot CPU has index 0 >>> s390x: smp: avoid hardcoded CPU addresses >>> s390x: firq: avoid hardcoded CPU addresses >>> s390x: skrf: avoid hardcoded CPU addresses >>> >>> lib/s390x/smp.h | 2 ++ >>> lib/s390x/smp.c | 28 ++++++++++++----- >>> s390x/firq.c | 17 +++++----- >>> s390x/skrf.c | 8 +++-- >>> s390x/smp.c | 83 ++++++++++++++++++++++++++----------------------- >>> 5 files changed, 79 insertions(+), 59 deletions(-) >>> >> >
On Thu, 3 Feb 2022 16:23:23 +0100 Janosch Frank <frankja@linux.ibm.com> wrote: > On 2/3/22 14:37, Claudio Imbrenda wrote: > > On Thu, 3 Feb 2022 09:45:56 +0100 > > Janosch Frank <frankja@linux.ibm.com> wrote: > > > >> On 1/28/22 19:54, Claudio Imbrenda wrote: > >>> On s390x there are no guarantees about the CPU addresses, except that > >>> they shall be unique. This means that in some environments, it's > >>> possible that there is no match between the CPU address and its > >>> position (index) in the list of available CPUs returned by the system. > >> > >> While I support this patch set I've yet to find an environment where > >> this gave me headaches. > >> > >>> > >>> This series fixes a small bug in the SMP initialization code, adds a > >>> guarantee that the boot CPU will always have index 0, and introduces > >>> some functions to allow tests to use CPU indexes instead of using > >>> hardcoded CPU addresses. This will allow the tests to run successfully > >>> in more environments (e.g. z/VM, LPAR). > >> > >> I'm wondering if we should do it the other way round and make the smp_* > >> functions take a idx instead of a cpu addr. The only instance where this > >> gets a bit ugly is the sigp calls which we would also need to convert. > > > > yes, in fact this is something I was already planning to do :) > > Do you want to add that in a v2 to reduce the additional churn? ok > > > > > for sigp, we can either convert, or add a wrapper with idx. > > How about adding a wrapper to smp.c? > > smp_cpu_sigp() > smp_cpu_sigp_retry() I was actually thinking about sigp_idx, but I like your names better > > > That would fall in line with the naming of the smp functions and it's > clear that we refer to a specific cpu from the smp lib. > > We can then leave the sigp.h functions as is so Nico can use them for > the invalid addr tests. yes, a "raw" sigp function should definitely stay there > > > > >> > >>> Some existing tests are adapted to take advantage of the new > >>> functionalities. > >>> > >>> Claudio Imbrenda (5): > >>> lib: s390x: smp: add functions to work with CPU indexes > >>> lib: s390x: smp: guarantee that boot CPU has index 0 > >>> s390x: smp: avoid hardcoded CPU addresses > >>> s390x: firq: avoid hardcoded CPU addresses > >>> s390x: skrf: avoid hardcoded CPU addresses > >>> > >>> lib/s390x/smp.h | 2 ++ > >>> lib/s390x/smp.c | 28 ++++++++++++----- > >>> s390x/firq.c | 17 +++++----- > >>> s390x/skrf.c | 8 +++-- > >>> s390x/smp.c | 83 ++++++++++++++++++++++++++----------------------- > >>> 5 files changed, 79 insertions(+), 59 deletions(-) > >>> > >> > > >
On 1/28/22 19:54, Claudio Imbrenda wrote: > On s390x there are no guarantees about the CPU addresses, except that > they shall be unique. This means that in some environments, it's > possible that there is no match between the CPU address and its > position (index) in the list of available CPUs returned by the system. > > This series fixes a small bug in the SMP initialization code, adds a > guarantee that the boot CPU will always have index 0, and introduces > some functions to allow tests to use CPU indexes instead of using > hardcoded CPU addresses. This will allow the tests to run successfully > in more environments (e.g. z/VM, LPAR). > > Some existing tests are adapted to take advantage of the new > functionalities. > > Claudio Imbrenda (5): > lib: s390x: smp: add functions to work with CPU indexes > lib: s390x: smp: guarantee that boot CPU has index 0 > s390x: smp: avoid hardcoded CPU addresses > s390x: firq: avoid hardcoded CPU addresses > s390x: skrf: avoid hardcoded CPU addresses > > lib/s390x/smp.h | 2 ++ > lib/s390x/smp.c | 28 ++++++++++++----- > s390x/firq.c | 17 +++++----- > s390x/skrf.c | 8 +++-- > s390x/smp.c | 83 ++++++++++++++++++++++++++----------------------- We use smp/sigp in uv-host.c and one of those uses looks a bit strange to me anyway. I think we also need to fix the sigp in cstart.S to only stop itself and not the cpu with the addr 0. Up to now we very much assumed that cpu 0 is always our boot cpu so if you start running the test with cpu addr 1 and 2 and leave out 0 you might find more problematic code. > 5 files changed, 79 insertions(+), 59 deletions(-) >
On Fri, 4 Feb 2022 16:01:54 +0100 Janosch Frank <frankja@linux.ibm.com> wrote: > On 1/28/22 19:54, Claudio Imbrenda wrote: > > On s390x there are no guarantees about the CPU addresses, except that > > they shall be unique. This means that in some environments, it's > > possible that there is no match between the CPU address and its > > position (index) in the list of available CPUs returned by the system. > > > > This series fixes a small bug in the SMP initialization code, adds a > > guarantee that the boot CPU will always have index 0, and introduces > > some functions to allow tests to use CPU indexes instead of using > > hardcoded CPU addresses. This will allow the tests to run successfully > > in more environments (e.g. z/VM, LPAR). > > > > Some existing tests are adapted to take advantage of the new > > functionalities. > > > > Claudio Imbrenda (5): > > lib: s390x: smp: add functions to work with CPU indexes > > lib: s390x: smp: guarantee that boot CPU has index 0 > > s390x: smp: avoid hardcoded CPU addresses > > s390x: firq: avoid hardcoded CPU addresses > > s390x: skrf: avoid hardcoded CPU addresses > > > > lib/s390x/smp.h | 2 ++ > > lib/s390x/smp.c | 28 ++++++++++++----- > > s390x/firq.c | 17 +++++----- > > s390x/skrf.c | 8 +++-- > > s390x/smp.c | 83 ++++++++++++++++++++++++++----------------------- > > We use smp/sigp in uv-host.c and one of those uses looks a bit strange > to me anyway. I had noticed that, it's fixed in the v2 (and that test will almost surely be rewritten anyway) > > I think we also need to fix the sigp in cstart.S to only stop itself and > not the cpu with the addr 0. not sure if that is needed right now. that is only used for snippets, right? or did you mean cstart64.S? but there, sigp is only used for SET_ARCHITECTURE, so it doesn't really matter I guess? > > Up to now we very much assumed that cpu 0 is always our boot cpu so if > you start running the test with cpu addr 1 and 2 and leave out 0 you > might find more problematic code. > > > > 5 files changed, 79 insertions(+), 59 deletions(-) > > >
On 2/4/22 16:14, Claudio Imbrenda wrote: > On Fri, 4 Feb 2022 16:01:54 +0100 > Janosch Frank <frankja@linux.ibm.com> wrote: > >> On 1/28/22 19:54, Claudio Imbrenda wrote: >>> On s390x there are no guarantees about the CPU addresses, except that >>> they shall be unique. This means that in some environments, it's >>> possible that there is no match between the CPU address and its >>> position (index) in the list of available CPUs returned by the system. >>> >>> This series fixes a small bug in the SMP initialization code, adds a >>> guarantee that the boot CPU will always have index 0, and introduces >>> some functions to allow tests to use CPU indexes instead of using >>> hardcoded CPU addresses. This will allow the tests to run successfully >>> in more environments (e.g. z/VM, LPAR). >>> >>> Some existing tests are adapted to take advantage of the new >>> functionalities. >>> >>> Claudio Imbrenda (5): >>> lib: s390x: smp: add functions to work with CPU indexes >>> lib: s390x: smp: guarantee that boot CPU has index 0 >>> s390x: smp: avoid hardcoded CPU addresses >>> s390x: firq: avoid hardcoded CPU addresses >>> s390x: skrf: avoid hardcoded CPU addresses >>> >>> lib/s390x/smp.h | 2 ++ >>> lib/s390x/smp.c | 28 ++++++++++++----- >>> s390x/firq.c | 17 +++++----- >>> s390x/skrf.c | 8 +++-- >>> s390x/smp.c | 83 ++++++++++++++++++++++++++----------------------- >> >> We use smp/sigp in uv-host.c and one of those uses looks a bit strange >> to me anyway. > > I had noticed that, it's fixed in the v2 (and that test will almost > surely be rewritten anyway) > >> >> I think we also need to fix the sigp in cstart.S to only stop itself and >> not the cpu with the addr 0. > > not sure if that is needed right now. that is only used for snippets, > right? Yes > > or did you mean cstart64.S? > but there, sigp is only used for SET_ARCHITECTURE, so it doesn't really > matter I guess? My bad, seems like it's Friday > >> >> Up to now we very much assumed that cpu 0 is always our boot cpu so if >> you start running the test with cpu addr 1 and 2 and leave out 0 you >> might find more problematic code. >> >> >>> 5 files changed, 79 insertions(+), 59 deletions(-) >>> >> >