Re: How to read a timestamp in kernel driver code?

From: Mike A. Harris (mharris@meteng.on.ca)
Date: Sat Mar 04 2000 - 15:26:36 EST


On Sun, 27 Feb 2000, Riley Williams wrote:

>Hi Mike, Alan.
>
>ALAN: I'm not quite sure about the ease with which the jiffies
>interval can be changed in current kernels. Memory says you're
>reasonably au fait with that, so can you advise on the current
>state of play please?

Well, I don't really need to read jiffies anymore. My SYSRQ
patch fixes SYSRQ by adding code which is activated by echoing 2
into the proc sysrq file instead. The code works regardless of
what keyboard you have, but SYSRQ works slightly differently on
keyboards when using my "2" method because it doesn't matter if
you let go of the SYSRQ key, so long as you hold ALT down.
Again, it only does this if you echo 2 into /proc..sysrq

> >> Reading through your description of what it's for, what
> >> you actually need is a simple go/no-go test that says
> >> "did these two codes arrive too fast?" with the
> >> definition of "too fast" being the problem.
>
> > No real problem. I put code to print the current "jiffies"
> > count upon SYSRQ make or break. When I press SYSRQ, it
> > prints the exact same time for both events.
>
>When you press any of the other keys, does it also print the
>exact same time for both events?

I had it only track SYSRQ presses, however once in a while the
jiffy count would be one higher for the break event than the make
event, indicating that the counter is counting ok, just the
events are happening faster than a single jiffy. It isn't hard
to believe that the keyboard sends scancodes at a rate faster
than the jiffy clock. I mean its only 100Hz which isn't really
that fast IMHO.

> > I figured I could just time how long it really takes and
> > get an idea of what an appropriate "threshold" value would
> > be. Unfortunately it didn't work. The values were indeed
> > the same.
>
>Is that just for the SYSRQ key, or for any key?

I have not explicitly tested it. It would be rather impossible
to generate info for other keys because other keys generate a
make code, and don't generate a break code until you let
go. Unless you're Bruce Jenner, or my friend Dave who has an
amazingly fast Nintendo trigger finger, you're likely going to
take several ms between make and break on any key.

If you are truely interested in knowing the time delays, I would
gladly make a quick hack to the keyboard code to do just
that. I'll bet that reading the jiffy timer is fine, and that
pressing and releasing a key manually will generate a difference
of several jiffies. If the keyboard sends multiple events in
between the time the jiffy counter rolls from one value to the
next however, all that shows is that the jiffy counter is not a
good candidate for an event counter for the given hardware (in
this case the keyboard).

I ditched it.

> > I pressed it several times to see if anything changed.
> > Sure enough, the odd press generated a jiffie count for
> > the break event that was +1 from the make event.
>
>That's to be expected.

Yep. I did that to see if it just always displayed the same
value, or if it would inc once in a while, and it did.

> > 2) The jiffy counter is not fast enough for my needs.
>
>Are you sure?
>
>Remember, all you need is to be able to distinguish between a
>keypress that sends both make and break codes together (which is
>wrong) and one that sends the codes as the event in question
>occurs (which is right).
>
>In your description above, you refer only to what happens when
>you press the SYSRQ key, and before you can make this claim, you
>need to know whether the same is true of the other keys as well.

Ahh, I see where you're heading. If the make/break are received
and the jiffy count does not increment, then I can assume that it
is a broken keyboard SYSRQ key. But it does increment by one
once in a while, so I'd have to allow for a difference of one
jiffy. That *MIGHT* work, and wouldn't affect other keys
whatsoever if the code is only inside the if block which tests
for SYSRQ (which it is), however different keyboards may respond
differently, and it would make the timing code dependant on the
jiffy clock's frequency. If sometime later HZ changes to 1024,
then the code will fail.

[SNIP]
>You would therefore need a counter that can distinguish between
>the arrival times of codes arriving at 180 codes per second to be
>able to time every gap.

That is an interesting analasis, however, the code which is
timing anything at all, is ONLY timing the make and break of
SYSRQ. Timing any other keys is meaningless - at least for what
I was trying to fix. The ONLY keycode in which the make/break
time matters is SYSRQ, so typing 400 wpm doesn't matter because
you certainly aren't typing 400wpm using the SYSRQ key
alone. ;o)

So what it really boils down to is: Is someone likely to press
SYSRQ and release it in less than 2 jiffies on a normal
non-broken keyboard? Even if they did, it would cause no
harm. If someone pressed SYSRQ that fast, then there is no way
that they'd press an actual SYSRQ key to do something in that
time too, and so everything would work no matter what.

> > So, I need an existing in kernel counter that is a bit
> > faster.
>
>Based on the above maths, you would need a counter that measures
>time units of 1/200 of a second or shorter to be able to measure
>the arrival times for all keys.

But you're complicating the fact of what it is I'm trying to
do. You're timing every key - I don't know why. I was only
interested in timing the SYSRQ key to determine a threshold value
for a keyboard generated make/break pair on SYSRQ only.

> > Jiffies are 1/100 of a second on x86 if I'm correct, so
> > that would be 10ms per jiffy.
>
>The DEFAULT on an x86 is 1/100 of a second, but I understand that
>can be changed by simply changing a single #define in the
>sources, I have certainly heard of people running Linux on x86
>with it set to use jiffies of 1/1024 second, which is the value
>used on the Alpha if my memory's correct.

Right, but I don't consider that a good reason to change the
default jiffies just to fix a SYSRQ problem on broken keyboards.
Especially not, because it may expose problems in the kernel or
userland apps that expect HZ=100. Sure the apps should likely be
fixed to be HZ independant, but I'm not going to overly
complicate things in such a wild way just to have SYSRQ working
(which it now does work with my existing patch I might add).

;o)

>Personally, I could see good reason for changing that counter to
>enable a working fix to be produced where one can't be produced
>otherwise, PROVIDING that it doesn't adversely affect the kernel
>in other ways.

Well said! I found an alternate fix though. ;o) I'll just keep
having fun replying to the rest of the message though. It is
interesting reading the ideas anyways! It is making me think of
some other cool things to do too. ;o)

> > The keyboard appears to send the two codes faster than
> > that, and I'm guessing not much faster, but a higher
> > resolution would be needed to catch things properly.
>
>Based on simple maths, the 1/100 jiffies counter should be able
>to distinguish the two cases for typists with speeds of 250 wpm
>or below, and for the usual case of speeds below 150 wpm, should
>have no problem whatsoever.

250wpm? Where is she? I want to marry her. ;o)

> > Yes, I'm familiar with the sticky keys concept, but I have
> > no use for it.
>
>Are you sure of that? Remember that just because your broken
>keyboard only shows one key with the problem doesn't mean that
>there aren't other keyboards that show the same problem for more
>than one key, or for other keys - and a solution that solves this
>problem for ALL keys is more likely to get in than one that only
>solves this problem for one specific key, no matter how important
>that key may be.

Then someone else can implement a fix for the other broken keys
if they have them. It doesn't make much sense to me to try and
fix a problem that in fact may not exist at all. I finally fixed
SYSRQ for me, and it does not in any way affect any other
keypresses except itself and the ALT break code which is
automaticly there if SYSRQ was received, so it just works. ;o)

> >> I suspect this is the tweak you refer to, and if so, and
> >> you can extend it to provide a full "sticky keys"
> >> implementation as standard in the kernel, then I for one
> >> would have no problem endorsing it in that disguise. I know
> >> several people with suchlike disabilities who would be
> >> willing to use Linux were it not for their disability.
>
> > No, thats not what its for. ;o)
>
>You appear to have completely missed the point: Whatever your
>patch is for, if it can also be used to solve other problems,
>then it WILL be used to solve them. In fact, it would almost
>certainly be to your advantage to write it such that it DOES
>solve related problems at the same time.

Perhaps I've missed your point, but perhaps you've missed the
problem I'm trying to solve. It may well be possible to solve it
in another manner, however here is the problem and solution:

Problem: I set out to fix SYSRQ to work on my keyboard.
Goals:
        1) Fix SYSRQ to work with my keyboard.
        2) It *MUST* *NOT* alter the current expected behavior
           for anybody, or interfere with keyboard handling at
           all.
        3) It *MUST* be a minimal patch, preferably contained
           within the existing CONFIG_MAGIC_SYSRQ defines.

Options:
        1) Have the code autodetect the broken keyboard and
           adjust to suit.

My attempt to time things was an attempt to implement option
number 1, however I learned that it is not possible without
making drastic changes to kernel defines, or adding other code
that isn't needed for anything else, and cannot be justified just
for the purpose of fixing a DEVELOPERS "hack" feature used by 1
in 100000 people using the kernel, and of which maybe 1 in 10000
have the bug. So option 1 was ditched.

Goal 1, 2, and 3 were met however, and unless I get a bug report
on it sometime in the next week or so, I'll assume that it works
just as good for everyone else, as it does for me. Then I'll
take out the debugging printk's and port it to 2.3.x, and send it
to Alan, etc..

> > What it is for is this: My keyboard sends SYSRQ make and
> > break simultaneously, thus SYSRQ doesn't work on my machine
> > with the existing SYSRQ keyboard code. The existing SYSRQ
> > code sets sysrq_enabled when the make code is captured, and
> > clears sysrq_enabled when the break code is captured. My
> > patch changes this so that the make code enables
> > sysrq_enabled, but the break code is completely ignored.
> > Instead, the ALT key's break code is used to clear
> > sysrq_enabled.
>
> > The idea is that ALT *HAS* to be pressed down anyways in
> > order for either SYSRQ make or break to be received, so we
> > can use the ALT break code to get out of SYSRQ mode.
>
>There is one problem with that scenario: WHat happens if the user
>tries to press ALT-SCRLK and hits the SYSRQ key whilst doing so?
>On a correctly working keyboard, the code sequence that would
>be presented to the kernel is...

Ahh! This is EXACTLY the type of discussion I wanted to see! It
helps to cover all possible scenarios. I have indeed written
down some "whatif" charts, and tried to cover the situations so
that SYSRQ only happens when it is supposed to, and any other
mistakes, etc just get ignored gracefully. So far it works fine,
but there may be times when it doesn't, so lets see.

> ALT MAKE
> SYSRQ MAKE
> SYSRQ BREAK
> SCRLK MAKE
> SCRLK BREAK
> ALT BREAK
>
>...and EXACTLY THE SAME sequence would show up on your broken
>keyboard, but the sequence that your patch would present to the
>kernel is DIFFERENT, namely...
>
> ALT MAKE
> SYSRQ MAKE
> SCRLK MAKE
> SCRLK BREAK
> ALT BREAK
> SYSRQ BREAK
>
>...and this needs dealing with before your patch works.
>
>One solution would be to define the sequence SYSRQ SCRLK as being
>identical to pressing SCRLK on its own, but that's for you to
>decide.

Evidently you haven't looked at my patch. ;o) My patch does NOT
in ANY WAY effect the sequence of keys the keyboard handler
sees. ALL of my changes are inside the code block which starts
with:

if (keycode == SYSRQ_KEY)

So if you never press SYSRQ, my code never EVER gets touched at
all. If you do press SYSRQ, it works the same way that it always
did - both on fixed and broken keyboards (when
sysrq_enabled=1). The only person who would turn on
sysrq_enabled=2, would be someone like me, who has a broken
keyboard.

With sysrq_enabled=1, what happens is: The existing 2.2.14 code
toggles the variable sysrq_pressed whenever the make OR break
code is received for SYSRQ.

With the sysrq_enabled=2, what happens is, my code ONLY toggles
(sysrq_pressed = !up_flag), when the SYSRQ *MAKE* code is
received. When the BREAK code is received, it ignores it by just
doing a "return" instead. This keeps sysrq_pressed in an
activated state, as if the key were pressed and held down. Then
of course we need a way to get OUT of SYSRQ mode, and that
happens when the ALT key's break code is received. The code
special cases everything so that any ALT key's break code works
to clear sysrq_pressed.

Any key pressed while sysrq_pressed is active, will be sent to
the sysrq handler for processing. If the key is not valid, then
the same thing will occur as if you pressed a bad key on a
working keyboard without my patch.

Please look at the patch to see exactly what it is doing
first. If after looking at it, you see some scenario that could
cause REAL TROUBLE to occur, then I certainly want to fix
it. The current patch more or less makes the SYSRQ key work like
a sticky key as long as ALT is held down. It ONLY does that
however if you enable it via proc first with:

echo 2 > /proc/sys/kernel/sysrq

> > The solution works and works well.
>
>Does that include the above scenario?

The above scenario is not possible. My patch doesn't change
the order of keyboard make/break events hitting the code. Here
is what happens with sysrq_enabled set to 1.

> ALT MAKE
> SYSRQ MAKE - enable's sysrq_pressed, now listening
> for special keypress
> SYSRQ BREAK - disable's sysrq_pressed, ignoring now
> SCRLK MAKE - processed normally
> SCRLK BREAK - processed normally
> ALT BREAK - processed normally

The above is how things happen on the existing kernel with ANY
keyboard. With my patch, with sysrq_enabled set to 1, the above
scenario is 100% identical.

Here is what happens when sysrq_enabled is set to 2 with my patch
applied:

ALT MAKE
SYSRQ MAKE - enable's sysrq_pressed, now listening
                  for special keypress
SYSRQ BREAK - we ignore this code and return, thus keeping
                  the value of sysrq_pressed the same, as if
                  SYSRQ's BREAK code were never received.
SCRLK MAKE - processed the same as if you pressed it without
                  my patch applied.
SCRLK BREAK - same as make code
ALT BREAK - toggles sysrq_pressed *ONLY* if sysrq_pressed
                  is active, then processed normally afterwards.

I haven't been able to trigger any bad behavior and I've tried to
do so a LOT on both a working keyboard and a broken one.

> > HOWEVER, it changes the way that SYSRQ works from what
> > people are used to.
>
>That may not be a bad thing - did you think of that?

No, you don't understand. It DOES NOT change the way SYSRQ works
AT ALL IN ANY WAY WHATSOEVER on broken OR fixed keyboards if you
enable SYSRQ with:

echo 1 > /proc/sys/kernel/sysrq

which is the recommended way to do so.

You need to use "echo 2" to enable my behavior, and if you do,
ALL keyboards will work with SYSRQ, however SYSRQ will act like a
sticky key until ALT is released. It works, and doesn't seem to
have any flaws so far.

No amount of bashing I've given it in the last day, with either
keyboard (good or bad) has caused any undesireable behaviour or
problems. If someone else has problems actually USING the patch,
I want to know what/how/why, and any comments they have.

If someone wants to analyze possible problem scenarios, that is
good too, but only if they've looked at my actual code and
understand it first. Prefereably, having also tried it for real.

> > Since this changes the behaviour of the SYSRQ key combo, I
> > figured that if I submitted the patch it would be rejected
> > because it changes the behaviour that everyone is used to,
> > just so that SYSRQ works on a foobar keyboard that maybe 1
> > in 500 developers have.
>
>If the solution is specific to just that one key, then yes, it
>would almost certainly be rejected. However, a solution that
>solved a more generic problem relating to ANY shift key on the
>keyboard would have a far better chance of getting in.

The problem occurs ONLY because of the way the current SYSRQ code
works. Since the SYSRQ code does not handle other keys such as
shift, then the problem can never EVER occur with them. Please
look at the keyboard.c file and see how SYSRQ works, and WHY it
does not work on keyboards that send both make and break
simultaneously. It is isolated issue direct with the SYSRQ key.

> > So... I figured if I could have the patch detect if a
> > broken keyboard was being used (by timing the time it takes
> > in between make and break on SYSRQ), then it could have the
> > existing behaviour on normal keyboards, and yet still work
> > on my foobar keyboard - but without having to hold down
> > SYSRQ because holding it down on my keyboard doesn't matter
> > anyways.
>
>Even then, it may get rejected as being code bloat - and you

Which is why I would only accept a solution that was no more than
a few small lines of code MAX. My patch, aside from comments -
which can be reduced if necessary, or removed if necessary, only
adds 8 lines of code, of which 4 lines are printk's for debugging
the patch. Once the printk's are removed, the patch adds only 4
lines of code to the actual code. Definitely not bloat - by far.

>would almost certainly need to include a new config option after
>the "Enable SYSRQ" option such that the following appeared in
>"make menuconfig" in the "Kernel hacking" screen...
>
> [*] Magic SysRq key
> [*] Fix keyboards with broken SysRq behaviour
>
>...as otherwise, you'd almost certainly have the anti-bloat
>brigade up in arms at the way you've "bloated the kernel to deal
>with a problem they don't have" as they usually claim.

Well, as I say, it is 4 lines of code, which probably results in
a kernel that is 20 bytes larger if you choose SYSRQ. Hardly
bloat. I'd certainly be willing to make it a config feature,
however I think that would bloat the size of the config code
unnecessarily to save 20 bytes of binary in the kernel image when
using SYSRQ.

Right now, I'm ONLY interested in if the thing WORKS for
everyone. Once that is done, THEN I'll consider sending it in
for general inclusion. If it is rejected, I'll try to change it
in whatever way recommended if it will be accepted. If it is
flat out rejected, then that is ok too for whatever reason. I'm
going to at least try nonetheless.

> > To do this, I need to time the thing, and it is less than a
> > single jiffie for sure. So I need a new counter to use.
>
>As stated previously, you don't actually need to time the event,
>but only to detect whether the make/break codes have come too
>fast. A similar problem occurs with the automatic decoding of
[SNIP]

Since the problem is solved, discussing the timing of keys is no
longer really relevant, so I'm going to chop it off here. Your
comments are definitely correct however, and if I'd not found
another way of doing the fix, I'd possibly have tried out some
more stuff WRT timing issues.

> > The other idea I had was to have /proc/sys/kernel/sysrq use
> > the existing code if you echo 1 into there to turn it on,
> > or to use my code if you echo 2 in there. I asked someone
> > for help on how to do this, but didn't get an answer.
>
>Linus has recently expressed his dislike of the use of /proc
>files for changing kernel behaviour, so I would tend to suspect
>that he would reject any solution incorporating this.

Perhaps he will, but I hope not. I'm NOT adding anything at all
to proc. Just allowing something that is there allready to take
on another value not previously used (2).

> > I have no idea how to use the proc stuff. Would I just
> > check the global variable that is set? Or does the code
> > that acts upon that clean up the value first?
>
>It's not a global variable in most cases, but one specific to the
>particular file you use. It's also not difficult to do.

Yes, I found that out as well. My initial solutions all added a
bunch of code. I wasn't happy with this, and would NEVER have
sent it in. My existing solution adds 4 lines of code to the
keyboard.c file in a non-bloat and non-intrusive manner.

Only one thing though: The patch that I sent out the other day,
was not the one I intended to... it was the first patch. ;o(

My second patch, is the one I intended to send out. Both patches
work, but the first patch makes the keyboard beep and you need to
switch VC's back and forth. The second patch works seemingly
without a glitch.

If it is not accepted, I can live with it. Sure is annoying not
being able to do SYSRQ without a patched kernel though, and I
really don't see any other way of fixing it other than right
inside the handle_scancode() function where SYSRQ is checked for.

Perhaps someone else can come up with a more elegant
solution.. I dunno..

Anyways, take care!

TTYL

-- 
Mike A. Harris                                     Linux advocate     
Computer Consultant                                  GNU advocate  
Capslock Consulting                          Open Source advocate

Suspicious Anagram #4: Word: PRESIDENT CLINTON OF THE USA Anagram: TO COPULATE HE FINDS INTERNS

- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Tue Mar 07 2000 - 21:00:17 EST