Re: [PATCH v4 3/7] ACPI: button: Access input device's users under appropriate mutex
On Thu, Jun 25, 2020 at 12:55:29PM +0200, Rafael J. Wysocki wrote: > On Thu, Jun 25, 2020 at 7:23 AM Dmitry Torokhov > wrote: > > > > On Wed, Jun 24, 2020 at 05:00:09PM +0200, Rafael J. Wysocki wrote: > > > On Mon, Jun 8, 2020 at 1:22 PM Andrzej Pietrasiewicz > > > wrote: > > > > > > > > Inspecting input device's 'users' member should be done under device's > > > > mutex, so add appropriate invocations. > > > > > > > > Signed-off-by: Andrzej Pietrasiewicz > > > > > > This looks like a fix that might be applied independently of the other > > > patches in the series. > > > > > > Do you want me to pick it up? > > > > If you pick it we'll have to have a dance with this series. Can I apply > > instead? > > Yes, please. > > Also feel free to add > > Acked-by: Rafael J. Wysocki > > to it. Looking at the driver I think the patch and the original use of input->users is not proper. I'll post another patch addressing this shortly. Thanks. -- Dmitry
Re: [PATCH v4 3/7] ACPI: button: Access input device's users under appropriate mutex
On Thu, Jun 25, 2020 at 7:23 AM Dmitry Torokhov wrote: > > On Wed, Jun 24, 2020 at 05:00:09PM +0200, Rafael J. Wysocki wrote: > > On Mon, Jun 8, 2020 at 1:22 PM Andrzej Pietrasiewicz > > wrote: > > > > > > Inspecting input device's 'users' member should be done under device's > > > mutex, so add appropriate invocations. > > > > > > Signed-off-by: Andrzej Pietrasiewicz > > > > This looks like a fix that might be applied independently of the other > > patches in the series. > > > > Do you want me to pick it up? > > If you pick it we'll have to have a dance with this series. Can I apply > instead? Yes, please. Also feel free to add Acked-by: Rafael J. Wysocki to it. > I do not think this change has any practical effect as nobody > attaches/detached input handlers or opening/closing input devices when > system goes through device resume phase. Indeed. Thanks!
Re: [PATCH v4 3/7] ACPI: button: Access input device's users under appropriate mutex
On Wed, Jun 24, 2020 at 05:00:09PM +0200, Rafael J. Wysocki wrote:
> On Mon, Jun 8, 2020 at 1:22 PM Andrzej Pietrasiewicz
> wrote:
> >
> > Inspecting input device's 'users' member should be done under device's
> > mutex, so add appropriate invocations.
> >
> > Signed-off-by: Andrzej Pietrasiewicz
>
> This looks like a fix that might be applied independently of the other
> patches in the series.
>
> Do you want me to pick it up?
If you pick it we'll have to have a dance with this series. Can I apply
instead?
I do not think this change has any practical effect as nobody
attaches/detached input handlers or opening/closing input devices when
system goes through device resume phase.
>
> > ---
> > drivers/acpi/button.c | 5 -
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
> > index 78cfc70cb320..ff7ab291f678 100644
> > --- a/drivers/acpi/button.c
> > +++ b/drivers/acpi/button.c
> > @@ -456,13 +456,16 @@ static int acpi_button_resume(struct device *dev)
> > {
> > struct acpi_device *device = to_acpi_device(dev);
> > struct acpi_button *button = acpi_driver_data(device);
> > + struct input_dev *input = button->input;
> >
> > button->suspended = false;
> > - if (button->type == ACPI_BUTTON_TYPE_LID && button->input->users) {
> > + mutex_lock(>mutex);
> > + if (button->type == ACPI_BUTTON_TYPE_LID && input->users) {
> > button->last_state = !!acpi_lid_evaluate_state(device);
> > button->last_time = ktime_get();
> > acpi_lid_initialize_state(device);
> > }
> > + mutex_unlock(>mutex);
> > return 0;
> > }
> > #endif
> > --
> > 2.17.1
> >
Thanks.
--
Dmitry
Re: [PATCH v4 3/7] ACPI: button: Access input device's users under appropriate mutex
On Mon, Jun 8, 2020 at 1:22 PM Andrzej Pietrasiewicz
wrote:
>
> Inspecting input device's 'users' member should be done under device's
> mutex, so add appropriate invocations.
>
> Signed-off-by: Andrzej Pietrasiewicz
This looks like a fix that might be applied independently of the other
patches in the series.
Do you want me to pick it up?
> ---
> drivers/acpi/button.c | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
> index 78cfc70cb320..ff7ab291f678 100644
> --- a/drivers/acpi/button.c
> +++ b/drivers/acpi/button.c
> @@ -456,13 +456,16 @@ static int acpi_button_resume(struct device *dev)
> {
> struct acpi_device *device = to_acpi_device(dev);
> struct acpi_button *button = acpi_driver_data(device);
> + struct input_dev *input = button->input;
>
> button->suspended = false;
> - if (button->type == ACPI_BUTTON_TYPE_LID && button->input->users) {
> + mutex_lock(>mutex);
> + if (button->type == ACPI_BUTTON_TYPE_LID && input->users) {
> button->last_state = !!acpi_lid_evaluate_state(device);
> button->last_time = ktime_get();
> acpi_lid_initialize_state(device);
> }
> + mutex_unlock(>mutex);
> return 0;
> }
> #endif
> --
> 2.17.1
>
[PATCH v4 3/7] ACPI: button: Access input device's users under appropriate mutex
Inspecting input device's 'users' member should be done under device's
mutex, so add appropriate invocations.
Signed-off-by: Andrzej Pietrasiewicz
---
drivers/acpi/button.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
index 78cfc70cb320..ff7ab291f678 100644
--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -456,13 +456,16 @@ static int acpi_button_resume(struct device *dev)
{
struct acpi_device *device = to_acpi_device(dev);
struct acpi_button *button = acpi_driver_data(device);
+ struct input_dev *input = button->input;
button->suspended = false;
- if (button->type == ACPI_BUTTON_TYPE_LID && button->input->users) {
+ mutex_lock(>mutex);
+ if (button->type == ACPI_BUTTON_TYPE_LID && input->users) {
button->last_state = !!acpi_lid_evaluate_state(device);
button->last_time = ktime_get();
acpi_lid_initialize_state(device);
}
+ mutex_unlock(>mutex);
return 0;
}
#endif
--
2.17.1
