[PATCH] rfkill-input doesn't work until 5 minutes after boot

[PATCH] rfkill-input doesn't work until 5 minutes after boot

[Matthew Garrett]

rfkill-input implements debounce as follows:

        if (time_after(jiffies, task->last + msecs_to_jiffies(200))) {

However, task->last is initialised to 0 while jiffies starts at -300*HZ. 
Any input within 5 minutes of kernel start is therefore ignored. Fix by 
initialising task->last correctly.

Signed-off-by: Matthew Garrett <mjg@redhat.com>

---

.27 material?

diff --git a/net/rfkill/rfkill-input.c b/net/rfkill/rfkill-input.c
index e5b6955..de75934 100644
--- a/net/rfkill/rfkill-input.c
+++ b/net/rfkill/rfkill-input.c
@@ -101,6 +101,7 @@ static void rfkill_schedule_toggle(struct rfkill_task *task)
 		.mutex = __MUTEX_INITIALIZER(n.mutex),		\
 		.lock = __SPIN_LOCK_UNLOCKED(n.lock),		\
 		.desired_state = RFKILL_STATE_UNBLOCKED,	\
+		.last = INITIAL_JIFFIES,			\
 	}
 
 static DEFINE_RFKILL_TASK(rfkill_wlan, RFKILL_TYPE_WLAN);

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] rfkill-input doesn't work until 5 minutes after boot

[Sitsofe Wheeler]

Matthew Garrett wrote:
> rfkill-input implements debounce as follows:
> 
>         if (time_after(jiffies, task->last + msecs_to_jiffies(200))) {
> 
> However, task->last is initialised to 0 while jiffies starts at -300*HZ. 
> Any input within 5 minutes of kernel start is therefore ignored. Fix by 
> initialising task->last correctly.
> 
> Signed-off-by: Matthew Garrett <mjg@redhat.com>

Works for me (saves a huge wait to be able to do wifi toggling via a 
hotkey).

Tested-by: Sitsofe Wheeler <sitsofe@yahoo.com>

-- 
Sitsofe | http://sucs.org/~sits/

Re: [PATCH] rfkill-input doesn't work until 5 minutes after boot

[Andrew Morton]

On Sat, 4 Oct 2008 21:43:43 +0100 Matthew Garrett <mjg59@srcf.ucam.org> wrote:

> rfkill-input implements debounce as follows:
> 
>         if (time_after(jiffies, task->last + msecs_to_jiffies(200))) {
> 
> However, task->last is initialised to 0 while jiffies starts at -300*HZ. 
> Any input within 5 minutes of kernel start is therefore ignored. Fix by 
> initialising task->last correctly.
> 
> Signed-off-by: Matthew Garrett <mjg@redhat.com>
> 
> 
> diff --git a/net/rfkill/rfkill-input.c b/net/rfkill/rfkill-input.c
> index e5b6955..de75934 100644
> --- a/net/rfkill/rfkill-input.c
> +++ b/net/rfkill/rfkill-input.c
> @@ -101,6 +101,7 @@ static void rfkill_schedule_toggle(struct rfkill_task *task)
>  		.mutex = __MUTEX_INITIALIZER(n.mutex),		\
>  		.lock = __SPIN_LOCK_UNLOCKED(n.lock),		\
>  		.desired_state = RFKILL_STATE_UNBLOCKED,	\
> +		.last = INITIAL_JIFFIES,			\
>  	}
>  
>  static DEFINE_RFKILL_TASK(rfkill_wlan, RFKILL_TYPE_WLAN);
> 

That'll only work as intended if CONFIG_RFKILL_INPUT=y?  If the module
is loaded 10 minutes after boot, the timestamp is still wrong.  It might
happily happen to work, but will still fail after 2^31 jiffies (or something
like that).

Generally speaking, INITIAL_JIFFIES is a secret internal debugging
detail and its use out in general kernel code is a red flag.

I think this initialisation should be done at runtime somehow.

> .27 material?

yup.

Re: [PATCH] rfkill-input doesn't work until 5 minutes after boot

[Matthew Garrett]

On Sat, Oct 04, 2008 at 03:39:29PM -0700, Andrew Morton wrote:

> That'll only work as intended if CONFIG_RFKILL_INPUT=y?  If the module
> is loaded 10 minutes after boot, the timestamp is still wrong.  It might
> happily happen to work, but will still fail after 2^31 jiffies (or something
> like that).

Mm. True. I'll fix it up to do it at load time.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

[PATCH v2] rfkill-input doesn't work until 5 minutes after boot

[Matthew Garrett]

rfkill-input implements debounce as follows:

        if (time_after(jiffies, task->last + msecs_to_jiffies(200))) {

However, task->last is initialised to 0 while jiffies starts at -300*HZ. 
Any input within 5 minutes of kernel start is therefore ignored. Fix by 
initialising task->last correctly.

Signed-off-by: Matthew Garrett <mjg@redhat.com>

---

Set the last event value at module load time, since otherwise we'll have 
a window of failure if someone loads the module in a few hundred million 
years. I look forward to being rewarded by the post-humans for caring so 
much about them.

diff --git a/net/rfkill/rfkill-input.c b/net/rfkill/rfkill-input.c
index e5b6955..86197bb 100644
--- a/net/rfkill/rfkill-input.c
+++ b/net/rfkill/rfkill-input.c
@@ -255,6 +255,11 @@ static struct input_handler rfkill_handler = {
 
 static int __init rfkill_handler_init(void)
 {
+	rfkill_wlan.last = jiffies - HZ/5;
+	rfkill_bt.last = jiffies - HZ/5;
+	rfkill_uwb.last = jiffies - HZ/5;
+	rfkill_wimax.last = jiffies - HZ/5;
+	rfkill_wwan.last = jiffies - HZ/5;
 	return input_register_handler(&rfkill_handler);
 }
 

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH v2] rfkill-input doesn't work until 5 minutes after boot

[Andrew Morton]

On Sun, 5 Oct 2008 01:43:34 +0100 Matthew Garrett <mjg59@srcf.ucam.org> wrote:

> rfkill-input implements debounce as follows:
> 
>         if (time_after(jiffies, task->last + msecs_to_jiffies(200))) {
> 
> However, task->last is initialised to 0 while jiffies starts at -300*HZ. 
> Any input within 5 minutes of kernel start is therefore ignored. Fix by 
> initialising task->last correctly.
> 
> Signed-off-by: Matthew Garrett <mjg@redhat.com>
> 
> ---
> 
> Set the last event value at module load time, since otherwise we'll have 
> a window of failure if someone loads the module in a few hundred million 
> years. I look forward to being rewarded by the post-humans for caring so 
> much about them.

Jiffies wraparound is 49.7 days at HZ=1000.

> diff --git a/net/rfkill/rfkill-input.c b/net/rfkill/rfkill-input.c
> index e5b6955..86197bb 100644
> --- a/net/rfkill/rfkill-input.c
> +++ b/net/rfkill/rfkill-input.c
> @@ -255,6 +255,11 @@ static struct input_handler rfkill_handler = {
>  
>  static int __init rfkill_handler_init(void)
>  {
> +	rfkill_wlan.last = jiffies - HZ/5;
> +	rfkill_bt.last = jiffies - HZ/5;
> +	rfkill_uwb.last = jiffies - HZ/5;
> +	rfkill_wimax.last = jiffies - HZ/5;
> +	rfkill_wwan.last = jiffies - HZ/5;
>  	return input_register_handler(&rfkill_handler);
>  }

If someone adds a new rfkill_foo there's a risk that they'll forget to
make the corresponding change here.  A comment at the definition sites
would help.


Or, better, do something like

static struct rfkill_task rfkill_tasks[] = {
	DEFINE_RFKILL_TASK(RFKILL_TYPE_WLAN),
	...
};

#define rfkill_wlan	rfkill_tasks[0]
...

	for (i = 0; i < ARRAY_SIZE(rfkill_tasks); i++)
		...


but the new definition of DEFINE_RFKILL_TASK gets tricky.

Re: [PATCH v2] rfkill-input doesn't work until 5 minutes after boot

[Matthew Garrett]

On Sat, Oct 04, 2008 at 09:43:41PM -0700, Andrew Morton wrote:

> Jiffies wraparound is 49.7 days at HZ=1000.

Damn those pesky 32-bit systems.

> If someone adds a new rfkill_foo there's a risk that they'll forget to
> make the corresponding change here.  A comment at the definition sites
> would help.

Sure.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

[PATCH v3] rfkill-input doesn't work until 5 minutes after boot

[Matthew Garrett]

rfkill-input implements debounce as follows:

        if (time_after(jiffies, task->last + msecs_to_jiffies(200))) {

However, task->last is initialised to 0 while jiffies starts at -300*HZ. 
Any input within 5 minutes of kernel start is therefore ignored. Fix by 
initialising task->last correctly.

Signed-off-by: Matthew Garrett <mjg@redhat.com>

---

Document the requirement to initialise the struct.

diff --git a/net/rfkill/rfkill-input.c b/net/rfkill/rfkill-input.c
index e5b6955..6c7baa0 100644
--- a/net/rfkill/rfkill-input.c
+++ b/net/rfkill/rfkill-input.c
@@ -103,6 +103,8 @@ static void rfkill_schedule_toggle(struct rfkill_task *task)
 		.desired_state = RFKILL_STATE_UNBLOCKED,	\
 	}
 
+/* Remember to initialise these in rfkill_handler_init if you add any */
+
 static DEFINE_RFKILL_TASK(rfkill_wlan, RFKILL_TYPE_WLAN);
 static DEFINE_RFKILL_TASK(rfkill_bt, RFKILL_TYPE_BLUETOOTH);
 static DEFINE_RFKILL_TASK(rfkill_uwb, RFKILL_TYPE_UWB);
@@ -255,6 +257,11 @@ static struct input_handler rfkill_handler = {
 
 static int __init rfkill_handler_init(void)
 {
+	rfkill_wlan.last = jiffies - HZ/5;
+	rfkill_bt.last = jiffies - HZ/5;
+	rfkill_uwb.last = jiffies - HZ/5;
+	rfkill_wimax.last = jiffies - HZ/5;
+	rfkill_wwan.last = jiffies - HZ/5;
 	return input_register_handler(&rfkill_handler);
 }
 

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH v3] rfkill-input doesn't work until 5 minutes after boot

[Henrique de Moraes Holschuh]

On Sun, 05 Oct 2008, Matthew Garrett wrote:
> rfkill-input implements debounce as follows:
> 
>         if (time_after(jiffies, task->last + msecs_to_jiffies(200))) {
> 
> However, task->last is initialised to 0 while jiffies starts at -300*HZ. 
> Any input within 5 minutes of kernel start is therefore ignored. Fix by 
> initialising task->last correctly.
> 
> Signed-off-by: Matthew Garrett <mjg@redhat.com>

FWIW:
Acked-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [PATCH v3] rfkill-input doesn't work until 5 minutes after boot

[Ivo van Doorn]

On Sunday 05 October 2008, Matthew Garrett wrote:
> rfkill-input implements debounce as follows:
> 
>         if (time_after(jiffies, task->last + msecs_to_jiffies(200))) {
> 
> However, task->last is initialised to 0 while jiffies starts at -300*HZ. 
> Any input within 5 minutes of kernel start is therefore ignored. Fix by 
> initialising task->last correctly.
>
> Signed-off-by: Matthew Garrett <mjg@redhat.com>

I am not too happy about the multiple jiffies - HZ/5 statements
regardless of the comment that it should be updated when new tasks are
defined.

But on the other hand I haven't seen any real alternatives yet either,
so I have no objections against this patch either.

Acked-by: Ivo van Doorn <IvDoorn@gmail.com>
 
> ---
> 
> Document the requirement to initialise the struct.
> 
> diff --git a/net/rfkill/rfkill-input.c b/net/rfkill/rfkill-input.c
> index e5b6955..6c7baa0 100644
> --- a/net/rfkill/rfkill-input.c
> +++ b/net/rfkill/rfkill-input.c
> @@ -103,6 +103,8 @@ static void rfkill_schedule_toggle(struct rfkill_task *task)
>  		.desired_state = RFKILL_STATE_UNBLOCKED,	\
>  	}
>  
> +/* Remember to initialise these in rfkill_handler_init if you add any */
> +
>  static DEFINE_RFKILL_TASK(rfkill_wlan, RFKILL_TYPE_WLAN);
>  static DEFINE_RFKILL_TASK(rfkill_bt, RFKILL_TYPE_BLUETOOTH);
>  static DEFINE_RFKILL_TASK(rfkill_uwb, RFKILL_TYPE_UWB);
> @@ -255,6 +257,11 @@ static struct input_handler rfkill_handler = {
>  
>  static int __init rfkill_handler_init(void)
>  {
> +	rfkill_wlan.last = jiffies - HZ/5;
> +	rfkill_bt.last = jiffies - HZ/5;
> +	rfkill_uwb.last = jiffies - HZ/5;
> +	rfkill_wimax.last = jiffies - HZ/5;
> +	rfkill_wwan.last = jiffies - HZ/5;
>  	return input_register_handler(&rfkill_handler);
>  }
>  
> 

Re: [PATCH v3] rfkill-input doesn't work until 5 minutes after boot

[Sitsofe Wheeler]

Matthew Garrett wrote:
> rfkill-input implements debounce as follows:
> 
>         if (time_after(jiffies, task->last + msecs_to_jiffies(200))) {
> 
> However, task->last is initialised to 0 while jiffies starts at -300*HZ. 
> Any input within 5 minutes of kernel start is therefore ignored. Fix by 
> initialising task->last correctly.

This patch also works for me.

Tested-by: Sitsofe Wheeler <sitsofe@yahoo.com>

-- 
Sitsofe | http://sucs.org/~sits/

Re: [PATCH v3] rfkill-input doesn't work until 5 minutes after boot

[Dmitry Torokhov]

Hi Matthew,

On Sun, Oct 05, 2008 at 12:02:26PM +0100, Matthew Garrett wrote:
> rfkill-input implements debounce as follows:
> 
>         if (time_after(jiffies, task->last + msecs_to_jiffies(200))) {
> 
> However, task->last is initialised to 0 while jiffies starts at -300*HZ. 
> Any input within 5 minutes of kernel start is therefore ignored. Fix by 
> initialising task->last correctly.
> 
> Signed-off-by: Matthew Garrett <mjg@redhat.com>
> 

I'd rather have something like below. It should probably go through
David's (networking) tree. since it is in net/.

-- 
Dmitry

Input: rfkill-input doesn't work until 5 minutes after boot

From: Matthew Garrett <mjg59@srcf.ucam.org>

rfkill-input implements debounce as follows:

        if (time_after(jiffies, task->last + msecs_to_jiffies(200))) {

However, task->last is initialised to 0 while jiffies starts at -300*HZ.
Any input within 5 minutes of kernel start is therefore ignored. Fix by
initialising task->last at module load.

Signed-off-by: Matthew Garrett <mjg@redhat.com>
Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 net/rfkill/rfkill-input.c |   28 ++++++++++++++++++----------
 1 files changed, 18 insertions(+), 10 deletions(-)


diff --git a/net/rfkill/rfkill-input.c b/net/rfkill/rfkill-input.c
index e5b6955..775c6ba 100644
--- a/net/rfkill/rfkill-input.c
+++ b/net/rfkill/rfkill-input.c
@@ -109,21 +109,24 @@ static DEFINE_RFKILL_TASK(rfkill_uwb, RFKILL_TYPE_UWB);
 static DEFINE_RFKILL_TASK(rfkill_wimax, RFKILL_TYPE_WIMAX);
 static DEFINE_RFKILL_TASK(rfkill_wwan, RFKILL_TYPE_WWAN);
 
+static struct rfkill_task *rfkill_tasks[] = {
+	&rfkill_wlan,
+	&rfkill_bt,
+	&rfkill_uwb,
+	&rfkill_wimax,
+	&rfkill_wwan,
+};
+
 static void rfkill_schedule_evsw_rfkillall(int state)
 {
+	int i;
+
 	/* EVERY radio type. state != 0 means radios ON */
 	/* handle EPO (emergency power off) through shortcut */
 	if (state) {
-		rfkill_schedule_set(&rfkill_wwan,
-				    RFKILL_STATE_UNBLOCKED);
-		rfkill_schedule_set(&rfkill_wimax,
-				    RFKILL_STATE_UNBLOCKED);
-		rfkill_schedule_set(&rfkill_uwb,
-				    RFKILL_STATE_UNBLOCKED);
-		rfkill_schedule_set(&rfkill_bt,
-				    RFKILL_STATE_UNBLOCKED);
-		rfkill_schedule_set(&rfkill_wlan,
-				    RFKILL_STATE_UNBLOCKED);
+		for (i = 0; i < ARRAY_SIZE(rfkill_tasks); i++)
+			rfkill_schedule_set(rfkill_tasks[i],
+					    RFKILL_STATE_UNBLOCKED);
 	} else
 		rfkill_schedule_epo();
 }
@@ -255,6 +258,11 @@ static struct input_handler rfkill_handler = {
 
 static int __init rfkill_handler_init(void)
 {
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(rfkill_tasks); i++)
+		rfkill_tasks[i]->last = jiffies - HZ / 5;
+
 	return input_register_handler(&rfkill_handler);
 }
 

Re: [PATCH v3] rfkill-input doesn't work until 5 minutes after boot

[Matthew Garrett]

On Sun, Oct 05, 2008 at 05:24:39PM -0400, Dmitry Torokhov wrote:

> I'd rather have something like below. It should probably go through
> David's (networking) tree. since it is in net/.

Fine with me, as long as Sitsofe can confirm it works?

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH v3] rfkill-input doesn't work until 5 minutes after boot

[Henrique de Moraes Holschuh]

On Sun, 05 Oct 2008, Ivo van Doorn wrote:
> I am not too happy about the multiple jiffies - HZ/5 statements
> regardless of the comment that it should be updated when new tasks are
> defined.
> 
> But on the other hand I haven't seen any real alternatives yet either,
> so I have no objections against this patch either.

It doesn't matter much.  The next rfkill patchset will remove most of that
code, anyway (and yes, I *will* make sure it doesn't have the same issue
this patch is fixing).  But that's 2.6.28/2.6.29 stuff, probably 2.6.29.

I didn't send them yet because I was still changing other parts of the code
too much.  I may send them soon as RFC.

In other words, don't worry too much about that maintenance annoyance in
rfkill-input.  It will not be around for long.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [PATCH v3] rfkill-input doesn't work until 5 minutes after boot

[Sitsofe Wheeler]

Matthew Garrett wrote:
> On Sun, Oct 05, 2008 at 05:24:39PM -0400, Dmitry Torokhov wrote:
> 
>> I'd rather have something like below. It should probably go through
>> David's (networking) tree. since it is in net/.
> 
> Fine with me, as long as Sitsofe can confirm it works?

Dmitry's patch also works for me.

Tested-by: Sitsofe Wheeler <sitsofe@yahoo.com>


-- 
Sitsofe | http://sucs.org/~sits/

Re: [PATCH v3] rfkill-input doesn't work until 5 minutes after boot

[Ivo van Doorn]

On Monday 06 October 2008, Sitsofe Wheeler wrote:
> Matthew Garrett wrote:
> > On Sun, Oct 05, 2008 at 05:24:39PM -0400, Dmitry Torokhov wrote:
> > 
> >> I'd rather have something like below. It should probably go through
> >> David's (networking) tree. since it is in net/.
> > 
> > Fine with me, as long as Sitsofe can confirm it works?
> 
> Dmitry's patch also works for me.
> 
> Tested-by: Sitsofe Wheeler <sitsofe@yahoo.com>

Dmitry's patch has my personal preference, because it
decreases the change of forgetting the initialization.

Acked-by: Ivo van Doorn <IvDoorn@gmail.com>