Hi Linus, going back to this one, I missed that bit earlier - the last three hunks of your patch replaced KERNEL_DS by USER_DATA, everywhere else it's replaced by SUPER_DATA. Typo, or something too subtle for me to grasp? Cheers, Michael Am 06.07.21 um 08:39 schrieb Linus Torvalds:
On Mon, Jul 5, 2021 at 1:46 AM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:Probably this should be select SET_FS if CPU_HAS_ADDRESS_SPACESActually, I don't think m68k has a single real "set_fs()" at all, and it should just be converted as-is to not use CONFIG_SET_FS. Yes, there is a "set_fs()" function, but none of the remaining uses actually are the traditional kernel style of "use kernel addresses as user addresses". So as far as the *kernel* is concerned, m68k already looks like a no-SET_FS architecture, and "set-fs()" is purely a syntactic thing. So I think the right thing to do looks something like this: - make the rule be that SFC/DFC is always normally USER_DATA - the special m68k sequences that need to play with special segments will always do preempt_disable(); set_segment(..whatever segment they need..); .. do the special operation .. set_segment(USER_DATA); preempt_enable(); - set_fs() goes away entirely, because the user access functions always work on USER_DATA and SFC/DFC is always right for them. Anyway, I'm attaching a COMPLETELY UNTESTED AND ALMOST CERTAINLY VERY VERY BROKEN patch that is likely not at all correct, but shows what I think the solution should be. The important thing is really just the removal of SET_FS entirely, the rest is me winging it. NOTE! The above very much assumes that all the special non-USER_DATA accesses can always be done with preemption disabled. Why? Because I also made the context switching always just save USER_DATA as the segment. I didn't *remove* the segment switching - because I'm not sure if that "just disable preemption across things that do segment games" is actually valid. So *if* that preempt_disable/enable is ok, then the segment switching can just be removed entirely at context switch time. And if it is *not* ok, then the preempt_disable/enable should go away, and the context switch should save/restore the actual SFC/DFC value. Again - let me be very very clear: the only m68k I've ever used was a 68008. It didn't have segments. This patch is COMPLETE GARBAGE. Do not trust it. Do not use it for anything but a "Linus suggests maybe something along these lines could work". This has not even been build-tested, and I'm pretty sure it won't build as-is. It really is a "Linus did some pattern matching and a few grep's". Oh, and if the magical SFC/DFC games can happen in interrupts, then the "preempt_disable/enable" actually needs to be a full interrupt disable/enable, or the code needs to re-introduce that "save old value, restore it". So that's another assumption this example patch makes - that the SFC/DFC games only happen in process context. Which may be complete garbage too. Did I mention that I think this patch is garbage? Because it really is. Linus
