From owner-freebsd-current Sun Feb 24 17:52: 9 2002 Delivered-To: freebsd-current@freebsd.org Received: from apollo.backplane.com (apollo.backplane.com [216.240.41.2]) by hub.freebsd.org (Postfix) with ESMTP id AAFBC37B400; Sun, 24 Feb 2002 17:52:05 -0800 (PST) Received: (from dillon@localhost) by apollo.backplane.com (8.11.6/8.9.1) id g1P1q1d27292; Sun, 24 Feb 2002 17:52:01 -0800 (PST) (envelope-from dillon) Date: Sun, 24 Feb 2002 17:52:01 -0800 (PST) From: Matthew Dillon Message-Id: <200202250152.g1P1q1d27292@apollo.backplane.com> To: Bruce Evans Cc: Terry Lambert , Alfred Perlstein , Bosko Milekic , Seigo Tanimura , , John Baldwin Subject: Re: Success! critical_enter()/critical_exit() revamp (was Re: malloc_bucket() idea (was Re: How to fix malloc.)) References: <20020225111741.S36145-100000@gamplex.bde.org> Sender: owner-freebsd-current@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG :> up. One of the best things about this patch set is that it is really :> flexible. We should be able to really make interrupts fly. In fact, :> it should even be possible for one cpu to steal another cpu's pending :> interrupt(s) fairly trivially, without having to resort to IPIs. : :Good idea. Stealing would be even easier if the mask were global :-). Well, for the moment I am attempting to avoid having to use lock;'d bus cycles to keep things efficient. When we get to the stealing part we might wind up using lock;, but I'll deal with that when the time comes. :> The second issue is that cpu_switch() does not save/restore the :> PSL_I (interrupt disablement mask). I added a PSL word to the PCB :> structure to take care of the problem. Without this if you have :> a thread switch away while holding interrupts hard-disabled, the :> next thread will inherit the hard disablement. I saw the sti's :> you added in various places but I figured the best solution was :> to simply save/restore the state. The original code didn't have : :cpu_switch() certainly needs to do this if it can be called with the :interrupt enable flag[s] in different states. I need the sti's (actually :enable_intr()'s because I don't want fast interrupts to be disabled :during context switches. This works because enabling interrupts is sure :to be safe, since we might be switching to a thread that will enable :them. Some sort of lock is needed to prevent interrupts interfering :with the switch. I think soft-masking them in critical_enter() is :sufficient in your version too. : :Bruce I don't think we want to make sched_lock any more complex then it already is, so at least for the foreseeable future we are not going to be able to actually execute an interrupt handler until the sched_lock is released in (typically) msleep(). I am rather annoyed that two levels of procedure have to be called with the sched_lock held (mi_switch() and cpu_switch()), leaving interrupts disabled for a fairly long period of time, but I don't see any way around it right now. Eventually (presumably) we will have per-cpu run queues. That combined with interrupt stealing may resolve the problem for us. I am still not convinced that making the various *pending* flags globals will be more efficient, because it introduces significant cache mastership issues. It might be easier to do this: interrupt_vector { if (td->td_critnest) { ... try to forward the interrupt to another cpu that is not in a critical section ... ... else set bit in local ipending mask. } else { ... take or schedule interrupt ... } } Or possibly: interrupt_vector { if (td->td_critnest) { if (!mtx_owned(sched_lock) && mtx_trylock(sched_lock)) { ... we can still schedule the interrupt even though we are in a critical section, we just can't switch to it yet ... } else { ... try to forward the interrupt to a cpu that is not in a critical section ... ... else set bit in local ipending mask. } } else { ... take or schedule interrupt ... } } -Matt Matthew Dillon To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-current" in the body of the message