All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel.vetter@ffwll.ch>
To: DRI Development <dri-devel@lists.freedesktop.org>
Cc: Du Cheng <ducheng2@gmail.com>,
	Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	Intel Graphics Development <intel-gfx@lists.freedesktop.org>,
	Zheyu Ma <zheyuma97@gmail.com>,
	Javier Martinez Canillas <javierm@redhat.com>,
	Matthew Wilcox <willy@infradead.org>,
	Nathan Chancellor <nathan@kernel.org>,
	Claudio Suarez <cssk@net-c.es>,
	Daniel Vetter <daniel.vetter@intel.com>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	Sam Ravnborg <sam@ravnborg.org>, Helge Deller <deller@gmx.de>,
	Guenter Roeck <linux@roeck-us.net>
Subject: [PATCH] fbcon: Fix delayed takeover locking
Date: Wed, 13 Apr 2022 10:21:28 +0200	[thread overview]
Message-ID: <20220413082128.348186-1-daniel.vetter@ffwll.ch> (raw)

I messed up the delayed takover path in the locking conversion in
6e7da3af008b ("fbcon: Move console_lock for register/unlink/unregister").

Fix it by re-extracting the lockless function and using it in the
delayed takeover path, where we need to hold the lock already to
iterate over the list of already registered fb. Well the current code
still is broken in there (since the list is protected by a
registration_lock, which we can't take here because it nests the other
way round with console_lock), but in the future this will be a list
protected by console_lock when this is all sorted out.

Reported-by: Nathan Chancellor <nathan@kernel.org>
Cc: Nathan Chancellor <nathan@kernel.org>
Fixes: 6e7da3af008b ("fbcon: Move console_lock for register/unlink/unregister")
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Du Cheng <ducheng2@gmail.com>
Cc: Claudio Suarez <cssk@net-c.es>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Zheyu Ma <zheyuma97@gmail.com>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Helge Deller <deller@gmx.de>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Javier Martinez Canillas <javierm@redhat.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/video/fbdev/core/fbcon.c | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 6a7d470beec7..b4e43b39d9a8 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -2772,7 +2772,6 @@ static void fbcon_unbind(void)
 static inline void fbcon_unbind(void) {}
 #endif /* CONFIG_VT_HW_CONSOLE_BINDING */
 
-/* called with console_lock held */
 void fbcon_fb_unbind(struct fb_info *info)
 {
 	int i, new_idx = -1;
@@ -2822,7 +2821,6 @@ void fbcon_fb_unbind(struct fb_info *info)
 	console_unlock();
 }
 
-/* called with console_lock held */
 void fbcon_fb_unregistered(struct fb_info *info)
 {
 	int i, idx;
@@ -2928,14 +2926,11 @@ MODULE_PARM_DESC(lockless_register_fb,
 	"Lockless framebuffer registration for debugging [default=off]");
 
 /* called with console_lock held */
-int fbcon_fb_registered(struct fb_info *info)
+static int do_fb_registered(struct fb_info *info)
 {
 	int ret = 0, i, idx;
 
-	if (!lockless_register_fb)
-		console_lock();
-	else
-		atomic_inc(&ignore_console_lock_warning);
+	WARN_CONSOLE_UNLOCKED();
 
 	fbcon_registered_fb[info->node] = info;
 	fbcon_num_registered_fb++;
@@ -2945,7 +2940,7 @@ int fbcon_fb_registered(struct fb_info *info)
 
 	if (deferred_takeover) {
 		pr_info("fbcon: Deferring console take-over\n");
-		goto out;
+		return 0;
 	}
 
 	if (info_idx == -1) {
@@ -2965,7 +2960,20 @@ int fbcon_fb_registered(struct fb_info *info)
 		}
 	}
 
-out:
+	return ret;
+}
+
+int fbcon_fb_registered(struct fb_info *info)
+{
+	int ret;
+
+	if (!lockless_register_fb)
+		console_lock();
+	else
+		atomic_inc(&ignore_console_lock_warning);
+
+	ret = do_fb_registered(info);
+
 	if (!lockless_register_fb)
 		console_unlock();
 	else
@@ -3280,7 +3288,7 @@ static void fbcon_register_existing_fbs(struct work_struct *work)
 	logo_shown = FBCON_LOGO_DONTSHOW;
 
 	fbcon_for_each_registered_fb(i)
-		fbcon_fb_registered(fbcon_registered_fb[i]);
+		do_fb_registered(fbcon_registered_fb[i]);
 
 	console_unlock();
 }
-- 
2.34.1


WARNING: multiple messages have this Message-ID (diff)
From: Daniel Vetter <daniel.vetter@ffwll.ch>
To: DRI Development <dri-devel@lists.freedesktop.org>
Cc: Du Cheng <ducheng2@gmail.com>,
	Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	Intel Graphics Development <intel-gfx@lists.freedesktop.org>,
	Zheyu Ma <zheyuma97@gmail.com>,
	Javier Martinez Canillas <javierm@redhat.com>,
	Matthew Wilcox <willy@infradead.org>,
	Nathan Chancellor <nathan@kernel.org>,
	Daniel Vetter <daniel.vetter@intel.com>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	Sam Ravnborg <sam@ravnborg.org>, Helge Deller <deller@gmx.de>,
	Guenter Roeck <linux@roeck-us.net>
Subject: [Intel-gfx] [PATCH] fbcon: Fix delayed takeover locking
Date: Wed, 13 Apr 2022 10:21:28 +0200	[thread overview]
Message-ID: <20220413082128.348186-1-daniel.vetter@ffwll.ch> (raw)

I messed up the delayed takover path in the locking conversion in
6e7da3af008b ("fbcon: Move console_lock for register/unlink/unregister").

Fix it by re-extracting the lockless function and using it in the
delayed takeover path, where we need to hold the lock already to
iterate over the list of already registered fb. Well the current code
still is broken in there (since the list is protected by a
registration_lock, which we can't take here because it nests the other
way round with console_lock), but in the future this will be a list
protected by console_lock when this is all sorted out.

Reported-by: Nathan Chancellor <nathan@kernel.org>
Cc: Nathan Chancellor <nathan@kernel.org>
Fixes: 6e7da3af008b ("fbcon: Move console_lock for register/unlink/unregister")
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Du Cheng <ducheng2@gmail.com>
Cc: Claudio Suarez <cssk@net-c.es>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Zheyu Ma <zheyuma97@gmail.com>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Helge Deller <deller@gmx.de>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Javier Martinez Canillas <javierm@redhat.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/video/fbdev/core/fbcon.c | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 6a7d470beec7..b4e43b39d9a8 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -2772,7 +2772,6 @@ static void fbcon_unbind(void)
 static inline void fbcon_unbind(void) {}
 #endif /* CONFIG_VT_HW_CONSOLE_BINDING */
 
-/* called with console_lock held */
 void fbcon_fb_unbind(struct fb_info *info)
 {
 	int i, new_idx = -1;
@@ -2822,7 +2821,6 @@ void fbcon_fb_unbind(struct fb_info *info)
 	console_unlock();
 }
 
-/* called with console_lock held */
 void fbcon_fb_unregistered(struct fb_info *info)
 {
 	int i, idx;
@@ -2928,14 +2926,11 @@ MODULE_PARM_DESC(lockless_register_fb,
 	"Lockless framebuffer registration for debugging [default=off]");
 
 /* called with console_lock held */
-int fbcon_fb_registered(struct fb_info *info)
+static int do_fb_registered(struct fb_info *info)
 {
 	int ret = 0, i, idx;
 
-	if (!lockless_register_fb)
-		console_lock();
-	else
-		atomic_inc(&ignore_console_lock_warning);
+	WARN_CONSOLE_UNLOCKED();
 
 	fbcon_registered_fb[info->node] = info;
 	fbcon_num_registered_fb++;
@@ -2945,7 +2940,7 @@ int fbcon_fb_registered(struct fb_info *info)
 
 	if (deferred_takeover) {
 		pr_info("fbcon: Deferring console take-over\n");
-		goto out;
+		return 0;
 	}
 
 	if (info_idx == -1) {
@@ -2965,7 +2960,20 @@ int fbcon_fb_registered(struct fb_info *info)
 		}
 	}
 
-out:
+	return ret;
+}
+
+int fbcon_fb_registered(struct fb_info *info)
+{
+	int ret;
+
+	if (!lockless_register_fb)
+		console_lock();
+	else
+		atomic_inc(&ignore_console_lock_warning);
+
+	ret = do_fb_registered(info);
+
 	if (!lockless_register_fb)
 		console_unlock();
 	else
@@ -3280,7 +3288,7 @@ static void fbcon_register_existing_fbs(struct work_struct *work)
 	logo_shown = FBCON_LOGO_DONTSHOW;
 
 	fbcon_for_each_registered_fb(i)
-		fbcon_fb_registered(fbcon_registered_fb[i]);
+		do_fb_registered(fbcon_registered_fb[i]);
 
 	console_unlock();
 }
-- 
2.34.1


             reply	other threads:[~2022-04-13  8:21 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-13  8:21 Daniel Vetter [this message]
2022-04-13  8:21 ` [Intel-gfx] [PATCH] fbcon: Fix delayed takeover locking Daniel Vetter
2022-04-13  9:16 ` Javier Martinez Canillas
2022-04-13  9:16   ` [Intel-gfx] " Javier Martinez Canillas
2022-04-13  9:21   ` Daniel Vetter
2022-04-13  9:21     ` [Intel-gfx] " Daniel Vetter
2022-04-13  9:31     ` Javier Martinez Canillas
2022-04-13  9:31       ` [Intel-gfx] " Javier Martinez Canillas
2022-04-13  9:25 ` Daniel Vetter
2022-04-13  9:25   ` [Intel-gfx] " Daniel Vetter
2022-04-13 15:20   ` Nathan Chancellor
2022-04-13 15:20     ` [Intel-gfx] " Nathan Chancellor
2022-04-13 20:50 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2022-04-13 21:12 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2022-04-13 23:13 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220413082128.348186-1-daniel.vetter@ffwll.ch \
    --to=daniel.vetter@ffwll.ch \
    --cc=cssk@net-c.es \
    --cc=daniel.vetter@intel.com \
    --cc=deller@gmx.de \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=ducheng2@gmail.com \
    --cc=geert@linux-m68k.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=javierm@redhat.com \
    --cc=linux@roeck-us.net \
    --cc=nathan@kernel.org \
    --cc=penguin-kernel@I-love.SAKURA.ne.jp \
    --cc=sam@ravnborg.org \
    --cc=tzimmermann@suse.de \
    --cc=willy@infradead.org \
    --cc=zheyuma97@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.