x86_64: O_EXCL on /dev/mcelog (resend)

PATCH In review Tim Hockin 2 participants 2 messages May 7, 2007 – May 9, 2007

lore

Share & tools

lore

Tim Hockin

author

2007-05-07 22:20 UTC #0 lore raw

PATCH 1/1 x86_64: O_EXCL on /dev/mcelog (resend)

From: Tim Hockin <[email protected]>

Background:
/dev/mcelog is a clear-on-read interface. It is currently possible for multiple users to open and read() the device. Users are protected from each other during any one read, but not across reads.

Description:
This patch adds support for O_EXCL to /dev/mcelog. If a user opens the device with O_EXCL, no other user may open the device (EBUSY). Likewise, any user that tries to open the device with O_EXCL while another user has the device will fail (EBUSY).

Result:
Applications can get exclusive access to /dev/mcelog. Applications that do not care will be unchanged.

Alternatives:
A simpler choice would be to only allow one open() at all, regardless of O_EXCL.

Testing:
I wrote an application that opens /dev/mcelog with O_EXCL and observed that any other app that tried to open /dev/mcelog would fail until the exclusive app had closed the device.

Caveats:
None.

Patch:
This patch is against 2.6.21.

Signed-off-by Tim Hockin <[email protected]>

---

This is the first version version of this patch. The simpler alternative of only one open() sounds better to me, but becomes a net change in behavior.

arch/x86_64/kernel/mce.c +36 -0

diff -pruN linux-2.6.20+th/arch/x86_64/kernel/mce.c linux-2.6.20+th1.5/arch/x86_64/kernel/mce.c
@@ -465,6 +465,40 @@ void __cpuinit mcheck_init(struct cpuinf
  * Character device to read and clear the MCE log.
  */
 
+static DEFINE_SPINLOCK(mce_state_lock);
+static int open_count;	/* #times opened */
+static int open_exclu;	/* already open exclusive? */
+
+static int mce_open(struct inode *inode, struct file *file)
+{
+	spin_lock(&mce_state_lock);
+
+	if (open_exclu || (open_count && (file->f_flags & O_EXCL))) {
+		spin_unlock(&mce_state_lock);
+		return -EBUSY;
+	}
+
+	if (file->f_flags & O_EXCL)
+		open_exclu = 1;
+	open_count++;
+
+	spin_unlock(&mce_state_lock);
+
+	return 0;
+}
+
+static int mce_release(struct inode *inode, struct file *file)
+{
+	spin_lock(&mce_state_lock);
+
+	open_count--;
+	open_exclu = 0;
+
+	spin_unlock(&mce_state_lock);
+
+	return 0;
+}
+
 static void collect_tscs(void *data) 
 { 
 	unsigned long *cpu_tsc = (unsigned long *)data;

@@ -553,6 +587,8 @@ static int mce_ioctl(struct inode *i, st
 }
 
 static const struct file_operations mce_chrdev_ops = {
+	.open = mce_open,
+	.release = mce_release,
 	.read = mce_read,
 	.ioctl = mce_ioctl,
 };

Andrew Morton

2007-05-09 23:46 UTC #1 lore raw

Quoted 45 lines · On Mon, 7 May 2007 15:19:01 -0700 [email protected] (Tim Hockin) wrote:

From: Tim Hockin <[email protected]>
Background:
/dev/mcelog is a clear-on-read interface. It is currently possible for multiple users to open and read() the device. Users are protected from each other during any one read, but not across reads.
Description:
This patch adds support for O_EXCL to /dev/mcelog. If a user opens the device with O_EXCL, no other user may open the device (EBUSY). Likewise, any user that tries to open the device with O_EXCL while another user has the device will fail (EBUSY).
Result:
Applications can get exclusive access to /dev/mcelog. Applications that do not care will be unchanged.
Alternatives:
A simpler choice would be to only allow one open() at all, regardless of O_EXCL.
Testing:
I wrote an application that opens /dev/mcelog with O_EXCL and observed that any other app that tried to open /dev/mcelog would fail until the exclusive app had closed the device.
Caveats:
None.
Patch:
This patch is against 2.6.21.
Signed-off-by Tim Hockin <[email protected]>
---
This is the first version version of this patch. The simpler alternative of only one open() sounds better to me, but becomes a net change in behavior.
diff -pruN linux-2.6.20+th/arch/x86_64/kernel/mce.c linux-2.6.20+th1.5/arch/x86_64/kernel/mce.c

arch/x86_64/kernel/mce.c void __cpuinit mcheck_init(struct cpuinf · line 470

Quoted 6 lines

arch/x86_64/kernel/mce.c +3 -0

@@ -465,6 +465,40 @@ void __cpuinit mcheck_init(struct cpuinf
  * Character device to read and clear the MCE log.
  */
 
+static DEFINE_SPINLOCK(mce_state_lock);
+static int open_count;	/* #times opened */
+static int open_exclu;	/* already open exclusive? */

"open_exclusive" ;)

Quoted 7 lines

arch/x86_64/kernel/mce.c +7 -0

+static int mce_open(struct inode *inode, struct file *file)
+{
+	spin_lock(&mce_state_lock);
+
+	if (open_exclu || (open_count && (file->f_flags & O_EXCL))) {
+		spin_unlock(&mce_state_lock);
+		return -EBUSY;

arch/x86_64/kernel/mce.c void __cpuinit mcheck_init(struct cpuinf · line 488

Quoted 10 lines

arch/x86_64/kernel/mce.c +10 -0

+	}
+
+	if (file->f_flags & O_EXCL)
+		open_exclu = 1;
+	open_count++;
+
+	spin_unlock(&mce_state_lock);
+
+	return 0;
+}

Does this guy support lseek? If not we should toss a nonseekable_open() call in here.

Quoted 11 lines

arch/x86_64/kernel/mce.c +11 -0

+static int mce_release(struct inode *inode, struct file *file)
+{
+	spin_lock(&mce_state_lock);
+
+	open_count--;
+	open_exclu = 0;
+
+	spin_unlock(&mce_state_lock);
+
+	return 0;
+}