On Wed, Nov 27, 2013 at 2:02 PM, H. Peter Anvin <h...@zytor.com> wrote: > For the record, this is the entire patch necessary to do the > sync_cores() system call -- and no potential interactions with security > frameworks or whatnot, simply because no security-sensitive operations > are performed of any kind. > > Comments/opinions appreciated.
If we just care about a core sync, then: - on_each_cpu() is overkill, since you really only want each CPU that can run that process. And we have that bitmask already: it's the same bitmask that we use for TLB flush purposes. - calling "sync_cores()" cross-cpu is overkill, since the IPI will already sync the other cores. And the current core is already going to be synchronized wrt the code change, since we're in kernel mode and the code is in user mode. So I actually think your patch does too much - although it's possible that we should have a system call argument saying "sync just this process" or "sync all cores" so that people can literally do some "modify global instruction in shared library" games if they really really want to. That said, the thing I really do *not* like about this patch is that it makes the "insert 'int3' instruction" visible to user space. That makes the "global instruction" replacement impossible, for example, because while we'd sync all cores, we have no way to protect against other processes getting the breakpoint exception and just SIGSEGV'ing randomly as a result of *that*. And even if we say "well, we don't care about the global case" (which is quite possibly the right thign to do), it's actually hard for threaded libraries to sanely catch SIGSGV for the breakpoint case too. And since the only reason for this existing IN THE FIRST PLACE is the threaded case, I have to say that I absolutely despise this "simpler" interface. The interface may be simpler, but it's garbage. I didn't like the first patch either, but the first patch with "replace the stupid pseudo-signal back-call with just waiting" at least seems to be a good interface. Unlike this kind of stuff. Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/