On Mon, Sep 14, 2020 at 11:24 PM Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> wrote: > > On Tue, Sep 15, 2020 at 09:20:59AM +0300, Ard Biesheuvel wrote: > > > > The documentation of kmap_atomic() states the following: > > > > * The use of kmap_atomic/kunmap_atomic is discouraged - kmap/kunmap > > * gives a more generic (and caching) interface. But kmap_atomic can > > * be used in IRQ contexts, so in some (very limited) cases we need > > * it. > > > > so if this is no longer accurate, perhaps we should fix it? > > This hasn't been accurate for at least ten years :) Yeah, that used to be true a long long time ago, but the comment is very stale. > > But another reason I tried to avoid kmap_atomic() is that it disables > > preemption unconditionally, even on 64-bit architectures where HIGHMEM > > is irrelevant. So using kmap_atomic() here means that the bulk of > > WireGuard packet encryption runs with preemption disabled, essentially > > for legacy reasons. > > Agreed. We should definitely fix that. Well, honestly, one big reason for that is debugging. The *semantics* of the kmap_atomic() is in the name - you can't sleep in between it and the kunmap_atomic(). On any sane architecture, kmap_atomic() ends up being a no-op from an implementation standpoint, and sleeping would work just fine. But we very much want to make sure that people don't then write code that doesn't work on the bad old 32-bit machines where it really needs that sequence to be safe from preemption. So it's mostly a debug thing. I say "mostly", because there might be small other details too, like shared code, and perhaps even a couple of users out in the wild that depend on the pagefault_disable() inherent in the current kmap_atomic(), who knows.. So no, the preemption disabling isn't inherent in the operation itself. But it does have some argument for it. Linus _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx