On Wed, Jul 7, 2021 at 7:25 AM Christoph Hellwig <hch@xxxxxx> wrote:
So I've come up with a whole (compile tested only) series. We don't
really need the preempt_disable either given the m68k saves and restores
the SFC/DFC registers on context switch:
Your commit f349b7ab5f34 ("m68k: hardcode the error value in
__{get,put}_user_asm") is wrong. The fact that
__constant_copy_to_user() passes in a positive "error" value may look
odd, but it is correct. It's not an error, it's a return value, and
"copy_to_user()" returns not -EFAULT, but the number of characters not
copied (because it needs to be able to restart - think partial
"read/write()" system calls etc).
That said, on x86, we've gotten rid of the small constant cases
entirely, because it just made such a mess of the uaccess.h files, and
we ended up just fixing any performance-sensitive code that used small
constant sizes to do the proper get/put_user() instead.
So one option is to just remove that constant-sized copy code entirely.
Also - can somebody verify that SFC/DFC is never modified in interrupts?
Because if that happens, then the "temporarily modify SFC/DFC"
sequences need to actually save/restore it, rather than just set it
back to USER_DATA.. But yes, you're right that switch_to() will
save/restore it for regular context switches.
Finally, talking about regular context switches - I think this series
should also rename "thread.fs" to "thread.segment" or something like
that.
Apart from those small details, I think that series looks very good.
Linus
