On Sun, 2006-09-07 at 16:19 +0200, Thomas Graf wrote: > * Jamal Hadi Salim <[EMAIL PROTECTED]> 2006-07-09 10:03 > > On Sun, 2006-09-07 at 15:33 +0200, Thomas Graf wrote: > > > That's not gonna work, dev->queue_lock may be held legimitately > > > by someone else than an underlying dev_queue_xmit() call. > > > > > > > If there is a legitimate reason then it wont work. I cant think of one > > though. > > See sch_generic.c, it's documented. A simple grep on queue_lock > would have told you the same. >
If you mean that the device will also try to grab the qlock there, then that is fine still for the serialization. It all starts at dev_queue_xmit. > > This is also another approach that would work. If you think its simpler > > go ahead and shoot a patch. > > It's not simpler, it's correct, while your patch is wrong. > Ok, calm down, will you? Man, you make it very hard to follow Daves Tibetan approach. Let me tell you exactly how i feel about your approach: It is unnecessarily complex. The approach i posted is not only fine, it works with only 4-5 lines of code; i have numerous tests against it over a long period of time. I was trying to be polite and follow Daves advice not to insist it has to be done my way - it almost seems impossible with you trying to nitpick on little unimportant details. > > A->*->A is a no-no. > > And in some cases it is fine to let the user just fsck themselves > > because then they will understand it is a bad idea [1] when shit > > happens. OTOH, if there was a KISS way of doing it (as in the ifb case, > > why not). > > I remind you that you started mentioning this A->*->A case while > talking about tx deadlocks that were supposed to be prevented with > the !from check or something along that lines. I can't really tell > because you explain it differently in every posting. > Because you are looking for preciseness at the micro/code level and i am trying to explain at the conceptual/macro level (I have to adjust to your mode but it is hard for me because i dont think at our level that matters). When i sense you didnt understand me the last time, I try to explain it better. The deadlock happens on transmit. If you want to be precise, it happens when dev_queue_xmit is invoked. If you want more preciseness, when the queue lock is contended. If you want more preciseness, the code sample that i showed you should help illustrate it. > > Yes, of course otherwise i wouldnt bother to comment on any patches. > > So maintain the code and fix your bugs. Ok, I dont think this is gonna work - I may have to give up on getting any resolution. Heres the suggestion again and you can still shoot a patch: - If we can avoid doing anything at mirred that is the most preferable approach. i.e get the change for free. In other words if it complicates things it is not worth it. Someone redirecting eth0 to eth0 on egress deserves whats happening to them. - Try it against Herberts patch. It may resolve something or may require an adjustment to Herberts patch so we can kill two birds with one stone i.e get it for free. I dont know. I guess i shouldnt say "i dont know" it is not precise, my point is you may find things when you attempt the change. The patch is in Daves 2.6.18 tree. - Iam fine with your suggestion to use a flag. The problem is not which scheme you use - it is whether the change at such a crucial path in the stack would be acceptable to other people. Anyways, I am off. cheers, jamal - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
