Re: [PATCH 1/1] thinkpad-acpi: kill hotkey_thread_mutex
On Tuesday, March 26, 2013 02:09:19 PM Oleg Nesterov wrote:
> On 03/26, Rafael J. Wysocki wrote:
> >
> > On Thursday, March 07, 2013 06:53:59 PM Oleg Nesterov wrote:
> > > hotkey_kthread() does try_to_freeze() under hotkey_thread_mutex.
> > >
> > > We can simply kill this mutex, hotkey_poll_stop_sync() does not need
> > > to serialize with hotkey_kthread(). When kthread_stop() returns the
> > > thread is already dead, it called do_exit()->complete_vfork_done().
> > >
> > > Reported-by: Artem Savkov
> > > Reported-by: Maciej Rutecki
> > > Signed-off-by: Oleg Nesterov
> >
> > I wonder if anyone is taking care of this patch?
>
> IIRC, it is already in -mm tree.
OK, thanks for letting me know.
> > > --- x/drivers/platform/x86/thinkpad_acpi.c
> > > +++ x/drivers/platform/x86/thinkpad_acpi.c
> > > @@ -1965,9 +1965,6 @@ struct tp_nvram_state {
> > > /* kthread for the hotkey poller */
> > > static struct task_struct *tpacpi_hotkey_task;
> > >
> > > -/* Acquired while the poller kthread is running, use to sync start/stop
> > > */
> > > -static struct mutex hotkey_thread_mutex;
> > > -
> > > /*
> > > * Acquire mutex to write poller control variables as an
> > > * atomic block.
> > > @@ -2463,8 +2460,6 @@ static int hotkey_kthread(void *data)
> > > unsigned int poll_freq;
> > > bool was_frozen;
> > >
> > > - mutex_lock(&hotkey_thread_mutex);
> > > -
> > > if (tpacpi_lifecycle == TPACPI_LIFE_EXITING)
> > > goto exit;
> > >
> > > @@ -2524,7 +2519,6 @@ static int hotkey_kthread(void *data)
> > > }
> > >
> > > exit:
> > > - mutex_unlock(&hotkey_thread_mutex);
> > > return 0;
> > > }
> > >
> > > @@ -2534,9 +2528,6 @@ static void hotkey_poll_stop_sync(void)
> > > if (tpacpi_hotkey_task) {
> > > kthread_stop(tpacpi_hotkey_task);
> > > tpacpi_hotkey_task = NULL;
> > > - mutex_lock(&hotkey_thread_mutex);
> > > - /* at this point, the thread did exit */
> > > - mutex_unlock(&hotkey_thread_mutex);
> > > }
> > > }
> > >
> > > @@ -3235,7 +3226,6 @@ static int __init hotkey_init(struct ibm
> > > mutex_init(&hotkey_mutex);
> > >
> > > #ifdef CONFIG_THINKPAD_ACPI_HOTKEY_POLL
> > > - mutex_init(&hotkey_thread_mutex);
> > > mutex_init(&hotkey_thread_data_mutex);
> > > #endif
> > >
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> > > the body of a message to majord...@vger.kernel.org
> > > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] thinkpad-acpi: kill hotkey_thread_mutex
On 03/26, Rafael J. Wysocki wrote:
>
> On Thursday, March 07, 2013 06:53:59 PM Oleg Nesterov wrote:
> > hotkey_kthread() does try_to_freeze() under hotkey_thread_mutex.
> >
> > We can simply kill this mutex, hotkey_poll_stop_sync() does not need
> > to serialize with hotkey_kthread(). When kthread_stop() returns the
> > thread is already dead, it called do_exit()->complete_vfork_done().
> >
> > Reported-by: Artem Savkov
> > Reported-by: Maciej Rutecki
> > Signed-off-by: Oleg Nesterov
>
> I wonder if anyone is taking care of this patch?
IIRC, it is already in -mm tree.
> Rafael
>
>
> > --- x/drivers/platform/x86/thinkpad_acpi.c
> > +++ x/drivers/platform/x86/thinkpad_acpi.c
> > @@ -1965,9 +1965,6 @@ struct tp_nvram_state {
> > /* kthread for the hotkey poller */
> > static struct task_struct *tpacpi_hotkey_task;
> >
> > -/* Acquired while the poller kthread is running, use to sync start/stop */
> > -static struct mutex hotkey_thread_mutex;
> > -
> > /*
> > * Acquire mutex to write poller control variables as an
> > * atomic block.
> > @@ -2463,8 +2460,6 @@ static int hotkey_kthread(void *data)
> > unsigned int poll_freq;
> > bool was_frozen;
> >
> > - mutex_lock(&hotkey_thread_mutex);
> > -
> > if (tpacpi_lifecycle == TPACPI_LIFE_EXITING)
> > goto exit;
> >
> > @@ -2524,7 +2519,6 @@ static int hotkey_kthread(void *data)
> > }
> >
> > exit:
> > - mutex_unlock(&hotkey_thread_mutex);
> > return 0;
> > }
> >
> > @@ -2534,9 +2528,6 @@ static void hotkey_poll_stop_sync(void)
> > if (tpacpi_hotkey_task) {
> > kthread_stop(tpacpi_hotkey_task);
> > tpacpi_hotkey_task = NULL;
> > - mutex_lock(&hotkey_thread_mutex);
> > - /* at this point, the thread did exit */
> > - mutex_unlock(&hotkey_thread_mutex);
> > }
> > }
> >
> > @@ -3235,7 +3226,6 @@ static int __init hotkey_init(struct ibm
> > mutex_init(&hotkey_mutex);
> >
> > #ifdef CONFIG_THINKPAD_ACPI_HOTKEY_POLL
> > - mutex_init(&hotkey_thread_mutex);
> > mutex_init(&hotkey_thread_data_mutex);
> > #endif
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] thinkpad-acpi: kill hotkey_thread_mutex
On Thursday, March 07, 2013 06:53:59 PM Oleg Nesterov wrote:
> hotkey_kthread() does try_to_freeze() under hotkey_thread_mutex.
>
> We can simply kill this mutex, hotkey_poll_stop_sync() does not need
> to serialize with hotkey_kthread(). When kthread_stop() returns the
> thread is already dead, it called do_exit()->complete_vfork_done().
>
> Reported-by: Artem Savkov
> Reported-by: Maciej Rutecki
> Signed-off-by: Oleg Nesterov
I wonder if anyone is taking care of this patch?
Rafael
> --- x/drivers/platform/x86/thinkpad_acpi.c
> +++ x/drivers/platform/x86/thinkpad_acpi.c
> @@ -1965,9 +1965,6 @@ struct tp_nvram_state {
> /* kthread for the hotkey poller */
> static struct task_struct *tpacpi_hotkey_task;
>
> -/* Acquired while the poller kthread is running, use to sync start/stop */
> -static struct mutex hotkey_thread_mutex;
> -
> /*
> * Acquire mutex to write poller control variables as an
> * atomic block.
> @@ -2463,8 +2460,6 @@ static int hotkey_kthread(void *data)
> unsigned int poll_freq;
> bool was_frozen;
>
> - mutex_lock(&hotkey_thread_mutex);
> -
> if (tpacpi_lifecycle == TPACPI_LIFE_EXITING)
> goto exit;
>
> @@ -2524,7 +2519,6 @@ static int hotkey_kthread(void *data)
> }
>
> exit:
> - mutex_unlock(&hotkey_thread_mutex);
> return 0;
> }
>
> @@ -2534,9 +2528,6 @@ static void hotkey_poll_stop_sync(void)
> if (tpacpi_hotkey_task) {
> kthread_stop(tpacpi_hotkey_task);
> tpacpi_hotkey_task = NULL;
> - mutex_lock(&hotkey_thread_mutex);
> - /* at this point, the thread did exit */
> - mutex_unlock(&hotkey_thread_mutex);
> }
> }
>
> @@ -3235,7 +3226,6 @@ static int __init hotkey_init(struct ibm
> mutex_init(&hotkey_mutex);
>
> #ifdef CONFIG_THINKPAD_ACPI_HOTKEY_POLL
> - mutex_init(&hotkey_thread_mutex);
> mutex_init(&hotkey_thread_data_mutex);
> #endif
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] thinkpad-acpi: kill hotkey_thread_mutex
On Thu, 07 Mar 2013, Mandeep Singh Baines wrote: > On Thu, Mar 7, 2013 at 9:53 AM, Oleg Nesterov wrote: > > hotkey_kthread() does try_to_freeze() under hotkey_thread_mutex. > > > > We can simply kill this mutex, hotkey_poll_stop_sync() does not need > > to serialize with hotkey_kthread(). When kthread_stop() returns the > > thread is already dead, it called do_exit()->complete_vfork_done(). > > > > Reported-by: Artem Savkov > > Reported-by: Maciej Rutecki > > Signed-off-by: Oleg Nesterov > > > > Reviewed-by: Mandeep Singh Baines Acked-by: Henrique de Moraes Holschuh -- "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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] thinkpad-acpi: kill hotkey_thread_mutex
On Thu, Mar 7, 2013 at 9:53 AM, Oleg Nesterov wrote:
> hotkey_kthread() does try_to_freeze() under hotkey_thread_mutex.
>
> We can simply kill this mutex, hotkey_poll_stop_sync() does not need
> to serialize with hotkey_kthread(). When kthread_stop() returns the
> thread is already dead, it called do_exit()->complete_vfork_done().
>
> Reported-by: Artem Savkov
> Reported-by: Maciej Rutecki
> Signed-off-by: Oleg Nesterov
>
Reviewed-by: Mandeep Singh Baines
> --- x/drivers/platform/x86/thinkpad_acpi.c
> +++ x/drivers/platform/x86/thinkpad_acpi.c
> @@ -1965,9 +1965,6 @@ struct tp_nvram_state {
> /* kthread for the hotkey poller */
> static struct task_struct *tpacpi_hotkey_task;
>
> -/* Acquired while the poller kthread is running, use to sync start/stop */
> -static struct mutex hotkey_thread_mutex;
> -
> /*
> * Acquire mutex to write poller control variables as an
> * atomic block.
> @@ -2463,8 +2460,6 @@ static int hotkey_kthread(void *data)
> unsigned int poll_freq;
> bool was_frozen;
>
> - mutex_lock(&hotkey_thread_mutex);
> -
> if (tpacpi_lifecycle == TPACPI_LIFE_EXITING)
> goto exit;
>
> @@ -2524,7 +2519,6 @@ static int hotkey_kthread(void *data)
> }
>
> exit:
> - mutex_unlock(&hotkey_thread_mutex);
> return 0;
> }
>
> @@ -2534,9 +2528,6 @@ static void hotkey_poll_stop_sync(void)
> if (tpacpi_hotkey_task) {
> kthread_stop(tpacpi_hotkey_task);
> tpacpi_hotkey_task = NULL;
> - mutex_lock(&hotkey_thread_mutex);
> - /* at this point, the thread did exit */
> - mutex_unlock(&hotkey_thread_mutex);
> }
> }
>
> @@ -3235,7 +3226,6 @@ static int __init hotkey_init(struct ibm
> mutex_init(&hotkey_mutex);
>
> #ifdef CONFIG_THINKPAD_ACPI_HOTKEY_POLL
> - mutex_init(&hotkey_thread_mutex);
> mutex_init(&hotkey_thread_data_mutex);
> #endif
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/1] thinkpad-acpi: kill hotkey_thread_mutex
hotkey_kthread() does try_to_freeze() under hotkey_thread_mutex.
We can simply kill this mutex, hotkey_poll_stop_sync() does not need
to serialize with hotkey_kthread(). When kthread_stop() returns the
thread is already dead, it called do_exit()->complete_vfork_done().
Reported-by: Artem Savkov
Reported-by: Maciej Rutecki
Signed-off-by: Oleg Nesterov
--- x/drivers/platform/x86/thinkpad_acpi.c
+++ x/drivers/platform/x86/thinkpad_acpi.c
@@ -1965,9 +1965,6 @@ struct tp_nvram_state {
/* kthread for the hotkey poller */
static struct task_struct *tpacpi_hotkey_task;
-/* Acquired while the poller kthread is running, use to sync start/stop */
-static struct mutex hotkey_thread_mutex;
-
/*
* Acquire mutex to write poller control variables as an
* atomic block.
@@ -2463,8 +2460,6 @@ static int hotkey_kthread(void *data)
unsigned int poll_freq;
bool was_frozen;
- mutex_lock(&hotkey_thread_mutex);
-
if (tpacpi_lifecycle == TPACPI_LIFE_EXITING)
goto exit;
@@ -2524,7 +2519,6 @@ static int hotkey_kthread(void *data)
}
exit:
- mutex_unlock(&hotkey_thread_mutex);
return 0;
}
@@ -2534,9 +2528,6 @@ static void hotkey_poll_stop_sync(void)
if (tpacpi_hotkey_task) {
kthread_stop(tpacpi_hotkey_task);
tpacpi_hotkey_task = NULL;
- mutex_lock(&hotkey_thread_mutex);
- /* at this point, the thread did exit */
- mutex_unlock(&hotkey_thread_mutex);
}
}
@@ -3235,7 +3226,6 @@ static int __init hotkey_init(struct ibm
mutex_init(&hotkey_mutex);
#ifdef CONFIG_THINKPAD_ACPI_HOTKEY_POLL
- mutex_init(&hotkey_thread_mutex);
mutex_init(&hotkey_thread_data_mutex);
#endif
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
