Re: rose timer t error displayed in /proc/net/rose

[Francois Romieu <romieu@fr.zoreil.com>]

[-- Attachment #1: Type: text/plain, Size: 1917 bytes --]

Thomas Osterried <thomas@osterried.de> :
[...]
> 1. why do you check for pending timer anymore?

s/do/don't/ ?

I don't check for pending timer because:
- the check is racy
- jiffies_delta_to_clock_t maxes negative delta to zero

> 2. I'm not really sure what value jiffies_delta_to_clock_t() returns. jiffies / HZ?
>    jiffies_delta_to_clock_t() returns clock_t.

I completely messed this part :o/

clock_t relates to USER_HZ like jiffies does to HZ.

[don't break the ax25]

Sure, we are in violent agreement here.

[...]
> 1. I expect rose->timer to be restarted soon. If it does not happen, is there a bug?

The relevant rose state in Bernard's sample was ROSE_STATE_2.

net/rose/rose_timer.c::rose_timer_expiry would had called
rose_disconnect(sk, ETIMEDOUT, -1, -1) so there should not
be any timer restart (afaiu, etc.).

[...]
>    => Negative may be handled due to Francois' patch now correctly.
> delta as signed long may be negative. max(0L, -nnnn) sould result to 0L.
>    This would result to 0. Perhaps proven by Francois, because he used this function and achieved a correct display of that idle value. Francois, am I correct, is "0" really displayed?

I must confess that I was not terribly professional this morning past 2AM.

The attached snippet illustrates the behavior wrt negative values
(make; insmod foo.ko ; sleep 1; rmmod foo.ko; dmesg | tail -n 2).
It also illustrates that I got the unit wrong.

This should be better:

diff --git a/net/ax25/ax25_timer.c b/net/ax25/ax25_timer.c
index 85865ebfdfa2..3c94e5a2d098 100644
--- a/net/ax25/ax25_timer.c
+++ b/net/ax25/ax25_timer.c
@@ -108,10 +108,9 @@ int ax25_t1timer_running(ax25_cb *ax25)
 
 unsigned long ax25_display_timer(struct timer_list *timer)
 {
-	if (!timer_pending(timer))
-		return 0;
+	long delta = timer->expires - jiffies;
 
-	return timer->expires - jiffies;
+	return max(0L, delta);
 }
 
 EXPORT_SYMBOL(ax25_display_timer);


[-- Attachment #2: Makefile --]
[-- Type: text/plain, Size: 129 bytes --]

obj-m := foo.o

KDIR := /lib/modules/$(shell uname -r)/build
PWD  := $(shell pwd)

default:
	$(MAKE) -C $(KDIR) M=$(PWD) modules

[-- Attachment #3: foo.c --]
[-- Type: text/plain, Size: 512 bytes --]

#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/init.h>

#include <linux/jiffies.h>

static unsigned long expires;

static void bar(void)
{
	long delta = expires - jiffies;

	printk(KERN_INFO "%lu %lu %lx\n", jiffies_delta_to_clock_t(delta),
	       jiffies_delta_to_clock_t(jiffies), delta);
}

static int __init foo_start(void)
{
	expires = jiffies;

	bar();

	return 0;
}

static void __exit foo_end(void)
{
	bar();
}

module_init(foo_start);
module_exit(foo_end);

MODULE_LICENSE("GPL");