From owner-freebsd-current@FreeBSD.ORG Tue Jan 19 01:43:55 2010 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 8D1851065670; Tue, 19 Jan 2010 01:43:55 +0000 (UTC) (envelope-from jroberson@jroberson.net) Received: from mail-yx0-f171.google.com (mail-yx0-f171.google.com [209.85.210.171]) by mx1.freebsd.org (Postfix) with ESMTP id 39BE68FC14; Tue, 19 Jan 2010 01:43:54 +0000 (UTC) Received: by yxe1 with SMTP id 1so34303yxe.3 for ; Mon, 18 Jan 2010 17:43:54 -0800 (PST) Received: by 10.101.134.39 with SMTP id l39mr10685505ann.130.1263865434376; Mon, 18 Jan 2010 17:43:54 -0800 (PST) Received: from ?10.0.1.198? (udp022762uds.hawaiiantel.net [72.234.79.107]) by mx.google.com with ESMTPS id 15sm2944405gxk.12.2010.01.18.17.43.52 (version=SSLv3 cipher=RC4-MD5); Mon, 18 Jan 2010 17:43:53 -0800 (PST) Date: Mon, 18 Jan 2010 15:47:33 -1000 (HST) From: Jeff Roberson X-X-Sender: jroberson@desktop To: Kohji Okuno In-Reply-To: <20100119.103858.29593248145858473.okuno.kohji@jp.panasonic.com> Message-ID: References: <3bbf2fe11001171858o4568fe38l9b2db54ec9856b50@mail.gmail.com> <20100118.155352.59640143160034670.okuno.kohji@jp.panasonic.com> <3bbf2fe11001172306m69ff6544i3aaf05e2540136e1@mail.gmail.com> <20100119.103858.29593248145858473.okuno.kohji@jp.panasonic.com> User-Agent: Alpine 2.00 (BSF 1167 2008-08-23) MIME-Version: 1.0 Content-Type: MULTIPART/MIXED; BOUNDARY="2547152148-1844873231-1263865655=:1027" Cc: attilio@freebsd.org, freebsd-current@freebsd.org Subject: Re: Bug about sched_4bsd? X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 19 Jan 2010 01:43:55 -0000 This message is in MIME format. The first part should be readable text, while the remaining parts are likely unreadable without MIME-aware tools. --2547152148-1844873231-1263865655=:1027 Content-Type: TEXT/PLAIN; charset=iso-8859-1; format=flowed Content-Transfer-Encoding: 8BIT On Tue, 19 Jan 2010, Kohji Okuno wrote: > Hello, Attilio, > > I think setpriority() can set priority to sleeping threads. > Is it really safe? I agree, in this code path maybe_resched is not properly locking curthread. curthread will be sched_lock and the sleeping thread will be a sleepq lock. I believe that since &sched_lock is ordered after container locks it would be sufficient to compare the two td_lock pointers and acquire sched_lock if they are not equal. Someone should look at other maybe_resched callers or add an assert that curthread->td_lock is always &sched_lock in this function. Thanks, Jeff > > Thank you, > Kohji Okuno > >> 2010/1/18 Kohji Okuno : >>> 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. >>> >>> <> >>> sched_4bsd.c:     maybe_resched() >>> sched_4bsd.c:     resetpriority_thread() >>> sched_4bsd.c:     sched_nice()                  get thread_lock(td) >>> kern_resource.c:  donice() >>> kern_resource.c:  setpriority()                 get PROC_LOCK() >>> >>> static void >>> maybe_resched(struct thread *td) >>> { >>>        THREAD_LOCK_ASSERT(td, MA_OWNED); >>>        if (td->td_priority < curthread->td_priority) >>>                curthread->td_flags |= 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 >> >> >> -- >> Peace can only be achieved by understanding - A. Einstein > --2547152148-1844873231-1263865655=:1027--