[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#667501: WARNING when trying to read a floppy disk with Nautilus



reassign 667501 src:linux-2.6 3.2.12-1
tags 667501 + upstream patch moreinfo
quit

rpnpif wrote:

> WARNING: at /build/buildd-linux-2.6_3.2.12-1~bpo60+1-i386-EZ37JT/linux-2.6-3.2.12/debian/build/source_i386_none/drivers/block/floppy.c:1041 setup_rw_floppy+0x1e8/0x2a1 [floppy]()
> Hardware name: MS-6380                         
> floppy_disable_hlt() scheduled for removal in 2012

Thanks.  This is from

	commit 3b70b2e5fcf6
	Author: Len Brown <len.brown@intel.com>
	Date:   Fri Apr 1 15:08:48 2011 -0400

	    x86 idle floppy: deprecate disable_hlt()

	    Plan to remove floppy_disable_hlt in 2012, an ancient
	    workaround with comments that it should be removed.

	    This allows us to remove clutter and a run-time branch
	    from the idle code.

	    WARN_ONCE() on invocation until it is removed.

	    cc: x86@kernel.org
	    cc: stable@kernel.org # .39.x
	    Signed-off-by: Len Brown <len.brown@intel.com>

Using an assertion (WARN_ONCE) means that Len thinks calls to
floppy_disable_hlt are a Bad Thing in some sense, presumably because
of their effect on power consumption.  So this might be a real bug or
just an overly aggressive warning.  The fix is

	commit f6365201d8a2
	Author: Len Brown <len.brown@intel.com>
	Date:   Thu Mar 29 14:49:17 2012 -0700

	    x86: Remove the ancient and deprecated disable_hlt() and enable_hlt() facility

and is not part of "master" yet.  I don't understand how anyone
thought applying the warning without the fix could be a good idea.

Please test the attached patch, for example using the following
instructions:

 0. Prerequisites.

	apt-get install git build-essential

 1. Get a copy of the kernel history, if you don't already have it.

	git clone \
	  git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git

 2. Fetch point releases:

	cd linux
	git remote add stable \
	  git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git
	git fetch stable

 3. Configure, build, and test:

	git checkout stable/linux-3.2.y
	cp /boot/config-$(uname -r) .config; # current configuration
	make localmodconfig; # optional: minimize configuration
	make deb-pkg; # optionally with -j<num> for parallel build
	dpkg -i ../<name of package>; # as root
	reboot
	... test test test ...

 4. Hopefully it produces the warning.  So try the patch:

	cd linux
	git am -3sc <path to patch>
	make deb-pkg; # maybe with -j4
	dpkg -i ../<name of package>; # as root
	reboot
	... test test test ...

   Hopefully it does not produce the warning.

   If you happen to have a cheap and ancient 486 or Cyrix 5510, see if
   floppy disk access makes the power supply go bad.

An alternative set of instructions is at [1].

Ben, I think we should take this patch.  Do you think it's a candidate
for gregkh's stable tree?

Thanks,
Jonathan

[1] http://kernel-handbook.alioth.debian.org/ch-common-tasks.html#s-common-official
or the corresponding page in the debian-kernel-handbook package
From: Len Brown <len.brown@intel.com>
Date: Thu, 29 Mar 2012 14:49:17 -0700
Subject: x86: Remove the ancient and deprecated disable_hlt() and enable_hlt() facility

commit f6365201d8a21fb347260f89d6e9b3e718d63c70 upstream.

The X86_32-only disable_hlt/enable_hlt mechanism was used by the
32-bit floppy driver. Its effect was to replace the use of the
HLT instruction inside default_idle() with cpu_relax() - essentially
it turned off the use of HLT.

This workaround was commented in the code as:

 "disable hlt during certain critical i/o operations"

 "This halt magic was a workaround for ancient floppy DMA
  wreckage. It should be safe to remove."

H. Peter Anvin additionally adds:

 "To the best of my knowledge, no-hlt only existed because of
  flaky power distributions on 386/486 systems which were sold to
  run DOS.  Since DOS did no power management of any kind,
  including HLT, the power draw was fairly uniform; when exposed
  to the much hhigher noise levels you got when Linux used HLT
  caused some of these systems to fail.

  They were by far in the minority even back then."

Alan Cox further says:

 "Also for the Cyrix 5510 which tended to go castors up if a HLT
  occurred during a DMA cycle and on a few other boxes HLT during
  DMA tended to go astray.

  Do we care ? I doubt it. The 5510 was pretty obscure, the 5520
  fixed it, the 5530 is probably the oldest still in any kind of
  use."

So, let's finally drop this.

Signed-off-by: Len Brown <len.brown@intel.com>
Signed-off-by: Josh Boyer <jwboyer@redhat.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Acked-by: "H. Peter Anvin" <hpa@zytor.com>
Acked-by: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: Stephen Hemminger <shemminger@vyatta.com
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: <stable@kernel.org>
Link: http://lkml.kernel.org/n/tip-3rhk9bzf0x9rljkv488tloib@git.kernel.org
[ If anyone cares then alternative instruction patching could be
  used to replace HLT with a one-byte NOP instruction. Much simpler. ]
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 Documentation/feature-removal-schedule.txt |    8 -------
 arch/x86/include/asm/system.h              |    7 ------
 arch/x86/kernel/process.c                  |   24 -------------------
 drivers/block/floppy.c                     |   36 ----------------------------
 4 files changed, 75 deletions(-)

diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
index 3d849122b5b1..47c4ec2fc674 100644
--- a/Documentation/feature-removal-schedule.txt
+++ b/Documentation/feature-removal-schedule.txt
@@ -6,14 +6,6 @@ be removed from this file.
 
 ---------------------------
 
-What:	x86 floppy disable_hlt
-When:	2012
-Why:	ancient workaround of dubious utility clutters the
-	code used by everybody else.
-Who:	Len Brown <len.brown@intel.com>
-
----------------------------
-
 What:	CONFIG_APM_CPU_IDLE, and its ability to call APM BIOS in idle
 When:	2012
 Why:	This optional sub-feature of APM is of dubious reliability,
diff --git a/arch/x86/include/asm/system.h b/arch/x86/include/asm/system.h
index 2d2f01ce6dcb..d75adff83784 100644
--- a/arch/x86/include/asm/system.h
+++ b/arch/x86/include/asm/system.h
@@ -93,10 +93,6 @@ do {									\
 			"memory");					\
 } while (0)
 
-/*
- * disable hlt during certain critical i/o operations
- */
-#define HAVE_DISABLE_HLT
 #else
 
 /* frame pointer must be last for get_wchan */
@@ -392,9 +388,6 @@ static inline void clflush(volatile void *__p)
 
 #define nop() asm volatile ("nop")
 
-void disable_hlt(void);
-void enable_hlt(void);
-
 void cpu_idle_wait(void);
 
 extern unsigned long arch_align_stack(unsigned long sp);
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index ee5d4fbd53b4..59b9b3770421 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -341,34 +341,10 @@ void (*pm_idle)(void);
 EXPORT_SYMBOL(pm_idle);
 #endif
 
-#ifdef CONFIG_X86_32
-/*
- * This halt magic was a workaround for ancient floppy DMA
- * wreckage. It should be safe to remove.
- */
-static int hlt_counter;
-void disable_hlt(void)
-{
-	hlt_counter++;
-}
-EXPORT_SYMBOL(disable_hlt);
-
-void enable_hlt(void)
-{
-	hlt_counter--;
-}
-EXPORT_SYMBOL(enable_hlt);
-
-static inline int hlt_use_halt(void)
-{
-	return (!hlt_counter && boot_cpu_data.hlt_works_ok);
-}
-#else
 static inline int hlt_use_halt(void)
 {
 	return 1;
 }
-#endif
 
 /*
  * We use this if we don't have any better
diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index 9955a53733b2..40bf4c2b6b61 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -1032,37 +1032,6 @@ static int fd_wait_for_completion(unsigned long delay, timeout_fn function)
 	return 0;
 }
 
-static DEFINE_SPINLOCK(floppy_hlt_lock);
-static int hlt_disabled;
-static void floppy_disable_hlt(void)
-{
-	unsigned long flags;
-
-	WARN_ONCE(1, "floppy_disable_hlt() scheduled for removal in 2012");
-	spin_lock_irqsave(&floppy_hlt_lock, flags);
-	if (!hlt_disabled) {
-		hlt_disabled = 1;
-#ifdef HAVE_DISABLE_HLT
-		disable_hlt();
-#endif
-	}
-	spin_unlock_irqrestore(&floppy_hlt_lock, flags);
-}
-
-static void floppy_enable_hlt(void)
-{
-	unsigned long flags;
-
-	spin_lock_irqsave(&floppy_hlt_lock, flags);
-	if (hlt_disabled) {
-		hlt_disabled = 0;
-#ifdef HAVE_DISABLE_HLT
-		enable_hlt();
-#endif
-	}
-	spin_unlock_irqrestore(&floppy_hlt_lock, flags);
-}
-
 static void setup_DMA(void)
 {
 	unsigned long f;
@@ -1107,7 +1076,6 @@ static void setup_DMA(void)
 	fd_enable_dma();
 	release_dma_lock(f);
 #endif
-	floppy_disable_hlt();
 }
 
 static void show_floppy(void);
@@ -1709,7 +1677,6 @@ irqreturn_t floppy_interrupt(int irq, void *dev_id)
 	fd_disable_dma();
 	release_dma_lock(f);
 
-	floppy_enable_hlt();
 	do_floppy = NULL;
 	if (fdc >= N_FDC || FDCS->address == -1) {
 		/* we don't even know which FDC is the culprit */
@@ -1858,8 +1825,6 @@ static void floppy_shutdown(unsigned long data)
 		show_floppy();
 	cancel_activity();
 
-	floppy_enable_hlt();
-
 	flags = claim_dma_lock();
 	fd_disable_dma();
 	release_dma_lock(flags);
@@ -4504,7 +4469,6 @@ static void floppy_release_irq_and_dma(void)
 #if N_FDC > 1
 	set_dor(1, ~8, 0);
 #endif
-	floppy_enable_hlt();
 
 	if (floppy_track_buffer && max_buffer_sectors) {
 		tmpsize = max_buffer_sectors * 1024;
-- 
1.7.10.rc3


Reply to: