[Resending, this time without html bloat...] Al, AFAIR (which isn't saying much), it's correct. sigreturn() is not a normal syscall. The code that's being jumped over normally ensures that the syscall args that were passed into a syscall are present in user-level upon returning from the syscall. However, since sigreturn() does a non-local return, that behavior would be wrong. For my own reference (and the other few people who still might have a copy of my book): this is described on page 233, last paragraph. --david On Tue, May 1, 2012 at 5:37 PM, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote: > > AFAICS, pNonSys had been used for several purposes from the very > beginning; one of them is suppression of syscall restarts and > since 2.3.99pre6-4 rt_sigreturn(2) is relying on that to prevent > a really unpleasant pile of problems. So far, so good, but that's > not the only thing conditional on pNonSys - there's also > (pNonSys) br.cond.dpnt dont_preserve_current_frame > COVER // add current frame into dirty partition and set cr.ifs > ;; > mov r19=ar.bsp // get new backing store pointer > rbs_switch: > sub r16=r16,r18 // krbs = old bsp - size of dirty partition > cmp.ne p9,p0=r0,r0 // clear p9 to skip restore of cr.ifs > ;; > sub r19=r19,r16 // calculate total byte size of dirty partition > add r18=64,r18 // don't force in0-in7 into memory... > ;; > shl r19=r19,16 // shift size of dirty partition into loadrs position > ;; > dont_preserve_current_frame: > > in .work_processed_kernel and I'd really like to know if skipping it on > sigreturn() is an intended effect. I don't know ia64 well enough to even > tell what that code is doing, let alone whether it should be done on > sigreturn() or not... > > The reason I'm asking is that we really need to loop after hitting > notify_resume_user() when we are returning to userland; not to mention > anything else, NEED_RESCHED might've been set while we had been building > sigframes. Moreover, handling all pending signals is the normal behaviour > on most of the architectures; some of the reasons for it are covered by > kinda-sorta looping inside do_signal() (to catch self-inflicted SIGSEGV), > but the full-blown variant is saner anyway and it's better to have that > logics consistent between the architectures. > > Of course, taking that loop into entry.S needs a different mechanism > for preventing double restarts; I wonder if using the same method that > is used by sigreturn() would be safe. FWIW, entry.S bits are very > small; the question is whether they are correct... > > Comments? > > diff --git a/arch/ia64/kernel/entry.S b/arch/ia64/kernel/entry.S > index 1ccbe12..27322e6 100644 > --- a/arch/ia64/kernel/entry.S > +++ b/arch/ia64/kernel/entry.S > @@ -1184,7 +1184,8 @@ skip_rbs_switch: > > .notify: > (pUStk) br.call.spnt.many rp=notify_resume_user > -.ret10: cmp.ne p6,p0=r0,r0 // p6 <- 0 (don't re-check) > +.ret10: > +(pUStk) cmp.eq.unc p6,p0=r0,r0 // p6 <- pUStk > (pLvSys)br.cond.sptk.few __paravirt_pending_syscall_end > br.cond.sptk.many .work_processed_kernel > > @@ -1250,6 +1251,7 @@ GLOBAL_ENTRY(notify_resume_user) > (pSys) mov out2=1 // out2==1 => we're in a syscall > ;; > (pNonSys) mov out2=0 // out2==0 => not a syscall > + cmp.eq pNonSys,pSys=r0,r0 // don't do restarts twice > .fframe 16 > .spillsp ar.unat, 16 > st8 [sp]=r9,-16 // allocate space for ar.unat and save it > -- > To unsubscribe from this list: send the line "unsubscribe linux-ia64" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- eGauge Systems LLC, http://egauge.net/, 1.877-EGAUGE1, fax 720.545.9768 -- To unsubscribe from this list: send the line "unsubscribe linux-ia64" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
|