Not really harmful not to, but also not harm in grabbing the lock. And
this shuts up a new WARNING I introduced in commit ddde3c18b700 ("vt:
More locking checks").
Reported-by: Jens Remus <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Nicolas Pitre <[email protected]>
Cc: Martin Hostettler <[email protected]>
Cc: Adam Borowski <[email protected]>
Cc: Mikulas Patocka <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Cc: Daniel Vetter <[email protected]>
Cc: Sam Ravnborg <[email protected]>
---
drivers/tty/vt/vt.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index ec92f36ab5c4..34aa39d1aed9 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -3771,7 +3771,11 @@ static ssize_t show_bind(struct device *dev, struct device_attribute *attr,
char *buf)
{
struct con_driver *con = dev_get_drvdata(dev);
- int bind = con_is_bound(con->con);
+ int bind;
+
+ console_lock();
+ bind = con_is_bound(con->con);
+ console_unlock();
return snprintf(buf, PAGE_SIZE, "%i\n", bind);
}
--
2.20.1
Hi Daniel.
Patch looks good. You can add my:
Acked-by: Sam Ravnborg <[email protected]>
For good measure I checked all other users of con_is_bound()
and they looked good from a locking perspective.
Then I looked a bit more for missing locking and lost
the overview.
On Thu, Jul 18, 2019 at 10:09:03AM +0200, Daniel Vetter wrote:
> Not really harmful not to, but also not harm in grabbing the lock. And
> this shuts up a new WARNING I introduced in commit ddde3c18b700 ("vt:
> More locking checks").
Maybe add the warning that Jens reported to the changelog, in case
someone hits something that looks like this warning.
Mainly for google fodder, but also in case changelogs are searched.
Sam
>
> Reported-by: Jens Remus <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Nicolas Pitre <[email protected]>
> Cc: Martin Hostettler <[email protected]>
> Cc: Adam Borowski <[email protected]>
> Cc: Mikulas Patocka <[email protected]>
> Signed-off-by: Daniel Vetter <[email protected]>
> Cc: Daniel Vetter <[email protected]>
> Cc: Sam Ravnborg <[email protected]>
> ---
> drivers/tty/vt/vt.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index ec92f36ab5c4..34aa39d1aed9 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -3771,7 +3771,11 @@ static ssize_t show_bind(struct device *dev, struct device_attribute *attr,
> char *buf)
> {
> struct con_driver *con = dev_get_drvdata(dev);
> - int bind = con_is_bound(con->con);
> + int bind;
> +
> + console_lock();
> + bind = con_is_bound(con->con);
> + console_unlock();
>
> return snprintf(buf, PAGE_SIZE, "%i\n", bind);
> }
> --
> 2.20.1
Am 18.07.2019 um 10:09 schrieb Daniel Vetter:
> Not really harmful not to, but also not harm in grabbing the lock. And
> this shuts up a new WARNING I introduced in commit ddde3c18b700 ("vt:
> More locking checks").
>
> Reported-by: Jens Remus <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Nicolas Pitre <[email protected]>
> Cc: Martin Hostettler <[email protected]>
> Cc: Adam Borowski <[email protected]>
> Cc: Mikulas Patocka <[email protected]>
> Signed-off-by: Daniel Vetter <[email protected]>
> Cc: Daniel Vetter <[email protected]>
> Cc: Sam Ravnborg <[email protected]>
> ---
> drivers/tty/vt/vt.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
Thank you for the quick fix! Looks fine to me. Did test with cat as well
as our dump2tar utility. The warning is gone.
Tested-by: Jens Remus <[email protected]>
Regards,
Jens Remus
--
Linux on Z and z/VSE Development & Service (D3229)
IBM Systems & Technology Group, Pure Systems & Modular Software Development
IBM Data Privacy Statement: https://www.ibm.com/privacy/us/en/
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
On Tue, Jul 23, 2019 at 09:38:20AM +0200, Daniel Vetter wrote:
> Hi Greg,
>
> Do you plan to pick this up in your console/vt/whatever-fixes branch?
Yes, now that the merge window is closed, I will be picking this up.
thanks,
greg k-h
Hi Greg,
Do you plan to pick this up in your console/vt/whatever-fixes branch?
There's no reason for me to route this through drm-fixes.
-Daniel
On Thu, Jul 18, 2019 at 10:09:03AM +0200, Daniel Vetter wrote:
> Not really harmful not to, but also not harm in grabbing the lock. And
> this shuts up a new WARNING I introduced in commit ddde3c18b700 ("vt:
> More locking checks").
>
> Reported-by: Jens Remus <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Nicolas Pitre <[email protected]>
> Cc: Martin Hostettler <[email protected]>
> Cc: Adam Borowski <[email protected]>
> Cc: Mikulas Patocka <[email protected]>
> Signed-off-by: Daniel Vetter <[email protected]>
> Cc: Daniel Vetter <[email protected]>
> Cc: Sam Ravnborg <[email protected]>
> ---
> drivers/tty/vt/vt.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index ec92f36ab5c4..34aa39d1aed9 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -3771,7 +3771,11 @@ static ssize_t show_bind(struct device *dev, struct device_attribute *attr,
> char *buf)
> {
> struct con_driver *con = dev_get_drvdata(dev);
> - int bind = con_is_bound(con->con);
> + int bind;
> +
> + console_lock();
> + bind = con_is_bound(con->con);
> + console_unlock();
>
> return snprintf(buf, PAGE_SIZE, "%i\n", bind);
> }
> --
> 2.20.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch