Re: [PATCH] m68knommu: remove set_fs()

On Wed, Jul 7, 2021 at 7:25 AM Christoph Hellwig <[email protected]> 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