On Fri, Oct 30 2020 at 13:28, Linus Torvalds wrote:
> On Fri, Oct 30, 2020 at 2:39 AM Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
>>
>> But then we really should not name it kmap_local. 'local' suggests
>> locality, think local_irq*, local_bh* ... kmap_task would be more
>> accurate then.
>
> So the main reason I'd like to see it is because I think on a
> non-highmem machine, the new kmap should be a complete no-op. IOW,
> we'd make sure that there are no costs, no need to increment any
> "restrict migration" counts etc.
Fair enough.
> It's been a bit of a pain to have kmap_atomic() have magical side
> semantics that people might then depend on.
kmap_atomic() will still have the side semantics :)
> I think "local" could still work as a name, because it would have to
> be thread-local (and maybe we'd want a debug mode where that gets
> verified, as actual HIGHMEM machines are getting rare).
>
> I'd avoid "task", because that implies (to me, at least) that it
> wouldn't be good for interrupts etc that don't have a task context.
>
> I think the main issue is that it has to be released in the same
> context as it was created (ie no passing those things around to other
> contexts). I think "local" is fine for that, but I could imagine other
> names. The ones that come to mind are somewhat cumbersome, though
> ("shortterm" or "local_ctx" or something along those lines).
Yeah, not really intuitive either.
Let's stick with _local and add proper function documentation which
clearly says, that the side effect of non-migratability applies only for
the 32bit highmem case in order to make it work at all.
So code which needs CPU locality cannot rely on it and we have enough
debug stuff to catch something like:
kmap_local()
this_cpu_write(....)
kunmap_local()
Let me redo the pile.
While at it I might have a look at that debug request from Willy in the
other end of this thread. Any comment on that?
https://lore.kernel.org/r/87k0v7mrrd.fsf@xxxxxxxxxxxxxxxxxxxxxxx
Thanks,
tglx
