Skip to content

Comments

[PATCH CLOUD-DEV v4] Modular Framework: schedulers#166

Closed
brbrooks wants to merge 3 commits intoOpenDataPlane:cloud-devfrom
brbrooks:cloud-dev
Closed

[PATCH CLOUD-DEV v4] Modular Framework: schedulers#166
brbrooks wants to merge 3 commits intoOpenDataPlane:cloud-devfrom
brbrooks:cloud-dev

Conversation

@brbrooks
Copy link
Contributor

@brbrooks brbrooks commented Sep 6, 2017

No description provided.

@muvarov muvarov changed the title Modular Framework: schedulers [PATCH CLOUD-DEV v1] Modular Framework: schedulers Sep 6, 2017
@brbrooks
Copy link
Contributor Author

brbrooks commented Sep 7, 2017

I don't think these 'helper' test case failures are related to this patch:

FAIL: chksum                                                                                
============                                                                                
                                                                                            
Mounting hugetlbfs                                                                          
Trying 1G pages                                                                             
Total number: 1                                                                             
Free pages: 1                                                                               
running ./chksum!                                                                           
EAL: Detected 4 lcore(s)                                                                    
EAL: No free hugepages reported in hugepages-2048kB                                         
EAL: Probing VFIO support...                                                                
EAL: Cannot obtain physical addresses: No such file or directory. Only vfio will function.  
EAL: WARNING: Master core has no memory on local socket!                                    
EAL: PCI device 0000:02:01.0 on NUMA socket -1                                              
EAL:   probe driver: 8086:100f net_e1000_em                                                 
EAL: failed to initialize crypto_openssl device                                             
EAL: Bus (virtual) probe failed.                                                            
EAL: FATAL: Cannot probe devices                                                            
                                                                                            
EAL: Cannot probe devices                                                                   
                                                                                            
odp_init.c:143:odp_init_dpdk():Cannot init the Intel DPDK EAL!                              
odp_init.c:311:odp_init_global():ODP dpdk init failed.                                      
chksum.c:42:main():Error: ODP global init failed.                                           
Unmounting hugetlbfs                                                                        
FAIL chksum (exit status: 1)

@muvarov muvarov changed the title [PATCH CLOUD-DEV v1] Modular Framework: schedulers [PATCH CLOUD-DEV v2] Modular Framework: schedulers Sep 7, 2017
@brbrooks
Copy link
Contributor Author

brbrooks commented Sep 7, 2017

2 of 3 checkpatch warnings are false positives. I corrected the 1 of 3 in next patch series.

Copy link
Contributor

@Bill-Fischofer-Linaro Bill-Fischofer-Linaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checkpatch issues need to be fixed. Travis-reported breakage in the linux-dpdk test is the more serious issue that must be addressed.

odp_api_proto(schedule, schedule_release_atomic) \
schedule_release_atomic;
odp_api_proto(schedule, schedule_release_ordered) \
schedule_release_ordered;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checkpatch legitimately flags these two statements. You don't need continuations here. If the line is too long just continue it without the backslash. Newline characters are just whitespace to C and needn't be escaped, unlike the C preprocessor.

endif

pool/dpdk.lo: AM_CFLAGS += -DIM_ACTIVE_MODULE
if ODP_SCHEDULE_SCALABLE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding is that we are building and registering all scheduler variants and here only set the active one.
Is my understanding OK?
For ODP_SCHEDULE_SP case, we will have both 'generic' and 'sp' built with IM_ACTIVE_MODULE. Is this OK from modular framework point of view?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, only one scheduler is marked as the 'active' scheduler. This is controlled by --enable-schedule-xxx configure flag.

@heyi-linaro is working on build system improvements in this area, so please expect this to evolve.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brbrooks actually when using --enable-schedule-sp you will have both 'generic' and 'sp' marked as active.
I was thinking at:

if ODP_SCHEDULE_SCALABLE
schedule/scalable.lo: AM_CFLAGS += -DIM_ACTIVE_MODULE
else
if ODP_SCHEDULE_SP
schedule/sp.lo: AM_CFLAGS += -DIM_ACTIVE_MODULE
else
if ODP_SCHEDULE_IQUERY
schedule/iquery.lo: AM_CFLAGS += -DIM_ACTIVE_MODULE
else
schedule/generic.lo: AM_CFLAGS += -DIM_ACTIVE_MODULE
endif
endif
endif

Is ugly, but does the job.
Yet, your code may work... this was more a question for @heyi-linaro .

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @bogdanPricope. It would be nice to have elif or else if. ☺️

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, but there is no 'elif' / 'else if' construction available from what I know....

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, in next plan these "ugly" conditional rules should be auto-generated, subsystem and module writers only need to "declare" their subsystem and modules nicely and concisely like:

SUBSYSTEMS += new_subsystem
__new_subsystem_MODULES += one:default
two:SELECT_FLAG_A
three: SELECT_FLAG_B ...

@GBalakrishna
Copy link
Contributor

GBalakrishna commented Sep 8, 2017

@brbrooks - from your log I see EAL: failed to initialize crypto_openssl device. have you built dpdk with OPENSSL PMD enabled ?. That is usually the main reason for the failure. If you have enabled it already, you need to check why the crypto initialization fails during dpdk init. In general it should be initialized without an issue. Read for more info on README from platform/linux-dpdk/ on how to enable crypto.

Signed-off-by: Brian Brooks <brian.brooks@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Reviewed-by: Yi He <yi.he@linaro.org>
@muvarov muvarov changed the title [PATCH CLOUD-DEV v2] Modular Framework: schedulers [PATCH CLOUD-DEV v3] Modular Framework: schedulers Sep 10, 2017
@brbrooks
Copy link
Contributor Author

@GBalakrishna I can't Reply to your comment directly, so I am replying here.

Travis CI builds linux-dpdk the same for all PRs. So, I don't know why these helper tests are failing for linux-dpdk build. It looks unrelated to the code changes in this PR.

@Bill-Fischofer-Linaro
Copy link
Contributor

Travis reports the following failures in the linux-dpdk tests:

5.50s$ sudo LD_LIBRARY_PATH="/usr/local/lib:$LD_LIBRARY_PATH" make check
Making check in platform/linux-dpdk
make[1]: Entering directory `/home/travis/build/Linaro/odp/platform/linux-dpdk'
make[1]: Nothing to be done for `check'.
make[1]: Leaving directory `/home/travis/build/Linaro/odp/platform/linux-dpdk'
Making check in helper
make[1]: Entering directory `/home/travis/build/Linaro/odp/helper'
make[1]: Leaving directory `/home/travis/build/Linaro/odp/helper'
Making check in helper/test
make[1]: Entering directory `/home/travis/build/Linaro/odp/helper/test'
make  check-TESTS
make[2]: Entering directory `/home/travis/build/Linaro/odp/helper/test'
make[3]: Entering directory `/home/travis/build/Linaro/odp/helper/test'
FAIL: chksum
FAIL: cuckootable
PASS: parse
FAIL: table
FAIL: iplookuptable
FAIL: odpthreads_as_processes
FAIL: odpthreads_as_pthreads

@GBalakrishna
Copy link
Contributor

@brbrooks pls try to see if you can reproduce it locally on your machine. Look at the travis file to see how are we building & running linux-dpdk. When I try to run locally with your PR, I see segfault on all the failed tests.

Signed-off-by: Brian Brooks <brian.brooks@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Reviewed-by: Kevin Wang <kevin.wang@arm.com>
Signed-off-by: Brian Brooks <brian.brooks@arm.com>
Reviewed-by: Ola Liljedahl <ola.liljedahl@arm.com>
@brbrooks
Copy link
Contributor Author

Found the problem...

schedule/generic.lo: AM_CFLAGS += -DIM_ACTIVE_MODULE

needs to be:

../linux-generic/schedule/generic.lo: AM_CFLAGS += -DIM_ACTIVE_MODULE

@muvarov muvarov changed the title [PATCH CLOUD-DEV v3] Modular Framework: schedulers [PATCH CLOUD-DEV v4] Modular Framework: schedulers Sep 11, 2017
@brbrooks
Copy link
Contributor Author

@Bill-Fischofer-Linaro ping. Travis CI is good now.

@heyi-arm
Copy link

merged and closed.

@heyi-arm heyi-arm closed this Sep 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants