Messages in this thread | | | Date | Thu, 10 Mar 2011 18:16:54 -0800 | From | Michel Lespinasse <> | Subject | Re: [PATCH] futex: cmpxchg_futex_value_locked API change |
| |
On Thu, Mar 10, 2011 at 07:55:05PM +0100, Thomas Gleixner wrote: > On Wed, 9 Mar 2011, Michel Lespinasse wrote: > > On Tue, Mar 08, 2011 at 09:17:11PM +0100, Thomas Gleixner wrote: > > Just looked at it again in detail before picking it up. Can we please > separate the s/int/u32/ changes from the real API change ? > > > - pagefault_disable(); /* implies preempt_disable() */ > > + /* Note that preemption is disabled by futex_atomic_cmpxchg_inatomic > > + * call sites. */ > > That wants to be a separate patch as well.
No problems. That makes it 3 patches, will send as replies to this.
> > -int futex_atomic_op_inuser(int encoded_op, int __user *uaddr) > > +int futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr) > > { > > int op = (encoded_op >> 28) & 7; > > int cmp = (encoded_op >> 24) & 15; > > @@ -197,7 +197,7 @@ int futex_atomic_op_inuser(int encoded_op, int __user *uaddr) > > if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28)) > > oparg = 1 << oparg; > > > > - if (!access_ok(VERIFY_WRITE, uaddr, sizeof(int))) > > + if (!access_ok(VERIFY_WRITE, uaddr, sizeof(u32))) > > return -EFAULT; > > > > pagefault_disable(); > > So following the reasoning above, shouldn't that be the same for > futex_atomic_op_inuser() ?
futex_atomic_op_inuser() is currently called by core futex code with page faults enabled. I think that's OK - for futex_atomic_cmpxchg_inatomic I fixed the arm implementation because it was inconsistent with the other ones, but for futex_atomic_op_inuser() every arch does it that way.
-- Michel "Walken" Lespinasse A program is never fully debugged until the last user dies.
| |