Re: [PATCH] kexec jump: fix compiling warning on xchg(&kexec_lock, 0) in kernel_kexec()

[Vivek Goyal <vgoyal@redhat.com>]

On Wed, Aug 13, 2008 at 01:41:18PM -0700, Andrew Morton wrote:
> On Wed, 13 Aug 2008 13:31:24 -0700 (PDT)
> Linus Torvalds <torvalds@linux-foundation.org> wrote:
> 
> > On Wed, 13 Aug 2008, Andrew Morton wrote:
> > > 
> > > #2:
> > 
> > I thought you said there were things that want to sleep in the region?
> 
> be reasonable - that was over five minutes ago.
> 
> --- a/kernel/kexec.c~kexec-use-a-bitop-for-locking-rather-than-xchg
> +++ a/kernel/kexec.c
> @@ -12,7 +12,7 @@
>  #include <linux/slab.h>
>  #include <linux/fs.h>
>  #include <linux/kexec.h>
> -#include <linux/spinlock.h>
> +#include <linux/mutex.h>
>  #include <linux/list.h>
>  #include <linux/highmem.h>
>  #include <linux/syscalls.h>
> @@ -924,19 +924,14 @@ static int kimage_load_segment(struct ki
>   */
>  struct kimage *kexec_image;
>  struct kimage *kexec_crash_image;
> -/*
> - * A home grown binary mutex.
> - * Nothing can wait so this mutex is safe to use
> - * in interrupt context :)
> - */
> -static int kexec_lock;
> +
> +static DEFINE_MUTEX(kexec_mutex);
>  
>  asmlinkage long sys_kexec_load(unsigned long entry, unsigned long nr_segments,
>  				struct kexec_segment __user *segments,
>  				unsigned long flags)
>  {
>  	struct kimage **dest_image, *image;
> -	int locked;
>  	int result;
>  
>  	/* We only trust the superuser with rebooting the system. */
> @@ -972,8 +967,7 @@ asmlinkage long sys_kexec_load(unsigned 
>  	 *
>  	 * KISS: always take the mutex.
>  	 */
> -	locked = xchg(&kexec_lock, 1);
> -	if (locked)
> +	if (!mutex_trylock(&kexec_mutex))
>  		return -EBUSY;
>  
>  	dest_image = &kexec_image;
> @@ -1015,8 +1009,7 @@ asmlinkage long sys_kexec_load(unsigned 
>  	image = xchg(dest_image, image);
>  
>  out:
> -	locked = xchg(&kexec_lock, 0); /* Release the mutex */
> -	BUG_ON(!locked);
> +	mutex_unlock(&kexec_mutex);
>  	kimage_free(image);
>  
>  	return result;
> @@ -1063,10 +1056,7 @@ asmlinkage long compat_sys_kexec_load(un
>  
>  void crash_kexec(struct pt_regs *regs)
>  {
> -	int locked;
> -
> -
> -	/* Take the kexec_lock here to prevent sys_kexec_load
> +	/* Take the kexec_mutex here to prevent sys_kexec_load
>  	 * running on one cpu from replacing the crash kernel
>  	 * we are using after a panic on a different cpu.
>  	 *
> @@ -1074,8 +1064,7 @@ void crash_kexec(struct pt_regs *regs)
>  	 * of memory the xchg(&kexec_crash_image) would be
>  	 * sufficient.  But since I reuse the memory...
>  	 */
> -	locked = xchg(&kexec_lock, 1);
> -	if (!locked) {
> +	if (mutex_trylock(&kexec_mutex)) {
>  		if (kexec_crash_image) {
>  			struct pt_regs fixed_regs;
>  			crash_setup_regs(&fixed_regs, regs);
> @@ -1083,8 +1072,7 @@ void crash_kexec(struct pt_regs *regs)
>  			machine_crash_shutdown(&fixed_regs);
>  			machine_kexec(kexec_crash_image);
>  		}
> -		locked = xchg(&kexec_lock, 0);
> -		BUG_ON(!locked);
> +		mutex_unlock(&kexec_mutex);
>  	}
>  }
>  
> @@ -1434,7 +1422,7 @@ int kernel_kexec(void)
>  {
>  	int error = 0;
>  
> -	if (xchg(&kexec_lock, 1))
> +	if (!mutex_trylock(&kexec_mutex))
>  		return -EBUSY;
>  	if (!kexec_image) {
>  		error = -EINVAL;
> @@ -1498,8 +1486,6 @@ int kernel_kexec(void)
>  #endif
>  
>   Unlock:
> -	if (!xchg(&kexec_lock, 0))
> -		BUG();
> -
> +	mutex_unlock(&kexec_mutex);
>  	return error;
>  }

I tested kexec and kudmp on 2.6.27-rc3 with this patch. Works for me on a
32 bit machine.

Thanks
Vivek