Skip to content

LOG: Make sure main thread is getting id=0 - #47

Merged
yosefe merged 3 commits into
openucx:masterfrom
yosefe:topic/log-thread-id
Dec 8, 2014
Merged

yosefe merged 3 commits into
openucx:masterfrom
yosefe:topic/log-thread-id

Conversation

@yosefe

@yosefe yosefe commented Dec 8, 2014

Copy link
Copy Markdown
Member

No description provided.

@mellanox-github

Copy link
Copy Markdown
Contributor

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
http://bgate.mellanox.com:8000/jenkins-secure/job/gh-ucx-pr/82/
Test PASSed.

Comment thread src/ucs/debug/log.c

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

line 52 is done not under lock
maybe it is better to split this function into actual init_thread() and get_thread_id() parts?
if get_thread_id function the var "threads_count" can be accessed w/o lock indeed.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

this is a double-checked lock paradigm so no need to take lock every time.
if current thread is not found, it is searched for again with lock held.
it assigns the threads according to the order they are calling this function
calling it on the first time from log_init forces thread 0 to be the main.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ok

@mike-dubman

Copy link
Copy Markdown
Contributor

👍

yosefe added a commit that referenced this pull request Dec 8, 2014
LOG: Make sure main thread is getting id=0
@yosefe
yosefe merged commit 42678ce into openucx:master Dec 8, 2014
@yosefe
yosefe deleted the topic/log-thread-id branch December 8, 2014 14:40
@shamisp

shamisp commented Dec 8, 2014

Copy link
Copy Markdown
Contributor

There is a gettid() call, why you don't use it

@yosefe

yosefe commented Dec 8, 2014

Copy link
Copy Markdown
Member Author

i wanted that the thread numbers will be 0..N-1, and not system-wide identifier

@shamisp

shamisp commented Dec 8, 2014

Copy link
Copy Markdown
Contributor
  1. Does it mean that that corresponds to threads id's in gdb or something ? How does it help identify actual thread

  2. It seems that the function finds thread id and if it can't set threads' id. Maybe it is better to separate to two separate calls ? (not critical)

I think it is useful info to have but it should correspond to some "real" value that is helpful for debug (gdb).

@yosefe

yosefe commented Dec 8, 2014

Copy link
Copy Markdown
Member Author

Unfortunately it does not correspond to gdb.
I think we should fix it to make it same as gdb (probably sort by tid?)

@shamisp

shamisp commented Dec 8, 2014

Copy link
Copy Markdown
Contributor

@shamisp

shamisp commented Dec 8, 2014

Copy link
Copy Markdown
Contributor

I may ask our debugger guy how the gdb magic works in older gdb versions.

amaslenn pushed a commit to amaslenn/ucx that referenced this pull request Jun 2, 2020
brminich pushed a commit to brminich/ucx that referenced this pull request Sep 30, 2020
TEST/APPS/IO_DEMO: Fix the failures when some of multiple servers are killed
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.

4 participants