On Wed, Mar 13, 2024 at 12:12 PM Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote: > > On Wed, Mar 13 2024 at 11:28, Pasha Tatashin wrote: > > On Wed, Mar 13, 2024 at 9:43 AM Pasha Tatashin > > <pasha.tatashin@xxxxxxxxxx> wrote: > >> Here's a potential solution that is fast, avoids locking, and ensures atomicity: > >> > >> 1. Kernel Stack VA Space > >> Dedicate a virtual address range ([KSTACK_START_VA - KSTACK_END_VA]) > >> exclusively for kernel stacks. This simplifies validation of faulting > >> addresses to be part of a stack. > >> > >> 2. Finding the faulty task > >> - Use ALIGN(fault_address, THREAD_SIZE) to calculate the end of the > >> topmost stack page (since stack addresses are aligned to THREAD_SIZE). > >> - Store the task_struct pointer as the last word on this topmost page, > >> that is always present as it is a pre-allcated stack page. > >> > >> 3. Stack Padding > >> Increase padding to 8 bytes on x86_64 (TOP_OF_KERNEL_STACK_PADDING 8) > >> to accommodate the task_struct pointer. > > > > Alternatively, do not even look-up the task_struct in > > dynamic_stack_fault(), but only install the mapping to the faulting > > address, store va in the per-cpu array, and handle the rest in > > dynamic_stack() during context switching. At that time spin locks can > > be taken, and we can do a find_vm_area(addr) call. > > > > This way, we would not need to modify TOP_OF_KERNEL_STACK_PADDING to > > keep task_struct in there. > > Why not simply doing the 'current' update right next to the stack > switching in __switch_to_asm() which has no way of faulting. > > That needs to validate whether anything uses current between the stack > switch and the place where current is updated today. I think nothing > should do so, but I would not be surprised either if it would be the > case. Such code would already today just work by chance I think, > > That should not be hard to analyze and fixup if necessary. > > So that's fixable, but I'm not really convinced that all of this is safe > and correct under all circumstances. That needs a lot more analysis than > just the trivial one I did for switch_to(). Agreed, if the current task pointer can be switched later, after loads and stores to the stack, that would be a better solution. I will incorporate this approach into my next version. I also concur that this proposal necessitates more rigorous analysis. This work remains in the investigative phase, where I am seeking a viable solution to the problem. The core issue is that kernel stacks consume excessive memory for certain workloads. However, we cannot simply reduce their size, as this leads to machine crashes in the infrequent instances where stacks do run deep. Thanks, Pasha
