* [PATCH] media: pvrusb2: Convert timers to use timer_setup()
@ 2017-10-24 15:22 Kees Cook
2017-10-25 14:14 ` Kees Cook
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Kees Cook @ 2017-10-24 15:22 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: Mike Isely, linux-media, linux-kernel
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.
Cc: Mike Isely <isely@pobox.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/media/usb/pvrusb2/pvrusb2-hdw.c | 64 ++++++++++++++++++---------------
1 file changed, 36 insertions(+), 28 deletions(-)
diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
index ad5b25b89699..8289ee482f49 100644
--- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
+++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
@@ -330,10 +330,10 @@ static void pvr2_hdw_state_log_state(struct pvr2_hdw *);
static int pvr2_hdw_cmd_usbstream(struct pvr2_hdw *hdw,int runFl);
static int pvr2_hdw_commit_setup(struct pvr2_hdw *hdw);
static int pvr2_hdw_get_eeprom_addr(struct pvr2_hdw *hdw);
-static void pvr2_hdw_quiescent_timeout(unsigned long);
-static void pvr2_hdw_decoder_stabilization_timeout(unsigned long);
-static void pvr2_hdw_encoder_wait_timeout(unsigned long);
-static void pvr2_hdw_encoder_run_timeout(unsigned long);
+static void pvr2_hdw_quiescent_timeout(struct timer_list *);
+static void pvr2_hdw_decoder_stabilization_timeout(struct timer_list *);
+static void pvr2_hdw_encoder_wait_timeout(struct timer_list *);
+static void pvr2_hdw_encoder_run_timeout(struct timer_list *);
static int pvr2_issue_simple_cmd(struct pvr2_hdw *,u32);
static int pvr2_send_request_ex(struct pvr2_hdw *hdw,
unsigned int timeout,int probe_fl,
@@ -2373,18 +2373,15 @@ struct pvr2_hdw *pvr2_hdw_create(struct usb_interface *intf,
}
if (!hdw) goto fail;
- setup_timer(&hdw->quiescent_timer, pvr2_hdw_quiescent_timeout,
- (unsigned long)hdw);
+ timer_setup(&hdw->quiescent_timer, pvr2_hdw_quiescent_timeout, 0);
- setup_timer(&hdw->decoder_stabilization_timer,
- pvr2_hdw_decoder_stabilization_timeout,
- (unsigned long)hdw);
+ timer_setup(&hdw->decoder_stabilization_timer,
+ pvr2_hdw_decoder_stabilization_timeout, 0);
- setup_timer(&hdw->encoder_wait_timer, pvr2_hdw_encoder_wait_timeout,
- (unsigned long)hdw);
+ timer_setup(&hdw->encoder_wait_timer, pvr2_hdw_encoder_wait_timeout,
+ 0);
- setup_timer(&hdw->encoder_run_timer, pvr2_hdw_encoder_run_timeout,
- (unsigned long)hdw);
+ timer_setup(&hdw->encoder_run_timer, pvr2_hdw_encoder_run_timeout, 0);
hdw->master_state = PVR2_STATE_DEAD;
@@ -3539,10 +3536,16 @@ static void pvr2_ctl_read_complete(struct urb *urb)
complete(&hdw->ctl_done);
}
+struct hdw_timer {
+ struct timer_list timer;
+ struct pvr2_hdw *hdw;
+};
-static void pvr2_ctl_timeout(unsigned long data)
+static void pvr2_ctl_timeout(struct timer_list *t)
{
- struct pvr2_hdw *hdw = (struct pvr2_hdw *)data;
+ struct hdw_timer *timer = from_timer(timer, t, timer);
+ struct pvr2_hdw *hdw = timer->hdw;
+
if (hdw->ctl_write_pend_flag || hdw->ctl_read_pend_flag) {
hdw->ctl_timeout_flag = !0;
if (hdw->ctl_write_pend_flag)
@@ -3564,7 +3567,10 @@ static int pvr2_send_request_ex(struct pvr2_hdw *hdw,
{
unsigned int idx;
int status = 0;
- struct timer_list timer;
+ struct hdw_timer timer = {
+ .hdw = hdw,
+ };
+
if (!hdw->ctl_lock_held) {
pvr2_trace(PVR2_TRACE_ERROR_LEGS,
"Attempted to execute control transfer without lock!!");
@@ -3621,8 +3627,8 @@ static int pvr2_send_request_ex(struct pvr2_hdw *hdw,
hdw->ctl_timeout_flag = 0;
hdw->ctl_write_pend_flag = 0;
hdw->ctl_read_pend_flag = 0;
- setup_timer(&timer, pvr2_ctl_timeout, (unsigned long)hdw);
- timer.expires = jiffies + timeout;
+ timer_setup_on_stack(&timer.timer, pvr2_ctl_timeout, 0);
+ timer.timer.expires = jiffies + timeout;
if (write_len && write_data) {
hdw->cmd_debug_state = 2;
@@ -3677,7 +3683,7 @@ status);
}
/* Start timer */
- add_timer(&timer);
+ add_timer(&timer.timer);
/* Now wait for all I/O to complete */
hdw->cmd_debug_state = 4;
@@ -3687,7 +3693,7 @@ status);
hdw->cmd_debug_state = 5;
/* Stop timer */
- del_timer_sync(&timer);
+ del_timer_sync(&timer.timer);
hdw->cmd_debug_state = 6;
status = 0;
@@ -3769,6 +3775,8 @@ status);
if ((status < 0) && (!probe_fl)) {
pvr2_hdw_render_useless(hdw);
}
+ destroy_timer_on_stack(&timer.timer);
+
return status;
}
@@ -4366,9 +4374,9 @@ static int state_eval_encoder_run(struct pvr2_hdw *hdw)
/* Timeout function for quiescent timer. */
-static void pvr2_hdw_quiescent_timeout(unsigned long data)
+static void pvr2_hdw_quiescent_timeout(struct timer_list *t)
{
- struct pvr2_hdw *hdw = (struct pvr2_hdw *)data;
+ struct pvr2_hdw *hdw = from_timer(hdw, t, quiescent_timer);
hdw->state_decoder_quiescent = !0;
trace_stbit("state_decoder_quiescent",hdw->state_decoder_quiescent);
hdw->state_stale = !0;
@@ -4377,9 +4385,9 @@ static void pvr2_hdw_quiescent_timeout(unsigned long data)
/* Timeout function for decoder stabilization timer. */
-static void pvr2_hdw_decoder_stabilization_timeout(unsigned long data)
+static void pvr2_hdw_decoder_stabilization_timeout(struct timer_list *t)
{
- struct pvr2_hdw *hdw = (struct pvr2_hdw *)data;
+ struct pvr2_hdw *hdw = from_timer(hdw, t, decoder_stabilization_timer);
hdw->state_decoder_ready = !0;
trace_stbit("state_decoder_ready", hdw->state_decoder_ready);
hdw->state_stale = !0;
@@ -4388,9 +4396,9 @@ static void pvr2_hdw_decoder_stabilization_timeout(unsigned long data)
/* Timeout function for encoder wait timer. */
-static void pvr2_hdw_encoder_wait_timeout(unsigned long data)
+static void pvr2_hdw_encoder_wait_timeout(struct timer_list *t)
{
- struct pvr2_hdw *hdw = (struct pvr2_hdw *)data;
+ struct pvr2_hdw *hdw = from_timer(hdw, t, encoder_wait_timer);
hdw->state_encoder_waitok = !0;
trace_stbit("state_encoder_waitok",hdw->state_encoder_waitok);
hdw->state_stale = !0;
@@ -4399,9 +4407,9 @@ static void pvr2_hdw_encoder_wait_timeout(unsigned long data)
/* Timeout function for encoder run timer. */
-static void pvr2_hdw_encoder_run_timeout(unsigned long data)
+static void pvr2_hdw_encoder_run_timeout(struct timer_list *t)
{
- struct pvr2_hdw *hdw = (struct pvr2_hdw *)data;
+ struct pvr2_hdw *hdw = from_timer(hdw, t, encoder_run_timer);
if (!hdw->state_encoder_runok) {
hdw->state_encoder_runok = !0;
trace_stbit("state_encoder_runok",hdw->state_encoder_runok);
--
2.7.4
--
Kees Cook
Pixel Security
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] media: pvrusb2: Convert timers to use timer_setup()
2017-10-24 15:22 [PATCH] media: pvrusb2: Convert timers to use timer_setup() Kees Cook
@ 2017-10-25 14:14 ` Kees Cook
2017-10-25 16:23 ` Mike Isely
2017-10-25 16:21 ` Mike Isely
2017-10-30 9:48 ` Hans Verkuil
2 siblings, 1 reply; 5+ messages in thread
From: Kees Cook @ 2017-10-25 14:14 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: Mike Isely, linux-media, LKML
Eek, sorry, this uses timer_setup_on_stack() which is only in -next.
If you can Ack this, I can carry it in the timer tree.
Thanks!
-Kees
On Tue, Oct 24, 2017 at 5:22 PM, Kees Cook <keescook@chromium.org> wrote:
> In preparation for unconditionally passing the struct timer_list pointer to
> all timer callbacks, switch to using the new timer_setup() and from_timer()
> to pass the timer pointer explicitly.
>
> Cc: Mike Isely <isely@pobox.com>
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> Cc: linux-media@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> drivers/media/usb/pvrusb2/pvrusb2-hdw.c | 64 ++++++++++++++++++---------------
> 1 file changed, 36 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
> index ad5b25b89699..8289ee482f49 100644
> --- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
> +++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
> @@ -330,10 +330,10 @@ static void pvr2_hdw_state_log_state(struct pvr2_hdw *);
> static int pvr2_hdw_cmd_usbstream(struct pvr2_hdw *hdw,int runFl);
> static int pvr2_hdw_commit_setup(struct pvr2_hdw *hdw);
> static int pvr2_hdw_get_eeprom_addr(struct pvr2_hdw *hdw);
> -static void pvr2_hdw_quiescent_timeout(unsigned long);
> -static void pvr2_hdw_decoder_stabilization_timeout(unsigned long);
> -static void pvr2_hdw_encoder_wait_timeout(unsigned long);
> -static void pvr2_hdw_encoder_run_timeout(unsigned long);
> +static void pvr2_hdw_quiescent_timeout(struct timer_list *);
> +static void pvr2_hdw_decoder_stabilization_timeout(struct timer_list *);
> +static void pvr2_hdw_encoder_wait_timeout(struct timer_list *);
> +static void pvr2_hdw_encoder_run_timeout(struct timer_list *);
> static int pvr2_issue_simple_cmd(struct pvr2_hdw *,u32);
> static int pvr2_send_request_ex(struct pvr2_hdw *hdw,
> unsigned int timeout,int probe_fl,
> @@ -2373,18 +2373,15 @@ struct pvr2_hdw *pvr2_hdw_create(struct usb_interface *intf,
> }
> if (!hdw) goto fail;
>
> - setup_timer(&hdw->quiescent_timer, pvr2_hdw_quiescent_timeout,
> - (unsigned long)hdw);
> + timer_setup(&hdw->quiescent_timer, pvr2_hdw_quiescent_timeout, 0);
>
> - setup_timer(&hdw->decoder_stabilization_timer,
> - pvr2_hdw_decoder_stabilization_timeout,
> - (unsigned long)hdw);
> + timer_setup(&hdw->decoder_stabilization_timer,
> + pvr2_hdw_decoder_stabilization_timeout, 0);
>
> - setup_timer(&hdw->encoder_wait_timer, pvr2_hdw_encoder_wait_timeout,
> - (unsigned long)hdw);
> + timer_setup(&hdw->encoder_wait_timer, pvr2_hdw_encoder_wait_timeout,
> + 0);
>
> - setup_timer(&hdw->encoder_run_timer, pvr2_hdw_encoder_run_timeout,
> - (unsigned long)hdw);
> + timer_setup(&hdw->encoder_run_timer, pvr2_hdw_encoder_run_timeout, 0);
>
> hdw->master_state = PVR2_STATE_DEAD;
>
> @@ -3539,10 +3536,16 @@ static void pvr2_ctl_read_complete(struct urb *urb)
> complete(&hdw->ctl_done);
> }
>
> +struct hdw_timer {
> + struct timer_list timer;
> + struct pvr2_hdw *hdw;
> +};
>
> -static void pvr2_ctl_timeout(unsigned long data)
> +static void pvr2_ctl_timeout(struct timer_list *t)
> {
> - struct pvr2_hdw *hdw = (struct pvr2_hdw *)data;
> + struct hdw_timer *timer = from_timer(timer, t, timer);
> + struct pvr2_hdw *hdw = timer->hdw;
> +
> if (hdw->ctl_write_pend_flag || hdw->ctl_read_pend_flag) {
> hdw->ctl_timeout_flag = !0;
> if (hdw->ctl_write_pend_flag)
> @@ -3564,7 +3567,10 @@ static int pvr2_send_request_ex(struct pvr2_hdw *hdw,
> {
> unsigned int idx;
> int status = 0;
> - struct timer_list timer;
> + struct hdw_timer timer = {
> + .hdw = hdw,
> + };
> +
> if (!hdw->ctl_lock_held) {
> pvr2_trace(PVR2_TRACE_ERROR_LEGS,
> "Attempted to execute control transfer without lock!!");
> @@ -3621,8 +3627,8 @@ static int pvr2_send_request_ex(struct pvr2_hdw *hdw,
> hdw->ctl_timeout_flag = 0;
> hdw->ctl_write_pend_flag = 0;
> hdw->ctl_read_pend_flag = 0;
> - setup_timer(&timer, pvr2_ctl_timeout, (unsigned long)hdw);
> - timer.expires = jiffies + timeout;
> + timer_setup_on_stack(&timer.timer, pvr2_ctl_timeout, 0);
> + timer.timer.expires = jiffies + timeout;
>
> if (write_len && write_data) {
> hdw->cmd_debug_state = 2;
> @@ -3677,7 +3683,7 @@ status);
> }
>
> /* Start timer */
> - add_timer(&timer);
> + add_timer(&timer.timer);
>
> /* Now wait for all I/O to complete */
> hdw->cmd_debug_state = 4;
> @@ -3687,7 +3693,7 @@ status);
> hdw->cmd_debug_state = 5;
>
> /* Stop timer */
> - del_timer_sync(&timer);
> + del_timer_sync(&timer.timer);
>
> hdw->cmd_debug_state = 6;
> status = 0;
> @@ -3769,6 +3775,8 @@ status);
> if ((status < 0) && (!probe_fl)) {
> pvr2_hdw_render_useless(hdw);
> }
> + destroy_timer_on_stack(&timer.timer);
> +
> return status;
> }
>
> @@ -4366,9 +4374,9 @@ static int state_eval_encoder_run(struct pvr2_hdw *hdw)
>
>
> /* Timeout function for quiescent timer. */
> -static void pvr2_hdw_quiescent_timeout(unsigned long data)
> +static void pvr2_hdw_quiescent_timeout(struct timer_list *t)
> {
> - struct pvr2_hdw *hdw = (struct pvr2_hdw *)data;
> + struct pvr2_hdw *hdw = from_timer(hdw, t, quiescent_timer);
> hdw->state_decoder_quiescent = !0;
> trace_stbit("state_decoder_quiescent",hdw->state_decoder_quiescent);
> hdw->state_stale = !0;
> @@ -4377,9 +4385,9 @@ static void pvr2_hdw_quiescent_timeout(unsigned long data)
>
>
> /* Timeout function for decoder stabilization timer. */
> -static void pvr2_hdw_decoder_stabilization_timeout(unsigned long data)
> +static void pvr2_hdw_decoder_stabilization_timeout(struct timer_list *t)
> {
> - struct pvr2_hdw *hdw = (struct pvr2_hdw *)data;
> + struct pvr2_hdw *hdw = from_timer(hdw, t, decoder_stabilization_timer);
> hdw->state_decoder_ready = !0;
> trace_stbit("state_decoder_ready", hdw->state_decoder_ready);
> hdw->state_stale = !0;
> @@ -4388,9 +4396,9 @@ static void pvr2_hdw_decoder_stabilization_timeout(unsigned long data)
>
>
> /* Timeout function for encoder wait timer. */
> -static void pvr2_hdw_encoder_wait_timeout(unsigned long data)
> +static void pvr2_hdw_encoder_wait_timeout(struct timer_list *t)
> {
> - struct pvr2_hdw *hdw = (struct pvr2_hdw *)data;
> + struct pvr2_hdw *hdw = from_timer(hdw, t, encoder_wait_timer);
> hdw->state_encoder_waitok = !0;
> trace_stbit("state_encoder_waitok",hdw->state_encoder_waitok);
> hdw->state_stale = !0;
> @@ -4399,9 +4407,9 @@ static void pvr2_hdw_encoder_wait_timeout(unsigned long data)
>
>
> /* Timeout function for encoder run timer. */
> -static void pvr2_hdw_encoder_run_timeout(unsigned long data)
> +static void pvr2_hdw_encoder_run_timeout(struct timer_list *t)
> {
> - struct pvr2_hdw *hdw = (struct pvr2_hdw *)data;
> + struct pvr2_hdw *hdw = from_timer(hdw, t, encoder_run_timer);
> if (!hdw->state_encoder_runok) {
> hdw->state_encoder_runok = !0;
> trace_stbit("state_encoder_runok",hdw->state_encoder_runok);
> --
> 2.7.4
>
>
> --
> Kees Cook
> Pixel Security
--
Kees Cook
Pixel Security
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] media: pvrusb2: Convert timers to use timer_setup()
2017-10-24 15:22 [PATCH] media: pvrusb2: Convert timers to use timer_setup() Kees Cook
2017-10-25 14:14 ` Kees Cook
@ 2017-10-25 16:21 ` Mike Isely
2017-10-30 9:48 ` Hans Verkuil
2 siblings, 0 replies; 5+ messages in thread
From: Mike Isely @ 2017-10-25 16:21 UTC (permalink / raw)
To: Kees Cook
Cc: Mauro Carvalho Chehab, linux-media, Linux Kernel Mailing List,
Mike Isely at pobox
Acked-By: Mike Isely <isely@pobox.com>
On Tue, 24 Oct 2017, Kees Cook wrote:
> In preparation for unconditionally passing the struct timer_list pointer to
> all timer callbacks, switch to using the new timer_setup() and from_timer()
> to pass the timer pointer explicitly.
>
> Cc: Mike Isely <isely@pobox.com>
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> Cc: linux-media@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> drivers/media/usb/pvrusb2/pvrusb2-hdw.c | 64 ++++++++++++++++++---------------
> 1 file changed, 36 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
> index ad5b25b89699..8289ee482f49 100644
> --- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
> +++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
> @@ -330,10 +330,10 @@ static void pvr2_hdw_state_log_state(struct pvr2_hdw *);
> static int pvr2_hdw_cmd_usbstream(struct pvr2_hdw *hdw,int runFl);
> static int pvr2_hdw_commit_setup(struct pvr2_hdw *hdw);
> static int pvr2_hdw_get_eeprom_addr(struct pvr2_hdw *hdw);
> -static void pvr2_hdw_quiescent_timeout(unsigned long);
> -static void pvr2_hdw_decoder_stabilization_timeout(unsigned long);
> -static void pvr2_hdw_encoder_wait_timeout(unsigned long);
> -static void pvr2_hdw_encoder_run_timeout(unsigned long);
> +static void pvr2_hdw_quiescent_timeout(struct timer_list *);
> +static void pvr2_hdw_decoder_stabilization_timeout(struct timer_list *);
> +static void pvr2_hdw_encoder_wait_timeout(struct timer_list *);
> +static void pvr2_hdw_encoder_run_timeout(struct timer_list *);
> static int pvr2_issue_simple_cmd(struct pvr2_hdw *,u32);
> static int pvr2_send_request_ex(struct pvr2_hdw *hdw,
> unsigned int timeout,int probe_fl,
> @@ -2373,18 +2373,15 @@ struct pvr2_hdw *pvr2_hdw_create(struct usb_interface *intf,
> }
> if (!hdw) goto fail;
>
> - setup_timer(&hdw->quiescent_timer, pvr2_hdw_quiescent_timeout,
> - (unsigned long)hdw);
> + timer_setup(&hdw->quiescent_timer, pvr2_hdw_quiescent_timeout, 0);
>
> - setup_timer(&hdw->decoder_stabilization_timer,
> - pvr2_hdw_decoder_stabilization_timeout,
> - (unsigned long)hdw);
> + timer_setup(&hdw->decoder_stabilization_timer,
> + pvr2_hdw_decoder_stabilization_timeout, 0);
>
> - setup_timer(&hdw->encoder_wait_timer, pvr2_hdw_encoder_wait_timeout,
> - (unsigned long)hdw);
> + timer_setup(&hdw->encoder_wait_timer, pvr2_hdw_encoder_wait_timeout,
> + 0);
>
> - setup_timer(&hdw->encoder_run_timer, pvr2_hdw_encoder_run_timeout,
> - (unsigned long)hdw);
> + timer_setup(&hdw->encoder_run_timer, pvr2_hdw_encoder_run_timeout, 0);
>
> hdw->master_state = PVR2_STATE_DEAD;
>
> @@ -3539,10 +3536,16 @@ static void pvr2_ctl_read_complete(struct urb *urb)
> complete(&hdw->ctl_done);
> }
>
> +struct hdw_timer {
> + struct timer_list timer;
> + struct pvr2_hdw *hdw;
> +};
>
> -static void pvr2_ctl_timeout(unsigned long data)
> +static void pvr2_ctl_timeout(struct timer_list *t)
> {
> - struct pvr2_hdw *hdw = (struct pvr2_hdw *)data;
> + struct hdw_timer *timer = from_timer(timer, t, timer);
> + struct pvr2_hdw *hdw = timer->hdw;
> +
> if (hdw->ctl_write_pend_flag || hdw->ctl_read_pend_flag) {
> hdw->ctl_timeout_flag = !0;
> if (hdw->ctl_write_pend_flag)
> @@ -3564,7 +3567,10 @@ static int pvr2_send_request_ex(struct pvr2_hdw *hdw,
> {
> unsigned int idx;
> int status = 0;
> - struct timer_list timer;
> + struct hdw_timer timer = {
> + .hdw = hdw,
> + };
> +
> if (!hdw->ctl_lock_held) {
> pvr2_trace(PVR2_TRACE_ERROR_LEGS,
> "Attempted to execute control transfer without lock!!");
> @@ -3621,8 +3627,8 @@ static int pvr2_send_request_ex(struct pvr2_hdw *hdw,
> hdw->ctl_timeout_flag = 0;
> hdw->ctl_write_pend_flag = 0;
> hdw->ctl_read_pend_flag = 0;
> - setup_timer(&timer, pvr2_ctl_timeout, (unsigned long)hdw);
> - timer.expires = jiffies + timeout;
> + timer_setup_on_stack(&timer.timer, pvr2_ctl_timeout, 0);
> + timer.timer.expires = jiffies + timeout;
>
> if (write_len && write_data) {
> hdw->cmd_debug_state = 2;
> @@ -3677,7 +3683,7 @@ status);
> }
>
> /* Start timer */
> - add_timer(&timer);
> + add_timer(&timer.timer);
>
> /* Now wait for all I/O to complete */
> hdw->cmd_debug_state = 4;
> @@ -3687,7 +3693,7 @@ status);
> hdw->cmd_debug_state = 5;
>
> /* Stop timer */
> - del_timer_sync(&timer);
> + del_timer_sync(&timer.timer);
>
> hdw->cmd_debug_state = 6;
> status = 0;
> @@ -3769,6 +3775,8 @@ status);
> if ((status < 0) && (!probe_fl)) {
> pvr2_hdw_render_useless(hdw);
> }
> + destroy_timer_on_stack(&timer.timer);
> +
> return status;
> }
>
> @@ -4366,9 +4374,9 @@ static int state_eval_encoder_run(struct pvr2_hdw *hdw)
>
>
> /* Timeout function for quiescent timer. */
> -static void pvr2_hdw_quiescent_timeout(unsigned long data)
> +static void pvr2_hdw_quiescent_timeout(struct timer_list *t)
> {
> - struct pvr2_hdw *hdw = (struct pvr2_hdw *)data;
> + struct pvr2_hdw *hdw = from_timer(hdw, t, quiescent_timer);
> hdw->state_decoder_quiescent = !0;
> trace_stbit("state_decoder_quiescent",hdw->state_decoder_quiescent);
> hdw->state_stale = !0;
> @@ -4377,9 +4385,9 @@ static void pvr2_hdw_quiescent_timeout(unsigned long data)
>
>
> /* Timeout function for decoder stabilization timer. */
> -static void pvr2_hdw_decoder_stabilization_timeout(unsigned long data)
> +static void pvr2_hdw_decoder_stabilization_timeout(struct timer_list *t)
> {
> - struct pvr2_hdw *hdw = (struct pvr2_hdw *)data;
> + struct pvr2_hdw *hdw = from_timer(hdw, t, decoder_stabilization_timer);
> hdw->state_decoder_ready = !0;
> trace_stbit("state_decoder_ready", hdw->state_decoder_ready);
> hdw->state_stale = !0;
> @@ -4388,9 +4396,9 @@ static void pvr2_hdw_decoder_stabilization_timeout(unsigned long data)
>
>
> /* Timeout function for encoder wait timer. */
> -static void pvr2_hdw_encoder_wait_timeout(unsigned long data)
> +static void pvr2_hdw_encoder_wait_timeout(struct timer_list *t)
> {
> - struct pvr2_hdw *hdw = (struct pvr2_hdw *)data;
> + struct pvr2_hdw *hdw = from_timer(hdw, t, encoder_wait_timer);
> hdw->state_encoder_waitok = !0;
> trace_stbit("state_encoder_waitok",hdw->state_encoder_waitok);
> hdw->state_stale = !0;
> @@ -4399,9 +4407,9 @@ static void pvr2_hdw_encoder_wait_timeout(unsigned long data)
>
>
> /* Timeout function for encoder run timer. */
> -static void pvr2_hdw_encoder_run_timeout(unsigned long data)
> +static void pvr2_hdw_encoder_run_timeout(struct timer_list *t)
> {
> - struct pvr2_hdw *hdw = (struct pvr2_hdw *)data;
> + struct pvr2_hdw *hdw = from_timer(hdw, t, encoder_run_timer);
> if (!hdw->state_encoder_runok) {
> hdw->state_encoder_runok = !0;
> trace_stbit("state_encoder_runok",hdw->state_encoder_runok);
>
--
Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] media: pvrusb2: Convert timers to use timer_setup()
2017-10-25 14:14 ` Kees Cook
@ 2017-10-25 16:23 ` Mike Isely
0 siblings, 0 replies; 5+ messages in thread
From: Mike Isely @ 2017-10-25 16:23 UTC (permalink / raw)
To: Kees Cook; +Cc: Mauro Carvalho Chehab, linux-media, LKML, Mike Isely at pobox
Ack'ed (separate formal reply)
-Mike
On Wed, 25 Oct 2017, Kees Cook wrote:
> Eek, sorry, this uses timer_setup_on_stack() which is only in -next.
> If you can Ack this, I can carry it in the timer tree.
>
> Thanks!
>
> -Kees
>
> On Tue, Oct 24, 2017 at 5:22 PM, Kees Cook <keescook@chromium.org> wrote:
> > In preparation for unconditionally passing the struct timer_list pointer to
> > all timer callbacks, switch to using the new timer_setup() and from_timer()
> > to pass the timer pointer explicitly.
> >
> > Cc: Mike Isely <isely@pobox.com>
> > Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> > Cc: linux-media@vger.kernel.org
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > ---
> > drivers/media/usb/pvrusb2/pvrusb2-hdw.c | 64 ++++++++++++++++++---------------
> > 1 file changed, 36 insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
> > index ad5b25b89699..8289ee482f49 100644
> > --- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
> > +++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
> > @@ -330,10 +330,10 @@ static void pvr2_hdw_state_log_state(struct pvr2_hdw *);
> > static int pvr2_hdw_cmd_usbstream(struct pvr2_hdw *hdw,int runFl);
> > static int pvr2_hdw_commit_setup(struct pvr2_hdw *hdw);
> > static int pvr2_hdw_get_eeprom_addr(struct pvr2_hdw *hdw);
> > -static void pvr2_hdw_quiescent_timeout(unsigned long);
> > -static void pvr2_hdw_decoder_stabilization_timeout(unsigned long);
> > -static void pvr2_hdw_encoder_wait_timeout(unsigned long);
> > -static void pvr2_hdw_encoder_run_timeout(unsigned long);
> > +static void pvr2_hdw_quiescent_timeout(struct timer_list *);
> > +static void pvr2_hdw_decoder_stabilization_timeout(struct timer_list *);
> > +static void pvr2_hdw_encoder_wait_timeout(struct timer_list *);
> > +static void pvr2_hdw_encoder_run_timeout(struct timer_list *);
> > static int pvr2_issue_simple_cmd(struct pvr2_hdw *,u32);
> > static int pvr2_send_request_ex(struct pvr2_hdw *hdw,
> > unsigned int timeout,int probe_fl,
> > @@ -2373,18 +2373,15 @@ struct pvr2_hdw *pvr2_hdw_create(struct usb_interface *intf,
> > }
> > if (!hdw) goto fail;
> >
> > - setup_timer(&hdw->quiescent_timer, pvr2_hdw_quiescent_timeout,
> > - (unsigned long)hdw);
> > + timer_setup(&hdw->quiescent_timer, pvr2_hdw_quiescent_timeout, 0);
> >
> > - setup_timer(&hdw->decoder_stabilization_timer,
> > - pvr2_hdw_decoder_stabilization_timeout,
> > - (unsigned long)hdw);
> > + timer_setup(&hdw->decoder_stabilization_timer,
> > + pvr2_hdw_decoder_stabilization_timeout, 0);
> >
> > - setup_timer(&hdw->encoder_wait_timer, pvr2_hdw_encoder_wait_timeout,
> > - (unsigned long)hdw);
> > + timer_setup(&hdw->encoder_wait_timer, pvr2_hdw_encoder_wait_timeout,
> > + 0);
> >
> > - setup_timer(&hdw->encoder_run_timer, pvr2_hdw_encoder_run_timeout,
> > - (unsigned long)hdw);
> > + timer_setup(&hdw->encoder_run_timer, pvr2_hdw_encoder_run_timeout, 0);
> >
> > hdw->master_state = PVR2_STATE_DEAD;
> >
> > @@ -3539,10 +3536,16 @@ static void pvr2_ctl_read_complete(struct urb *urb)
> > complete(&hdw->ctl_done);
> > }
> >
> > +struct hdw_timer {
> > + struct timer_list timer;
> > + struct pvr2_hdw *hdw;
> > +};
> >
> > -static void pvr2_ctl_timeout(unsigned long data)
> > +static void pvr2_ctl_timeout(struct timer_list *t)
> > {
> > - struct pvr2_hdw *hdw = (struct pvr2_hdw *)data;
> > + struct hdw_timer *timer = from_timer(timer, t, timer);
> > + struct pvr2_hdw *hdw = timer->hdw;
> > +
> > if (hdw->ctl_write_pend_flag || hdw->ctl_read_pend_flag) {
> > hdw->ctl_timeout_flag = !0;
> > if (hdw->ctl_write_pend_flag)
> > @@ -3564,7 +3567,10 @@ static int pvr2_send_request_ex(struct pvr2_hdw *hdw,
> > {
> > unsigned int idx;
> > int status = 0;
> > - struct timer_list timer;
> > + struct hdw_timer timer = {
> > + .hdw = hdw,
> > + };
> > +
> > if (!hdw->ctl_lock_held) {
> > pvr2_trace(PVR2_TRACE_ERROR_LEGS,
> > "Attempted to execute control transfer without lock!!");
> > @@ -3621,8 +3627,8 @@ static int pvr2_send_request_ex(struct pvr2_hdw *hdw,
> > hdw->ctl_timeout_flag = 0;
> > hdw->ctl_write_pend_flag = 0;
> > hdw->ctl_read_pend_flag = 0;
> > - setup_timer(&timer, pvr2_ctl_timeout, (unsigned long)hdw);
> > - timer.expires = jiffies + timeout;
> > + timer_setup_on_stack(&timer.timer, pvr2_ctl_timeout, 0);
> > + timer.timer.expires = jiffies + timeout;
> >
> > if (write_len && write_data) {
> > hdw->cmd_debug_state = 2;
> > @@ -3677,7 +3683,7 @@ status);
> > }
> >
> > /* Start timer */
> > - add_timer(&timer);
> > + add_timer(&timer.timer);
> >
> > /* Now wait for all I/O to complete */
> > hdw->cmd_debug_state = 4;
> > @@ -3687,7 +3693,7 @@ status);
> > hdw->cmd_debug_state = 5;
> >
> > /* Stop timer */
> > - del_timer_sync(&timer);
> > + del_timer_sync(&timer.timer);
> >
> > hdw->cmd_debug_state = 6;
> > status = 0;
> > @@ -3769,6 +3775,8 @@ status);
> > if ((status < 0) && (!probe_fl)) {
> > pvr2_hdw_render_useless(hdw);
> > }
> > + destroy_timer_on_stack(&timer.timer);
> > +
> > return status;
> > }
> >
> > @@ -4366,9 +4374,9 @@ static int state_eval_encoder_run(struct pvr2_hdw *hdw)
> >
> >
> > /* Timeout function for quiescent timer. */
> > -static void pvr2_hdw_quiescent_timeout(unsigned long data)
> > +static void pvr2_hdw_quiescent_timeout(struct timer_list *t)
> > {
> > - struct pvr2_hdw *hdw = (struct pvr2_hdw *)data;
> > + struct pvr2_hdw *hdw = from_timer(hdw, t, quiescent_timer);
> > hdw->state_decoder_quiescent = !0;
> > trace_stbit("state_decoder_quiescent",hdw->state_decoder_quiescent);
> > hdw->state_stale = !0;
> > @@ -4377,9 +4385,9 @@ static void pvr2_hdw_quiescent_timeout(unsigned long data)
> >
> >
> > /* Timeout function for decoder stabilization timer. */
> > -static void pvr2_hdw_decoder_stabilization_timeout(unsigned long data)
> > +static void pvr2_hdw_decoder_stabilization_timeout(struct timer_list *t)
> > {
> > - struct pvr2_hdw *hdw = (struct pvr2_hdw *)data;
> > + struct pvr2_hdw *hdw = from_timer(hdw, t, decoder_stabilization_timer);
> > hdw->state_decoder_ready = !0;
> > trace_stbit("state_decoder_ready", hdw->state_decoder_ready);
> > hdw->state_stale = !0;
> > @@ -4388,9 +4396,9 @@ static void pvr2_hdw_decoder_stabilization_timeout(unsigned long data)
> >
> >
> > /* Timeout function for encoder wait timer. */
> > -static void pvr2_hdw_encoder_wait_timeout(unsigned long data)
> > +static void pvr2_hdw_encoder_wait_timeout(struct timer_list *t)
> > {
> > - struct pvr2_hdw *hdw = (struct pvr2_hdw *)data;
> > + struct pvr2_hdw *hdw = from_timer(hdw, t, encoder_wait_timer);
> > hdw->state_encoder_waitok = !0;
> > trace_stbit("state_encoder_waitok",hdw->state_encoder_waitok);
> > hdw->state_stale = !0;
> > @@ -4399,9 +4407,9 @@ static void pvr2_hdw_encoder_wait_timeout(unsigned long data)
> >
> >
> > /* Timeout function for encoder run timer. */
> > -static void pvr2_hdw_encoder_run_timeout(unsigned long data)
> > +static void pvr2_hdw_encoder_run_timeout(struct timer_list *t)
> > {
> > - struct pvr2_hdw *hdw = (struct pvr2_hdw *)data;
> > + struct pvr2_hdw *hdw = from_timer(hdw, t, encoder_run_timer);
> > if (!hdw->state_encoder_runok) {
> > hdw->state_encoder_runok = !0;
> > trace_stbit("state_encoder_runok",hdw->state_encoder_runok);
> > --
> > 2.7.4
> >
> >
> > --
> > Kees Cook
> > Pixel Security
>
>
>
>
--
Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] media: pvrusb2: Convert timers to use timer_setup()
2017-10-24 15:22 [PATCH] media: pvrusb2: Convert timers to use timer_setup() Kees Cook
2017-10-25 14:14 ` Kees Cook
2017-10-25 16:21 ` Mike Isely
@ 2017-10-30 9:48 ` Hans Verkuil
2 siblings, 0 replies; 5+ messages in thread
From: Hans Verkuil @ 2017-10-30 9:48 UTC (permalink / raw)
To: Kees Cook, Mauro Carvalho Chehab; +Cc: Mike Isely, linux-media, linux-kernel
On 10/24/2017 05:22 PM, Kees Cook wrote:
> In preparation for unconditionally passing the struct timer_list pointer to
> all timer callbacks, switch to using the new timer_setup() and from_timer()
> to pass the timer pointer explicitly.
>
> Cc: Mike Isely <isely@pobox.com>
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> Cc: linux-media@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
Regards,
Hans
> ---
> drivers/media/usb/pvrusb2/pvrusb2-hdw.c | 64 ++++++++++++++++++---------------
> 1 file changed, 36 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
> index ad5b25b89699..8289ee482f49 100644
> --- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
> +++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
> @@ -330,10 +330,10 @@ static void pvr2_hdw_state_log_state(struct pvr2_hdw *);
> static int pvr2_hdw_cmd_usbstream(struct pvr2_hdw *hdw,int runFl);
> static int pvr2_hdw_commit_setup(struct pvr2_hdw *hdw);
> static int pvr2_hdw_get_eeprom_addr(struct pvr2_hdw *hdw);
> -static void pvr2_hdw_quiescent_timeout(unsigned long);
> -static void pvr2_hdw_decoder_stabilization_timeout(unsigned long);
> -static void pvr2_hdw_encoder_wait_timeout(unsigned long);
> -static void pvr2_hdw_encoder_run_timeout(unsigned long);
> +static void pvr2_hdw_quiescent_timeout(struct timer_list *);
> +static void pvr2_hdw_decoder_stabilization_timeout(struct timer_list *);
> +static void pvr2_hdw_encoder_wait_timeout(struct timer_list *);
> +static void pvr2_hdw_encoder_run_timeout(struct timer_list *);
> static int pvr2_issue_simple_cmd(struct pvr2_hdw *,u32);
> static int pvr2_send_request_ex(struct pvr2_hdw *hdw,
> unsigned int timeout,int probe_fl,
> @@ -2373,18 +2373,15 @@ struct pvr2_hdw *pvr2_hdw_create(struct usb_interface *intf,
> }
> if (!hdw) goto fail;
>
> - setup_timer(&hdw->quiescent_timer, pvr2_hdw_quiescent_timeout,
> - (unsigned long)hdw);
> + timer_setup(&hdw->quiescent_timer, pvr2_hdw_quiescent_timeout, 0);
>
> - setup_timer(&hdw->decoder_stabilization_timer,
> - pvr2_hdw_decoder_stabilization_timeout,
> - (unsigned long)hdw);
> + timer_setup(&hdw->decoder_stabilization_timer,
> + pvr2_hdw_decoder_stabilization_timeout, 0);
>
> - setup_timer(&hdw->encoder_wait_timer, pvr2_hdw_encoder_wait_timeout,
> - (unsigned long)hdw);
> + timer_setup(&hdw->encoder_wait_timer, pvr2_hdw_encoder_wait_timeout,
> + 0);
>
> - setup_timer(&hdw->encoder_run_timer, pvr2_hdw_encoder_run_timeout,
> - (unsigned long)hdw);
> + timer_setup(&hdw->encoder_run_timer, pvr2_hdw_encoder_run_timeout, 0);
>
> hdw->master_state = PVR2_STATE_DEAD;
>
> @@ -3539,10 +3536,16 @@ static void pvr2_ctl_read_complete(struct urb *urb)
> complete(&hdw->ctl_done);
> }
>
> +struct hdw_timer {
> + struct timer_list timer;
> + struct pvr2_hdw *hdw;
> +};
>
> -static void pvr2_ctl_timeout(unsigned long data)
> +static void pvr2_ctl_timeout(struct timer_list *t)
> {
> - struct pvr2_hdw *hdw = (struct pvr2_hdw *)data;
> + struct hdw_timer *timer = from_timer(timer, t, timer);
> + struct pvr2_hdw *hdw = timer->hdw;
> +
> if (hdw->ctl_write_pend_flag || hdw->ctl_read_pend_flag) {
> hdw->ctl_timeout_flag = !0;
> if (hdw->ctl_write_pend_flag)
> @@ -3564,7 +3567,10 @@ static int pvr2_send_request_ex(struct pvr2_hdw *hdw,
> {
> unsigned int idx;
> int status = 0;
> - struct timer_list timer;
> + struct hdw_timer timer = {
> + .hdw = hdw,
> + };
> +
> if (!hdw->ctl_lock_held) {
> pvr2_trace(PVR2_TRACE_ERROR_LEGS,
> "Attempted to execute control transfer without lock!!");
> @@ -3621,8 +3627,8 @@ static int pvr2_send_request_ex(struct pvr2_hdw *hdw,
> hdw->ctl_timeout_flag = 0;
> hdw->ctl_write_pend_flag = 0;
> hdw->ctl_read_pend_flag = 0;
> - setup_timer(&timer, pvr2_ctl_timeout, (unsigned long)hdw);
> - timer.expires = jiffies + timeout;
> + timer_setup_on_stack(&timer.timer, pvr2_ctl_timeout, 0);
> + timer.timer.expires = jiffies + timeout;
>
> if (write_len && write_data) {
> hdw->cmd_debug_state = 2;
> @@ -3677,7 +3683,7 @@ status);
> }
>
> /* Start timer */
> - add_timer(&timer);
> + add_timer(&timer.timer);
>
> /* Now wait for all I/O to complete */
> hdw->cmd_debug_state = 4;
> @@ -3687,7 +3693,7 @@ status);
> hdw->cmd_debug_state = 5;
>
> /* Stop timer */
> - del_timer_sync(&timer);
> + del_timer_sync(&timer.timer);
>
> hdw->cmd_debug_state = 6;
> status = 0;
> @@ -3769,6 +3775,8 @@ status);
> if ((status < 0) && (!probe_fl)) {
> pvr2_hdw_render_useless(hdw);
> }
> + destroy_timer_on_stack(&timer.timer);
> +
> return status;
> }
>
> @@ -4366,9 +4374,9 @@ static int state_eval_encoder_run(struct pvr2_hdw *hdw)
>
>
> /* Timeout function for quiescent timer. */
> -static void pvr2_hdw_quiescent_timeout(unsigned long data)
> +static void pvr2_hdw_quiescent_timeout(struct timer_list *t)
> {
> - struct pvr2_hdw *hdw = (struct pvr2_hdw *)data;
> + struct pvr2_hdw *hdw = from_timer(hdw, t, quiescent_timer);
> hdw->state_decoder_quiescent = !0;
> trace_stbit("state_decoder_quiescent",hdw->state_decoder_quiescent);
> hdw->state_stale = !0;
> @@ -4377,9 +4385,9 @@ static void pvr2_hdw_quiescent_timeout(unsigned long data)
>
>
> /* Timeout function for decoder stabilization timer. */
> -static void pvr2_hdw_decoder_stabilization_timeout(unsigned long data)
> +static void pvr2_hdw_decoder_stabilization_timeout(struct timer_list *t)
> {
> - struct pvr2_hdw *hdw = (struct pvr2_hdw *)data;
> + struct pvr2_hdw *hdw = from_timer(hdw, t, decoder_stabilization_timer);
> hdw->state_decoder_ready = !0;
> trace_stbit("state_decoder_ready", hdw->state_decoder_ready);
> hdw->state_stale = !0;
> @@ -4388,9 +4396,9 @@ static void pvr2_hdw_decoder_stabilization_timeout(unsigned long data)
>
>
> /* Timeout function for encoder wait timer. */
> -static void pvr2_hdw_encoder_wait_timeout(unsigned long data)
> +static void pvr2_hdw_encoder_wait_timeout(struct timer_list *t)
> {
> - struct pvr2_hdw *hdw = (struct pvr2_hdw *)data;
> + struct pvr2_hdw *hdw = from_timer(hdw, t, encoder_wait_timer);
> hdw->state_encoder_waitok = !0;
> trace_stbit("state_encoder_waitok",hdw->state_encoder_waitok);
> hdw->state_stale = !0;
> @@ -4399,9 +4407,9 @@ static void pvr2_hdw_encoder_wait_timeout(unsigned long data)
>
>
> /* Timeout function for encoder run timer. */
> -static void pvr2_hdw_encoder_run_timeout(unsigned long data)
> +static void pvr2_hdw_encoder_run_timeout(struct timer_list *t)
> {
> - struct pvr2_hdw *hdw = (struct pvr2_hdw *)data;
> + struct pvr2_hdw *hdw = from_timer(hdw, t, encoder_run_timer);
> if (!hdw->state_encoder_runok) {
> hdw->state_encoder_runok = !0;
> trace_stbit("state_encoder_runok",hdw->state_encoder_runok);
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-10-30 9:48 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-24 15:22 [PATCH] media: pvrusb2: Convert timers to use timer_setup() Kees Cook
2017-10-25 14:14 ` Kees Cook
2017-10-25 16:23 ` Mike Isely
2017-10-25 16:21 ` Mike Isely
2017-10-30 9:48 ` Hans Verkuil
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).