On 12/25/2015 09:25 PM, David Miller wrote:
From: Rob Gardner <rob.gardner@xxxxxxxxxx>
Date: Thu, 24 Dec 2015 10:39:05 -0700
Sorry I should have noted this in the log message, but we
intentionally did not use get_fs() and set_fs() there because they are
not safe to use in a "nested" interrupt context. n.b. get_fs() is not
guaranteed to report a value consistent with %asi while we're
executing the perf interrupt handler, because it may have interrupted
kernel code where %asi is inconsistent with the thread_info current_ds
value. This is common, e.g. right in NGmemcpy.
Is that a real problem?
The return from trap will restore the %asi register properly from
the %tstate register.
It's not a problem for the context that was interrupted since as you
say, %asi will be restored properly. But in the perf interrupt context
it seems a little messy to me because get_fs() can (and will) return a
value inconsistent with %asi, so at the end of perf_callchain_user()
when set_fs() is done, it will not truly restore %asi to what it was
when the function began. So up until the return from trap, %asi contains
an unintended value. I didn't track down all possibilities to see if
anything bad might happen, but it just seemed wrong to leave things in
that state.
Also, in the code we submitted, there was an optimization in which %asi
is read, and then only set to ASI_AIUS if necessary. This drastically
reduces the number of writes to the %asi register since most of the time
%asi will contain ASI_AIUS. This seems like a reasonable optimization,
since this function may be called thousands of times per second on every
cpu. But this doesn't work at all using get_fs() since it is
inconsistent with %asi, and that is why we went with the inline
assembler to read and write %asi directly.
Merry Christmas.
Rob
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
