UCT/API/UCP: Hide slow-path progress elem and refcount in uct_worker. #1583

yosefe · 2017-06-06T20:22:58Z

No description provided.

This commit does some preparations in UCT for the change in callback queue API: + Block async context outside of callback queue calls. + Keep callback reference count in a handle in uct_base_iface_t/ + Instead of exposing slow-path callback queue element in UCT API, hide it inside uct worker code. This will allocate the element internally and release when finished.

mellanox-github · 2017-06-06T23:08:25Z

Test PASSed.
See http://bgate.mellanox.com/jenkins/job/gh-ucx-pr/1814/ for details.

mellanox-github · 2017-06-07T00:08:25Z

Test PASSed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/3754/ for details (Mellanox internal link).

yosefe · 2017-06-07T07:33:35Z

@brminich @shamisp pls review

shamisp · 2017-06-07T14:13:44Z

adding @bbenton for API part

brminich

I'm a bit confused with namings. So now uct_worker_progress_register_safe is used for adding to slow path? Is the intention that user should not add to fast path at all?

brminich · 2017-06-07T14:25:40Z

src/uct/sm/mm/mm_iface.c

@@ -403,8 +403,12 @@ static void uct_mm_iface_recv_messages(uct_mm_iface_t *iface)
        ret = recvfrom(iface->signal_fd, &sig, sizeof(sig), 0, NULL, 0);
        if (ret == sizeof(sig)) {
            ucs_debug("mm_iface %p: got connect message", iface);
-            ucs_callbackq_add_safe(&iface->super.worker->progress_q,
-                                   uct_mm_iface_progress, iface);
+            if (iface->super.prog.refcount++ == 0) {


why uct_worker_progress_register is not used instead?

because we need to use "safe" mode here - it's called from async thread

but ucs_callbackq_add_safe is the same as ucs_callbackq_add. Also UCS_ASYNC_BLOCK is called in
uct_worker_progress_register

ucs_callbackq_add_safe is currently the same as ucs_callbackq_add, but the user of callbackq should not take advantage of that knowldge, since according to API _safe version should be used if calling from another thread

ASYNC_BLOCK is not needed from the async context itself

so this will be changed to calling uct_worker_progress_register_safe with some flag in the next PR?

the goal of this PR is mainly to make next PR fit into 500 lines

yosefe · 2017-06-07T14:37:10Z

@brminich this PR allows users register only to slow-path. After next PR it would be possible to register to fast-path by passing flags.

brminich

👍

MattBBaker · 2017-06-07T20:21:33Z

bot:ornl:retest

yosefe · 2017-06-13T08:31:54Z

@bbenton @shamisp can you pls review?

mellanox-github · 2017-06-13T09:47:57Z

Test PASSed.
See http://bgate.mellanox.com/jenkins/job/gh-ucx-pr/1858/ for details.

mellanox-github · 2017-06-13T10:47:57Z

Test PASSed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/3826/ for details (Mellanox internal link).

shamisp · 2017-06-13T15:54:10Z

@MattBBaker did u test it on cray ?

MattBBaker · 2017-06-13T17:16:18Z

@shamisp Builds and runs fine on Cray.

shamisp · 2017-06-13T18:24:47Z

src/uct/api/uct.h

- *
- * @note This operation could potentially be slow.
+ * @param [in]    worker        Handle to worker.
+ * @param [in]    func          Pointer to callback function.


Pointer to the callback function

shamisp · 2017-06-13T18:25:06Z

src/uct/api/uct.h

- * @note This operation could potentially be slow.
+ * @param [in]    worker        Handle to worker.
+ * @param [in]    func          Pointer to callback function.
+ * @param [in]    arg           Argument to the function.


Argument for the callback function

bbenton · 2017-06-13T18:47:29Z

@yosefe,:q
I still see the following problems in uct.h:

uct_worker_slowpath_progress_register, rather than uct_worker_progress_register_safe, is still referenced in the description of uct_worker_progress_unregister_safe()
uct_worker_progress_register is referenced in the description of uct_worker_progress_register_safe, yet it is no longer an exposed function, right?

Am I missing something?

…slow-elem

yosefe · 2017-06-14T10:27:54Z

@bbenton @shamisp fixed the doc comments

mellanox-github · 2017-06-14T11:33:16Z

Test PASSed.
See http://bgate.mellanox.com/jenkins/job/gh-ucx-pr/1862/ for details.

mellanox-github · 2017-06-14T12:18:36Z

Test FAILed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/3830/ for details (Mellanox internal link).

mellanox-github · 2017-06-14T12:33:35Z

Test PASSed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/3831/ for details (Mellanox internal link).

bbenton · 2017-06-14T15:06:10Z

👍 for Doc/API

shamisp · 2017-06-14T15:23:26Z

@yosefe - it is ready to go

yosefe added API Cleanup labels Jun 6, 2017

yosefe force-pushed the topic/uct-api-no-slow-elem branch from 03d8b3c to 58bf758 Compare June 6, 2017 20:27

yosefe added the Ready for Review label Jun 7, 2017

brminich reviewed Jun 7, 2017

View reviewed changes

shamisp approved these changes Jun 7, 2017

View reviewed changes

brminich approved these changes Jun 7, 2017

View reviewed changes

shamisp requested changes Jun 13, 2017

View reviewed changes

yosefe force-pushed the topic/uct-api-no-slow-elem branch from 3ec6b75 to 6eed080 Compare June 14, 2017 10:24

yosefe added 2 commits June 14, 2017 13:27

Merge remote-tracking branch 'upstream/master' into topic/uct-api-no-…

3356e90

…slow-elem

UCT/API: Update documentation for progress register/unregister.

e290cb0

yosefe force-pushed the topic/uct-api-no-slow-elem branch from 6eed080 to e290cb0 Compare June 14, 2017 10:27

shamisp approved these changes Jun 14, 2017

View reviewed changes

yosefe merged commit 44ba878 into openucx:master Jun 14, 2017

yosefe deleted the topic/uct-api-no-slow-elem branch June 14, 2017 15:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UCT/API/UCP: Hide slow-path progress elem and refcount in uct_worker. #1583

UCT/API/UCP: Hide slow-path progress elem and refcount in uct_worker. #1583

yosefe commented Jun 6, 2017

mellanox-github commented Jun 6, 2017

mellanox-github commented Jun 7, 2017

yosefe commented Jun 7, 2017

shamisp commented Jun 7, 2017

brminich left a comment

brminich Jun 7, 2017

yosefe Jun 7, 2017 •

edited

Loading

brminich Jun 7, 2017

yosefe Jun 7, 2017 •

edited

Loading

brminich Jun 7, 2017

yosefe Jun 7, 2017

yosefe Jun 7, 2017

yosefe commented Jun 7, 2017

brminich left a comment

MattBBaker commented Jun 7, 2017

yosefe commented Jun 13, 2017

mellanox-github commented Jun 13, 2017

mellanox-github commented Jun 13, 2017

shamisp commented Jun 13, 2017

MattBBaker commented Jun 13, 2017

shamisp Jun 13, 2017

shamisp Jun 13, 2017

bbenton commented Jun 13, 2017

yosefe commented Jun 14, 2017

mellanox-github commented Jun 14, 2017

mellanox-github commented Jun 14, 2017

mellanox-github commented Jun 14, 2017

bbenton commented Jun 14, 2017

shamisp commented Jun 14, 2017

UCT/API/UCP: Hide slow-path progress elem and refcount in uct_worker. #1583

UCT/API/UCP: Hide slow-path progress elem and refcount in uct_worker. #1583

Conversation

yosefe commented Jun 6, 2017

mellanox-github commented Jun 6, 2017

mellanox-github commented Jun 7, 2017

yosefe commented Jun 7, 2017

shamisp commented Jun 7, 2017

brminich left a comment

Choose a reason for hiding this comment

brminich Jun 7, 2017

Choose a reason for hiding this comment

yosefe Jun 7, 2017 • edited Loading

Choose a reason for hiding this comment

brminich Jun 7, 2017

Choose a reason for hiding this comment

yosefe Jun 7, 2017 • edited Loading

Choose a reason for hiding this comment

brminich Jun 7, 2017

Choose a reason for hiding this comment

yosefe Jun 7, 2017

Choose a reason for hiding this comment

yosefe Jun 7, 2017

Choose a reason for hiding this comment

yosefe commented Jun 7, 2017

brminich left a comment

Choose a reason for hiding this comment

MattBBaker commented Jun 7, 2017

yosefe commented Jun 13, 2017

mellanox-github commented Jun 13, 2017

mellanox-github commented Jun 13, 2017

shamisp commented Jun 13, 2017

MattBBaker commented Jun 13, 2017

shamisp Jun 13, 2017

Choose a reason for hiding this comment

shamisp Jun 13, 2017

Choose a reason for hiding this comment

bbenton commented Jun 13, 2017

yosefe commented Jun 14, 2017

mellanox-github commented Jun 14, 2017

mellanox-github commented Jun 14, 2017

mellanox-github commented Jun 14, 2017

bbenton commented Jun 14, 2017

shamisp commented Jun 14, 2017

yosefe Jun 7, 2017 •

edited

Loading

yosefe Jun 7, 2017 •

edited

Loading