On Fri, 2013-08-23 at 18:40 -0500, James Yang wrote: > Scott's been able to put enough doubt in me to think that this is not > entirely safe, even though the testing and code generation show it to > work. Please reject this patch. > > I think there is still value in getting the unnecessary loads to be > removed since it would also allow unnecessary conditional branches to > be removed. I'll think about alternate ways to do this.
Hrm, The problem has to do with PACA accesses moving around accross preempt boundaries, it's a bit tricky, but in the case of "current" shouldn't be a problem... while the rest of the PACA might change (CPU# etc...) current remains stable for the point of view of a given thread. So I think the patch is fine. Scott ? Now, we do need some serious rework of PACA accesses. I'm very *VERY* nervous with what we have now. A bit of grepping shows dozens of cases where gcc copies r13 into another register or even saves/restores it, it scares the shit out of me :-) My thinking is to make r13 a hidden reg like we do (or used to) on ppc32 with r2 and break down paca access into two forms: - Direct access of a single field -> asm loads/stores inline - Anything else, uses a get_paca/put_paca construct that includes a preempt_disable/enable (and maybe along with a __get_paca/__put_paca pair that doesn't). This basically does a mr of r13 into another register and basically hides the whole lot from gcc. The former would be used for single fields, the latter, while adding a potentially unnecessary mr, will be much safer vs. gcc playing games with r13. Any volunteer ? Haven't had time to do it myself so far :-) Cheers, Ben. _______________________________________________ Linuxppc-dev mailing list [email protected] https://lists.ozlabs.org/listinfo/linuxppc-dev
