[Drbd-dev] DRBD-8 - after_state_ch() can attempt to terminate worker thread with wait==true

[Drbd-dev] DRBD-8 - after_state_ch() can attempt to terminate worker thread with wait==true

[Graham, Simon]

Near the end of after_state_ch() there are several clauses that stop
(and restart) receiver and worker threads synchronously - since this
function is called from both the worker and receiver thread context,
these calls cannot be made synchronously (and an ASSERT in
_drbd_thread_stop will fire).

There seem to be a couple of issues here:

1. Is it the intention that after_state_ch is always called from the
worker thread? If so, then the calls in drbd_receiver and
   drbd_fs need to change to submit work items

2. No matter what, it should not issue synchronous calls to
drbd_thread_stop for non-current threads.

3. In receive_state() -- should after_state_ch() be called at all if the
preceding call to _drbd_set_state fails? It seems a
   bit wrong to me to pass in the new state to after_state_ch when you
know this does not represent the actual new state...

4. Ditto receive_outdate().

Simon

Re: [Drbd-dev] DRBD-8 - after_state_ch() can attempt to terminate worker thread with wait==true

[Philipp Reisner]

Am Montag, 7. August 2006 15:33 schrieb Graham, Simon:
> 3. In receive_state() -- should after_state_ch() be called at all if the
> preceding call to _drbd_set_state fails? It seems a
>    bit wrong to me to pass in the new state to after_state_ch when you
> know this does not represent the actual new state...
>
> 4. Ditto receive_outdate().

Ok, forgot to fix the issue 4 up to now. 
Fied it with this commit.
http://lists.linbit.com/pipermail/drbd-cvs/2006-August/001184.html

-Phil
-- 
: Dipl-Ing Philipp Reisner                      Tel +43-1-8178292-50 :
: LINBIT Information Technologies GmbH          Fax +43-1-8178292-82 :
: Schönbrunnerstr 244, 1120 Vienna, Austria    http://www.linbit.com :

Re: [Drbd-dev] DRBD-8 - after_state_ch() can attempt to terminate worker thread with wait==true

[Philipp Reisner]

Am Montag, 7. August 2006 15:33 schrieb Graham, Simon:
> Near the end of after_state_ch() there are several clauses that stop
> (and restart) receiver and worker threads synchronously - since this
> function is called from both the worker and receiver thread context,
> these calls cannot be made synchronously (and an ASSERT in
> _drbd_thread_stop will fire).
>
> There seem to be a couple of issues here:
>
> 1. Is it the intention that after_state_ch is always called from the
> worker thread? If so, then the calls in drbd_receiver and
>    drbd_fs need to change to submit work items
>
> 2. No matter what, it should not issue synchronous calls to
> drbd_thread_stop for non-current threads.
>

I have to admit that I do not know myself how to do this right by now.

I wanted to get all those drbd_thread_stop*() calls spread all over the
code (usually happening if a state change was successfull) into a
central place.

The right solution probably is to use the asynchronous versions
in the after_state_ch() function, and to create aditional functions
with the semantics of wait_thread() , that can be used in that
some places after the successfull state change....

Will do this somewhen next week.

-Phil
-- 
: Dipl-Ing Philipp Reisner                      Tel +43-1-8178292-50 :
: LINBIT Information Technologies GmbH          Fax +43-1-8178292-82 :
: Schönbrunnerstr 244, 1120 Vienna, Austria    http://www.linbit.com :