04 Jul, 2012

1 commit

  • In commit 070ad7e793dc ("floppy: convert to delayed work and
    single-thread wq") the 'fd_timeout' timer was converted to a delayed
    work. However, the "del_timer(&fd_timeout)" was lost in the process,
    and any previous pending timeouts would stay active when we then
    re-queued the timeout.

    This resulted in the floppy probe sequence having a (stale) 20s timeout
    rather than the intended 3s timeout, and thus made booting with the
    floppy driver (but no actual floppy controller) take much longer than it
    should.

    Of course, there's little reason for most people to compile the floppy
    driver into the kernel at all, which is why most people never noticed.

    Canceling the delayed work where we used to do the del_timer() fixes the
    issue, and makes the floppy probing use the proper new timeout instead.
    The three second timeout is still very wasteful, but better than the 20s
    one.

    Reported-and-tested-by: Andi Kleen
    Reported-and-tested-by: Calvin Walton
    Cc: Jiri Kosina
    Signed-off-by: Linus Torvalds

    Linus Torvalds
     

18 May, 2012

2 commits

  • Block layer now handles O_EXCL in a generic way for block devices.

    The semantics is however different for floppy and all other block devices,
    as floppy driver contains its own O_EXCL handling.

    The semantics for all-but-floppy bdevs is "there can be at most one O_EXCL
    open of this file", while for floppy bdev the semantics is "if someone has
    the bdev open with O_EXCL, noone else can open it".

    There is actual userspace-observable change in behavior because of this
    since commit e525fd89d380c ("block: make blkdev_get/put() handle exclusive
    access") -- on kernels containing this commit, mount of /dev/fd0 causes
    the fd0 block device be claimed with _EXCL, preventing subsequent
    open(/dev/fd0).

    Bring things back into shape, i.e. make it possible, analogically to
    other block devices, to mount the floppy and open() it afterwards --
    remove the floppy-specific handling and let the generic bdev code O_EXCL
    handling take over.

    Signed-off-by: Jiri Kosina
    Acked-by: Tejun Heo
    Acked-by: NeilBrown
    Signed-off-by: Andrew Morton
    Signed-off-by: Jiri Kosina

    Jiri Kosina
     
  • There are several races in floppy driver between bottom half
    (scheduled_work) and timers (fd_timeout, fd_timer). Due to slowness
    of the actual floppy devices, those races are never (at least to my
    knowledge) triggered on a bare floppy metal. However on virtualized
    (emulated) floppy drives, which are of course magnitudes faster
    than the real ones, these races trigger reliably. They usually exhibit
    themselves as NULL pointer dereferences during DMA setup, such as

    BUG: unable to handle kernel NULL pointer dereference at 0000000a
    [ ... snip ... ]
    EIP: 0060:[] EFLAGS: 00010293 CPU: 0
    EAX: ffffe000 EBX: 0000000a ECX: 00000000 EDX: 0000000a
    ESI: c05d2718 EDI: 00000000 EBP: 00000000 ESP: f540fe44
    DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068
    Process swapper (pid: 0, ti=f540e000 task=c082d5a0 task.ti=c0826000)
    Stack:
    ffffe000 00001ffc 00000000 00000000 00000000 c05d2718 c0708b40 f540fe80
    c020470f c05d2718 c0708b40 00000000 f540fe80 0000000a f540fee4 00000000
    c0708b40 f540fee4 00000000 00000000 c020526b 00000000 c05d2718 c0708b40
    Call Trace:
    [] dump_trace+0xaf/0x110
    [] show_trace_log_lvl+0x4b/0x60
    [] show_trace+0x18/0x20
    [] dump_stack+0x6d/0x72
    [] warn_slowpath_common+0x77/0xb0
    [] warn_slowpath_fmt+0x33/0x40
    [] setup_DMA+0x14c/0x210 [floppy]
    [] setup_rw_floppy+0x105/0x190 [floppy]
    [] run_timer_softirq+0x168/0x2a0
    [] __do_softirq+0xc2/0x1c0
    [] do_softirq+0x7d/0xb0
    [] 0xf54d89ff

    but other instances can be easily seen as well. This can be observed at least under
    VMWare, VirtualBox and KVM.

    This patch converts all the timers and bottom halfs to be processed in a single
    workqueue. This aproach has been already discussed back in 2010 if I remember
    correctly, and Acked by Linus [1], but it then never made it to the tree.

    This all is based on original idea and code of Stephen Hemminger. I have
    ported original Stepen's code to the current state of the floppy driver, and
    performed quite some testing (on real hardware), which didn't reveal any issues
    (this includes not only writing and reading data, but also formatting
    (unfortunately I didn't find any Double-Density disks any more)). Ability to
    handle errors properly (supplying known bad floppies) has also been verified.

    [1] http://kerneltrap.org/mailarchive/linux-kernel/2010/6/11/4582092

    Based-on-patch-by: Stephen Hemminger
    Acked-by: Linus Torvalds
    Signed-off-by: Jiri Kosina

    Jiri Kosina
     

30 Mar, 2012

1 commit

  • 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
    Signed-off-by: Josh Boyer
    Signed-off-by: Andrew Morton
    Acked-by: "H. Peter Anvin"
    Acked-by: Alan Cox
    Cc: Stephen Hemminger
    Cc:
    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

    Len Brown
     

29 Mar, 2012

1 commit


06 Mar, 2012

1 commit


09 Feb, 2012

2 commits

  • floppy driver does not call add_disk() on all the drives hence we don't take
    gendisk reference on request queue for these drives. Don't call put_disk()
    with disk->queue set, otherwise we try to put the reference we never took.

    Reported-and-tested-by: Dirk Gouders
    Signed-off-by: Vivek Goyal
    Acked-by: Tejun Heo
    Signed-off-by: Jens Axboe

    Vivek Goyal
     
  • add_disk() takes gendisk reference on request queue. If driver failed during
    initialization and never called add_disk() then that extra reference is not
    taken. That reference is put in put_disk(). floppy driver allocates the
    disk, allocates queue, sets disk->queue and then relizes that floppy
    controller is not present. It tries to tear down everything and tries to
    put a reference down in put_disk() which was never taken.

    In such error cases cleanup disk->queue before calling put_disk() so that
    we never try to put down a reference which was never taken in first place.

    Reported-and-tested-by: Suresh Jayaraman
    Tested-by: Dirk Gouders
    Signed-off-by: Vivek Goyal
    Acked-by: Tejun Heo
    Signed-off-by: Jens Axboe

    Vivek Goyal
     

04 Jan, 2012

1 commit

  • Move invalidate_bdev, block_sync_page into fs/block_dev.c. Export
    kill_bdev as well, so brd doesn't have to open code it. Reduce
    buffer_head.h requirement accordingly.

    Removed a rather large comment from invalidate_bdev, as it looked a bit
    obsolete to bother moving. The small comment replacing it says enough.

    Signed-off-by: Nick Piggin
    Cc: Al Viro
    Cc: Christoph Hellwig
    Signed-off-by: Andrew Morton
    Signed-off-by: Al Viro

    Al Viro
     

21 Sep, 2011

1 commit

  • When no floppy is found the module code can be released while a timer
    function is pending or about to be executed.

    CPU0 CPU1
    floppy_init()
    timer_softirq()
    spin_lock_irq(&base->lock);
    detach_timer();
    spin_unlock_irq(&base->lock);
    -> Interrupt
    del_timer();
    return -ENODEV;
    module_cleanup();

    Signed-off-by: Thomas Gleixner
    Cc: Jens Axboe
    Cc:
    Signed-off-by: Andrew Morton
    Signed-off-by: Jens Axboe

    Carsten Emde
     

30 May, 2011

1 commit

  • * 'idle-release' of git://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux-idle-2.6:
    x86 idle: deprecate mwait_idle() and "idle=mwait" cmdline param
    x86 idle: deprecate "no-hlt" cmdline param
    x86 idle APM: deprecate CONFIG_APM_CPU_IDLE
    x86 idle floppy: deprecate disable_hlt()
    x86 idle: EXPORT_SYMBOL(default_idle, pm_idle) only when APM demands it
    x86 idle: clarify AMD erratum 400 workaround
    idle governor: Avoid lock acquisition to read pm_qos before entering idle
    cpuidle: menu: fixed wrapping timers at 4.294 seconds

    Linus Torvalds
     

29 May, 2011

1 commit

  • 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
     

22 Apr, 2011

1 commit

  • In-kernel disk event polling doesn't matter for legacy/fringe drivers
    and may lead to infinite event loop if ->check_events() implementation
    generates events on level condition instead of edge.

    Now that block layer supports suppressing exporting unlisted events,
    simply leaving disk->events cleared allows these drivers to keep the
    internal revalidation behavior intact while avoiding weird
    interactions with userland event handler.

    Signed-off-by: Tejun Heo
    Cc: Kay Sievers
    Signed-off-by: Jens Axboe

    Tejun Heo
     

10 Mar, 2011

3 commits


24 Feb, 2011

1 commit

  • There are two cases when we call flush_disk.
    In one, the device has disappeared (check_disk_change) so any
    data will hold becomes irrelevant.
    In the oter, the device has changed size (check_disk_size_change)
    so data we hold may be irrelevant.

    In both cases it makes sense to discard any 'clean' buffers,
    so they will be read back from the device if needed.

    In the former case it makes sense to discard 'dirty' buffers
    as there will never be anywhere safe to write the data. In the
    second case it *does*not* make sense to discard dirty buffers
    as that will lead to file system corruption when you simply enlarge
    the containing devices.

    flush_disk calls __invalidate_devices.
    __invalidate_device calls both invalidate_inodes and invalidate_bdev.

    invalidate_inodes *does* discard I_DIRTY inodes and this does lead
    to fs corruption.

    invalidate_bev *does*not* discard dirty pages, but I don't really care
    about that at present.

    So this patch adds a flag to __invalidate_device (calling it
    __invalidate_device2) to indicate whether dirty buffers should be
    killed, and this is passed to invalidate_inodes which can choose to
    skip dirty inodes.

    flusk_disk then passes true from check_disk_change and false from
    check_disk_size_change.

    dm avoids tripping over this problem by calling i_size_write directly
    rathher than using check_disk_size_change.

    md does use check_disk_size_change and so is affected.

    This regression was introduced by commit 608aeef17a which causes
    check_disk_size_change to call flush_disk, so it is suitable for any
    kernel since 2.6.27.

    Cc: stable@kernel.org
    Acked-by: Jeff Moyer
    Cc: Andrew Patterson
    Cc: Jens Axboe
    Signed-off-by: NeilBrown

    NeilBrown
     

14 Jan, 2011

1 commit


24 Dec, 2010

1 commit


08 Nov, 2010

1 commit


06 Nov, 2010

2 commits

  • While scanning the floopy code due to c093ee4f07f4 ("floppy: fix
    use-after-free in module load failure path"), I found one more instance
    of trying to access disk->queue pointer after doing put_disk() on
    gendisk. For some reason , floppy moule still loads/unloads fine. The
    object is probably still around with right pointer values.

    o There seems to be one more instance of trying to cleanup the request
    queue after we have called put_disk() on associated gendisk.

    o This fix is more out of code inspection. Even without this fix for
    some reason I am able to load/unload floppy module without any
    issues.

    o Floppy module loads/unloads fine after the fix.

    Signed-off-by: Vivek Goyal
    Signed-off-by: Linus Torvalds

    Vivek Goyal
     
  • Commit 488211844e0c ("floppy: switch to one queue per drive instead of
    sharing a queue") introduced a use-after-free. We do "put_disk()" on
    the disk device _before_ we then clean up the queue associated with that
    disk.

    Move the put_disk() down to avoid dereferencing a free'd data structure.

    Cc: Jens Axboe
    Cc: Vivek Goyal
    Reported-and-tested-by: Randy Dunlap
    Signed-off-by: Linus Torvalds

    Linus Torvalds
     

23 Oct, 2010

1 commit

  • * 'for-2.6.37/drivers' of git://git.kernel.dk/linux-2.6-block: (95 commits)
    cciss: fix PCI IDs for new Smart Array controllers
    drbd: add race-breaker to drbd_go_diskless
    drbd: use dynamic_dev_dbg to optionally log uuid changes
    dynamic_debug.h: Fix dynamic_dev_dbg() macro if CONFIG_DYNAMIC_DEBUG not set
    drbd: cleanup: change "s
    drbd: add explicit drbd_md_sync to drbd_resync_finished
    drbd: Do not log an ASSERT for P_OV_REQUEST packets while C_CONNECTED
    drbd: fix for possible deadlock on IO error during resync
    drbd: fix unlikely access after free and list corruption
    drbd: fix for spurious fullsync (uuids rotated too fast)
    drbd: allow for explicit resync-finished notifications
    drbd: preparation commit, using full state in receive_state()
    drbd: drbd_send_ack_dp must not rely on header information
    drbd: Fix regression in recv_bm_rle_bits (compressed bitmap)
    drbd: Fixed a stupid copy and paste error
    drbd: Allow larger values for c-fill-target.
    ...

    Fix up trivial conflict in drivers/block/ataflop.c due to BKL removal

    Linus Torvalds
     

05 Oct, 2010

1 commit

  • The block device drivers have all gained new lock_kernel
    calls from a recent pushdown, and some of the drivers
    were already using the BKL before.

    This turns the BKL into a set of per-driver mutexes.
    Still need to check whether this is safe to do.

    file=$1
    name=$2
    if grep -q lock_kernel ${file} ; then
    if grep -q 'include.*linux.mutex.h' ${file} ; then
    sed -i '/include.*/d' ${file}
    else
    sed -i 's/include.*.*$/include /g' ${file}
    fi
    sed -i ${file} \
    -e "/^#include.*linux.mutex.h/,$ {
    1,/^\(static\|int\|long\)/ {
    /^\(static\|int\|long\)/istatic DEFINE_MUTEX(${name}_mutex);

    } }" \
    -e "s/\(un\)*lock_kernel\>[ ]*()/mutex_\1lock(\&${name}_mutex)/g" \
    -e '/[ ]*cycle_kernel_lock();/d'
    else
    sed -i -e '/include.*\/d' ${file} \
    -e '/cycle_kernel_lock()/d'
    fi

    Signed-off-by: Arnd Bergmann

    Arnd Bergmann
     

22 Sep, 2010

1 commit

  • Pretty straight forward conversion. Note that we do round-robin
    between the drives that have available requests, before we simply
    used the drive that the IO scheduler told us to. Since the IO
    scheduler doesn't care about multiple devices per queue, the resulting
    sort would not have made sense.

    Fixed by Vivek to get rid of a double lock problem in set_next_request()

    Signed-off-by: Jens Axboe
    Signed-off-by: Vivek Goyal

    Jens Axboe
     

08 Aug, 2010

11 commits

  • The struct cont_t is just a set of virtual function pointers.

    Signed-off-by: Stephen Hemminger
    Signed-off-by: Andrew Morton
    Signed-off-by: Jens Axboe

    Stephen Hemminger
     
  • The open and release block_device_operations are currently
    called with the BKL held. In order to change that, we must
    first make sure that all drivers that currently rely
    on this have no regressions.

    This blindly pushes the BKL into all .open and .release
    operations for all block drivers to prepare for the
    next step. The drivers can subsequently replace the BKL
    with their own locks or remove it completely when it can
    be shown that it is not needed.

    The functions blkdev_get and blkdev_put are the only
    remaining users of the big kernel lock in the block
    layer, besides a few uses in the ioctl code, none
    of which need to serialize with blkdev_{get,put}.

    Most of these two functions is also under the protection
    of bdev->bd_mutex, including the actual calls to
    ->open and ->release, and the common code does not
    access any global data structures that need the BKL.

    Signed-off-by: Arnd Bergmann
    Acked-by: Christoph Hellwig
    Signed-off-by: Jens Axboe

    Arnd Bergmann
     
  • As a preparation for the removal of the big kernel
    lock in the block layer, this removes the BKL
    from the common ioctl handling code, moving it
    into every single driver still using it.

    Signed-off-by: Arnd Bergmann
    Acked-by: Christoph Hellwig
    Signed-off-by: Jens Axboe

    Arnd Bergmann
     
  • Convert assertions to use WARN(). There are several error checks in the
    code for things that should never happen. Convert them to standard
    warnings so kerneloops.org will see them.

    Signed-off-by: Stephen Hemminger
    Signed-off-by: Andrew Morton
    Signed-off-by: Jens Axboe

    Stephen Hemminger
     
  • Convert wait loops to use wait_event_ macros.

    Signed-off-by: Stephen Hemminger
    Signed-off-by: Andrew Morton
    Signed-off-by: Jens Axboe

    Stephen Hemminger
     
  • Ioctl cmd value is unsigned, so change normalize_ioctl

    Signed-off-by: Stephen Hemminger
    Signed-off-by: Andrew Morton
    Signed-off-by: Jens Axboe

    Stephen Hemminger
     
  • As reported by sparse, cmos attribute is local.

    Signed-off-by: Stephen Hemminger
    Signed-off-by: Andrew Morton
    Signed-off-by: Jens Axboe

    Stephen Hemminger
     
  • The usage_count was being protected by a lock which was only there to
    create an atomic counter.

    Signed-off-by: Stephen Hemminger
    Signed-off-by: Andrew Morton
    Signed-off-by: Jens Axboe

    Stephen Hemminger
     
  • The first thing the floppy does is read block 0 to test geometry and to
    test for disk presence. If disk is not present this causes a console
    warning message about failed I/O. Set flag to silence.

    Signed-off-by: Stephen Hemminger
    Signed-off-by: Andrew Morton
    Signed-off-by: Jens Axboe

    Stephen Hemminger
     
  • These routines are all big enough that is better to let the compiler
    decide to inline or not.

    Signed-off-by: Stephen Hemminger
    Signed-off-by: Andrew Morton
    Signed-off-by: Jens Axboe

    Stephen Hemminger
     
  • Set debug jiffies offset at initialization. Avoids wierd values showing
    up if debugging enabled.

    Signed-off-by: Stephen Hemminger
    Signed-off-by: Andrew Morton
    Signed-off-by: Jens Axboe

    Stephen Hemminger
     

13 Mar, 2010

4 commits