Re: Patch for critical_enter()/critical_exit() interrupt assem
:I agree that the use of cpu_critical_enter/exit could use cleaning up. :Can you give an example of where critical_enter is used improperly? :You mean in fork_exit? Your changes to cpu_switch solve that problem :with critical_exit almost unchanged. The savecrit stuff should really :just be made MD. If cpu_critical_exit was changed to take the thread :as its argument as I suggested before this would be easy. fork_exit() is a good example. The existing code explicitly initializes td-td_critnest to 1 and then sets td-td_savecrit to CRITICAL_FORK: td-td_critnest = 1; td-td_savecrit = CRITICAL_FORK; It then proceeds to unlock the sched_lock spin lock. This code only works because interrupts are hard-disabled in the fork trampoline code. In otherwords, the code assumes that cpu_critical_enter() and cpu_critical_exit() play with hard interrupt disablement. If interrupts were enabled there would be two severe race conditions in the code: The first upon entering fork_exit prior to ke_oncpu being set, and the second when td-td_critnest is set to 1 prior to td_savecrit being set to CRITICAL_FORK. The current ast() code is another example. This code calls cpu_critical_enter() and cpu_critical_exit() without bothering to bump the critical nesting count (i.e. bypasses critical_enter() and critical_exit()). Peter's hack to fix IPI deadlocks (no longer in -current) is a third example. He enters a critical section using critical_enter() and then calls cpu_critical_exit() while still holding td_critnest = 1, which breaks even a conservative reading of the API :-) -- There are also a huge number of places where cpu_critical_*() is used specifically in the I386 code for interrupt disablement. I am happy to clean it up, but not until after the multiple stages of my current patch set are in the tree because there are just too many places that have to be modified for me to feel comfortable doing both at once. Nor do I believe it makes sense to clean up cpu_critical_*() just to try to keep critical_*() in MI code (see below two sections). :Specifically I think that all of the current uses of cpu_critical_enter :exit outside of critical_enter exit are bugs. The use in ktr is bogus because Certainly in MI code, I agree. These cases can be attacked incrementally. fork_exit() will be fixed completely by my patches since it has to be for my code to work properly. I was planning on leaving ast() alone for the moment (because I am not planning on making cpu_critical_enter() a NOP any time soon). I would be willing to work on this after my patch is staged in (carrot and stick). :there in the first place. I think that witness is an example of where :we need a different specifically defined to be hard interrupt disable api. :This is why I suggested the intr_disable/intr_restore api, which should only :be used in MD code and in dire circumstances otherwise, of which this case :qualifies. The code in ast is just structured wrong. I think that before :the loop was in assembler outside of the routine itself, so that it didn't Witness is a special case allright. I don't know if I agree with extending a hard interrupt disablement API out to MI code though. :make so many assumptions about interrupt disablement. doreti which calls :it should look like this: : :loop: : cli; : if (there is an ast we can handle) : goto ast; : iret; :ast: : sti; : call ast; : goto loop; : :In which case ast doesn't need to loop or use critical sections at all. :All of the MD code I could find I think should use a different api for :hard interrupt disablement. They are all very MD and need this specifically, :which cpu_critical_enter need not necessarily do. I would agree with this methodology. :With these changes I don't see why the critical_enter/exit functions can't :or shouldn't remain MI. Cleaning up cpu_critical_enter()/exit would require a huge number of changes that I am not prepared to do right now, not only in i386, but in all architectures using cpu_critical_enter() and cpu_critical_exit() - alpha, i386 (many places), and ia64, not to mention ast(), fork_exit(), witness, subr_trap.c, and ktr.c. I suppose the routines could simply be renamed in the MD sections but the MI sections are going to be harder to fix. It doesn't make much sense to me to spend so much effort to leave two essentially one-line procedures, critical_enter() and critical_exit(), (++td-td_critnest + MD call, --td-td_critnest + MD call) MI when all the rest of the work they have to do is MD. Why are you so rabid about keeping critical_enter() and critical_exit() in MI sections? From my point of view leaving them MI simply makes the code more obfuscated, less
Re: Patch for critical_enter()/critical_exit() interrupt assem
On Thu, 7 Mar 2002, Jake Burkholder wrote: Apparently, On Thu, Mar 07, 2002 at 05:21:21PM -0500, Robert Watson said words to the effect of; The primary objections I've seen from Jake, and he posted them as part of the earlier thread prior to the commit, was that the API changes proposed by Matt don't make sense for the sparc64 implementation, uni-processor or multi-processor, and that while these changes might be appropriate for i386, he wanted to see the APIs set up in such a way that the differences in architectures were hidden in the MD code. This suggests working some more on the API before moving on, and my reading of earlier posts in the thread from John was that that was what he had in mind also. Yes. What I would like and what I mentioned before is for this to be hidden behind cpu_critical_enter/exit. Specifically, cpu_critical_enter would be a null macro for i386, which would turn critical_enter into just an increment, as Matt wanted. cpu_critical_exit would do all the magic of unpending interrupts. The reason for this is that I would like to see critical_exit handled any pending preemptions for a thread. We do not yet know exactly how this will work so I would like this to be done in MI code to start with. The code must not make assumptions that are not valid on all platforms. This is easiest if they use the same code. This is not about duplication of code in several MD files. There must be something that disables all interrupts, since fast interrupt handlers and other very-time-critical code needs to disable all interrupts. That something must have an MI interface, and it is currently named cpu_critical_enter(), so cpu_critical_enter() (or whatever it is named) must not be null. Your scheme basically needs another level of criticality between cpu_critical and plain critical (currently, criticality is not very hierarchial, but it should be). This level and the plain level must not mask fast interrupts (if any), since masking interrupts in not-very-critical code like the scheduler makes them non-fast. Bruce To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
On 08-Mar-02 Matthew Dillon wrote: :I agree that the use of cpu_critical_enter/exit could use cleaning up. :Can you give an example of where critical_enter is used improperly? :You mean in fork_exit? Your changes to cpu_switch solve that problem :with critical_exit almost unchanged. The savecrit stuff should really :just be made MD. If cpu_critical_exit was changed to take the thread :as its argument as I suggested before this would be easy. Hmm, I agree with getting rid of td_savecrit as an MI field and changing the API for cpu_critical_*. I also agree that cpu_critical_* is abused in many cases. I just fixed a few that can be fixed. I think most others can be fixed with the explicit intr_disable/intr_restore API which shouldn't be a problem since it's basically what cpu_critical_* is now but just misnamed. This would fix the remaining instance in witness for example. ast() is harder to solve, and although I don't like having stuff duplicated all over the place making maintenance harder, moving the loop and test out to MD code in either asm or C as Jake suggested would work fine. fork_exit() is a good example. The existing code explicitly initializes td-td_critnest to 1 and then sets td-td_savecrit to CRITICAL_FORK: td-td_critnest = 1; td-td_savecrit = CRITICAL_FORK; It then proceeds to unlock the sched_lock spin lock. This code only works because interrupts are hard-disabled in the fork trampoline code. In otherwords, the code assumes that cpu_critical_enter() and cpu_critical_exit() play with hard interrupt disablement. If interrupts were enabled there would be two severe race conditions in the code: The first upon entering fork_exit prior to ke_oncpu being set, and the second when td-td_critnest is set to 1 prior to td_savecrit being set to CRITICAL_FORK. No, the savecrit does assume that, but the critnest is still correct and will still work fine. By definition, you don't switch inside a critical section (taht's what critical sections do) so critnest will always be 1 here. Any interrupt or what not that comes in if interrutps aren't disabled might dink with teh count but the count will still be 1 by the time we get to releasing sched_lock. Alternatively, we could set td_critnest to 1 in fork() which would be ok with me. Peter's hack to fix IPI deadlocks (no longer in -current) is a third example. He enters a critical section using critical_enter() and then calls cpu_critical_exit() while still holding td_critnest = 1, which breaks even a conservative reading of the API :-) I always said that Peter's hack was gross. I didn't like it when he did it originally either. Actually, doing cpu_critical_exit() though is safe since all we need is for the critnest to be = 1 to prevent context switches (which is all critical sections do if you read the manpage that documents them). We only ever need to defer bottom-half code (or Primary Interrupt Context as Apple likes to call it) while holding a spin mutex. The MD code does abuse cpu_critical_* which I am quite happy to fix using intr_*(). Once this is done cpu_critical_* should be out of bogus land and you should be able to adjust your patches to change that API's implementation. I'm willing to do all the work to fixup the cpu_critical_* abuse right now before doing anything else. It really needs to be done anyway. :With these changes I don't see why the critical_enter/exit functions can't :or shouldn't remain MI. Cleaning up cpu_critical_enter()/exit would require a huge number of changes that I am not prepared to do right now, not only in i386, but in all architectures using cpu_critical_enter() and cpu_critical_exit() - alpha, i386 (many places), and ia64, not to mention ast(), fork_exit(), witness, subr_trap.c, and ktr.c. I suppose the routines could simply be renamed in the MD sections but the MI sections are going to be harder to fix. It doesn't make much sense to me to spend so much effort to leave two essentially one-line procedures, critical_enter() and critical_exit(), (++td-td_critnest + MD call, --td-td_critnest + MD call) MI when all the rest of the work they have to do is MD. They won't stay one line. That's the whole point. The entire reason we have MI versions is that I committed part of the preemption tree specifically to try and minimize the amount of the preemption code not currently checked in. The MI versions are still a WIP part of the preemption tree. They wouldn't even exist if it weren't for the preemption tree. I defer to Jake's opinions on the style and profiling issues. All that I ask is that you let me cleanup and fix the cpu_critical_* bogusness which will then allow you to implement your changes in that API while not breaking the kernel preemption WIP. -- John Baldwin [EMAIL PROTECTED]http://www.FreeBSD.org/~jhb/ Power Users
Re: Patch for critical_enter()/critical_exit() interrupt assem
Apparently, On Fri, Mar 08, 2002 at 01:23:01AM -0800, Matthew Dillon said words to the effect of; :I agree that the use of cpu_critical_enter/exit could use cleaning up. :Can you give an example of where critical_enter is used improperly? :You mean in fork_exit? Your changes to cpu_switch solve that problem :with critical_exit almost unchanged. The savecrit stuff should really :just be made MD. If cpu_critical_exit was changed to take the thread :as its argument as I suggested before this would be easy. fork_exit() is a good example. The existing code explicitly initializes td-td_critnest to 1 and then sets td-td_savecrit to CRITICAL_FORK: td-td_critnest = 1; td-td_savecrit = CRITICAL_FORK; It then proceeds to unlock the sched_lock spin lock. This code only works because interrupts are hard-disabled in the fork trampoline code. In otherwords, the code assumes that cpu_critical_enter() and cpu_critical_exit() play with hard interrupt disablement. If interrupts were enabled there would be two severe race conditions in the code: The first upon entering fork_exit prior to ke_oncpu being set, and the second when td-td_critnest is set to 1 prior to td_savecrit being set to CRITICAL_FORK. The current ast() code is another example. This code calls cpu_critical_enter() and cpu_critical_exit() without bothering to bump the critical nesting count (i.e. bypasses critical_enter() and critical_exit()). Peter's hack to fix IPI deadlocks (no longer in -current) is a third example. He enters a critical section using critical_enter() and then calls cpu_critical_exit() while still holding td_critnest = 1, which breaks even a conservative reading of the API :-) -- There are also a huge number of places where cpu_critical_*() is used specifically in the I386 code for interrupt disablement. I am happy to clean it up, but not until after the multiple stages of my current patch set are in the tree because there are just too many places that have to be modified for me to feel comfortable doing both at once. Nor do I believe it makes sense to clean up cpu_critical_*() just to try to keep critical_*() in MI code (see below two sections). :Specifically I think that all of the current uses of cpu_critical_enter :exit outside of critical_enter exit are bugs. The use in ktr is bogus because Certainly in MI code, I agree. These cases can be attacked incrementally. fork_exit() will be fixed completely by my patches since it has to be for my code to work properly. I was planning on leaving ast() alone for the moment (because I am not planning on making cpu_critical_enter() a NOP any time soon). I would be willing to work on this after my patch is staged in (carrot and stick). Ok. :there in the first place. I think that witness is an example of where :we need a different specifically defined to be hard interrupt disable api. :This is why I suggested the intr_disable/intr_restore api, which should only :be used in MD code and in dire circumstances otherwise, of which this case :qualifies. The code in ast is just structured wrong. I think that before :the loop was in assembler outside of the routine itself, so that it didn't Witness is a special case allright. I don't know if I agree with extending a hard interrupt disablement API out to MI code though. I agree that it should be avoided. John would know better why exactly this is needed and he may have already fixed it. :make so many assumptions about interrupt disablement. doreti which calls :it should look like this: : :loop: : cli; : if (there is an ast we can handle) : goto ast; : iret; :ast: : sti; : call ast; : goto loop; : :In which case ast doesn't need to loop or use critical sections at all. :All of the MD code I could find I think should use a different api for :hard interrupt disablement. They are all very MD and need this specifically, :which cpu_critical_enter need not necessarily do. I would agree with this methodology. :With these changes I don't see why the critical_enter/exit functions can't :or shouldn't remain MI. Cleaning up cpu_critical_enter()/exit would require a huge number of changes that I am not prepared to do right now, not only in i386, but in all architectures using cpu_critical_enter() and cpu_critical_exit() - alpha, i386 (many places), and ia64, not to mention ast(), fork_exit(), witness, subr_trap.c, and ktr.c. I suppose the routines could simply be renamed in the MD sections but the MI sections are going to be harder to fix. It doesn't make much sense to me to spend so much effort to leave two essentially one-line procedures, critical_enter() and critical_exit(),
Re: Patch for critical_enter()/critical_exit() interrupt assem
:On 08-Mar-02 Matthew Dillon wrote:
:
::I agree that the use of cpu_critical_enter/exit could use cleaning up.
::Can you give an example of where critical_enter is used improperly?
::You mean in fork_exit? Your changes to cpu_switch solve that problem
::with critical_exit almost unchanged. The savecrit stuff should really
::just be made MD. If cpu_critical_exit was changed to take the thread
::as its argument as I suggested before this would be easy.
:
:Hmm, I agree with getting rid of td_savecrit as an MI field and changing the
:API for cpu_critical_*. I also agree that cpu_critical_* is abused in many
:cases. I just fixed a few that can be fixed. I think most others can be fixed
:with the explicit intr_disable/intr_restore API which shouldn't be a problem
:since it's basically what cpu_critical_* is now but just misnamed. This would
:fix the remaining instance in witness for example. ast() is harder to solve,
:and although I don't like having stuff duplicated all over the place making
:maintenance harder, moving the loop and test out to MD code in either asm or C
:as Jake suggested would work fine.
:
: fork_exit() is a good example. The existing code explicitly
: initializes td-td_critnest to 1 and then sets td-td_savecrit
: to CRITICAL_FORK:
:
: td-td_critnest = 1;
: td-td_savecrit = CRITICAL_FORK;
:
: It then proceeds to unlock the sched_lock spin lock.
:
: This code only works because interrupts are hard-disabled in the
: fork trampoline code. In otherwords, the code assumes that
: cpu_critical_enter() and cpu_critical_exit() play with hard interrupt
: disablement. If interrupts were enabled there would be two severe
: race conditions in the code: The first upon entering fork_exit
: prior to ke_oncpu being set, and the second when td-td_critnest is set
: to 1 prior to td_savecrit being set to CRITICAL_FORK.
:
:No, the savecrit does assume that, but the critnest is still correct and will
:still work fine. By definition, you don't switch inside a critical section
:(taht's what critical sections do) so critnest will always be 1 here. Any
:interrupt or what not that comes in if interrutps aren't disabled might dink
:with teh count but the count will still be 1 by the time we get to releasing
:sched_lock. Alternatively, we could set td_critnest to 1 in fork() which would
:be ok with me.
Huh? Are you actually advocating that it is ok to allow an interrupt
to occur while curthread is in an inconsistent state? I mean, we aren't
talking about a little inconsistency here John, we are talking about
the sched_lock being in a weird state, and we are talking about
curthread's ke_oncpu not being set properly.
We could set td_critnest to 1 in fork(). I considered doing that but
decided it was easier to keep my code as close to the existing code
as possible so I clean it up in the fork trampoline code while
interrupts really are disabled. With the critical section in a
completely consistent state I can then enable interrupts prior
to calling fork_exit().
:I always said that Peter's hack was gross. I didn't like it when he did it
:originally either. Actually, doing cpu_critical_exit() though is safe since
:all we need is for the critnest to be = 1 to prevent context switches (which
:is all critical sections do if you read the manpage that documents them). We
:only ever need to defer bottom-half code (or Primary Interrupt Context as Apple
:likes to call it) while holding a spin mutex.
By my read of the code it is not safe to allow interrupts to be enabled
while critnest = 1 because while the interrupt may not context
switch, it will still attempt to schedule a new thread which requires
interactions with spin locks which the interrupted process may be
holding. The result: the interrupted process is not protected
by the spin lock it thought protected it because the interrupt can
pop in and do something horrible.
:The MD code does abuse cpu_critical_* which I am quite happy to fix using
:intr_*(). Once this is done cpu_critical_* should be out of bogus land and you
:should be able to adjust your patches to change that API's implementation.
:I'm willing to do all the work to fixup the cpu_critical_* abuse right now
:before doing anything else. It really needs to be done anyway.
:...
:They won't stay one line. That's the whole point. The entire reason we have
:MI versions is that I committed part of the preemption tree specifically to
:try and minimize the amount of the preemption code not currently checked in.
:The MI versions are still a WIP part of the preemption tree. They wouldn't
:even exist if it weren't for the preemption tree.
I find it highly unlikely that they will ever exceed four or five lines.
About the worst I can envision is a conditional like this:
critical_exit()
{
if (--td-td_critnest == 0) {this
Re: Patch for critical_enter()/critical_exit() interrupt assem
:The reason is that if they are in MI code they automatically apply to all :platforms and can't get out sync. When they are modified to handle preemption :the freebsd kernel will be fully preemptive. Not, it works on i386 and its :believed to work on alpha and powerpc is not preemptive at all and we don't If the routines were large this would be an issue, but we are talking between 1 and 5 lines of MI code verses potentially many more lines of MD code and I find it virtually impossible for such a small amount of code to 'get out of sync'. :even know about ia64 (not to dump on other platforms, this is just example of :how things go sometimes). I understand that this argument may be sentimental :and may not hold water in a technial discussion. As such I will not stop you :from making them MD if you disagree (not to imply that I have the power to do :so, I don't), I just think that keeping them MI is the right thing to do. : :I must admit that having them be MD will allow me to make optimizations for :sparc64 that I have wanted to. However, I do not think that this is better :for freebsd as a whole. : : : :... :... :The point is not wether its easy or not, the point is that this is an :important feature that may have been forgotten about. I use this on :a daily basis in sparc64 development and I would be upset if it was :broken there for any amount of time, not that this patch will affect it. :I am confident that you will fix any problems that arise, but I would :rather the next person that tries to do some debugging not be confused :by something being different, or by it not working and having to wait for :a fix, even if that amount of time is insignificant. If you do not feel :that this needs to be looked at before you commit then that is fine, again :I cannot stop you. I know many other committers who would not feel that :way about a patch of their own and I think that standard is worth adhereing :to. I apologize if this sounds like a lecture or if this offends you, it :is just how I feel. : :Jake I'm not sure what you are refering to here. The fast interrupt deferral stuff only applies to I386. In anycase, I certainly haven't forgotten about debugger support for I386, but I didn't go to great lengths to test whether the DDB backtrace operates as expected for the fast-interrupt-restart case either. -Matt Matthew Dillon [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
In message: [EMAIL PROTECTED] Matthew Dillon [EMAIL PROTECTED] writes: : We have very different development models, and different priorities. : I'm not sure why you are attempting to impose your development model : and priorities on me. This is the work I want to do. It's my time : here, not yours, and if you believe that the work will make your job : harder in some future unspecified commit then you have only to bring : up the issue down the road when you are actually ready to work on : that future unspecified commit and we can work it out. But I don't : think it's appropriate for you to impose such a strict set of requirements : based on work that does not exist in -current anywhere as far as I can : tell. While your time is your time, it isn't your tree. It isn't my tree. The tree is a shared resource that we all must respect. There are times that it is reasonable to have restrictions on what can go into the tree. Since smpng is being coordinated amoung several developers in the tree, there are some restrictions that may come with that. I think that such restrictions are reasonable and the price of doing business in a shared resource. I think that's a fundamental decision about the tree that has been made that your development model is at odds with and the source of much of the current dispute over commit or not to commit the changes. In the past there have been a number of restrictions on what can and can't go into the tree. I needn't enumerate them here, but I will add that there have been times where special restrictions were in place that weren't in place at other times. We're entering the glide path to 5.0, so I would expect there to be more such restrictions coming from the release engineer and maybe others as we move towards that release. I'm not saying that your patches necessarily fall under these restrictions, but I am saying that to assert that your time is yours and therefore anything that you do can go into the tree is a false premise. Warner To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
The API is intended for the stage-2 commit. The stage-1 commit is intended to do all the 'hard stuff' in as straightforward a manner as possible. The sysctl / option you talk about here existed in my patch set 2 and a half weeks ago. The API and getting it right is more important than the hard stuff. Its not as fun, but the API will outlive the code (consider the next arch, and the next). It may take more of your time due to discussion and feedback to get the API in place, but that's part of the cost of collaboration. I really wish you people would at least read the patch and commit message before you comment on it. I didn't have to read the patch to know that there were concerns and on-going discussion about the API. In this instance, the issues are not large and can be remedied quickly - all the more reason for the discussion to take place before the commit. I didn't have to read the patch to know that you didn't seem to care about getting the API right before commiting the code. The API was something to be cleaned up later. If you have problems with my API, I'll even help merge your changes into mine, but I need to commit now. Lets discuss this after the commit. This project is not about a race to commit. [Perhaps my viewpoint here is a bit different then some seeing as the CAM stuff was 1.5 years old by the time it was committed to the tree. I know all about painful merge conflicts.] I didn't have to read the patch to know that you would only remove your commit if someone could point to specific defects in the hard part of your submission. The hard part, how it works, and whether it contained any bugs or not was not the real issue. This is why you and John were talking right past each other. Don't get me wrong, Matt. I don't think you should be unduley held off from committing either. From my perspective of the discussion so far, other than the delay for John to get back home from BSDcon, we could have finished the discussion about the API and committed this stuff a week ago if the focus wasn't on how you were personally wronged, or John is holding up the whole tree, or my changes are correct, my changes are correct. All of the above may be true, but focusing on the particular events instead of the *process to avoid them recurring in the future* will only lead to more frustration and you wanting to work on -stable instead of -current, etc. You're a good engineer Matt. You've shown before that you can colloborate and coordinate with others in this project. The only issue is how that colloboration takes place, and the *process* for getting things done. If the process makes it twice as hard for us to get things done, then it's broken. In that case, *attack the process*, not the person. The process can and will change but only if we set aside personal indignity (why should I have to put up with this crap!?!?) and attack our problems and goals as engineers. Sometimes the flare-ups in FreeBSD remind me of the middle-east. Each side accuses the other side of starting it, or having the blame. In reality, both sides were jerks, own portions of the blame, and have concentrated on continuing the conflagration rather than mitigating it. I hope we have a better perspective on reality. -- Justin To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
Can someone please end this nightmare? If you've reviewed the patch and found substantive reason to contest it then speak now else the patch should be commited today so that forward progress can continue. I mean, cut the crap and state the facts you see about the code or sit in silence while those who do know the facts work them out. To bitch and moan because you assume it might interrupt someone elses work is as antiproductive as anything can be. From what I've seen I think Matt should be allowed to commit. Pete... To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
Trim your cc's. I'm sorry, but simply not liking the idea of someone else doing a particular optimization now verses later is not a good enough reason to require that 40+ hours worth of work be thrown away when that other person has stated, repeatedly, that he will support said code. Given that not optimising the code is consistent with the current SMPng approach, and given that you went and did this work unsolicited, the situation is unfortunate but inevitable. So far not one person, not even you, has been able to explain why the patch should not go in. It should not be committed because the current SMPng direction is establishment and implementation, not optimisation. Is that plain enough for you? You're doing the wrong work, as far as the general consensus goes. I am angry because you and a number of others are not willing to take the work at face value and instead insist on making ridiculous extremist assumption into it and then using that opinion to justify not allowing the patch to go in. You are angry because you've decided that you want to do something, and you don't want to be told you can't by someone else. Had you been reasonable about the situation, your code would probably have been committed and you could have spent the last week or more doing something else more interesting. Instead, you've tried to hold the Project to ransom for your ego. We don't tolerate this from anyone, no matter how skillful or useful they might otherwise be. This is a collaborative effort, and you need to collaborate on the group's terms, not your own. I don't see how you can possibly tell me that I am not being a team player when I am being told to throw the code away completely. If you were a team player, you would throw the code away. (Actually, I doubt that throwing the code away is the correct solution; my point here is simply that you've completely failed to understand what being part of a team is actually about.) I can't fight your opinion of me, but you can't expect me to listen to you if you can't justify it. You have already shown very clearly that you aren't even interested in looking at the code, that you don't care what it does, but you are against it anyway. That's because this is not about the code; it's about your behaviour. This is not about being part of a team. You don't have to be forced into using someone else's methodology to be part of a team. Being part of a team means respecting other people's metholodogies and it means compromising. I find this quite funny. You abuse John for not compromising, yet you're not willing to compromise on the issue that's really at hand here (and which has been highlighted repeatedly since the first day of dispute). You're asking for respect, but not returning it. This is about being part of a team, not having a fan club. This IS about team work, but you are barking up the wrong tree if you think I'm the one who's not being a team player here. Do we see you subordinating your personal goals to those of the team? Say what? Who said anything about me not wanting to discuss API changes? That's all I've been TRYING to do for the last three fucking weeks! No, you have been arguing to get your code committed, as-is, philosophically unchanged. Are you discussing API changes? No, you are basically just bashing me. That's all that you people have been doing for three fucking weeks. This is a consequence of your behaviour. Had you been more reasonable, you'd have had less bashing. Now who is being unreasonable? Why do you believe that it is absolutely necessary that I be prevented from committing the work? Because allowing you to commit it at this stage would represent giving in to your attempt to hold the Project to ransom. Had you moderated your tone and worked within the dictates of the situation, you'd probably have found a path that would have allowed your code to be committed with little effort. Unfortunately, you appear not to have learned from your previous mistakes, and (again) took an approach which deliberately antagonises your co-developers. You've successfully manufactured a situation from which it is almost impossible to proceed forward; any benefit to the Project that might be derived from your code has already been more than offset by time wasted in response to your behaviour, and actually giving your your head without seeing due process served is simply not consistent with our intention that development be managed by consensus. Despite your repeated attempts to transfer blame elsewhere, you are ultimately the root cause of your current dilemma, and likewise you are the only person that can resolve it. Such a resolution is going to require a change in approach from you; what you want is eminently achievable, you just have to work out that tricky
Re: Patch for critical_enter()/critical_exit() interrupt assem
On Thu, 7 Mar 2002, Michael Smith wrote: Trim your cc's. I'm sorry, but simply not liking the idea of someone else doing a particular optimization now verses later is not a good enough reason to require that 40+ hours worth of work be thrown away when that other person has stated, repeatedly, that he will support said code. Given that not optimising the code is consistent with the current SMPng approach, and given that you went and did this work unsolicited, the situation is unfortunate but inevitable. Geeze Mike I expect bettrer from you.. So far not one person, not even you, has been able to explain why the patch should not go in. It should not be committed because the current SMPng direction is establishment and implementation, not optimisation. The code actually fixes bugs too. Improvement in performance is a bonus. Is that plain enough for you? You're doing the wrong work, as far as the general consensus goes. No, If he had asked you first (and john didn't exist) whould you have honestly said I think that you should be forbidden from looking at fixing that code. I am angry because you and a number of others are not willing to take the work at face value and instead insist on making ridiculous extremist assumption into it and then using that opinion to justify not allowing the patch to go in. You are angry because you've decided that you want to do something, and you don't want to be told you can't by someone else. This is a volunteer process.. Who is to say that anyone is forbidden from fixing certain problems, especially when the problems are not in any particular person's Baliwick. These changes are no more in JHB's area than they are in BDE's. If BDE had committed them there would not have been a peep out of anyone. (And that's a certainty). Had you been reasonable about the situation, your code would probably have been committed and you could have spent the last week or more doing something else more interesting. Instead, you've tried to hold the Project to ransom for your ego. We don't tolerate this from anyone, no matter how skillful or useful they might otherwise be. Don't be melodramatic. There was whole week of silence on this topic when NOT ONE SINGLE PERSON bothered to review the patch. It's interesting that the loudest people against this patch have not read it. This is a collaborative effort, and you need to collaborate on the group's terms, not your own. In a collaborative effort someone would have reviewed the code by now. I can't fight your opinion of me, but you can't expect me to listen to you if you can't justify it. You have already shown very clearly that you aren't even interested in looking at the code, that you don't care what it does, but you are against it anyway. That's because this is not about the code; it's about your behaviour. The behaviour was ok until he got picked on. The patch was backed out within hours of the request and he's been waiting for a technical review ever since. I find this quite funny. You abuse John for not compromising, yet you're not willing to compromise on the issue that's really at hand here (and which has been highlighted repeatedly since the first day of dispute). Matt has put his code up for view and stated that he's willing to discuss it with anyone. John has hidden his code in 1MB of patches and refused to comment on Matt's patch other than to say that he wouldn't have done it. This is a consequence of your behaviour. Had you been more reasonable, you'd have had less bashing. May you be on the receiving end sometime. The initial objections were because some one SUSPECTED that jhb had code in that area. After the patch was backed out, not a single person has bothered to review it. Core simply washed it's hands of the deal. After that I'll tell you that I just about recommended to my company that we switch to Linux. It was so sickenning. Because allowing you to commit it at this stage would represent giving in to your attempt to hold the Project to ransom. No core's refusal to allow a commit shows a childish attempt to show who's boss, and to ensure that an uppity developer gets what's comming to him. Ther was a whole week of silence on the topic when Matt waited for comment, review and critisism. The only critisism he got was in the form of abuse. Had you moderated your tone and worked within the dictates of the situation, you'd probably have found a path that would have allowed your code to be committed with little effort. that obviously doesn't work.. see above. Unfortunately, you appear not to have learned from your previous mistakes, and (again) took an approach which deliberately antagonises your co-developers. You've successfully manufactured a situation from which it is almost impossible to proceed forward; any benefit to
Re: Patch for critical_enter()/critical_exit() interrupt assem
If memory serves me right, Bruce A. Mah wrote: [snip] Disregard. I hit the wrong button in my MUA. Sorry for the spammage. Bruce. To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
: those reviews object on a purely subjective manner. : : : I think this is premature : : I don't consider that to be a technical review? do you? If that's what he said, no. However, that's not all that he said. Also, Jake had several objections from a sparc64 perspective that weren't answered by dillon. My memory is different than yours. But all this he said she said stuff isn't getting us anywhere. Warner To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
:The API is intended for the stage-2 commit. The stage-1 commit :is intended to do all the 'hard stuff' in as straightforward :a manner as possible. The sysctl / option you talk about here :existed in my patch set 2 and a half weeks ago. : :The API and getting it right is more important than the hard stuff. :Its not as fun, but the API will outlive the code (consider the next :arch, and the next). It may take more of your time due to discussion :and feedback to get the API in place, but that's part of the cost of :collaboration. :... :I really wish you people would at least read the patch and commit :message before you comment on it. : :I didn't have to read the patch to know that there were concerns and :on-going discussion about the API. In this instance, the issues are :not large and can be remedied quickly - all the more reason for the :discussion to take place before the commit. Again, bullshit. You should have read the patch. How can you possibly justify a lecture on what I should and should not be doing when you don't even read to bother the material under discussion? :I didn't have to read the patch to know that you didn't seem to care :about getting the API right before commiting the code. The API Again, complete and utter bullshit. You actually believe that because I want to commit this stuff in multiple stages and decided that the API change would be in stage 2, that this somehow magically means that I don't seem to care? How the hell did you come to that conclusion? Did you even bother to read the commit message? I'll tell you why I want to commit the stuff in multple stages... BECAUSE THAT ALLOWS THE CORE OF THE CODE TO BE TESTED WHILE THE REMAINING STUFF CAN BE DISCUSSED AND/OR WORKED ON. That's why. I am not going to spend months integrating change after change into a patchset, having to retest the whole mess every time, and then only after months commit the final result. That is not how I work and I strongly oppose that kind of methodology. I prefer to work in smaller chunks and yet here you are trying to justify your nonsense by making further attacks on my methodologies and code that are completely absurd and completely unsupported by any evidence whatsoever. This is complete and utter nonsense. You seem to believe you know a lot about my motives without reading one goddamn line of code. Because you know what? The commit message laid this all out in very clear terms. :I didn't have to read the patch to know that you would only remove your :commit if someone could point to specific defects in the hard part of :your submission. The hard part, how it works, and whether it contained :any bugs or not was not the real issue. This is why you and John were :talking right past each other. Excuse me, but I think whether something contains bugs or not is a more serious issue then which source file the code winds up being in. And, again, you seem to be making ridiculous assumptions that are simply not true. Change the API? I am doing no such thing. I am expanding one portion of the existing API. The procedural interface has not changed. The procedure names have not changed. The location of the procedures change, and the capabilities of the procedures have changed, and certain restrictive assumptions regarding hard interrupt disablement have been changed. But you, and others, do not appear to be willing to face up to the code or its straightforward intent. Intead you are reading all sorts of garbage into its meaning, WITHOUT EVEN BOTHERING TO READ IT. It is unbelievable to me that a professional such as yourself would stoop to such tactics without backing it up with one shred of evidence and not even having bothered to read the code in question. And instead of standing up and apologizing for that, you instead feel that you have to start making wild accusations without one single hard reference to ANYTHING I have done. :.. :you can colloborate and coordinate with others in this project. The only :issue is how that colloboration takes place, and the *process* for getting :things done. If the process makes it twice as hard for us to get things :done, then it's broken. In that case, *attack the process*, not the person. :The process can and will change but only if we set aside personal :indignity (why should I have to put up with this crap!?!?) and attack :our problems and goals as engineers. Hey, I'm the one being attacked here. This whole mess started with people attacking me. If people had let well enough alone.. if people had simply read the goddamn patch rather then attack its intent (if you even know what it's intent is since so far you are batting 0 on that front), then we would not be in this situation. Instead it's attacked because
Re: Patch for critical_enter()/critical_exit() interrupt assem
Apparently, On Thu, Mar 07, 2002 at 05:21:21PM -0500, Robert Watson said words to the effect of; On Thu, 7 Mar 2002, Warner Losh wrote: In message [EMAIL PROTECTED] Julian Elischer writes: : : : On Thu, 7 Mar 2002, Justin T. Gibbs wrote: : : Then do the right things so it will. : : Unfortunatly that has been proven to not work. : : after reverting the change and silently waiting for a week : 1/ no person bothered to review it. : 2/ people assumed the patch had gone away. Ummm, There are reviews in the archives that object to the API as it relates to optimization and those objections haven't been sanely answered with anything more constructive than BS. The primary objections I've seen from Jake, and he posted them as part of the earlier thread prior to the commit, was that the API changes proposed by Matt don't make sense for the sparc64 implementation, uni-processor or multi-processor, and that while these changes might be appropriate for i386, he wanted to see the APIs set up in such a way that the differences in architectures were hidden in the MD code. This suggests working some more on the API before moving on, and my reading of earlier posts in the thread from John was that that was what he had in mind also. Yes. What I would like and what I mentioned before is for this to be hidden behind cpu_critical_enter/exit. Specifically, cpu_critical_enter would be a null macro for i386, which would turn critical_enter into just an increment, as Matt wanted. cpu_critical_exit would do all the magic of unpending interrupts. The reason for this is that I would like to see critical_exit handled any pending preemptions for a thread. We do not yet know exactly how this will work so I would like this to be done in MI code to start with. The code must not make assumptions that are not valid on all platforms. This is easiest if they use the same code. This is not about duplication of code in several MD files. Bruce also had some comments which were shrugged off, I thought they were important. Specifically, please do not make unnecessary changes to the assembler code. Macros do not need to be defined before they are used, I believe this was the justification for some of the reordering in apic_vector.s which makes the patch confusing. Please do not tab out the ; \ at the end of the lines in the INTR and FAST_INTR macros in icu_vector.s. This just makes unnecessary diffs. The PUSH_DUMMY macro must push a reasonable eip value, in addition to the code segment, so that profiling and stack traces work right. If you have not already, please make sure that a trace from inside an interrupt handler that was unpended looks somewhat normal. Please also make sure that kgdb is able to decode the frame properly. It assumes that the eip of the frame will be near certain kernel symbols in order to determine what kind of frame it is. Jake To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
:I didn't have to read the patch to know that there were concerns and :on-going discussion about the API. In this instance, the issues are :not large and can be remedied quickly - all the more reason for the :discussion to take place before the commit. Again, bullshit. You should have read the patch. How can you possibly justify a lecture on what I should and should not be doing when you don't even read to bother the material under discussion? Because it isn't relavent. Until you see that, you will continue to be frustrated. You will continue to get angry. You will continue to butt heads. You will continue to act unprofessionally on our lists. You will continue to piss people off. And most importantly, you will continue to lose the battles that you care about. Is your code in the tree right now? No. That above all else shows that your tactics are the wrong ones. :I didn't have to read the patch to know that you didn't seem to care :about getting the API right before commiting the code. The API Again, complete and utter bullshit. You actually believe that because I want to commit this stuff in multiple stages and decided that the API change would be in stage 2, that this somehow magically means that I don't seem to care? How the hell did you come to that conclusion? Did you even bother to read the commit message? You came to the conclusion that only *your decision* on what was an appropriate proceedure was the one that mattered. That's not the way this project works. You can't act unilaterally. When people ask you to hold off (and they even asked politely!) so discussion can take place, that is not the time to commit. I'll tell you why I want to commit the stuff in multple stages... BECAUSE THAT ALLOWS THE CORE OF THE CODE TO BE TESTED WHILE THE REMAINING STUFF CAN BE DISCUSSED AND/OR WORKED ON. That's why. One week of discussion will not prevent the code from being tested. I am not going to spend months integrating change after change into a patchset, having to retest the whole mess every time, and then only after months commit the final result. I've counted weeks so far and those weeks weren't even necessary. Now your expecting months and years of delay? Please. The only way it will get delayed that long is if you spend all of your time stomping up and down, writing in all caps, and tell the rest of us that we have to follow the proceedures you think are appropriate. That's not how colaboration works. You need to compromise and not get all pissed off if the process requires you to delay your commit for a bit. That is not how I work and I strongly oppose that kind of methodology. I think you've made that clear already. The question is whether you are willing to compromise so you can be part of a team or not. That means, for all of us, that we will not necessarily be able to work in the way we would personally want, all of the time. That's what happens in a group environment. That's life. I prefer to work in smaller chunks and yet here you are trying to justify your nonsense by making further attacks on my methodologies and code that are completely absurd and completely unsupported by any evidence whatsoever. This is not an attack on any such thing. This is about *process*. You want to commit in small chucks? Great! You want to get this stuff into the tree quickly? Great! You want to make the code so it can be dynamically configured for testability? Great! You don't want to discuss API changes that have affects on other ports, and other subsystems prior to committing them? That's unacceptable when people are asking you to explain them first, regardless of how well thought out or implemented they are. This is not a race to commit. This is about developing a well designed system without killing each other in the process. This is complete and utter nonsense. You seem to believe you know a lot about my motives without reading one goddamn line of code. Because you know what? The commit message laid this all out in very clear terms. For this kind of change, the commit message should have been the last, not the first public forum to have expressed these ideas. :I didn't have to read the patch to know that you would only remove your :commit if someone could point to specific defects in the hard part of :your submission. The hard part, how it works, and whether it contained :any bugs or not was not the real issue. This is why you and John were :talking right past each other. Excuse me, but I think whether something contains bugs or not is a more serious issue then which source file the code winds up being in. Did I say anything about source files? This is about discussion prior to commit. Nothing more. And, again, you seem to be making ridiculous assumptions that are simply not true. Change the API? I am doing no such thing. I am
Re: Patch for critical_enter()/critical_exit() interrupt assem
: :You came to the conclusion that only *your decision* on what was :an appropriate proceedure was the one that mattered. That's not :the way this project works. You can't act unilaterally. When people :ask you to hold off (and they even asked politely!) so discussion :can take place, that is not the time to commit. I did no such thing. I came to the conclusion because not a single goddamn person bothered to read the patch and instead the only argument I got was wait for John and John's only argument is I don't like the idea of optimizing this routine right now as if he would be the only one responsible for dealing with the consequences. I'm sorry, but simply not liking the idea of someone else doing a particular optimization now verses later is not a good enough reason to require that 40+ hours worth of work be thrown away when that other person has stated, repeatedly, that he will support said code. So far not one person, not even you, has been able to explain why the patch should not go in. John's only excuse is that he doesn't like the idea of optimizing it now. John has stated on several occassions that he believes I am breaking future work (like preemption). I have responded every time by asking him to clarify exactly what he thought would be broken. He has consistently refused to clarify his statement. I have read his tiny little 10-page SMP document and there is absolutely nothing in that document which is at odds with my patch. Not one thing. I am angry because you and a number of others are not willing to take the work at face value and instead insist on making ridiculous extremist assumption into it and then using that opinion to justify not allowing the patch to go in. I have said, REPEATEDLY, but you can't seem to understand, that I am totally open to making adjustments to the code. I am not willing to throw it away, but I am willing to make adjustments to the code. I don't see why the patch should be held up. But you know something? You and others don't seem to be interested in having a conversation about possible improvements. You seem to be out for blood, to prevent the patch from going in at all costs no matter how good it is, without even having bothered to read or understand it, and that makes me unbelievably angry, I don't see how you can possibly tell me that I am not being a team player when I am being told to throw the code away completely. I think it's John who is not being the team player here, and others, who seem to believe that me throwing the code away is the correct solution to their ills. :I'll tell you why I want to commit the stuff in multple stages... :BECAUSE THAT ALLOWS THE CORE OF THE CODE TO BE TESTED WHILE THE :REMAINING STUFF CAN BE DISCUSSED AND/OR WORKED ON. That's why. : :One week of discussion will not prevent the code from being tested. Coming on THREE weeks now. Three weeks of my time wasted arguing with people who don't even bother to take the time to understand what I am trying to accomplish, who seem to regard a relatively straightforward patch as somehow being an attack on John's leadership when that could not be farther from the truth, who seem inclined to do nothing but bash me personally and supply opinions without one shred of evidence to back them up. I can't fight your opinion of me, but you can't expect me to listen to you if you can't justify it. You have already shown very clearly that you aren't even interested in looking at the code, that you don't care what it does, but you are against it anyway. You response is to ignore the points I have brought up and then start bashing me personally. :I am not going to spend months integrating change after change into :a patchset, having to retest the whole mess every time, and then only :after months commit the final result. : :I've counted weeks so far and those weeks weren't even necessary. Now :your expecting months and years of delay? Please. Gee, lets see... why don't YOU ask JOHN how long he intends to wait before he allows this sort of optimization to be made? Eh? Please. : :That is not how I work and I strongly oppose that kind of methodology. : :I think you've made that clear already. The question is whether you :are willing to compromise so you can be part of a team or not. That :means, for all of us, that we will not necessarily be able to work in :the way we would personally want, all of the time. That's what happens :in a group environment. That's life. This is not about being part of a team. You don't have to be forced into using someone else's methodology to be part of a team. Being part of a team means respecting other people's metholodogies and it means compromising. There has been no
Re: Patch for critical_enter()/critical_exit() interrupt assem
On Thu, 7 Mar 2002, Justin T. Gibbs wrote: The only way it will get delayed that long is if you spend all of your time stomping up and down, writing in all caps, and tell the rest of us that we have to follow the proceedures you think are appropriate. That's not how colaboration works. You need to compromise and not get all pissed off if the process requires you to delay your commit for a bit. Justin, the stuff was backed out when requested, and has been so for quite a while now. NOT ONE SINGLE of the dog-in-the-manger people has bothered to review it in that week. How long is he expected to wait? John has only said that somehow it aesthetically offends his sensibilities in some way. He has certainly not given any techincal reason to not do it, (that I have seen). How long is a person supposed to wait when no-one can give a reason that it should not be committed and there are reasons that it should? I prefer to work in smaller chunks and yet here you are trying to justify your nonsense by making further attacks on my methodologies and code that are completely absurd and completely unsupported by any evidence whatsoever. This is not an attack on any such thing. This is about *process*. You want to commit in small chucks? Great! You want to get this stuff into the tree quickly? Great! You want to make the code so it can be dynamically configured for testability? Great! You don't want to discuss API changes that have affects on other ports, and other subsystems prior to committing them? That's unacceptable when people are asking you to explain them first, regardless of how well thought out or implemented they are. This is not a race to commit. This is about developing a well designed system without killing each other in the process. Talk about puting words into another person's mouth! NOT ONE SINGLE PERSON has asked to discuss this work! Matt has already said that he would GLADLY discuss it with anyone who's interested to do so. So far tehre ahve been no takers. For this kind of change, the commit message should have been the last, not the first public forum to have expressed these ideas. It was being discusse on mailing lists as well as privatly before teh commit. Did I say anything about source files? This is about discussion prior to commit. Nothing more. How is expanding an API not a change to that API? Why is it that when other developers express a need to discuss these changes prior to them being committed, their concerns don't matter to you? Are you the only person in the project that can't keep changes in your local tree for a few days while you present your ideas? I have done no such thing. The facts are very simple: Looks very much like it from this direction. Matt: I'm going to commit this code to the tree Others: We should discuss the rammifications first. Matt: We can discuss it after its in. Others: No, we need to discuss it first. Matt: Commit Others: Back it out until we discuss it. Matt: !@#$%^$@#. Every time I try to do something its blocked Matt: Backs out: Ok so what do you want to discuss? Others: .. Oh, nothing really, we just felt like stuffing you around again. That's not how I perceived this at all. John needs to be active in the discussion on changes so they are resolved quickly and don't hold you back. I have not said one thing about your changes not going in other than to say that they should be discussed first. If John doesn't want to have to merge with you, he'll have to get his patches into the tree just like anyone else. No, Core has just said that he doesn't because he can over-rule any change in the kernel. And there is no requirement for him to justify it. Regardless, you have to be willing to discuss and perhaps refine your changes prior to committing them. This is true for anyone on the project, not just you. He's been sitting around for a week waiting for a single person to want to discuss it. I challenge you to find a single person who has asked to discuss the design. (and actually done it). It is only a huge deal because it was made into a huge deal. If the change had been discussed prior to being pushed into the tree, this would never have happened. I don't think that John, or anyone else, is opposed to the change going in once some small issues are discussed first. Just get over this I've been abused bit already and discuss the changes! We all want to move on and I'd rather see us moving on with your changes than without. No it's only a huge deal because OTHERS made it a huge deal. -- Justin To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
No, Core has just said that he doesn't because he can over-rule any change in the kernel. And there is no requirement for him to justify it. That is definately *NOT* what core has said. Please go re-read our announcement of the SMPng tech lead appointment. We specifically indicate that the tech lead is obligated to provide justification for any decisions that he may make. -DG David Greenman Co-founder, The FreeBSD Project - http://www.freebsd.org President, TeraSolutions, Inc. - http://www.terasolutions.com President, Download Technologies, Inc. - http://www.downloadtech.com Pave the road of life with opportunities. To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
On Thu, 7 Mar 2002, David Greenman wrote: No, Core has just said that he doesn't because he can over-rule any change in the kernel. And there is no requirement for him to justify it. That is definately *NOT* what core has said. Please go re-read our announcement of the SMPng tech lead appointment. We specifically indicate that the tech lead is obligated to provide justification for any decisions that he may make. So, where is it? -DG David Greenman Co-founder, The FreeBSD Project - http://www.freebsd.org President, TeraSolutions, Inc. - http://www.terasolutions.com President, Download Technologies, Inc. - http://www.downloadtech.com Pave the road of life with opportunities. To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
: :You came to the conclusion that only *your decision* on what was :an appropriate proceedure was the one that mattered. That's not :the way this project works. You can't act unilaterally. When people :ask you to hold off (and they even asked politely!) so discussion :can take place, that is not the time to commit. I did no such thing. Let me quote you from below: So, you see, I didn't just commit it out of nowhere. I waited what I believed to be a reasonable period of time. So your oppinion on what was a reasonable period of time was the only one that mattered. Q.E.D. I came to the conclusion because not a single goddamn person bothered to read the patch and instead the only argument I got was wait for John and John's only argument is I don't like the idea of optimizing this routine right now as if he would be the only one responsible for dealing with the consequences. Actually, John's reaction to the patch is a secondary issue. He wasn't even able to read the lists when this thing blew up. He could have fallen over backward with love for your changes and you still would have strewn cuss words all over our lists. [More talk about the irrelevant contents of the patch and 40 hours of work being thrown away paranoia.] I am angry because you and a number of others are not willing to take the work at face value and instead insist on making ridiculous extremist assumption into it and then using that opinion to justify not allowing the patch to go in. How many times do I have to say this? The patch is not the issue. Most likely it will be incorperated into the tree shortly. Yawn I'm sorry Matt, but even if these changes are gold lined, it doesn't change the fact that you acted unilaterally in a manner that is not conducive to team work. That it. That's really it. Now do you want me to go chew out John too. Okay. John isn't being super professional either. The fact that you started this doesn't change that. You both have done things that you shouldn't have. Now instead of trying to convince us that you are completely without reproach, why not move forward in some constructive manner? Aren't you out of breath yet? Aren't your fingers tired of typing the same old worn out argument, My code excuses my behavior! again and again? :One week of discussion will not prevent the code from being tested. Coming on THREE weeks now. Three weeks of my time wasted arguing with people who don't even bother to take the time to understand what I am trying to accomplish... This is your choice Matt. You may not realize it, but you are in control of how long this wears on. Gee, lets see... why don't YOU ask JOHN how long he intends to wait before he allows this sort of optimization to be made? Eh? Please. Hey John. Can you comment on whatever issues you have with the content of these changes? If the API is not compatible with what you are doing, please explain why and how those conflicts might be resolved. Assuming that these issues can be addressed and the optimization can be disabled via a configuration option, what further reasons are there to not allow this change to go into the tree? :That is not how I work and I strongly oppose that kind of methodology. : :I think you've made that clear already. The question is whether you :are willing to compromise so you can be part of a team or not. That :means, for all of us, that we will not necessarily be able to work in :the way we would personally want, all of the time. That's what happens :in a group environment. That's life. This is not about being part of a team. I've played hide and seek with people that feel this way. 1, 2, 3 Seems like a reasonable amount of time to me... Ha Ha found you! You don't have to be forced into using someone else's methodology to be part of a team. No, you have to accept the team's methodology in areas that affect the team. As we say in the States, your personal liberties end where they infrindge on mine. This is no different. The CVS repository is not yours to commit to any way you like. The team has a methodology for that and as soon as that methodology is broken, we fall into situations just like this one. This IS about team work, but you are barking up the wrong tree if you think I'm the one who's not being a team player here. I know you believe this. Just as you believed you were reasonable in committing that code when you were asked not to. Just as you continue to insist that the content of your patch is the issue here. I can't convince you otherwise, but perhaps I can convince you to drop your self righteousness for a bit so we can move on? I HAVE NEVER SAID THAT THE CODE COULD NOT BE CHANGED. Hello? Are you even listening to what I am saying? Actually? No. This isn't about the code, so your comments about it have absolutely no sway with me. You should save your breath for
Re: Patch for critical_enter()/critical_exit() interrupt assem
On Thu, 7 Mar 2002, Justin T. Gibbs wrote: The only way it will get delayed that long is if you spend all of your time stomping up and down, writing in all caps, and tell the rest of us that we have to follow the proceedures you think are appropriate. That's not how colaboration works. You need to compromise and not get all pissed off if the process requires you to delay your commit for a bit. Justin, the stuff was backed out when requested, and has been so for quite a while now. Sure. That doesn't excuse the fact that it was committed in the first place. NOT ONE SINGLE of the dog-in-the-manger people has bothered to review it in that week. How long is he expected to wait? So, after screaming obsenities at John and creating a big stink on our lists, you want to know why those in the know haven't felt like engaging Matt? I'm not saying that is an excuse, just a possible reason. I've already asked John to move this stuff along in my other mail, so don't try to pin me up as an obstructionist. He's been sitting around for a week waiting for a single person to want to discuss it. Hey, if Matt wants to believe that he can only be productive once his changes are committed, that is his problem. It is only a huge deal because it was made into a huge deal. If the change had been discussed prior to being pushed into the tree, this would never have happened. I don't think that John, or anyone else, is opposed to the change going in once some small issues are discussed first. Just get over this I've been abused bit already and discuss the changes! We all want to move on and I'd rather see us moving on with your changes than without. No it's only a huge deal because OTHERS made it a huge deal. So we're supposed to ignore it when Matt breaks the rules just because he is such a good hacker? Been there. Tried that. Now where here again. -- Justin To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
If memory serves me right, Justin T. Gibbs wrote: : :You came to the conclusion that only *your decision* on what was :an appropriate proceedure was the one that mattered. That's not :the way this project works. You can't act unilaterally. When people :ask you to hold off (and they even asked politely!) so discussion :can take place, that is not the time to commit. I did no such thing. Let me quote you from below: So, you see, I didn't just commit it out of nowhere. I waited what I believed to be a reasonable period of time. So your oppinion on what was a reasonable period of time was the only one that mattered. Q.E.D. I came to the conclusion because not a single goddamn person bothered to read the patch and instead the only argument I got was wait for John and John's only argument is I don't like the idea of optimizing this routine right now as if he would be the only one responsible for dealing with the consequences. Actually, John's reaction to the patch is a secondary issue. He wasn't even able to read the lists when this thing blew up. He could have fallen over backward with love for your changes and you still would have strewn cuss words all over our lists. [More talk about the irrelevant contents of the patch and 40 hours of work being thrown away paranoia.] I am angry because you and a number of others are not willing to take the work at face value and instead insist on making ridiculous extremist assumption into it and then using that opinion to justify not allowing the patch to go in. How many times do I have to say this? The patch is not the issue. Most likely it will be incorperated into the tree shortly. Yawn I'm sorry Matt, but even if these changes are gold lined, it doesn't change the fact that you acted unilaterally in a manner that is not conducive to team work. That it. That's really it. Now do you want me to go chew out John too. Okay. John isn't being super professional either. The fact that you started this doesn't change that. You both have done things that you shouldn't have. Now instead of trying to convince us that you are completely without reproach, why not move forward in some constructive manner? Aren't you out of breath yet? Aren't your fingers tired of typing the same old worn out argument, My code excuses my behavior! again and again? :One week of discussion will not prevent the code from being tested. Coming on THREE weeks now. Three weeks of my time wasted arguing with people who don't even bother to take the time to understand what I am trying to accomplish... This is your choice Matt. You may not realize it, but you are in control of how long this wears on. Gee, lets see... why don't YOU ask JOHN how long he intends to wait before he allows this sort of optimization to be made? Eh? Please. Hey John. Can you comment on whatever issues you have with the content of these changes? If the API is not compatible with what you are doing, please explain why and how those conflicts might be resolved. Assuming that these issues can be addressed and the optimization can be disabled via a configuration option, what further reasons are there to not allow this change to go into the tree? :That is not how I work and I strongly oppose that kind of methodology. : :I think you've made that clear already. The question is whether you :are willing to compromise so you can be part of a team or not. That :means, for all of us, that we will not necessarily be able to work in :the way we would personally want, all of the time. That's what happens :in a group environment. That's life. This is not about being part of a team. I've played hide and seek with people that feel this way. 1, 2, 3 Seems like a reasonable amount of time to me... Ha Ha found you! You don't have to be forced into using someone else's methodology to be part of a team. No, you have to accept the team's methodology in areas that affect the team. As we say in the States, your personal liberties end where they infrindge on mine. This is no different. The CVS repository is not yours to commit to any way you like. The team has a methodology for that and as soon as that methodology is broken, we fall into situations just like this one. This IS about team work, but you are barking up the wrong tree if you think I'm the one who's not being a team player here. I know you believe this. Just as you believed you were reasonable in committing that code when you were asked not to. Just as you continue to insist that the content of your patch is the issue here. I can't convince you otherwise, but perhaps I can convince you to drop your self righteousness for a bit so we can move on? I HAVE NEVER SAID THAT THE CODE COULD NOT BE CHANGED. Hello? Are you even listening
Re: Patch for critical_enter()/critical_exit() interrupt assem
:Yes. What I would like and what I mentioned before is for this to be :hidden behind cpu_critical_enter/exit. Specifically, cpu_critical_enter :would be a null macro for i386, which would turn critical_enter into :just an increment, as Matt wanted. cpu_critical_exit would do all the :magic of unpending interrupts. The reason for this is that I would :like to see critical_exit handled any pending preemptions for a thread. :We do not yet know exactly how this will work so I would like this to be :done in MI code to start with. The code must not make assumptions that are :not valid on all platforms. This is easiest if they use the same code. :This is not about duplication of code in several MD files. We can't, because the MI code makes some rather blatent assumptions in regards to cpu_critical_enter and exit. Take the witness code, for example. So we cannot safely null-out those macros. In fact, the MI code also makes some rather blatent assumptions in regards to critical_enter() and exit which I have also had to clean up (and which will need considerably more cleanup). These assumptions include, just as an example: The witness code calling cpu_critical_enter()/exit without holding td_critnest count, and Peter's now withdrawn code which explicitly released the cpu critical section while holding a non-zero td_critnest count. In otherwords, really aweful hacks. So we can't just NULL out those inlines. Trying to keep critical_enter()/critical_exit() MI and cpu_critical_enter()/cpu_critical_exit() MD and separated doesn't make much sense to me, because frankly critical_enter() and critical_exit() are tiny little routines (and will remain tiny even after SWI and preemption is added) and I can't think of any reason to force higher complexity in the routines due to the separation. In short, critical_enter()/critical_exit() are TIGHTLY INTEGRATED with cpu_critical_enter/exit despite your attempt to make critical_enter/exit MI. What my patch does is accept as true the fact that the two sets of routines are, in fact, tightly integrated, and then implements them in a more sensible way (or, more to the point, allows each architecture to implement them in the proper manner as befits the architecture). The API's are still MI, but the implementation is MD. :Bruce also had some comments which were shrugged off, I thought they :were important. Specifically, please do not make unnecessary changes :to the assembler code. Macros do not need to be defined before they :are used, I believe this was the justification for some of the reordering :in apic_vector.s which makes the patch confusing. Please do not tab :out the ; \ at the end of the lines in the INTR and FAST_INTR macros :in icu_vector.s. This just makes unnecessary diffs. The PUSH_DUMMY :macro must push a reasonable eip value, in addition to the code segment, :so that profiling and stack traces work right. If you have not already, :please make sure that a trace from inside an interrupt handler that was :unpended looks somewhat normal. Please also make sure that kgdb is able :to decode the frame properly. It assumes that the eip of the frame will :be near certain kernel symbols in order to determine what kind of frame :it is. : :Jake Actually all I did there was square up icu_vector.s so it looked almost the same as apic_vector.s. I would consider that an improvement. Besides, I find it very difficult to work on those macros without tabbing out the backslashes. I moved some of the #defines around due to it otherwise being a forward reference. This turned out not to be an issue since the macros aren't actually used until later in the code, but I don't like forward referenced macros anyway so I left it in. Insofar as diffs go, I'm afraid even the most conservative changes to the assembly to support interrupt deferral would make for a pretty big diff set. I'm not going to change them now, no, but I don't see this as being a show stopper in the least. -Matt Matthew Dillon [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
At 7:04 PM -0800 3/7/02, Matthew Dillon wrote: :Bruce also had some comments which were shrugged off, I thought they :were important. Specifically, please do not make unnecessary changes :to the assembler code. Macros do not need to be defined before they :are used, I believe this was the justification for some of the :reordering in apic_vector.s which makes the patch confusing. Please :do not tab out the ; \ at the end of the lines in the INTR and :FAST_INTR macros in icu_vector.s. This just makes unnecessary diffs. : :Jake Actually all I did there was square up icu_vector.s so it looked almost the same as apic_vector.s. I would consider that an improvement. Perhaps that part of the update could be considered cosmetic, and thus be done as a separate update -- just so people can tell which lines are cosmetic changes and which ones are substantive. That is more work for you, but it makes it easier on reviewers, and it is certainly consistent with what developers are asked to do on other updates they make. You'll still probably have a debate on the wonderfulness of that cosmetic change, but at least it makes it easier for the major changes to looked at and commented on separately. -- Garance Alistair Drosehn= [EMAIL PROTECTED] Senior Systems Programmer or [EMAIL PROTECTED] Rensselaer Polytechnic Instituteor [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
On Thu, 7 Mar 2002, Justin T. Gibbs wrote: Then do the right things so it will. Unfortunatly that has been proven to not work. after reverting the change and silently waiting for a week 1/ no person bothered to review it. 2/ people assumed the patch had gone away. To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
In message [EMAIL PROTECTED] Julian Elischer writes: : : : On Thu, 7 Mar 2002, Justin T. Gibbs wrote: : : Then do the right things so it will. : : Unfortunatly that has been proven to not work. : : after reverting the change and silently waiting for a week : 1/ no person bothered to review it. : 2/ people assumed the patch had gone away. Ummm, There are reviews in the archives that object to the API as it relates to optimization and those objections haven't been sanely answered with anything more constructive than BS. Warner To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
those reviews object on a purely subjective manner. I think this is premature I don't consider that to be a technical review? do you? they do not even comment on the bug-fixing aspect of the patch. On Thu, 7 Mar 2002, Warner Losh wrote: In message [EMAIL PROTECTED] Julian Elischer writes: : : : On Thu, 7 Mar 2002, Justin T. Gibbs wrote: : : Then do the right things so it will. : : Unfortunatly that has been proven to not work. : : after reverting the change and silently waiting for a week : 1/ no person bothered to review it. : 2/ people assumed the patch had gone away. Ummm, There are reviews in the archives that object to the API as it relates to optimization and those objections haven't been sanely answered with anything more constructive than BS. Warner To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
On 07-Mar-02 Matthew Dillon wrote:
:Search for paper John Baldwin and find link 6:
:
:http://www.FreeBSD.org/cgi/getmsg.cgi?fetch=199282+204026+/usr/local/www/db/
:text/2002/freebsd-arch/20020303.freebsd-arch
:
:The actual paper is at:
: http://www.FreeBSD.org/~jhb/smpng/design/article.{ps,pdf}
:
:-J
:--
:Jeroen C. van Gelderen - [EMAIL PROTECTED]
Ok... I've read it. The sections on interrupts and critical sections
are fully compatible with my patch. The one section... basically
the last sentence of the last paragraph, is exactly the piece that
my patch cleans up and makes more flexible. Instead of requiring that
cpu_critical_*() always be used for the initial critical_enter() my
patch makes it optional, and for I386 I use that flexibility to allow
critical_*() to NOT have to call cpu_critical_*().
You seemed to have missed the entire part where we handle deferred preemptions
in MI code in critical_exit(). The critical_enter/exit stuff really exists to
support the preemption code and to get rid of the various FOO_NOSWITCH flags.
I think it is ok to remove the linkage between critical_enter/exit and
cpu_critical_* (possibly even renaming cpu_critical_* to a better name) and to
allow arch's to optimize cpu_critical_* but leave critical_* as MI code.
That's what I've asked for from the start and Jake even suggested it from the
start.
I'm still not comfortable with the optimiation, but changing the MI critical_*
code is by far my biggest objection to the code.
-Matt
Matthew Dillon
[EMAIL PROTECTED]
--
John Baldwin [EMAIL PROTECTED]http://www.FreeBSD.org/~jhb/
Power Users Use the Power to Serve! - http://www.FreeBSD.org/
To Unsubscribe: send mail to [EMAIL PROTECTED]
with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
On Thu, 7 Mar 2002, Warner Losh wrote: In message [EMAIL PROTECTED] Julian Elischer writes: : : : On Thu, 7 Mar 2002, Justin T. Gibbs wrote: : : Then do the right things so it will. : : Unfortunatly that has been proven to not work. : : after reverting the change and silently waiting for a week : 1/ no person bothered to review it. : 2/ people assumed the patch had gone away. Ummm, There are reviews in the archives that object to the API as it relates to optimization and those objections haven't been sanely answered with anything more constructive than BS. The primary objections I've seen from Jake, and he posted them as part of the earlier thread prior to the commit, was that the API changes proposed by Matt don't make sense for the sparc64 implementation, uni-processor or multi-processor, and that while these changes might be appropriate for i386, he wanted to see the APIs set up in such a way that the differences in architectures were hidden in the MD code. This suggests working some more on the API before moving on, and my reading of earlier posts in the thread from John was that that was what he had in mind also. I don't pretend to understand all the issues here, but I think it's important to recognize that there have been several coherrent responses to the current patch that do need to be addressed. I think the preference I've seen from a number of developers is that the be addressed before the commit, rather than after. Robert N M Watson FreeBSD Core Team, TrustedBSD Project [EMAIL PROTECTED] NAI Labs, Safeport Network Services To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
: :In otherwords. You acted unilaterally. You seem to be making my :arguments for me. Again. Thanks! No, I am not acting unilaterally, but neither do I feel it is appropriate to be told to wait indefinitely (and I am STILL waiting by the way!). When I commit something it isn't set in stone. If there's a problem with the commit, or it doesn't integrate just right with something else, etc etc etc... I have no problem discussing and modifying the source. To me, the commit is a way to stage the work out, not the final chapter. This may not have been discussed with John prior to my wanting to commit it, but it was discussed. It was discussed on the lists, in public, and the discussion was quite reasonable in my view until I tried to commit it. Now contrast that with, say, Peter's PMAP commit. No discussion on the public lists AT ALL, no warning, no heads up, nothing. Did anyone complain? No. Contrast that commit with my critical_*() work. (Now, in fact, I am not faulting Peter nor saying that he should not have done the commit. I am simply contrasting it with my own work). My making a commit is not only not acting unilaterally, it is not setting the work in stone and it is not terminating debate or review either. It is just a stepping stone and I believe it is wholely inappropriate for you or anyone else to ask that a single stone not even be set down on the table when I have an armful of them without giving a reasonable reason for the request. You are at fault for not researching the situation before opening your mouth, and that you get burned for it is nobody's fault but your own. You are at fault for misunderstanding the purpose of the commit and ignoring my repeated statements regarding further discussion. You are at fault for assuming that my making this commit somehow means that I am not willing to continue a reasoned debate on the issues involved. Justin, you need to take a step back and really think about what you are tring to prove here. -Matt To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
:: after reverting the change and silently waiting for a week :: 1/ no person bothered to review it. :: 2/ people assumed the patch had gone away. : :Ummm, There are reviews in the archives that object to the API as it :relates to optimization and those objections haven't been sanely :answered with anything more constructive than BS. : :Warner I think John has a right to object to the work based on it being an optimization, but that should not sufficient reason in anyone's book to veto a commit, especially something that is as straightforward and obvious as I believe my work to be in this case. Unlike John, I am not the type of person who leaves hundreds of kilobytes of patches laying around my tree. For me completed work is either committable, or it should be thrown away. John's other objections - in regards to interference with future work, are completely unsubstantiated. He has not explained any reasoning for his objections AT ALL other then to make vague, undirected comments about how it doesn't fit with his idea of the world. He pointed me to a 10-page mini paper and I even after reading it I do not see how any of my work inteferes with anything he is doing. Not a single person beyond John has voiced ANY objection to work beyond the wait for John to get back objection and the talk to John objection. Not one person. -Matt Matthew Dillon [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
:You seemed to have missed the entire part where we handle deferred preemptions :in MI code in critical_exit(). The critical_enter/exit stuff really exists to :support the preemption code and to get rid of the various FOO_NOSWITCH flags. :I think it is ok to remove the linkage between critical_enter/exit and :cpu_critical_* (possibly even renaming cpu_critical_* to a better name) and to :allow arch's to optimize cpu_critical_* but leave critical_* as MI code. :That's what I've asked for from the start and Jake even suggested it from the :start. : :I'm still not comfortable with the optimiation, but changing the MI critical_* :code is by far my biggest objection to the code. : :John Baldwin [EMAIL PROTECTED]http://www.FreeBSD.org/~jhb/ Who said anything about not handling deferred preemptions in MI code in critical_exit()? You mean just because I am moving less then 30 lines of code from MI to MD you object to the concept? What is so difficult about having the code, in the MD source files, do this: if (td-whatever_preemption_request) mi_function_to_deal_with_preemption(); We do this sort of thing all the time in MD code. It's useful because it allows us to focus a critical piece of code in a single source file rather then forcing us to split the critical piece of code into three or four source files as your current code does... all in the name of placing a bare 30 lines of code (less!) in kern/ instead of in arch/.../? Because in my view that is ALL you are talking about here... I don't see why the above code has to be in an MI procedure. It can just as easily be in an MD implementation of critical_*() and I believe the implementation of the API is far easier to work with and far more flexible with these two procedures sitting in MD rather then MI. I don't see how my code can possibly interfere with deferred preemption. Two lines of code in arch does not constitute interference, and I am certainly willing to deal with that myself... you don't have to lift one bloody finger. All you need to do is write your MI preemption handling code and you would have to do that in any case. Look at all the hacks you ALREADY have to deal with the split you impose between critical_*() and cpu_critical_*(). Look at the hack peter did and look at the incorrect assumptions you already have made in MI code due to the way you constructed the original API (hard interrupt disablement). And, most of all, I don't see how something so trivial should result in you vetoing my commit. I mean, it's no skin off your nose if down the line it turns out that critical_*() gets complex enough to warrent an MI split, because I would be the one doing it. But, right now, even with the preemption stuff you are talking about, there is NO REASON to keep a forced split and plenty of reasons to move it to MD. -Matt Matthew Dillon [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
:The primary objections I've seen from Jake, and he posted them as part of :the earlier thread prior to the commit, was that the API changes proposed :by Matt don't make sense for the sparc64 implementation, uni-processor or :multi-processor, and that while these changes might be appropriate for :i386, he wanted to see the APIs set up in such a way that the differences Huh? I have made no API changes that have any effect whatsoever on these architectures. Jake is free to implement whatever he wants for them, all I intend to do is make it easier and more flexible to do so. Sure, I intend to move critical_enter() and critical_exit() from MI to MD, but all that means for non-i386 architectures is that the EXACT procedures for critical_enter() and critical_exit() now sitting in kern/kern_switch.c will be copied into arch/arch/critical.c. That's the only effect for these other architectures. How our architectures handle things like deferred interrupts and SWI is architecture specific. It always has been and always will be. That doesn't mean we have to duplicate a large piece of code in arch/arch/critical.c (for example, SWI handling or deferred context switch support), it simply means that these functions will be written in an MI section and the MD critical_*() functions will simply call the MI functions. This work will come to a grand total of 2 or 4 lines of code per architecture. Big fucking deal! -Matt Matthew Dillon [EMAIL PROTECTED] :I don't pretend to understand all the issues here, but I think it's :important to recognize that there have been several coherrent responses to :the current patch that do need to be addressed. I think the preference :I've seen from a number of developers is that the be addressed before the :commit, rather than after. : :Robert N M Watson FreeBSD Core Team, TrustedBSD Project :[EMAIL PROTECTED] NAI Labs, Safeport Network Services To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
On Thu, Mar 07, 2002 at 02:43:36PM -0700, Warner Losh wrote: In message [EMAIL PROTECTED] Julian Elischer writes: : : : On Thu, 7 Mar 2002, Justin T. Gibbs wrote: : : Then do the right things so it will. : : Unfortunatly that has been proven to not work. : : after reverting the change and silently waiting for a week : 1/ no person bothered to review it. : 2/ people assumed the patch had gone away. Ummm, There are reviews in the archives that object to the API as it relates to optimization and those objections haven't been sanely answered with anything more constructive than BS. They are BS. Saying it's not good because it's an optimization is not a technical argument. I've already mentionned this before but I'm going to do it one more time, for the record: optimizations can be anything in SMPng. Heck, you could say that locking down a subsystem is an optimization. Even doing lockdowns (what John wants everyone to be doing) is something that is likely to change in the future. You can't prevent API changes from happening, they are part of regular evolutionary steps. Just look at what happened to the mbuf subsystem. I initially locked it down and it has changed an incredible amount. The API changed, other things changed. It's normal because it's part of regular code evolution. Therefore, you can't just say that the work some of us are doing is an optimization and that because it is that we shouldn't be doing it right now. These are things that have been in the TODO for a while now and are part of the general direction of this project and delaying them by claiming that it's too early and that the API might change is just preventing them from getting tested. Sure, the API may change. That's normal. You can't possibly get the API right at step 1. But you _can_ get the backend stuff at least working in the right direction. The same thing happened to our mutex code. The API totally changed (I cleaned it up myself). But does that mean that all the initial work done on mutex locks was wrong? I hope not. Without it, we never could have evolved to where we have. I've looked at both JHB's paper and Matt's patch. To me, it doesn't look like the direction we're supposed to be headed in is wrong. I don't see a technical argument (yet) saying otherwise besides for this it's an optimization and shouldn't go in thing. Please, if there is one, state it. The same applies to the ithread lightweight stuff I've been working on. Warner -- Bosko Milekic [EMAIL PROTECTED] [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
On Tuesday, 5 March 2002 at 9:43:29 -0800, Matthew Dillon wrote: phk wrote: you have a right to bully the only person who have consistently chugged away at the SMPng project when practically everybody else (you included) defected. This is an extreme misrepresentation of the facts. I stated very clearly at the original Yahoo SMP summit that I would soon not be available. I did what work I could before I became unavailable, and then I was, SURPRISE! Unavailable for 2 years! I did not abandon anyone. I wrote that code that is the basis for allowing us to remove Giant from syscalls, I wrote the original idle process code including all the hard assembly stuff. I cleaned up the pre-SMPng SPL masks (cpl and cml). I did what I could in the time I had. I've got to defend Matt here. This happened exactly the way he says. This in no way detracts from the work John has done, of course. Greg -- See complete headers for address and phone numbers To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
On Thursday, 7 March 2002 at 14:30:47 -0800, Matthew Dillon wrote: after reverting the change and silently waiting for a week 1/ no person bothered to review it. 2/ people assumed the patch had gone away. Ummm, There are reviews in the archives that object to the API as it relates to optimization and those objections haven't been sanely answered with anything more constructive than BS. Warner I think John has a right to object to the work based on it being an optimization, but that should not sufficient reason in anyone's book to veto a commit, especially something that is as straightforward and obvious as I believe my work to be in this case. I think this is a good point. We're in a development phase now, and we shouldn't step on each others' feet. But Matt has gone to some trouble to ensure it can be turned off at will. Come a release, it's possible that the project manager might decide to chop it out again, but I'm betting on it staying. Another issue: sure, it's partially an optimization. Sure, it contains MD code. But it also fixes bugs. Should a policy of structure now, optimize later mean we should reject bug fixes which also perform better? And will we, in the long term, be able to eliminate MD code and still have good performance? I think the issue of i386 only is a non-issue. It has to start somewhere, and it doesn't break the other platforms. Unlike John, I am not the type of person who leaves hundreds of kilobytes of patches laying around my tree. For me completed work is either committable, or it should be thrown away. Without prejudice to John (I don't know how much uncommitted stuff he has), I think that Matt's right here too. Where practicable, smaller increments are easier to handle. John's other objections - in regards to interference with future work, are completely unsubstantiated. He has not explained any reasoning for his objections AT ALL other then to make vague, undirected comments about how it doesn't fit with his idea of the world. He pointed me to a 10-page mini paper and I even after reading it I do not see how any of my work inteferes with anything he is doing. Core has asked John to describe his objections. Based on the current flam^H^H^H^Hdiscussion, I'm sure people will agree that it's probably better to discuss things in a smaller group first. The results will, of course, be made known. Greg -- See complete headers for address and phone numbers To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
:Perhaps that part of the update could be considered cosmetic, and :thus be done as a separate update -- just so people can tell which :lines are cosmetic changes and which ones are substantive. That is :more work for you, but it makes it easier on reviewers, and it is :certainly consistent with what developers are asked to do on other :updates they make. : :You'll still probably have a debate on the wonderfulness of that :cosmetic change, but at least it makes it easier for the major :changes to looked at and commented on separately. : :-- :Garance Alistair Drosehn= [EMAIL PROTECTED] Ugh. I'd rather not. It would take a while to unwind the tabbing from the deferred interrupts. i.e. I didn't just cleanup the syntax in icu_vector.s, I also had to implement the same stuff I put in apic_vector.s to support deferred interrupts. -Matt To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
Apparently, On Thu, Mar 07, 2002 at 07:04:49PM -0800, Matthew Dillon said words to the effect of; :Yes. What I would like and what I mentioned before is for this to be :hidden behind cpu_critical_enter/exit. Specifically, cpu_critical_enter :would be a null macro for i386, which would turn critical_enter into :just an increment, as Matt wanted. cpu_critical_exit would do all the :magic of unpending interrupts. The reason for this is that I would :like to see critical_exit handled any pending preemptions for a thread. :We do not yet know exactly how this will work so I would like this to be :done in MI code to start with. The code must not make assumptions that are :not valid on all platforms. This is easiest if they use the same code. :This is not about duplication of code in several MD files. We can't, because the MI code makes some rather blatent assumptions in regards to cpu_critical_enter and exit. Take the witness code, for example. So we cannot safely null-out those macros. In fact, the MI code also makes some rather blatent assumptions in regards to critical_enter() and exit which I have also had to clean up (and which will need considerably more cleanup). I agree that the use of cpu_critical_enter/exit could use cleaning up. Can you give an example of where critical_enter is used improperly? You mean in fork_exit? Your changes to cpu_switch solve that problem with critical_exit almost unchanged. The savecrit stuff should really just be made MD. If cpu_critical_exit was changed to take the thread as its argument as I suggested before this would be easy. Specifically I think that all of the current uses of cpu_critical_enter exit outside of critical_enter exit are bugs. The use in ktr is bogus because the increment of ktr_idx is done atomically. I don't know why this was there in the first place. I think that witness is an example of where we need a different specifically defined to be hard interrupt disable api. This is why I suggested the intr_disable/intr_restore api, which should only be used in MD code and in dire circumstances otherwise, of which this case qualifies. The code in ast is just structured wrong. I think that before the loop was in assembler outside of the routine itself, so that it didn't make so many assumptions about interrupt disablement. doreti which calls it should look like this: loop: cli; if (there is an ast we can handle) goto ast; iret; ast: sti; call ast; goto loop; In which case ast doesn't need to loop or use critical sections at all. All of the MD code I could find I think should use a different api for hard interrupt disablement. They are all very MD and need this specifically, which cpu_critical_enter need not necessarily do. With these changes I don't see why the critical_enter/exit functions can't or shouldn't remain MI. These assumptions include, just as an example: The witness code calling cpu_critical_enter()/exit without holding td_critnest count, and Peter's now withdrawn code which explicitly released the cpu critical section while holding a non-zero td_critnest count. In otherwords, really aweful hacks. So we can't just NULL out those inlines. Trying to keep critical_enter()/critical_exit() MI and cpu_critical_enter()/cpu_critical_exit() MD and separated doesn't make much sense to me, because frankly critical_enter() and critical_exit() are tiny little routines (and will remain tiny even after SWI and preemption is added) and I can't think of any reason to force higher complexity in the routines due to the separation. In short, critical_enter()/critical_exit() are TIGHTLY INTEGRATED with cpu_critical_enter/exit despite your attempt to make critical_enter/exit MI. What my patch does is accept as true the fact that the two sets of routines are, in fact, tightly integrated, and then implements them in a more sensible way (or, more to the point, allows each architecture to implement them in the proper manner as befits the architecture). I do not agree that having cpu_critical_enter separate in any way hinders an architecture's ability to implement critical_enter properly. The MI code that calls it expects it to do certain things, one of them is to call cpu_critical_enter and cpu_critical_exit exactly once for each non-nested critical_enter exit pair. Wether or not this actually disables interrupts is up to the MD code. The API's are still MI, but the implementation is MD. :Bruce also had some comments which were shrugged off, I thought they :were important. Specifically, please do not make unnecessary changes :to the assembler code. Macros do not need to be defined before they :are used, I believe this was the justification for some of the reordering :in apic_vector.s which makes the patch
Re: Patch for critical_enter()/critical_exit() interrupt assem
On 05-Mar-02 Matthew Dillon wrote: : It makes no sense whatsoever to me to be told not to commit something : due to stale code that may not be quite compatible sitting in P4 that : you can't make work in any reasonable time frame. You should stop : trying to screw over my work and instead look to your own. You have : so many balls in the air constructed in a fine mesh that you can't seem : to accept a single change to your perfectly meshing subsystems, half : of which are going stale in P4. This is greatly restricting your : ability to consider deviation. : :*sigh* The only reason I'm not spending my cycles tracking down the :preemption :bugs so that preemption can go in is that I have decided that at the moment :there is more gain to be found by doing actual locking work. This means that :preemption will eventually go in, just Not Right Now. To that extent, I :don't :think premature optimizations based on observations of a kernel locked :entirely :by Giant that alter the API's preemption will change (and that were :originally :introduced when I committed all the bits of the preemption code that I could :w/o breaking the kernel) should go in. Those are your cycles, not mine. Why should I be forced to sit on my heals for an unknown period of time (but so far 3 weeks waiting for the ucred changes and 2 weeks waiting for critical_*()), twiddling my thumbs wasting MY cycles just because of your own scheduling issues? That's really the crux of this whole situation. I don't think it is appropriate for you to impose your development methodology on me or anyone else, and I most especially do not think it is appropriate for you to arbitrarily hold off the hard work that I have done in order to fit it into your schedule. I have said very clearly what I intend these critical_*() patches to do. I have said, repeatedly, that I do not believe they would intefere with your work in any significant manner. You have yet to explain in any detail why you think they would. I have said, repeatedly, that I am perfectly willing to work with you later on when you ARE ready to move forward on your own work. That's the crux of the situation, John. I do not believe you have the right to hold this work off, at least not based on any of the explanations you have given so far. Have you read the paper I posted to arch? It quite clearly (I thought) explained the role of the critical_* and the cpu_critical_* in the preemption code. It should be rather obvious from that why I don't want the critical_* stuff to change from their current model. I also think that just as it is too early to optimize to light weight ithread switches (sparc64 is going to optimize all kthread switches, not just ithreads by using a more general approach, and we may do that on other arch's as well) it is too early to optimize and add complexity for certain API's when we aren't sure what effect they will really have on the final product. For example, right now Giant blocks almost all of the kernel so we are going to contest it on almost every case. Contesting involves grabbing the sched_lock which can result in executing the critical section implementation more than we will end up doing later. I would rather wait on optimizations until the system is farther along and we can judge what things really need optimization and which ones don't. Optimizations are usually a tradeoff as far as increased complexity vs. performance and I personally at least would like to keep things simple for the time being. However, I would not be opposed to the code if it fit into the design in the document I posted to arch. A couple of notes though based on a quick preliminary glance at the code: - The swi code has been changed to not be limited to a bitfield so that it can support an arbtirary number of swi's. Right now we still support a small number but we may end up with several netisr's for example. We also may move away from scheduling netisr's via swi_sched anyways and using a semaphore or some such instead. Presently your patch goes back to assuming a fixed number of SWI's. - More of a minor niglet that is easy to be fixed: you added MD fields to the MI pcpu bits of struct pcpu. MD fields belong in the PCPU_MD_FIELDS macro in sys/i386/include/pcpu.h. If you intend for the fields to be MI, then that is more of a problem as it assumes too much about interrupts for different platforms. Alpha interrupts will not easily fit into a simple bitmask for example. -- John Baldwin [EMAIL PROTECTED]http://www.FreeBSD.org/~jhb/ Power Users Use the Power to Serve! - http://www.FreeBSD.org/ To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
:Have you read the paper I posted to arch? It quite clearly (I thought) :explained the role of the critical_* and the cpu_critical_* in the preemption :code. It should be rather obvious from that why I don't want the critical_* I'm sorry John, I have no idea what you are refering to here. You have given no reference that I can lookup... you have hundreds of postings on arch. Why don't you just post the reasoning? Nothing I have seen anywhere so far would create an issue. I've heard you repeatedly reference documents but I have no idea which documents you are talking about or which portion of said documents you are refering to. So instead of continuing to post ghost references why don't you just lay the problem out as you see it in this thread and explain why you do not believe the critical section work I am doing would work with it. :stuff to change from their current model. I also think that just as it is too :early to optimize to light weight ithread switches (sparc64 is going to :optimize all kthread switches, not just ithreads by using a more general We have very different development models, and different priorities. I'm not sure why you are attempting to impose your development model and priorities on me. This is the work I want to do. It's my time here, not yours, and if you believe that the work will make your job harder in some future unspecified commit then you have only to bring up the issue down the road when you are actually ready to work on that future unspecified commit and we can work it out. But I don't think it's appropriate for you to impose such a strict set of requirements based on work that does not exist in -current anywhere as far as I can tell. :approach, and we may do that on other arch's as well) it is too early to :optimize and add complexity for certain API's when we aren't sure what effect :they will really have on the final product. For example, right now Giant :blocks almost all of the kernel so we are going to contest it on almost every :case. Contesting involves grabbing the sched_lock which can result in :executing the critical section implementation more than we will end up doing :later. I would rather wait on optimizations until the system is farther along :and we can judge what things really need optimization and which ones don't. Again, different development models. I see a set of APIs being severely misused by commits, the most recent being Peter's-that-he-backed-out, and I see a considerable need down the road for an efficient critical section API. You may not be interested in doing the work on these now but you != me. This work is my interest. Again, I don't understand why you are trying to impose your personal tastes on me. :A couple of notes though based on a quick preliminary glance at the code: : :- The swi code has been changed to not be limited to a bitfield so that it can : support an arbtirary number of swi's. Right now we still support a small : number but we may end up with several netisr's for example. We also may move : away from scheduling netisr's via swi_sched anyways and using a semaphore or : some such instead. Presently your patch goes back to assuming a fixed number : of SWI's. I see an swi scheduler, which has nothing to do with the critical section work I have done. If there is other SWI stuff I don't see it in the -current tree. Nor does the work I have done in any way prevent the use of or in any way make it more difficult to use a list-based SWI queueing system instead of a bitmap. The bitmap stuff in my critical section patch only applies to i386... it's a specific implementation for soft and statclock IPI forwarding for the deferred case, nothing more. I didn't even bother to integrate AST (saving that for later work). It does not in any way prevent one from redoing that bit of code or even simply keeping the code and adding an SWI queue feature - something that could in fact be done either MD or MI depending on the needs of the architectures. If you believe there to be a problem here, perhaps explaining the SWI mechanism you are refering to would help. I am confident that it would not impose anything incompatible with the work I've done. :- More of a minor niglet that is easy to be fixed: you added MD fields to the MI : pcpu bits of struct pcpu. MD fields belong in the PCPU_MD_FIELDS macro in : sys/i386/include/pcpu.h. If you intend for the fields to be MI, then that is : more of a problem as it assumes too much about interrupts for different : platforms. Alpha interrupts will not easily fit into a simple bitmask for : example. : :-- : :John Baldwin [EMAIL PROTECTED]http://www.FreeBSD.org/~jhb/ These fields do in fact belong in MD PCPU_MD_FIELDS. I didn't notice it, perhaps because there is not a single
Re: Patch for critical_enter()/critical_exit() interrupt assem
On Wed, 6 Mar 2002, Jeroen C.van Gelderen wrote: Search for paper John Baldwin and find link 6: A good start though incomplete. Unfortunate that it took such a fight to get it to be written. Hopefully it's existance can prevent soem further bloodshed. Is it possible for other people to add to this or is it a closed document? To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
:Search for paper John Baldwin and find link 6:
:
:http://www.FreeBSD.org/cgi/getmsg.cgi?fetch=199282+204026+/usr/local/www/db/
:text/2002/freebsd-arch/20020303.freebsd-arch
:
:The actual paper is at:
: http://www.FreeBSD.org/~jhb/smpng/design/article.{ps,pdf}
:
:-J
:--
:Jeroen C. van Gelderen - [EMAIL PROTECTED]
Ok... I've read it. The sections on interrupts and critical sections
are fully compatible with my patch. The one section... basically
the last sentence of the last paragraph, is exactly the piece that
my patch cleans up and makes more flexible. Instead of requiring that
cpu_critical_*() always be used for the initial critical_enter() my
patch makes it optional, and for I386 I use that flexibility to allow
critical_*() to NOT have to call cpu_critical_*().
-Matt
Matthew Dillon
[EMAIL PROTECTED]
To Unsubscribe: send mail to [EMAIL PROTECTED]
with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
:stuff to change from their current model. I also think that just as it :is too early to optimize to light weight ithread switches (sparc64 is :going to optimize all kthread switches, not just ithreads by using a more :general We have very different development models, and different priorities. [...] This same issue came up at the BSDCon developers conference in regard to ithreads. Is it better to optimize some bit of code because it is the fun and interesting thing to do, or to build a simple, yet stable and easily verified foundation, that we can later optimize in a controlled manner? This is not about whether these particular changes are correct. The concern is that they may contain a subtle bug that makes it difficult to verify other portions of the system. I don't think anyone believes that we are at a point in current where we can convince ourselves that the system does work, no matter how slowly, in all cases. If we start to optimize before we reach such a point, how will we ever determine the robustness of the system? An optimization put in place today may have a bug or may expose a bug somewhere else in the system. In the current situation it is difficult to know which scenario is true (bug or side-effect) because we don't have a solid, verified, foundation. My 2 cents? Work with John to get the APIs that affect this particular code stable. That means discussion and perhaps that discussion will take some time (if this blow-up hadn't occurred the discussion would already be over now ;-). Once the APIs are in place, commit your code in a format that makes it completely optional, just like your Giant lock optimizations. This means that those interested in validating and determining the impact of your code can easily do so by flipping a switch. Those who would rather debug their own subsystem problems in a slower, but simpler, environment can do so too. -- Justin To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
:This same issue came up at the BSDCon developers conference in :regard to ithreads. Is it better to optimize some bit of code :because it is the fun and interesting thing to do, or to build a simple, :yet stable and easily verified foundation, that we can later optimize :in a controlled manner? This is not about whether these particular :changes are correct. The concern is that they may contain a subtle :bug that makes it difficult to verify other portions of the system. :... :-- :Justin Well now hold on a second here Justin. Did you even look at the patch? Or are you just making a generalization that is totally unrelated to the patch? Perhaps you are unaware of the sysctl instrumentation that allows the interrupts-enabled-during-critical-section mechanism to be turned on and off on a whim? Perhaps you are unaware that this patch is not JUST an optmization. Far from it, it solves a number of what I consider to be critical issues. For example, it solves the IPI deadlock problem, and it allows us to cleanup some fairly aweful hacks in the code, e.g. in fork_exit(). I'm all for a discussion, but I would prefer it if the discussion were based on the actual work that I am trying to get committed and not some incorrect generalization that is being used to justify some other approach. None of this is secret. I've stated these facts very clearly a half dozen times now AND in the commit message. Don't ignore it or assume that I am some beginner who's just throwing random commits in and destabilizing the tree. I have gone to great lengths to do the precise opposite of that. -Matt Matthew Dillon [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
:My 2 cents? Work with John to get the APIs that affect this particular :code stable. That means discussion and perhaps that discussion will :take some time (if this blow-up hadn't occurred the discussion :would already be over now ;-). Once the APIs are in place, commit your :code in a format that makes it completely optional, just like your Giant :lock optimizations. This means that those interested in validating :and determining the impact of your code can easily do so by flipping :a switch. Those who would rather debug their own subsystem problems :in a slower, but simpler, environment can do so too. : :-- :Justin The API is intended for the stage-2 commit. The stage-1 commit is intended to do all the 'hard stuff' in as straightforward a manner as possible. The sysctl / option you talk about here existed in my patch set 2 and a half weeks ago. I really wish you people would at least read the patch and commit message before you comment on it. -Matt To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
On 01-Mar-02 Matthew Dillon wrote: : : :On 28-Feb-02 Matthew Dillon wrote: : Not to put too fine a point on it, but, I don't see how this can : possibly justify preventing me from committing my critical_*() stuff. : You've just stated publically that your preemption stuff is unusable : as it currently stands. Why am I supposed to wait an arbitrary period : of time for you to fix, test, and commit it? : : I would REALLY like to commit my critical_*() stuff, and for the record : this will also include the stage-2 stuff described in the original : commit : comments that will be made a few days after the current commit. : :Because the critical_* changes you are doing don't match up to the overall :design. See my mail to -arch for more on that though. At some point in the :future I think that this work can fit in rather well to the cpu_critical_foo :stuff (which can be split out from critical_enter/exit now). However, at :this :stage I would rather avoid complex optimizations of APIs that may change in :the :future. There is more to be gained from locking subsystems. :... :John Baldwin [EMAIL PROTECTED]http://www.FreeBSD.org/~jhb/ :Power Users Use the Power to Serve! - http://www.FreeBSD.org/ I strongly disagree. I have yet to see any technical description of this so-called overall design that shows any incompatibility, and what I decide to do with my time is my business. I don't really care whether you are ready for 'optimizations' or not. I seem to recall that you had similar objections when I started pushing Giant into the system calls, yet that work is in hind sight an obvious enabler for other work people have been doing and committing. No, I actually requested that Giant be pushed down into the syscalls and Maxime Henrion is working on finishing that work right now. I decided to address a serious issue that I've had with those routines for months because you consistently refused to even entertain the notion that you may have constructed the API wrong. Frankly, I feel that these changes both optimize and properly abstract the critical_*() code. I strongly believe that the abstraction you have in there now is improper. My patches have been tested, they work, and they ARE going to be committed as soon as I am able to do so. I believe you are overstepping your bounds as a committer by attempting to scrap them and I also believe that you do not fully understand the implications of the code, because you are blinded by the notion that I am making adjustments to your API. But I do, BDE does, Julian does, as do others. To me these changes are essential, and they must go in now before even more hacks are committed to the codebase to get around existing limitations. There are already hacks in the trampoline/fork_exit and ast code that seriously pollutes the MD/MI boundry, all of which you've flicked off your shoulder and explained away as being allowed by your API, and Peter added a third hack with his pmap commit (which got backed-out when he backed out the commit). These hacks make it crystal clear to me that you critical_*()/cpu_critical*() API is seriously flawed. I am addressing the issue and cleaning up the hacks to create a proper MD/MI separation of the code. Actually, I responded saying that I was willing to talk about how to properly abstract CRITICAL_FORK (which is gross I admit). If you will read the design doc, you will see that actually critical_foo and cpu_critical_foo can now be safely split up and made independent of each other. To this extent td_critnest would still stay MI, but td_savecrit could end up going away if the MD code fully handles the saving/restoring the state for whatever cpu_critical_* become. It makes no sense whatsoever to me to be told not to commit something due to stale code that may not be quite compatible sitting in P4 that you can't make work in any reasonable time frame. You should stop trying to screw over my work and instead look to your own. You have so many balls in the air constructed in a fine mesh that you can't seem to accept a single change to your perfectly meshing subsystems, half of which are going stale in P4. This is greatly restricting your ability to consider deviation. *sigh* The only reason I'm not spending my cycles tracking down the preemption bugs so that preemption can go in is that I have decided that at the moment there is more gain to be found by doing actual locking work. This means that preemption will eventually go in, just Not Right Now. To that extent, I don't think premature optimizations based on observations of a kernel locked entirely by Giant that alter the API's preemption will change (and that were originally introduced when I committed all the bits of the preemption code
Re: Patch for critical_enter()/critical_exit() interrupt assem
: It makes no sense whatsoever to me to be told not to commit something : due to stale code that may not be quite compatible sitting in P4 that : you can't make work in any reasonable time frame. You should stop : trying to screw over my work and instead look to your own. You have : so many balls in the air constructed in a fine mesh that you can't seem : to accept a single change to your perfectly meshing subsystems, half : of which are going stale in P4. This is greatly restricting your : ability to consider deviation. : :*sigh* The only reason I'm not spending my cycles tracking down the preemption :bugs so that preemption can go in is that I have decided that at the moment :there is more gain to be found by doing actual locking work. This means that :preemption will eventually go in, just Not Right Now. To that extent, I don't :think premature optimizations based on observations of a kernel locked entirely :by Giant that alter the API's preemption will change (and that were originally :introduced when I committed all the bits of the preemption code that I could :w/o breaking the kernel) should go in. Those are your cycles, not mine. Why should I be forced to sit on my heals for an unknown period of time (but so far 3 weeks waiting for the ucred changes and 2 weeks waiting for critical_*()), twiddling my thumbs wasting MY cycles just because of your own scheduling issues? That's really the crux of this whole situation. I don't think it is appropriate for you to impose your development methodology on me or anyone else, and I most especially do not think it is appropriate for you to arbitrarily hold off the hard work that I have done in order to fit it into your schedule. I have said very clearly what I intend these critical_*() patches to do. I have said, repeatedly, that I do not believe they would intefere with your work in any significant manner. You have yet to explain in any detail why you think they would. I have said, repeatedly, that I am perfectly willing to work with you later on when you ARE ready to move forward on your own work. That's the crux of the situation, John. I do not believe you have the right to hold this work off, at least not based on any of the explanations you have given so far. -Matt Matthew Dillon [EMAIL PROTECTED] :-- : :John Baldwin [EMAIL PROTECTED]http://www.FreeBSD.org/~jhb/ :Power Users Use the Power to Serve! - http://www.FreeBSD.org/ : :To Unsubscribe: send mail to [EMAIL PROTECTED] :with unsubscribe freebsd-current in the body of the message : To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
In message [EMAIL PROTECTED], Matthew Dillon wri tes: That's the crux of the situation, John. I do not believe you have the right to hold this work off, at least not based on any of the explanations you have given so far. Why don't you for a change, just stop being so ego-centered and instead head to the page at http://www.freebsd.org/smp/ and find yourself a task which is free for grabs, rather than insist that you have a right to bully the only person who have consistently chugged away at the SMPng project when practically everybody else (you included) defected. One could point at such a gem as: add locking to NFS which I am sure John and the rest of us would love to see you tackle... Give us peace Matt... -- Poul-Henning Kamp | UNIX since Zilog Zeus 3.20 [EMAIL PROTECTED] | TCP/IP since RFC 956 FreeBSD committer | BSD since 4.3-tahoe Never attribute to malice what can adequately be explained by incompetence. To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
On Tue, Mar 05, 2002 at 06:30:08PM +0100, Poul-Henning Kamp wrote: In message [EMAIL PROTECTED], Matthew Dillon wri tes: That's the crux of the situation, John. I do not believe you have the right to hold this work off, at least not based on any of the explanations you have given so far. Why don't you for a change, just stop being so ego-centered and instead head to the page at http://www.freebsd.org/smp/ and find yourself a task which is free for grabs, rather than insist that you have a right to bully the only person who have consistently chugged away at the SMPng project when practically everybody else (you included) defected. One could point at such a gem as: add locking to NFS which I am sure John and the rest of us would love to see you tackle... Hey, that's not fair. Instead of occasionally throwing in the do what John tells you argument, why don't you either present a real technical argument as to why Matt's stuff is bad (I'm not saying there isn't one) or, for that matter, add locking to NFS yourself? Give us peace Matt... -- Poul-Henning Kamp | UNIX since Zilog Zeus 3.20 [EMAIL PROTECTED] | TCP/IP since RFC 956 FreeBSD committer | BSD since 4.3-tahoe Never attribute to malice what can adequately be explained by incompetence. -- Bosko Milekic [EMAIL PROTECTED] [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
Matthew Dillon wrote: My patches have been tested, they work, and they ARE going to be committed as soon as I am able to do so. Tut, tut, looks like rain! -- Winnie the Pooh; A. A. Milne If you guys spent as much energy documenting your designs as you do bitching at each other, everyone would now have a clear idea of which side they should be on, or if there were even sides to begin with -- since the worst thing John has said about the patches is that they'd be a premature optimization that he expects to be needed later, but inappropriate now. There are already hacks in the trampoline/fork_exit and ast code that seriously pollutes the MD/MI boundry, all of which you've flicked off your shoulder and explained away as being allowed by your API, and Peter added a third hack with his pmap commit (which got backed-out when he backed out the commit). Peter's pmap code was good work, but he ran head on into an undocumented processor bug that FreeBSD had escaped earlier by serendipity. Remember the whole AMD/Intel/AGP discussion? You have so many balls in the air constructed in a fine mesh that you can't seem to accept a single change to your perfectly meshing subsystems, half of which are going stale in P4. This is greatly restricting your ability to consider deviation. Whether deviation is called for or not is still an open question, to my mind. However, you have a point about the uncommitted work. On the other hand, it was you who have been arguing so strenously that the size of the patches that need to go in in one go make them too dangerous. And John has a point about the uncommitted work, in that the SMP system doesn't make it to single user mode with the preemption patches. I think the right thing to do is to commit the cred changes, and stabilize them, if there's even a problem, as you expect from your comments about dangerous. I think the right thing to do on the cred front, *after* that, is to commit the patches -- John, if it won't boot to SMP afterward, you will have the eyes of everyone who uses SMP -current to help you discover why, and to correct the problems, which will happen *much faster* with a large number of eyes on it. I will repeat, if you want to discuss specific technical issues related to these commits after I put them in, I am all ears. I will repeat, I see nothing in this code that prevents you from accomplishing anything that you've brought up to date (which so far as just been the non-working preemption code you have sitting in P4). -- Terry To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
On Fri, 1 Mar 2002, Terry Lambert wrote: I think the right thing to do is to commit the cred changes, and stabilize them, if there's even a problem, as you expect from your comments about dangerous. John already committed a majority of all the cred changes. I never saw a commit message for it but the changes have appeared in the tree. My guess is that somethign went wrong with mail at that time if you didn't see the commit either.. To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
On 26-Feb-02 Julian Elischer wrote: On Tue, 26 Feb 2002, John Baldwin wrote: My suggestion will be to back it out. I would rather not have to make said suggestion. Can you please try to fit this into the existing framework rather than ripping it all up? We need to finalize and test the design before we hardcode too many assumptions about the implementation into the interface. You have pointed out some issues with the current interface which are valid and I would like to address those, however, there are still changes to the MI implementation that need to go in once it doesn't crash right and left. If you wish I could commit the code and make current a living hell for everyone, but my ethics don't permit me to test code that I know is broken. You know john, I wish you would commit more often and let it break things occasionally. It's REALLY HARD for anyone else to comment and help if you keep doing on P4 which is NOT the project Souce control system. Even with cvsup assistanace, it's just no-where near as convenient as having it checked in. And after you HAVE checked it in, others can help find and fix problems.. as it is you are on your own. (This is the reason I will shortly check in the KSE diffs on a branch) *sigh* Preemptive kernels don't even make it out of single user mode for SMP machines, ok? We aren't talking minor breakage here, we are talking _extreme_ breakage. If people want to play with it, preempt.patch on freefall is updated via a cron job every half hour or so. Unfortunately, however, it's in a limbo atm due to KSE and needing to sort out how the priorities are going to work. It will really be better to let KSE settle into the scheduler first adn then add preemption to the scheduler itself afterwards. The reason I'm not pushing preemption into the tree fully (I've already committed half of the original patch) is that there is other work (proc locking for example) that gets us more bang for the buck. -- John Baldwin [EMAIL PROTECTED]http://www.FreeBSD.org/~jhb/ Power Users Use the Power to Serve! - http://www.FreeBSD.org/ To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
yes but thete are subcommits that you could go ahead with... the td_ucred stuff could have been checked in directly into -current. On Thu, 28 Feb 2002, John Baldwin wrote: On 26-Feb-02 Julian Elischer wrote: On Tue, 26 Feb 2002, John Baldwin wrote: My suggestion will be to back it out. I would rather not have to make said suggestion. Can you please try to fit this into the existing framework rather than ripping it all up? We need to finalize and test the design before we hardcode too many assumptions about the implementation into the interface. You have pointed out some issues with the current interface which are valid and I would like to address those, however, there are still changes to the MI implementation that need to go in once it doesn't crash right and left. If you wish I could commit the code and make current a living hell for everyone, but my ethics don't permit me to test code that I know is broken. You know john, I wish you would commit more often and let it break things occasionally. It's REALLY HARD for anyone else to comment and help if you keep doing on P4 which is NOT the project Souce control system. Even with cvsup assistanace, it's just no-where near as convenient as having it checked in. And after you HAVE checked it in, others can help find and fix problems.. as it is you are on your own. (This is the reason I will shortly check in the KSE diffs on a branch) *sigh* Preemptive kernels don't even make it out of single user mode for SMP machines, ok? We aren't talking minor breakage here, we are talking _extreme_ breakage. If people want to play with it, preempt.patch on freefall is updated via a cron job every half hour or so. Unfortunately, however, it's in a limbo atm due to KSE and needing to sort out how the priorities are going to work. It will really be better to let KSE settle into the scheduler first adn then add preemption to the scheduler itself afterwards. The reason I'm not pushing preemption into the tree fully (I've already committed half of the original patch) is that there is other work (proc locking for example) that gets us more bang for the buck. -- John Baldwin [EMAIL PROTECTED]http://www.FreeBSD.org/~jhb/ Power Users Use the Power to Serve! - http://www.FreeBSD.org/ To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
Not to put too fine a point on it, but, I don't see how this can possibly justify preventing me from committing my critical_*() stuff. You've just stated publically that your preemption stuff is unusable as it currently stands. Why am I supposed to wait an arbitrary period of time for you to fix, test, and commit it? I would REALLY like to commit my critical_*() stuff, and for the record this will also include the stage-2 stuff described in the original commit comments that will be made a few days after the current commit. -Matt Matthew Dillon [EMAIL PROTECTED] : : Preemptive kernels don't even make it out of single user mode for SMP machines, : ok? We aren't talking minor breakage here, we are talking _extreme_ breakage. : If people want to play with it, preempt.patch on freefall is updated via a cron : job every half hour or so. Unfortunately, however, it's in a limbo atm due to : KSE and needing to sort out how the priorities are going to work. It will : really be better to let KSE settle into the scheduler first adn then add : preemption to the scheduler itself afterwards. : : The reason I'm not pushing preemption into the tree fully (I've already : committed half of the original patch) is that there is other work (proc locking : for example) that gets us more bang for the buck. : : -- : : John Baldwin [EMAIL PROTECTED]http://www.FreeBSD.org/~jhb/ : Power Users Use the Power to Serve! - http://www.FreeBSD.org/ : To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
On 28-Feb-02 Matthew Dillon wrote: Not to put too fine a point on it, but, I don't see how this can possibly justify preventing me from committing my critical_*() stuff. You've just stated publically that your preemption stuff is unusable as it currently stands. Why am I supposed to wait an arbitrary period of time for you to fix, test, and commit it? I would REALLY like to commit my critical_*() stuff, and for the record this will also include the stage-2 stuff described in the original commit comments that will be made a few days after the current commit. Because the critical_* changes you are doing don't match up to the overall design. See my mail to -arch for more on that though. At some point in the future I think that this work can fit in rather well to the cpu_critical_foo stuff (which can be split out from critical_enter/exit now). However, at this stage I would rather avoid complex optimizations of APIs that may change in the future. There is more to be gained from locking subsystems. -Matt Matthew Dillon [EMAIL PROTECTED] : : Preemptive kernels don't even make it out of single user mode for SMP : machines, : ok? We aren't talking minor breakage here, we are talking _extreme_ : breakage. : If people want to play with it, preempt.patch on freefall is updated via a : cron : job every half hour or so. Unfortunately, however, it's in a limbo atm due : to : KSE and needing to sort out how the priorities are going to work. It will : really be better to let KSE settle into the scheduler first adn then add : preemption to the scheduler itself afterwards. : : The reason I'm not pushing preemption into the tree fully (I've already : committed half of the original patch) is that there is other work (proc : locking : for example) that gets us more bang for the buck. : : -- : : John Baldwin [EMAIL PROTECTED]http://www.FreeBSD.org/~jhb/ : Power Users Use the Power to Serve! - http://www.FreeBSD.org/ : -- John Baldwin [EMAIL PROTECTED]http://www.FreeBSD.org/~jhb/ Power Users Use the Power to Serve! - http://www.FreeBSD.org/ To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
: : :On 28-Feb-02 Matthew Dillon wrote: : Not to put too fine a point on it, but, I don't see how this can : possibly justify preventing me from committing my critical_*() stuff. : You've just stated publically that your preemption stuff is unusable : as it currently stands. Why am I supposed to wait an arbitrary period : of time for you to fix, test, and commit it? : : I would REALLY like to commit my critical_*() stuff, and for the record : this will also include the stage-2 stuff described in the original commit : comments that will be made a few days after the current commit. : :Because the critical_* changes you are doing don't match up to the overall :design. See my mail to -arch for more on that though. At some point in the :future I think that this work can fit in rather well to the cpu_critical_foo :stuff (which can be split out from critical_enter/exit now). However, at this :stage I would rather avoid complex optimizations of APIs that may change in the :future. There is more to be gained from locking subsystems. :... :John Baldwin [EMAIL PROTECTED]http://www.FreeBSD.org/~jhb/ :Power Users Use the Power to Serve! - http://www.FreeBSD.org/ I strongly disagree. I have yet to see any technical description of this so-called overall design that shows any incompatibility, and what I decide to do with my time is my business. I don't really care whether you are ready for 'optimizations' or not. I seem to recall that you had similar objections when I started pushing Giant into the system calls, yet that work is in hind sight an obvious enabler for other work people have been doing and committing. I decided to address a serious issue that I've had with those routines for months because you consistently refused to even entertain the notion that you may have constructed the API wrong. Frankly, I feel that these changes both optimize and properly abstract the critical_*() code. I strongly believe that the abstraction you have in there now is improper. My patches have been tested, they work, and they ARE going to be committed as soon as I am able to do so. I believe you are overstepping your bounds as a committer by attempting to scrap them and I also believe that you do not fully understand the implications of the code, because you are blinded by the notion that I am making adjustments to your API. But I do, BDE does, Julian does, as do others. To me these changes are essential, and they must go in now before even more hacks are committed to the codebase to get around existing limitations. There are already hacks in the trampoline/fork_exit and ast code that seriously pollutes the MD/MI boundry, all of which you've flicked off your shoulder and explained away as being allowed by your API, and Peter added a third hack with his pmap commit (which got backed-out when he backed out the commit). These hacks make it crystal clear to me that you critical_*()/cpu_critical*() API is seriously flawed. I am addressing the issue and cleaning up the hacks to create a proper MD/MI separation of the code. It makes no sense whatsoever to me to be told not to commit something due to stale code that may not be quite compatible sitting in P4 that you can't make work in any reasonable time frame. You should stop trying to screw over my work and instead look to your own. You have so many balls in the air constructed in a fine mesh that you can't seem to accept a single change to your perfectly meshing subsystems, half of which are going stale in P4. This is greatly restricting your ability to consider deviation. I will repeat, if you want to discuss specific technical issues related to these commits after I put them in, I am all ears. I will repeat, I see nothing in this code that prevents you from accomplishing anything that you've brought up to date (which so far as just been the non-working preemption code you have sitting in P4). -Matt Matthew Dillon [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
In message [EMAIL PROTECTED], Matthew Dillon wri tes: :Because the critical_* changes you are doing don't match up to the overall :design. See my mail to -arch for more on that though. At some point in the :future I think that this work can fit in rather well to the cpu_critical_foo :stuff (which can be split out from critical_enter/exit now). However, at this :stage I would rather avoid complex optimizations of APIs that may change in the :future. There is more to be gained from locking subsystems. :... :John Baldwin [EMAIL PROTECTED]http://www.FreeBSD.org/~jhb/ :Power Users Use the Power to Serve! - http://www.FreeBSD.org/ I strongly disagree. I have yet to see any technical description of this so-called overall design that shows any incompatibility, and what I decide to do with my time is my business. Matt, That particular protest is rather hollow, considering that you were one of the first people to not show up for the SMPng work whereas John has been consistently chugging along on the job all the way. At this point in time, until he is officially unseated John is our designated SMPng architect and his word is pretty final. -- Poul-Henning Kamp | UNIX since Zilog Zeus 3.20 [EMAIL PROTECTED] | TCP/IP since RFC 956 FreeBSD committer | BSD since 4.3-tahoe Never attribute to malice what can adequately be explained by incompetence. To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
: :I strongly disagree. I have yet to see any technical description of :this so-called overall design that shows any incompatibility, and what :I decide to do with my time is my business. : :Matt, : :That particular protest is rather hollow, considering that you were :one of the first people to not show up for the SMPng work whereas :John has been consistently chugging along on the job all the way. :At this point in time, until he is officially unseated John is our :designated SMPng architect and his word is pretty final. : :-- :Poul-Henning Kamp | UNIX since Zilog Zeus 3.20 Oooh... so you mean that since I was working full time and unable to contribute, somehow this disqualifies me now? Sorry, that doesn't hold water. In fact at the yahoo meeting oh so long ago I even said, quite clearly, that I was not going to have time in the near future. And, indeed, I did not. But I do now. While I have great respect for the work John has done, his word is most certainly NOT final in even the most generous consideration of his position. There is absolutely nothing in the charter or rules that give John authority over dozens of core source files in sys/, there is nothing that gives him veto power over anything in the source tree, and there is nothing that allows him to lock-up so many files for such a long period of time. It just isn't there so, frankly, your assertion is pretty damn hollow Poul. -Matt To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
In message [EMAIL PROTECTED], Matthew Dillon wri tes: : :I strongly disagree. I have yet to see any technical description of :this so-called overall design that shows any incompatibility, and what :I decide to do with my time is my business. : :Matt, : :That particular protest is rather hollow, considering that you were :one of the first people to not show up for the SMPng work whereas :John has been consistently chugging along on the job all the way. :At this point in time, until he is officially unseated John is our :designated SMPng architect and his word is pretty final. : :-- :Poul-Henning Kamp | UNIX since Zilog Zeus 3.20 Oooh... so you mean that since I was working full time and unable to contribute, somehow this disqualifies me now? No, but it does make you, like the rest of us, underlings to John architectural direction. -- Poul-Henning Kamp | UNIX since Zilog Zeus 3.20 [EMAIL PROTECTED] | TCP/IP since RFC 956 FreeBSD committer | BSD since 4.3-tahoe Never attribute to malice what can adequately be explained by incompetence. To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
:John has been consistently chugging along on the job all the way. :At this point in time, until he is officially unseated John is our :designated SMPng architect and his word is pretty final. : :-- :Poul-Henning Kamp | UNIX since Zilog Zeus 3.20 Oooh... so you mean that since I was working full time and unable to contribute, somehow this disqualifies me now? No, but it does make you, like the rest of us, underlings to John architectural direction. Just a quick note that the core team is still having a lively discussion about this issue. All options are still on the table, including appointing an offical SMPng architect/manager. Please be patient. -DG David Greenman Co-founder, The FreeBSD Project - http://www.freebsd.org President, TeraSolutions, Inc. - http://www.terasolutions.com President, Download Technologies, Inc. - http://www.downloadtech.com Pave the road of life with opportunities. To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
On 26-Feb-02 Matthew Dillon wrote: :1) I had an ugly panic testing it on the alpha. After a good deal of :sleuthing, : I've determined that we still have some preemption related bugs in : possibly : the alpha pmap, but that td_ucred isn't the problem. :2) I've been thinking about the Giant instrumentation a bit. I figured out :that : most of my problems were with the implementation and have come up with : some : ideas that might make it a bit more scalable and easier from a long-term : perspective though I think it still has some scaling issues. This is precisely the problem. You are making so many modifications in your local tree that NONE of the work winds up getting committed for months and months. No, If I had one local tree I would be rather up the creek. Thankfully I have multiple trees so I can switch from one task to another without having them all tangled up in each other. :) :In regards to the critical section stuff: : :The critical section stuff currently in current is part of the original :preemption patches I wrote at Usenix last year. They aren't in the tree :because they aren't stable yet. We still have problems on the alpha and :problems with IPI deadlocks on SMP machines that need to be worked out. :Since :this API is still very much in flux I'd prefer to keep it simple for now and :not make the code overly complex with optimizations until after we have :settled :on the design. : :You've brought up some issues with the critical section stuff as far as its :assumptions in fork_exit() and a few other places. For now I would prefer to :leave that code alone as it works with our current model. However, I am :interested in fixing assumptions that would allow the cpu_critical_enter/exit :API to be changed but still maintain an MI interface. This could mean :changing :the API for cpu_critical_enter/exit and possibly the API (perhaps some MD :macros?) for the fork_exit() fixup code. : : :John Baldwin [EMAIL PROTECTED]http://www.FreeBSD.org/~jhb/ I'm sorry John, but I am going to go ahead with my commit. I strongly disagree with your premise and, frankly, my changes clean the code up rather then make it more complex. You have your fingers in just about every single goddamn file in the system and you and others have cried wolf one too many times. I am through with playing that game. The commit goes in. I am open to any suggestions you have for stage-2, which will be a furtherance of the cleanup, but I absolutely refuse to allow you to prevent me from contributing to the SMP work in -current for a forth time because it doesn't exactly match your dream or N-month-old stale patches you might have sitting around in P4. My suggestion will be to back it out. I would rather not have to make said suggestion. Can you please try to fit this into the existing framework rather than ripping it all up? We need to finalize and test the design before we hardcode too many assumptions about the implementation into the interface. You have pointed out some issues with the current interface which are valid and I would like to address those, however, there are still changes to the MI implementation that need to go in once it doesn't crash right and left. If you wish I could commit the code and make current a living hell for everyone, but my ethics don't permit me to test code that I know is broken. -Matt Matthew Dillon [EMAIL PROTECTED] -- John Baldwin [EMAIL PROTECTED]http://www.FreeBSD.org/~jhb/ Power Users Use the Power to Serve! - http://www.FreeBSD.org/ To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
On 26-Feb-02 Bruce Evans wrote: On Tue, 26 Feb 2002, John Baldwin wrote: The critical section stuff currently in current is part of the original preemption patches I wrote at Usenix last year. They aren't in the tree because they aren't stable yet. We still have problems on the alpha and problems with IPI deadlocks on SMP machines that need to be worked out. Since this API is still very much in flux I'd prefer to keep it simple for now and not make the code overly complex with optimizations until after we have settled on the design. The change is mostly part of the design change needed to un-break fast interrupt handlers. critical_enter() mask not mask hardware interrupts for the same reason than splhigh() must not mask them: there is too much MI code that thinks it is critical and short but really isn't (e.g., everything under spinlocks in -current). Changing cpu_critical_enter() to a null version to prevent spinlocks masking interrupts doesn't work very well because it is used for other things that really do need to mask interrupts. Having 2 levels for cpu_critical_enter() (on that masks normal interrupts and one that masks fast interrupts) would work but I think there are already too many levels of critical_enter()s. I think having the sparc-like API of intr_disable() / intr_restore() that we used to use to fix the places that assume too many implementation details of cpu_critical_foo() would be a good first step so that cpu_critical_foo() can be relegated back to supporting non-preemption (and no other spinlocks) and nothing else. Bruce -- John Baldwin [EMAIL PROTECTED]http://www.FreeBSD.org/~jhb/ Power Users Use the Power to Serve! - http://www.FreeBSD.org/ To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
:.. : cpu_critical_enter() to a null version to prevent spinlocks masking : interrupts doesn't work very well because it is used for other things : that really do need to mask interrupts. Having 2 levels for : cpu_critical_enter() (on that masks normal interrupts and one that : masks fast interrupts) would work but I think there are already too : many levels of critical_enter()s. : :I think having the sparc-like API of intr_disable() / intr_restore() that we :used to use to fix the places that assume too many implementation details of :cpu_critical_foo() would be a good first step so that cpu_critical_foo() can be :relegated back to supporting non-preemption (and no other spinlocks) and :nothing else. : : Bruce : :-- : :John Baldwin [EMAIL PROTECTED]http://www.FreeBSD.org/~jhb/ I consider this a mistake that inappropriately locks MI code into architecture-specific (MD) features, especially considering that there are only two places in the codebase that actually make this assumption right now and both can be easily fixed. It makes no sense to me to abstract out a third API (let alone making cpu_critical_*() available to the MI code) when the existing critical_*() API will do the job just fine. -Matt Matthew Dillon [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
On Tue, 26 Feb 2002, John Baldwin wrote: My suggestion will be to back it out. I would rather not have to make said suggestion. Can you please try to fit this into the existing framework rather than ripping it all up? We need to finalize and test the design before we hardcode too many assumptions about the implementation into the interface. You have pointed out some issues with the current interface which are valid and I would like to address those, however, there are still changes to the MI implementation that need to go in once it doesn't crash right and left. If you wish I could commit the code and make current a living hell for everyone, but my ethics don't permit me to test code that I know is broken. You know john, I wish you would commit more often and let it break things occasionally. It's REALLY HARD for anyone else to comment and help if you keep doing on P4 which is NOT the project Souce control system. Even with cvsup assistanace, it's just no-where near as convenient as having it checked in. And after you HAVE checked it in, others can help find and fix problems.. as it is you are on your own. (This is the reason I will shortly check in the KSE diffs on a branch) To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
On 26-Feb-02 Peter Wemm wrote: Matthew Dillon wrote: : :On Mon, 25 Feb 2002, Matthew Dillon wrote: : : Unless an unforseen problem arises, I am going to commit this : tomorrow : and then start working on a cleanup patch. I have decided to : :Please wait for jhb's opinion on it. He seems to be offline again. :I think he has plans and maybe even code for more code in critical_enter(). :I think we don't agree with these plans, but they are just as valid :as ours, and our versions undo many of his old changes. I am not going to predicate my every move on permission from JHB nor do I intend to repeat the last debacle which held-up (and is still holdin g up) potential commits for a week and a half now. JHB hasn't even committed *HIS* patches and I am beginning to wonder what the point is when *NOTHING* goes in. If he had code he damn well should have said something on the lists two days ago. As it is, I have invested a great deal of time and effort on this patch and it is damn well going to go in so I can move on. So, your great deal of time and effort over the last week is more important than our time and effort over the last few months? Kernel preemption was first written at Usenix last June. It's been stable on UP x86 for months but we still have preemption bugs on other systems that prevent it from going in at the time being. Having a fully preemptive kernel simplifies many things and is the same design path used by other major multithreaded Unix-like kernels. -- John Baldwin [EMAIL PROTECTED]http://www.FreeBSD.org/~jhb/ Power Users Use the Power to Serve! - http://www.FreeBSD.org/ To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
On 26-Feb-02 Matthew Dillon wrote: : :On Mon, 25 Feb 2002, Matthew Dillon wrote: : : Unless an unforseen problem arises, I am going to commit this tomorrow : and then start working on a cleanup patch. I have decided to : :Please wait for jhb's opinion on it. He seems to be offline again. :I think he has plans and maybe even code for more code in critical_enter(). :I think we don't agree with these plans, but they are just as valid :as ours, and our versions undo many of his old changes. I am not going to predicate my every move on permission from JHB nor do I intend to repeat the last debacle which held-up (and is still holding up) potential commits for a week and a half now. JHB hasn't even committed *HIS* patches and I am beginning to wonder what the point is when *NOTHING* goes in. If he had code he damn well should have said something on the lists two days ago. As it is, I have invested a great deal of time and effort on this patch and it is damn well going to go in so I can move on. I've been taking the weekend off so I could have some think to cool off and think and not get all emotional. The reasons I haven't committed the td_ucred stuff are: 1) I had an ugly panic testing it on the alpha. After a good deal of sleuthing, I've determined that we still have some preemption related bugs in possibly the alpha pmap, but that td_ucred isn't the problem. 2) I've been thinking about the Giant instrumentation a bit. I figured out that most of my problems were with the implementation and have come up with some ideas that might make it a bit more scalable and easier from a long-term perspective though I think it still has some scaling issues. In regards to the critical section stuff: The critical section stuff currently in current is part of the original preemption patches I wrote at Usenix last year. They aren't in the tree because they aren't stable yet. We still have problems on the alpha and problems with IPI deadlocks on SMP machines that need to be worked out. Since this API is still very much in flux I'd prefer to keep it simple for now and not make the code overly complex with optimizations until after we have settled on the design. You've brought up some issues with the critical section stuff as far as its assumptions in fork_exit() and a few other places. For now I would prefer to leave that code alone as it works with our current model. However, I am interested in fixing assumptions that would allow the cpu_critical_enter/exit API to be changed but still maintain an MI interface. This could mean changing the API for cpu_critical_enter/exit and possibly the API (perhaps some MD macros?) for the fork_exit() fixup code. -Matt -- John Baldwin [EMAIL PROTECTED]http://www.FreeBSD.org/~jhb/ Power Users Use the Power to Serve! - http://www.FreeBSD.org/ To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
:1) I had an ugly panic testing it on the alpha. After a good deal of sleuthing, : I've determined that we still have some preemption related bugs in possibly : the alpha pmap, but that td_ucred isn't the problem. :2) I've been thinking about the Giant instrumentation a bit. I figured out that : most of my problems were with the implementation and have come up with some : ideas that might make it a bit more scalable and easier from a long-term : perspective though I think it still has some scaling issues. This is precisely the problem. You are making so many modifications in your local tree that NONE of the work winds up getting committed for months and months. :In regards to the critical section stuff: : :The critical section stuff currently in current is part of the original :preemption patches I wrote at Usenix last year. They aren't in the tree :because they aren't stable yet. We still have problems on the alpha and :problems with IPI deadlocks on SMP machines that need to be worked out. Since :this API is still very much in flux I'd prefer to keep it simple for now and :not make the code overly complex with optimizations until after we have settled :on the design. : :You've brought up some issues with the critical section stuff as far as its :assumptions in fork_exit() and a few other places. For now I would prefer to :leave that code alone as it works with our current model. However, I am :interested in fixing assumptions that would allow the cpu_critical_enter/exit :API to be changed but still maintain an MI interface. This could mean changing :the API for cpu_critical_enter/exit and possibly the API (perhaps some MD :macros?) for the fork_exit() fixup code. : : :John Baldwin [EMAIL PROTECTED]http://www.FreeBSD.org/~jhb/ I'm sorry John, but I am going to go ahead with my commit. I strongly disagree with your premise and, frankly, my changes clean the code up rather then make it more complex. You have your fingers in just about every single goddamn file in the system and you and others have cried wolf one too many times. I am through with playing that game. The commit goes in. I am open to any suggestions you have for stage-2, which will be a furtherance of the cleanup, but I absolutely refuse to allow you to prevent me from contributing to the SMP work in -current for a forth time because it doesn't exactly match your dream or N-month-old stale patches you might have sitting around in P4. -Matt Matthew Dillon [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
On Tue, 26 Feb 2002, John Baldwin wrote: The critical section stuff currently in current is part of the original preemption patches I wrote at Usenix last year. They aren't in the tree because they aren't stable yet. We still have problems on the alpha and problems with IPI deadlocks on SMP machines that need to be worked out. Since this API is still very much in flux I'd prefer to keep it simple for now and not make the code overly complex with optimizations until after we have settled on the design. The change is mostly part of the design change needed to un-break fast interrupt handlers. critical_enter() mask not mask hardware interrupts for the same reason than splhigh() must not mask them: there is too much MI code that thinks it is critical and short but really isn't (e.g., everything under spinlocks in -current). Changing cpu_critical_enter() to a null version to prevent spinlocks masking interrupts doesn't work very well because it is used for other things that really do need to mask interrupts. Having 2 levels for cpu_critical_enter() (on that masks normal interrupts and one that masks fast interrupts) would work but I think there are already too many levels of critical_enter()s. Bruce To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Patch for critical_enter()/critical_exit() interrupt assem
: The critical section stuff currently in current is part of the original : preemption patches I wrote at Usenix last year. They aren't in the tree : because they aren't stable yet. We still have problems on the alpha and : problems with IPI deadlocks on SMP machines that need to be worked out. Since : this API is still very much in flux I'd prefer to keep it simple for now and : not make the code overly complex with optimizations until after we have settled : on the design. : :The change is mostly part of the design change needed to un-break fast :interrupt handlers. critical_enter() mask not mask hardware :interrupts for the same reason than splhigh() must not mask them: :there is too much MI code that thinks it is critical and short but :really isn't (e.g., everything under spinlocks in -current). Changing :cpu_critical_enter() to a null version to prevent spinlocks masking :interrupts doesn't work very well because it is used for other things :that really do need to mask interrupts. Having 2 levels for :cpu_critical_enter() (on that masks normal interrupts and one that :masks fast interrupts) would work but I think there are already too :many levels of critical_enter()s. : :Bruce Not to mention the fact that this will lead to an ability to break into the debugger via the serial concsole even if the code is stuck in a spin mutex or in the middle of the scheduler. Not with this commit or the followup cleanup commit, not quite, but maybe the one after. It puts us on the right road. -Matt Matthew Dillon [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
