Date: Mon, 18 Jan 2010 08:06:51 +0100 From: Attilio Rao <attilio@freebsd.org> To: Kohji Okuno <okuno.kohji@jp.panasonic.com> Cc: freebsd-current@freebsd.org, jroberson@jroberson.net Subject: Re: Bug about sched_4bsd? Message-ID: <3bbf2fe11001172306m69ff6544i3aaf05e2540136e1@mail.gmail.com> In-Reply-To: <20100118.155352.59640143160034670.okuno.kohji@jp.panasonic.com> References: <20100117.142200.321689433999177718.okuno.kohji@jp.panasonic.com> <20100117.152835.119882392487126976.okuno.kohji@jp.panasonic.com> <3bbf2fe11001171858o4568fe38l9b2db54ec9856b50@mail.gmail.com> <20100118.155352.59640143160034670.okuno.kohji@jp.panasonic.com>
next in thread | previous in thread | raw e-mail | index | archive | help
2010/1/18 Kohji Okuno <okuno.kohji@jp.panasonic.com>:
> Hello,
>
> Thank you, Attilio.
> I checked your patch. I think that your patch is better.
> I tested the patch quickly, and I think it's OK.
> # This probrem does not occur easily :-<
>
>
> What do you think about maybe_resched()?
> I have never experienced about maybe_resched(), but I think that the
> race condition may occur.
>
> <<Back Trace>>
> sched_4bsd.c: =C2=A0 =C2=A0 maybe_resched()
> sched_4bsd.c: =C2=A0 =C2=A0 resetpriority_thread()
> sched_4bsd.c: =C2=A0 =C2=A0 sched_nice() =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0get thread_lock(td)
> kern_resource.c: =C2=A0donice()
> kern_resource.c: =C2=A0setpriority() =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 get PROC_LOCK()
>
> static void
> maybe_resched(struct thread *td)
> {
> =C2=A0 =C2=A0 =C2=A0 =C2=A0THREAD_LOCK_ASSERT(td, MA_OWNED);
> =C2=A0 =C2=A0 =C2=A0 =C2=A0if (td->td_priority < curthread->td_priority)
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0curthread->td_flag=
s |=3D TDF_NEEDRESCHED;
> }
>
> I think, when td->td_lock is not &sched_lock, curthread->td_lock is
> not locked in maybe_resched().
I didn't look closely to the maybe_resched() callers but I think it is
ok. The thread_lock() function works in a way that the callers don't
need to know which container lock is present in a particular moment,
there is always a guarantee that the contenders will spin if the lock
on the struct can't be held.
In the case you outlined something very particular was happening.
Basically, we get &sched_lock but sched_lock was not the lock present
on td_lock. That means all the other paths willing to access to
td_lock for that thread (via thread_lock()) were allowed to do that
even if we wanted to keep the critical path closed.
Attilio
--=20
Peace can only be achieved by understanding - A. Einstein
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3bbf2fe11001172306m69ff6544i3aaf05e2540136e1>