Re: [Xenomai-core] [BUG] racy xnshadow_harden under CONFIG_PREEMPT

[Dmitry Adamushko <dmitry.adamushko@domain.hid>]

[-- Attachment #1: Type: text/plain, Size: 3268 bytes --]

On 30/01/06, Jan Kiszka <jan.kiszka@domain.hid> wrote:
>
> Dmitry Adamushko wrote:
> >>> ...
> >
> >> I have not checked it yet but my presupposition that something as easy
> as
> >> :
> >>> preempt_disable()
> >>>
> >>> wake_up_interruptible_sync();
> >>> schedule();
> >>>
> >>> preempt_enable();
> >> It's a no-go: "scheduling while atomic". One of my first attempts to
> >> solve it.
> >
> >
> > My fault. I meant the way preempt_schedule() and preempt_irq_schedule()
> call
> > schedule() while being non-preemptible.
> > To this end, ACTIVE_PREEMPT is set up.
> > The use of preempt_enable/disable() here is wrong.
> >
> >
> > The only way to enter schedule() without being preemptible is via
> >> ACTIVE_PREEMPT. But the effect of that flag should be well-known now.
> >> Kind of Gordian knot. :(
> >
> >
> > Maybe I have missed something so just for my curiosity : what does
> prevent
> > the use of PREEMPT_ACTIVE here?
> > We don't have a "preempted while atomic" message here as it seems to be
> a
> > legal way to call schedule() with that flag being set up.
>
> When PREEMPT_ACTIVE is set, task gets /preempted/ but not removed from
> the run queue - independent of its current status.


Err...  that's exactly the reason I have explained in my first mail for this
thread :) Blah.. I wish I was smoking something special before so I would
point that as the reason of my forgetfulness.

Actually, we could use PREEMPT_ACTIVE indeed + something else (probably
another flag) to distinguish between a case when PREEMPT_ACTIVE is set by
Linux and another case when it's set by xnshadow_harden().

xnshadow_harden()
{
struct task_struct *this_task = current;
...
xnthread_t *thread = xnshadow_thread(this_task);

if (!thread)
    return;

...
gk->thread = thread;

+ add_preempt_count(PREEMPT_ACTIVE);

// should be checked in schedule()
+ xnthread_set_flags(thread, XNATOMIC_TRANSIT);

set_current_state(TASK_INTERRUPTIBLE);
wake_up_interruptible_sync(&gk->waitq);
+ schedule();

+ sub_preempt_count(PREEMPT_ACTIVE);
...
}

Then, something like the following code should be called from schedule() :

void ipipe_transit_cleanup(struct task_struct *task, runqueue_t *rq)
{
xnthread_t *thread = xnshadow_thread(task);

if (!thread)
    return;

if (xnthread_test_flags(thread, XNATOMIC_TRANSIT))
    {
    xnthread_clear_flags(thread, XNATOMIC_TRANSIT);
    deactivate_task(task, rq);
    }
}

-----

schedule.c :
...
    switch_count = &prev->nivcsw;
    if (prev->state && !(preempt_count() & PREEMPT_ACTIVE))
switch_count = &prev->nvcsw;
        if (unlikely((prev->state & TASK_INTERRUPTIBLE) &&
                unlikely(signal_pending(prev)) ))
            prev->state = TASK_RUNNING;
        else {
            if (prev->state == TASK_UNINTERRUPTIBLE)
                rq->nr_uninterruptible++;
            deactivate_task(prev, rq);
        }
    }

// removes a task from the active queue if PREEMPT_ACTIVE + //
XNATOMIC_TRANSIT

+ #ifdef CONFIG_IPIPE
+ ipipe_transit_cleanup(prev, rq);
+ #endif /* CONFIG_IPIPE */
...

 Not very gracefully maybe, but could work or am I missing something
important?

--
Best regards,
Dmitry Adamushko

[-- Attachment #2: Type: text/html, Size: 4629 bytes --]