Skip to content

Commit

Permalink
x86, msr: Remove the bkl from msr_open()
Browse files Browse the repository at this point in the history
Remove the big kernel lock from msr_open() as it doesn't protect
anything there.

The only racy event that can happen here is a concurrent cpu shutdown.

So let's look at what could be racy during/after the above event:

- The cpu_online() check is racy, but the bkl doesn't help about
  that anyway it disables preemption but we may be chcking another
  cpu than the current one.
  Also the cpu can still become offlined between open and read calls.

- The cpu_data(cpu) returns a safe pointer too. It won't be released on
  cpu offlining. But some fields can be changed from
  arch/x86/kernel/smpboot.c:remove_siblinginfo() :

	- phys_proc_id
	- cpu_core_id

  Those are not read from msr_open(). What we are checking is the
  x86_capability that is left untouched on offlining.

So this removal looks safe.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: John Kacur <jkacur@redhat.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Sven-Thorsten Dietrich <sdietrich@suse.de>
LKML-Reference: <1254944602-7382-1-git-send-email-fweisbec@gmail.com>
Signed-off-by: H. Peter Anvin <hpa@zytor.com>
  • Loading branch information
Frederic Weisbecker authored and H. Peter Anvin committed Oct 7, 2009
1 parent 98059e3 commit d6c3040
Showing 1 changed file with 6 additions and 10 deletions.
16 changes: 6 additions & 10 deletions arch/x86/kernel/msr.c
Original file line number Diff line number Diff line change
Expand Up @@ -174,21 +174,17 @@ static int msr_open(struct inode *inode, struct file *file)
{
unsigned int cpu = iminor(file->f_path.dentry->d_inode);
struct cpuinfo_x86 *c = &cpu_data(cpu);
int ret = 0;

lock_kernel();
cpu = iminor(file->f_path.dentry->d_inode);

if (cpu >= nr_cpu_ids || !cpu_online(cpu)) {
ret = -ENXIO; /* No such CPU */
goto out;
}
if (cpu >= nr_cpu_ids || !cpu_online(cpu))
return -ENXIO; /* No such CPU */

c = &cpu_data(cpu);
if (!cpu_has(c, X86_FEATURE_MSR))
ret = -EIO; /* MSR not supported */
out:
unlock_kernel();
return ret;
return -EIO; /* MSR not supported */

return 0;
}

/*
Expand Down

0 comments on commit d6c3040

Please sign in to comment.