On Thu, 2009-08-06 at 14:58 +1000, Paul Mackerras wrote:
> +
> +#else /* CONFIG_PPC64 */
> +/*
> + * On 32-bit we just access the address and let hash_page create a
> + * HPTE if necessary, so there is no need to fall back to reading
> + * the page tables. Since this is called at interrupt level,
> + * do_page_fault() won't treat a DSI as a page fault.
> + */
Minor nit here... The comment makes it think there's only hash based
32-bit processors :-) In fact, there's a little issue with non-hash ones
here, which is that they rely on
do_page_fault->handle_mm_fault->ptep_set_access_flags to set
_PAGE_ACCESSED, and the TLB miss handlers are going to fault if that's
not set.
Not a big deal, but it does mean that if you have stack pages that
aren't young, they will fail to backtrace (though that's probably
unlikely unless you spend a lot of time very deep down a huge call
chain).
> +static int read_user_stack_32(unsigned int __user *ptr, unsigned int *ret)
> +{
> + if ((unsigned long)ptr > TASK_SIZE - sizeof(unsigned int) ||
> + ((unsigned long)ptr & 3))
> + return -EFAULT;
> +
> + return __get_user_inatomic(*ret, ptr);
> +}
> +
> +static inline void perf_callchain_user_64(struct pt_regs *regs,
> + struct perf_callchain_entry *entry)
> +{
> +}
> +
> +static inline int current_is_64bit(void)
> +{
> + return 0;
> +}
> +
> +static inline int valid_user_sp(unsigned long sp, int is_64)
> +{
> + if (!sp || (sp & 7) || sp > TASK_SIZE - 32)
I know the above is right but I would still have preferred () around
TASK_SIZE - 32 :-) In fact, || has lower precedence than & (I checked !)
so in theory if you really wanted to get rid of braces, you could have
written
if (!sp || sp & 7 || sp > TASK_SIZE - 32)
But heh, that sucks :-)
> +struct signal_frame_32 {
> + char dummy[__SIGNAL_FRAMESIZE32];
> + struct sigcontext32 sctx;
> + struct mcontext32 mctx;
> + int abigap[56];
> +};
> +
> +/*
> + * Layout for RT signal frames
> + */
> +struct rt_signal_frame_32 {
> + char dummy[__SIGNAL_FRAMESIZE32 + 16];
> + compat_siginfo_t info;
> + struct ucontext32 uc;
> + int abigap[56];
> +};
Should we put those somewhere shared ? They are almost the same
as the ones in signal_32.c apart from the initial gap... oh well, no big
deal if you want to keep them here for now.
Overall looks fine and I suppose it also works but I may have missed
something subtle.
Cheers,
Ben.
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
