On Sat, May 03, 2008 at 11:11:48AM -0700, Paul E. McKenney wrote: > On Sat, May 03, 2008 at 07:49:30AM +0200, Nick Piggin wrote: > > > This is perfectly deadlock free when wait=0 and it just returns -ENOMEM > > > on allocation failure. > > > > Yeah, I'm just talking about the wait=0 case. (btw. I'd rather the core > > API takes some data rather than allocates some itself, eg because you > > might want to have it on the stack). > > But taking data on the stack is safe only in the wait=1 case, right? Sure, but the API would be general enough to handle it. > > For the wait=1 case, something very clever such as processing pending > > requests in a polling loop might be cool... however I'd rather not add > > such complexity until someone needs it (you could stick a comment in > > there outlining your algorithm). But I'd just rather not have peole rely > > on it yet. > > In that case we may need to go back to the global lock with only one > request being processed at a time. Otherwise, if two wait=1 requests > happen at the same time, they deadlock waiting for each other to process > their request. (See Keith Owens: http://lkml.org/lkml/2008/5/2/183). > > In other words, if you want to allow parallel calls to > smp_call_function(), the simplest way to do it seems to be to do the > polling loop. The other ways I have come up with thus far are uglier > and less effective (see http://lkml.org/lkml/2008/4/30/164). > > Now, what I -could- do would be to prohibit the wait=1 case from > irq-disable state from polling -- that would make sense, as the caller > probably had a reason to mask irqs, and might not take kindly to having > them faked behind the caller's back. ;-) I think we're talking past each other a little bit. There is no irq-disabled calls as yet, therefore I don't think we should add a lot of complex code just to _allow_ for it; at least, not until a really compelling user comes up. The main thing is to parallelise the code. The fact that we can trivially support irq-disabled calls for nowait case (if the caller supplies the data or can handle failure) is just a bonus. > > > It it doesn't return -ENOMEM I know its been queued and will be > > > processed at some point, if it does fail, I can deal with it in another > > > way. > > > > At least with IPIs I think we can guarantee they will be processed on > > the target after we queue them. > > OK, so let me make sure I understand what is needed. One example might be > some code called from scheduler_tick(), which runs with irqs disabled. > Without the ability to call smp_call_function() directly, you have > to fire off a work queue or something. Now, if smp_call_function() > can hand you an -ENOMEM or (maybe) an -EBUSY, then you still have to > fire off the work queue, but you probably only have to do it rarely, > minimizing the performance impact. > > Another possibility is when it is -nice- to call smp_call_function(), > but can just try again on the next scheduler_tick() -- ignoring dynticks > idle for the moment. In this case, you might still test the error return > to set a flag that you will check on the next scheduler_tick() call. > > Is this where you guys are coming from? > > And you are all OK with smp_call_function() called with irqs enabled > never being able to fail, right? (Speaking of spaghetti code, why > foist unnecessary failure checks on the caller...) Having the fallback is fine, yes. I'd say it shouldn't often get called. > > > I know I'd like to do that and I suspect Nick has a few use cases up his > > > sleeve as well. > > > > It would be handy. The "quickly kick something off on another CPU" is > > pretty nice in mm/ when you have per-cpu queues or caches that might > > want to be flushed. > > OK, I think I might be seeing what you guys are getting at. Here is > what I believe you guys need: > > 1. No deadlocks, ever, not even theoretical "low probability" > deadlocks. Of course ;) > 2. No failure returns when called with irqs enabled. On the other > hand, when irqs are disabled, failure is possible. Though hopefully > unlikely. I think I'd like to keep existing smp_call_function that disallows irq-disabled calls and can't fail. Introduce a new one for irq-disabled case. But sure, the existing smp_call_function implementation can't fail. > 3. Parallel execution of multiple smp_call_function() requests > is required, even when called with irqs disabled. I think so. At least for the call_function_single case. > 4. The wait=1 case with irqs disabled is prohibited. > > 5. If you call smp_call_function() with irqs disabled, then you > are guaranteed that no other CPU's smp_call_function() handler > will be invoked while smp_call_function() is executing. > > Anything I am missing? For the last cases, I actually think your polling loop is pretty cool ;) So I don't completely object to it, I just don't think we should add it in until something wants it... Don't let me dictate the requirements though, the only real one I had was to make call_function_single scalable and faster, and call_function be as optimal as possible. -- To unsubscribe from this list: send the line "unsubscribe linux-arch" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html