linux-kernel - Re: [PATCH, RFC] Char dev BKL pushdown v2

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 19 May 2008 22:07:28 +0200 (CEST)
From:	Stefan Richter <stefanr@...6.in-berlin.de>
To:	Jonathan Corbet <corbet@....net>,
	linux1394-devel@...ts.sourceforge.net
cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Ingo Molnar <mingo@...e.hu>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Thomas Gleixner <tglx@...utronix.de>,
	Alan Cox <alan@...rguk.ukuu.org.uk>,
	Alexander Viro <viro@....linux.org.uk>,
	linux-kernel@...r.kernel.org,
	Stephen Rothwell <sfr@...b.auug.org.au>
Subject: Re: [PATCH, RFC] Char dev BKL pushdown v2

On 19 May, Stefan Richter wrote at LKML:
> I wrote:
>> Jonathan Corbet wrote:
>>>  drivers/ieee1394/dv1394.c                    |    6 +++-
>>>  drivers/ieee1394/raw1394.c                   |    3 ++
>>>  drivers/ieee1394/video1394.c                 |   18 +++++++++---
>> ...
>>>       ieee1394: cdev lock_kernel() pushdown
>> 
>> I have yet to look at these drivers in detail whether they need BKL or 
>> not.  They likely don't.
> 
> video1394 needs it to serialize module init vs. open.  The race 
> condition there can be prevented by splitting hpsb_register_highlevel() 
> into an _init and a _register function.  I will follow up with a patch.
> 
> dv1394 and raw1394 do not need the BKL in their open() methods AFAICS.


From: Stefan Richter <stefanr@...6.in-berlin.de>
Subject: ieee1394: video1394: reorder module init, prepare BKL removal

This prepares video1394 for removal of the BKL (big kernel lock):
It allows video1394_open() to be called while video1394_init_module()
is still in progress.

Signed-off-by: Stefan Richter <stefanr@...6.in-berlin.de>
---
 drivers/ieee1394/highlevel.c |    4 +---
 drivers/ieee1394/highlevel.h |   13 ++++++++++++-
 drivers/ieee1394/video1394.c |    2 ++
 3 files changed, 15 insertions(+), 4 deletions(-)

Index: linux/drivers/ieee1394/highlevel.c
===================================================================
--- linux.orig/drivers/ieee1394/highlevel.c
+++ linux/drivers/ieee1394/highlevel.c
@@ -228,10 +228,8 @@ void hpsb_register_highlevel(struct hpsb
 {
 	unsigned long flags;
 
+	hpsb_init_highlevel(hl);
 	INIT_LIST_HEAD(&hl->addr_list);
-	INIT_LIST_HEAD(&hl->host_info_list);
-
-	rwlock_init(&hl->host_info_lock);
 
 	down_write(&hl_drivers_sem);
 	list_add_tail(&hl->hl_list, &hl_drivers);
Index: linux/drivers/ieee1394/highlevel.h
===================================================================
--- linux.orig/drivers/ieee1394/highlevel.h
+++ linux/drivers/ieee1394/highlevel.h
@@ -2,7 +2,7 @@
 #define IEEE1394_HIGHLEVEL_H
 
 #include <linux/list.h>
-#include <linux/spinlock_types.h>
+#include <linux/spinlock.h>
 #include <linux/types.h>
 
 struct module;
@@ -103,6 +103,17 @@ int highlevel_lock64(struct hpsb_host *h
 void highlevel_fcp_request(struct hpsb_host *host, int nodeid, int direction,
 			   void *data, size_t length);
 
+/**
+ * hpsb_init_highlevel - initialize a struct hpsb_highlevel
+ *
+ * This is only necessary if hpsb_get_hostinfo_bykey can be called
+ * before hpsb_register_highlevel.
+ */
+static inline void hpsb_init_highlevel(struct hpsb_highlevel *hl)
+{
+	rwlock_init(&hl->host_info_lock);
+	INIT_LIST_HEAD(&hl->host_info_list);
+}
 void hpsb_register_highlevel(struct hpsb_highlevel *hl);
 void hpsb_unregister_highlevel(struct hpsb_highlevel *hl);
 
Index: linux/drivers/ieee1394/video1394.c
===================================================================
--- linux.orig/drivers/ieee1394/video1394.c
+++ linux/drivers/ieee1394/video1394.c
@@ -1503,6 +1503,8 @@ static int __init video1394_init_module 
 {
 	int ret;
 
+	hpsb_init_highlevel(&video1394_highlevel);
+
 	cdev_init(&video1394_cdev, &video1394_fops);
 	video1394_cdev.owner = THIS_MODULE;
 	ret = cdev_add(&video1394_cdev, IEEE1394_VIDEO1394_DEV, 16);

-- 
Stefan Richter
-=====-==--- -=-= =--==
http://arcgraph.de/sr/

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/