On Tue, May 10, 2022 at 07:45:49PM +0000, Song Liu wrote: > >> A KLP transition preempt notifier would help those > >> kernel threads transition to the new KLP version at > >> any time they reschedule. > > > > ... unless cond_resched() is a no-op due to CONFIG_PREEMPT? > > Based on my understanding (and a few other folks we chatted with), > a kernel thread can legally run for extended time, as long as it > calls cond_resched() at a reasonable frequency. Therefore, I > think we should be able to patch such thread easily, unless it > calls cond_resched() with being-patched function in the stack, > of course. But again, with CONFIG_PREEMPT, that doesn't work. > OTOH, Petr's mindset of allowing many minutes for the patch > transition is new to me. I need to think more about it. > Josh, what’s you opinion on this? IIUC, kpatch is designed to > only wait up to 60 seconds (no option to overwrite the time). I wouldn't be necessarily opposed to changing the kpatch timeout to something bigger, or eliminating it altogether in favor of a WARN() after x minutes. > >> How much it will help is hard to predict, but I should > >> be able to get results from a fairly large sample size > >> of systems within a few weeks :) > > > > As Peter said, keep in mind that we will need to fix other cases beyond > > Facebook, i.e., CONFIG_PREEMPT combined with non-x86 arches which don't > > have ORC so they can't reliably unwind from an IRQ. > > I think livepatch transition may fail in different cases, and we > don't need to address all of them in one shoot. Fixing some cases > is an improvement as long as we don't slow down other cases. I > understand that adding tiny overhead to __cond_resched() may end > up as a visible regression. But maybe adding it to > preempt_schedule_common() is light enough? > > Did I miss/misunderstand something? If it's a real bug, we should fix it everywhere, not just for Facebook. Otherwise CONFIG_PREEMPT and/or non-x86 arches become second-class citizens. -- Josh
