| Message ID | 1437598135-20291-1-git-send-email-anders.roxell@linaro.org |
|---|---|
| State | New |
| Headers | show |
I think that patch is not correct. Main problem that odp source for linux-generic has to be arch independent. But generated after ./scripts/builddeb opendataplane-1.x.tar.gz does not contain any other arches for timer. It has only linux-generic/arch/x86/. So in that case you can only compile that source on x86 and you can not compile it on lets say mips. That need to be fixed. Best regards, Maxim. On 07/22/15 23:48, Anders Roxell wrote: > Building generated this warning message: > "copying selected object files to avoid basename conflicts..." > > Reported-by: Mile Holmes <mike.holmes@linaro.org> > Signed-off-by: Anders Roxell <anders.roxell@linaro.org> > --- > platform/linux-generic/Makefile.am | 2 +- > .../linux-generic/arch/linux/{odp_time.c => odp_time_optimisation.c} | 0 > .../linux-generic/arch/mips64/{odp_time.c => odp_time_optimisation.c} | 0 > platform/linux-generic/arch/x86/{odp_time.c => odp_time_optimisation.c} | 0 > 4 files changed, 1 insertion(+), 1 deletion(-) > rename platform/linux-generic/arch/linux/{odp_time.c => odp_time_optimisation.c} (100%) > rename platform/linux-generic/arch/mips64/{odp_time.c => odp_time_optimisation.c} (100%) > rename platform/linux-generic/arch/x86/{odp_time.c => odp_time_optimisation.c} (100%) > > diff --git a/platform/linux-generic/Makefile.am b/platform/linux-generic/Makefile.am > index 4ee781c..937f082 100644 > --- a/platform/linux-generic/Makefile.am > +++ b/platform/linux-generic/Makefile.am > @@ -157,4 +157,4 @@ __LIB__libodp_la_SOURCES = \ > odp_timer.c \ > odp_version.c \ > odp_weak.c \ > - arch/@ARCH@/odp_time.c > + arch/@ARCH@/odp_time_optimisation.c > diff --git a/platform/linux-generic/arch/linux/odp_time.c b/platform/linux-generic/arch/linux/odp_time_optimisation.c > similarity index 100% > rename from platform/linux-generic/arch/linux/odp_time.c > rename to platform/linux-generic/arch/linux/odp_time_optimisation.c > diff --git a/platform/linux-generic/arch/mips64/odp_time.c b/platform/linux-generic/arch/mips64/odp_time_optimisation.c > similarity index 100% > rename from platform/linux-generic/arch/mips64/odp_time.c > rename to platform/linux-generic/arch/mips64/odp_time_optimisation.c > diff --git a/platform/linux-generic/arch/x86/odp_time.c b/platform/linux-generic/arch/x86/odp_time_optimisation.c > similarity index 100% > rename from platform/linux-generic/arch/x86/odp_time.c > rename to platform/linux-generic/arch/x86/odp_time_optimisation.c
On 23 July 2015 at 05:34, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > I think that patch is not correct. Main problem that odp source for > linux-generic has to be arch independent. > We decided to allow ARCH optimization where it did not compromise linux-generics goal of being a clear implementation vs a performance one. We then moved the dependencies to this directory so I disagree that it has to be independent. > But generated after ./scripts/builddeb opendataplane-1.x.tar.gz does not > contain any other arches for timer. It has only linux-generic/arch/x86/. So in that case you can only compile > that source on x86 and you can not > compile it on lets say mips. We could add a flag that ensures that generic code is always built and thus it is platform independent, that is what arch/linux is supposed to be, no ARCH touch ups at all. > That need to be fixed. > We we can fix that with a compile flag to force the use of arch/linux or completely revisit any CPU core optimisations in linux-generic, but I think that is orthogonal to this fix which allows the current build structure to work without a warning whilst that discussion happens. Maybe it should be a bug ? FYI the traffic_mngr code that is on its way also gains significant improvement from similar ARCH dependant touches so if we throw out all optimisations we need a new place to keep store the touch ups for reuse by other platforms. > > Best regards, > Maxim. > > On 07/22/15 23:48, Anders Roxell wrote: > >> Building generated this warning message: >> "copying selected object files to avoid basename conflicts..." >> >> Reported-by: Mile Holmes <mike.holmes@linaro.org> >> Signed-off-by: Anders Roxell <anders.roxell@linaro.org> >> >> --- >> platform/linux-generic/Makefile.am >> | 2 +- >> .../linux-generic/arch/linux/{odp_time.c => odp_time_optimisation.c} >> | 0 >> .../linux-generic/arch/mips64/{odp_time.c => odp_time_optimisation.c} >> | 0 >> platform/linux-generic/arch/x86/{odp_time.c => odp_time_optimisation.c} >> | 0 >> 4 files changed, 1 insertion(+), 1 deletion(-) >> rename platform/linux-generic/arch/linux/{odp_time.c => >> odp_time_optimisation.c} (100%) >> rename platform/linux-generic/arch/mips64/{odp_time.c => >> odp_time_optimisation.c} (100%) >> rename platform/linux-generic/arch/x86/{odp_time.c => >> odp_time_optimisation.c} (100%) >> >> diff --git a/platform/linux-generic/Makefile.am >> b/platform/linux-generic/Makefile.am >> index 4ee781c..937f082 100644 >> --- a/platform/linux-generic/Makefile.am >> +++ b/platform/linux-generic/Makefile.am >> @@ -157,4 +157,4 @@ __LIB__libodp_la_SOURCES = \ >> odp_timer.c \ >> odp_version.c \ >> odp_weak.c \ >> - arch/@ARCH@/odp_time.c >> + arch/@ARCH@/odp_time_optimisation.c >> diff --git a/platform/linux-generic/arch/linux/odp_time.c >> b/platform/linux-generic/arch/linux/odp_time_optimisation.c >> similarity index 100% >> rename from platform/linux-generic/arch/linux/odp_time.c >> rename to platform/linux-generic/arch/linux/odp_time_optimisation.c >> diff --git a/platform/linux-generic/arch/mips64/odp_time.c >> b/platform/linux-generic/arch/mips64/odp_time_optimisation.c >> similarity index 100% >> rename from platform/linux-generic/arch/mips64/odp_time.c >> rename to platform/linux-generic/arch/mips64/odp_time_optimisation.c >> diff --git a/platform/linux-generic/arch/x86/odp_time.c >> b/platform/linux-generic/arch/x86/odp_time_optimisation.c >> similarity index 100% >> rename from platform/linux-generic/arch/x86/odp_time.c >> rename to platform/linux-generic/arch/x86/odp_time_optimisation.c >> > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp >
On 07/23/15 23:30, Mike Holmes wrote: > > > On 23 July 2015 at 05:34, Maxim Uvarov <maxim.uvarov@linaro.org > <mailto:maxim.uvarov@linaro.org>> wrote: > > I think that patch is not correct. Main problem that odp source > for linux-generic has to be arch independent. > > > We decided to allow ARCH optimization where it did not compromise > linux-generics goal of being a clear implementation vs a performance > one. We then moved the dependencies to this directory so I disagree > that it has to be independent. > > But generated after ./scripts/builddeb opendataplane-1.x.tar.gz > does not contain any other arches for timer. > > It has only linux-generic/arch/x86/. So in that case you can only > compile that source on x86 and you can not > compile it on lets say mips. > > > We could add a flag that ensures that generic code is always built and > thus it is platform independent, that is what arch/linux is supposed > to be, no ARCH touch ups at all. > > That need to be fixed. > > > We we can fix that with a compile flag to force the use of arch/linux > or completely revisit any CPU core optimisations in linux-generic, but > I think that is orthogonal to this fix which allows the current build > structure to work without a warning whilst that discussion happens. > Maybe it should be a bug ? > > FYI the traffic_mngr code that is on its way also gains significant > improvement from similar ARCH dependant touches so if we throw out all > optimisations we need a new place to keep store the touch ups for > reuse by other platforms. > Optimizations are fine. But all arch optimization source code (c and h files) have to be packaged to release tag.gz. So that you can take that tag.gz and compile on any arch. That is my point. Maxim. > > Best regards, > Maxim. > > On 07/22/15 23:48, Anders Roxell wrote: > > Building generated this warning message: > "copying selected object files to avoid basename conflicts..." > > Reported-by: Mile Holmes <mike.holmes@linaro.org > <mailto:mike.holmes@linaro.org>> > Signed-off-by: Anders Roxell <anders.roxell@linaro.org > <mailto:anders.roxell@linaro.org>> > > --- > platform/linux-generic/Makefile.am | 2 +- > .../linux-generic/arch/linux/{odp_time.c => > odp_time_optimisation.c} | 0 > .../linux-generic/arch/mips64/{odp_time.c => > odp_time_optimisation.c} | 0 > platform/linux-generic/arch/x86/{odp_time.c => > odp_time_optimisation.c} | 0 > 4 files changed, 1 insertion(+), 1 deletion(-) > rename platform/linux-generic/arch/linux/{odp_time.c => > odp_time_optimisation.c} (100%) > rename platform/linux-generic/arch/mips64/{odp_time.c => > odp_time_optimisation.c} (100%) > rename platform/linux-generic/arch/x86/{odp_time.c => > odp_time_optimisation.c} (100%) > > diff --git a/platform/linux-generic/Makefile.am > b/platform/linux-generic/Makefile.am > index 4ee781c..937f082 100644 > --- a/platform/linux-generic/Makefile.am > +++ b/platform/linux-generic/Makefile.am > @@ -157,4 +157,4 @@ __LIB__libodp_la_SOURCES = \ > odp_timer.c \ > odp_version.c \ > odp_weak.c \ > - arch/@ARCH@/odp_time.c > + arch/@ARCH@/odp_time_optimisation.c > diff --git a/platform/linux-generic/arch/linux/odp_time.c > b/platform/linux-generic/arch/linux/odp_time_optimisation.c > similarity index 100% > rename from platform/linux-generic/arch/linux/odp_time.c > rename to > platform/linux-generic/arch/linux/odp_time_optimisation.c > diff --git a/platform/linux-generic/arch/mips64/odp_time.c > b/platform/linux-generic/arch/mips64/odp_time_optimisation.c > similarity index 100% > rename from platform/linux-generic/arch/mips64/odp_time.c > rename to > platform/linux-generic/arch/mips64/odp_time_optimisation.c > diff --git a/platform/linux-generic/arch/x86/odp_time.c > b/platform/linux-generic/arch/x86/odp_time_optimisation.c > similarity index 100% > rename from platform/linux-generic/arch/x86/odp_time.c > rename to platform/linux-generic/arch/x86/odp_time_optimisation.c > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> > https://lists.linaro.org/mailman/listinfo/lng-odp > > > > > -- > Mike Holmes > Technical Manager - Linaro Networking Group > Linaro.org <http://www.linaro.org/>***│ *Open source software for ARM SoCs >
On 2015-07-23 12:34, Maxim Uvarov wrote: > I think that patch is not correct. Disagree this patch does fix the warning. > Main problem that odp source for > linux-generic has to be arch independent. I think we agreed that we wanted linux-generic to be optimised for timers, thats why it looks this way. > But generated after ./scripts/builddeb opendataplane-1.x.tar.gz does not > contain any other arches for timer. > It has only linux-generic/arch/x86/. So in that case you can only compile > that source on x86 and you can not > compile it on lets say mips. That need to be fixed. I agree, that should to be fixed too. However, that is a separate issue. This patch is needed even if we fix the missing source files for mips64 and linux. Cheers, Anders > > Best regards, > Maxim. > > On 07/22/15 23:48, Anders Roxell wrote: > >Building generated this warning message: > >"copying selected object files to avoid basename conflicts..." > > > >Reported-by: Mile Holmes <mike.holmes@linaro.org> > >Signed-off-by: Anders Roxell <anders.roxell@linaro.org> > >--- > > platform/linux-generic/Makefile.am | 2 +- > > .../linux-generic/arch/linux/{odp_time.c => odp_time_optimisation.c} | 0 > > .../linux-generic/arch/mips64/{odp_time.c => odp_time_optimisation.c} | 0 > > platform/linux-generic/arch/x86/{odp_time.c => odp_time_optimisation.c} | 0 > > 4 files changed, 1 insertion(+), 1 deletion(-) > > rename platform/linux-generic/arch/linux/{odp_time.c => odp_time_optimisation.c} (100%) > > rename platform/linux-generic/arch/mips64/{odp_time.c => odp_time_optimisation.c} (100%) > > rename platform/linux-generic/arch/x86/{odp_time.c => odp_time_optimisation.c} (100%) > > > >diff --git a/platform/linux-generic/Makefile.am b/platform/linux-generic/Makefile.am > >index 4ee781c..937f082 100644 > >--- a/platform/linux-generic/Makefile.am > >+++ b/platform/linux-generic/Makefile.am > >@@ -157,4 +157,4 @@ __LIB__libodp_la_SOURCES = \ > > odp_timer.c \ > > odp_version.c \ > > odp_weak.c \ > >- arch/@ARCH@/odp_time.c > >+ arch/@ARCH@/odp_time_optimisation.c > >diff --git a/platform/linux-generic/arch/linux/odp_time.c b/platform/linux-generic/arch/linux/odp_time_optimisation.c > >similarity index 100% > >rename from platform/linux-generic/arch/linux/odp_time.c > >rename to platform/linux-generic/arch/linux/odp_time_optimisation.c > >diff --git a/platform/linux-generic/arch/mips64/odp_time.c b/platform/linux-generic/arch/mips64/odp_time_optimisation.c > >similarity index 100% > >rename from platform/linux-generic/arch/mips64/odp_time.c > >rename to platform/linux-generic/arch/mips64/odp_time_optimisation.c > >diff --git a/platform/linux-generic/arch/x86/odp_time.c b/platform/linux-generic/arch/x86/odp_time_optimisation.c > >similarity index 100% > >rename from platform/linux-generic/arch/x86/odp_time.c > >rename to platform/linux-generic/arch/x86/odp_time_optimisation.c > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp
Hello Anders, I still think that 2 issues are linked together. If not can you please send patch to fix both of them? Also sorry, but I'm not convinced about _optimisation prefix. Fox example in: arch/x86/odp_time_optimisation.c It's clear that everything under arch/x86/ is x86 optimizations. To have _optimisation prefix for all files in that directory looks very strange. (yes, also I don't like long file names.) So I would left original warning and drop your patch, then do such renaming. Maxim. On 08/03/15 23:23, Anders Roxell wrote: > On 2015-07-23 12:34, Maxim Uvarov wrote: >> I think that patch is not correct. > Disagree this patch does fix the warning. > >> Main problem that odp source for >> linux-generic has to be arch independent. > I think we agreed that we wanted linux-generic to be optimised for > timers, thats why it looks this way. > >> But generated after ./scripts/builddeb opendataplane-1.x.tar.gz does not >> contain any other arches for timer. >> It has only linux-generic/arch/x86/. So in that case you can only compile >> that source on x86 and you can not >> compile it on lets say mips. That need to be fixed. > I agree, that should to be fixed too. However, that is a separate issue. > This patch is needed even if we fix the missing source files for > mips64 and linux. > > Cheers, > Anders > >> Best regards, >> Maxim. >> >> On 07/22/15 23:48, Anders Roxell wrote: >>> Building generated this warning message: >>> "copying selected object files to avoid basename conflicts..." >>> >>> Reported-by: Mile Holmes <mike.holmes@linaro.org> >>> Signed-off-by: Anders Roxell <anders.roxell@linaro.org> >>> --- >>> platform/linux-generic/Makefile.am | 2 +- >>> .../linux-generic/arch/linux/{odp_time.c => odp_time_optimisation.c} | 0 >>> .../linux-generic/arch/mips64/{odp_time.c => odp_time_optimisation.c} | 0 >>> platform/linux-generic/arch/x86/{odp_time.c => odp_time_optimisation.c} | 0 >>> 4 files changed, 1 insertion(+), 1 deletion(-) >>> rename platform/linux-generic/arch/linux/{odp_time.c => odp_time_optimisation.c} (100%) >>> rename platform/linux-generic/arch/mips64/{odp_time.c => odp_time_optimisation.c} (100%) >>> rename platform/linux-generic/arch/x86/{odp_time.c => odp_time_optimisation.c} (100%) >>> >>> diff --git a/platform/linux-generic/Makefile.am b/platform/linux-generic/Makefile.am >>> index 4ee781c..937f082 100644 >>> --- a/platform/linux-generic/Makefile.am >>> +++ b/platform/linux-generic/Makefile.am >>> @@ -157,4 +157,4 @@ __LIB__libodp_la_SOURCES = \ >>> odp_timer.c \ >>> odp_version.c \ >>> odp_weak.c \ >>> - arch/@ARCH@/odp_time.c >>> + arch/@ARCH@/odp_time_optimisation.c >>> diff --git a/platform/linux-generic/arch/linux/odp_time.c b/platform/linux-generic/arch/linux/odp_time_optimisation.c >>> similarity index 100% >>> rename from platform/linux-generic/arch/linux/odp_time.c >>> rename to platform/linux-generic/arch/linux/odp_time_optimisation.c >>> diff --git a/platform/linux-generic/arch/mips64/odp_time.c b/platform/linux-generic/arch/mips64/odp_time_optimisation.c >>> similarity index 100% >>> rename from platform/linux-generic/arch/mips64/odp_time.c >>> rename to platform/linux-generic/arch/mips64/odp_time_optimisation.c >>> diff --git a/platform/linux-generic/arch/x86/odp_time.c b/platform/linux-generic/arch/x86/odp_time_optimisation.c >>> similarity index 100% >>> rename from platform/linux-generic/arch/x86/odp_time.c >>> rename to platform/linux-generic/arch/x86/odp_time_optimisation.c >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> https://lists.linaro.org/mailman/listinfo/lng-odp
diff --git a/platform/linux-generic/Makefile.am b/platform/linux-generic/Makefile.am index 4ee781c..937f082 100644 --- a/platform/linux-generic/Makefile.am +++ b/platform/linux-generic/Makefile.am @@ -157,4 +157,4 @@ __LIB__libodp_la_SOURCES = \ odp_timer.c \ odp_version.c \ odp_weak.c \ - arch/@ARCH@/odp_time.c + arch/@ARCH@/odp_time_optimisation.c
Building generated this warning message: "copying selected object files to avoid basename conflicts..." Reported-by: Mile Holmes <mike.holmes@linaro.org> Signed-off-by: Anders Roxell <anders.roxell@linaro.org> --- platform/linux-generic/Makefile.am | 2 +- .../linux-generic/arch/linux/{odp_time.c => odp_time_optimisation.c} | 0 .../linux-generic/arch/mips64/{odp_time.c => odp_time_optimisation.c} | 0 platform/linux-generic/arch/x86/{odp_time.c => odp_time_optimisation.c} | 0 4 files changed, 1 insertion(+), 1 deletion(-) rename platform/linux-generic/arch/linux/{odp_time.c => odp_time_optimisation.c} (100%) rename platform/linux-generic/arch/mips64/{odp_time.c => odp_time_optimisation.c} (100%) rename platform/linux-generic/arch/x86/{odp_time.c => odp_time_optimisation.c} (100%) diff --git a/platform/linux-generic/arch/linux/odp_time.c b/platform/linux-generic/arch/linux/odp_time_optimisation.c similarity index 100% rename from platform/linux-generic/arch/linux/odp_time.c rename to platform/linux-generic/arch/linux/odp_time_optimisation.c diff --git a/platform/linux-generic/arch/mips64/odp_time.c b/platform/linux-generic/arch/mips64/odp_time_optimisation.c similarity index 100% rename from platform/linux-generic/arch/mips64/odp_time.c rename to platform/linux-generic/arch/mips64/odp_time_optimisation.c diff --git a/platform/linux-generic/arch/x86/odp_time.c b/platform/linux-generic/arch/x86/odp_time_optimisation.c similarity index 100% rename from platform/linux-generic/arch/x86/odp_time.c rename to platform/linux-generic/arch/x86/odp_time_optimisation.c