On Fri, Dec 06, 2024 at 12:39:32PM +0200, Mike Rapoport wrote: > Hi Peter, > > On Thu, Dec 05, 2024 at 09:41:28AM +0100, Peter Zijlstra wrote: > > On Wed, Nov 06, 2024 at 08:50:41AM +0200, Mike Rapoport wrote: > > > > Anyway, while noodling with all that, I think there's a fairly > > fundamental error in all of this. The mem->rw_copy should not be a whole > > second allocation, it should be a (page granular) RW alias of the > > (large) ROX map. > > I've made mem->rw_copy an allocation because Andy raised a concern [1] that > we'll have an executable mapping of semi-formed code. > With an actual copy rather than an alias the ROX pages contain invalid > instructions up to the point we're through with relocations and > alternatives patching and the actual code appears in ROX mappings in one > go. > > [1] https://lore.kernel.org/all/a17c65c6-863f-4026-9c6f-a04b659e9ab4@xxxxxxxxxxxxxxxx Right, but at the same time he also calls out the problem with this; you're now forced to carry two pointers, one to indicate where the code is going to live and one to actually read/write to. (Not to mention the disparity of module_writable_address() only implying writes, but in actual reality it is required for reads too.) Pushing this dual pointer through all the pre-existing code that rewrites our code is done haphazardly and incomplete (and is quite ugly and error prone -- as evidenced by my ADL's current inability to load modules). He also then goes on to suggest a better approach would be to remap inplace to RW and back to RX. Now the problem with this is ofcourse that the remap to RW will shatter the large pages we're so very keen on preserving, but this can be rectified at the point where er remap back to RX -- it's just that the current code does not. All in all, I really don't like the place where we've end up, it is quite horrible :/
