On Mon Feb 1, 2021 at 10:15 AM CST, David Laight wrote: > From: Christopher M. Riedl > > Sent: 01 February 2021 15:56 > > > > On Thu Jan 28, 2021 at 4:38 AM CST, David Laight wrote: > > > From: Christopher M. Riedl > > > > Sent: 28 January 2021 04:04 > > > > > > > > Reuse the "safe" implementation from signal.c except for calling > > > > unsafe_copy_from_user() to copy into a local buffer. > > > > > > > > Signed-off-by: Christopher M. Riedl <c...@codefail.de> > > > > --- > > > > arch/powerpc/kernel/signal.h | 33 +++++++++++++++++++++++++++++++++ > > > > 1 file changed, 33 insertions(+) > > > > > > > > diff --git a/arch/powerpc/kernel/signal.h b/arch/powerpc/kernel/signal.h > > > > index 2559a681536e..c18402d625f1 100644 > > > > --- a/arch/powerpc/kernel/signal.h > > > > +++ b/arch/powerpc/kernel/signal.h > > > > @@ -53,6 +53,33 @@ unsigned long copy_ckfpr_from_user(struct > > > > task_struct *task, void __user *from); > > > > &buf[i], label);\ > > > > } while (0) > > > > > > > > +#define unsafe_copy_fpr_from_user(task, from, label) do { > > > > \ > > > > + struct task_struct *__t = task; > > > > \ > > > > + u64 __user *__f = (u64 __user *)from; > > > > \ > > > > + u64 buf[ELF_NFPREG]; > > > > \ > > > > > > How big is that buffer? > > > Isn't is likely to be reasonably large compared to a reasonable > > > kernel stack frame. > > > Especially since this isn't even a leaf function. > > > > > > > I think Christophe answered this - I don't really have an opinion either > > way. What would be a 'reasonable' kernel stack frame for reference? > > Zero :-) >
Hehe good point! > > > > > > + int i; > > > > \ > > > > + > > > > \ > > > > + unsafe_copy_from_user(buf, __f, ELF_NFPREG * sizeof(double), > > > > \ > > > > + label); > > > > \ > > > > + for (i = 0; i < ELF_NFPREG - 1; i++) > > > > \ > > > > + __t->thread.TS_FPR(i) = buf[i]; > > > > \ > > > > + __t->thread.fp_state.fpscr = buf[i]; > > > > \ > > > > +} while (0) > > On further reflection, since you immediately loop through the buffer > why not just use user_access_begin() and unsafe_get_user() in the loop. Christophe had suggested this a few revisions ago as well. When I tried this approach, the signal handling performance took a pretty big hit: https://lists.ozlabs.org/pipermail/linuxppc-dev/2020-October/219351.html I included some numbers on v3 as well but decided to drop the approach altogether for this one since it just didn't seem worth the hit. > > David > > - > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, > MK1 1PT, UK > Registration No: 1397386 (Wales)