10 Apr, 2014

1 commit

  • Martin reported that his test system would not boot with
    current git, it oopsed with this:

    BUG: unable to handle kernel paging request at ffff88046c6c9e80
    IP: [] blk_queue_start_tag+0x90/0x150
    PGD 1ddf067 PUD 1de2067 PMD 47fc7d067 PTE 800000046c6c9060
    Oops: 0002 [#1] SMP DEBUG_PAGEALLOC
    Modules linked in: sd_mod lpfc(+) scsi_transport_fc scsi_tgt oracleasm
    rpcsec_gss_krb5 ipv6 igb dca i2c_algo_bit i2c_core hwmon
    CPU: 3 PID: 87 Comm: kworker/u17:1 Not tainted 3.14.0+ #246
    Hardware name: Supermicro X9DRX+-F/X9DRX+-F, BIOS 3.00 07/09/2013
    Workqueue: events_unbound async_run_entry_fn
    task: ffff8802743c2150 ti: ffff880273d02000 task.ti: ffff880273d02000
    RIP: 0010:[] []
    blk_queue_start_tag+0x90/0x150
    RSP: 0018:ffff880273d03a58 EFLAGS: 00010092
    RAX: ffff88046c6c9e78 RBX: ffff880077208e78 RCX: 00000000fffc8da6
    RDX: 00000000fffc186d RSI: 0000000000000009 RDI: 00000000fffc8d9d
    RBP: ffff880273d03a88 R08: 0000000000000001 R09: ffff8800021c2410
    R10: 0000000000000005 R11: 0000000000015b30 R12: ffff88046c5bb8a0
    R13: ffff88046c5c0890 R14: 000000000000001e R15: 000000000000001e
    FS: 0000000000000000(0000) GS:ffff880277b00000(0000)
    knlGS:0000000000000000
    CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
    CR2: ffff88046c6c9e80 CR3: 00000000018f6000 CR4: 00000000000407e0
    Stack:
    ffff880273d03a98 ffff880474b18800 0000000000000000 ffff880474157000
    ffff88046c5c0890 ffff880077208e78 ffff880273d03ae8 ffffffff813b9e62
    ffff880200000010 ffff880474b18968 ffff880474b18848 ffff88046c5c0cd8
    Call Trace:
    [] scsi_request_fn+0xf2/0x510
    [] __blk_run_queue+0x37/0x50
    [] blk_execute_rq_nowait+0xb3/0x130
    [] blk_execute_rq+0x64/0xf0
    [] ? bit_waitqueue+0xd0/0xd0
    [] scsi_execute+0xe5/0x180
    [] scsi_execute_req_flags+0x9a/0x110
    [] sd_spinup_disk+0x94/0x460 [sd_mod]
    [] ? __unmap_hugepage_range+0x200/0x2f0
    [] sd_revalidate_disk+0xaa/0x3f0 [sd_mod]
    [] sd_probe_async+0xd8/0x200 [sd_mod]
    [] async_run_entry_fn+0x3f/0x140
    [] process_one_work+0x175/0x410
    [] worker_thread+0x123/0x400
    [] ? manage_workers+0x160/0x160
    [] kthread+0xce/0xf0
    [] ? kthread_freezable_should_stop+0x70/0x70
    [] ret_from_fork+0x7c/0xb0
    [] ? kthread_freezable_should_stop+0x70/0x70
    Code: 48 0f ab 11 72 db 48 81 4b 40 00 00 10 00 89 83 08 01 00 00 48 89
    df 49 8b 04 24 48 89 1c d0 e8 f7 a8 ff ff 49 8b 85 28 05 00 00 89
    58 08 48 89 03 49 8d 85 28 05 00 00 48 89 43 08 49 89 9d
    RIP [] blk_queue_start_tag+0x90/0x150
    RSP
    CR2: ffff88046c6c9e80

    Martin bisected and found this to be the problem patch;

    commit 6d113398dcf4dfcd9787a4ead738b186f7b7ff0f
    Author: Jan Kara
    Date: Mon Feb 24 16:39:54 2014 +0100

    block: Stop abusing rq->csd.list in blk-softirq

    and the problem was immediately apparent. The patch states that
    it is safe to reuse queuelist at completion time, since it is
    no longer used. However, that is not true if a device is using
    block enabled tagging. If that is the case, then the queuelist
    is reused to keep track of busy tags. If a device also ended
    up using softirq completions, we'd reuse ->queuelist for the
    IPI handling while block tagging was still using it. Boom.

    Fix this by adding a new ipi_list list head, and share the
    memory used with the request hash table. The hash table is
    never used after the request is moved to the dispatch list,
    which happens long before any potential completion of the
    request. Add a new request bit for this, so we don't have
    cases that check rq->hash while it could potentially have
    been reused for the IPI completion.

    Reported-by: Martin K. Petersen
    Tested-by: Benjamin Herrenschmidt
    Signed-off-by: Jens Axboe

    Jens Axboe
     

24 Nov, 2013

1 commit

  • Immutable biovecs are going to require an explicit iterator. To
    implement immutable bvecs, a later patch is going to add a bi_bvec_done
    member to this struct; for now, this patch effectively just renames
    things.

    Signed-off-by: Kent Overstreet
    Cc: Jens Axboe
    Cc: Geert Uytterhoeven
    Cc: Benjamin Herrenschmidt
    Cc: Paul Mackerras
    Cc: "Ed L. Cashin"
    Cc: Nick Piggin
    Cc: Lars Ellenberg
    Cc: Jiri Kosina
    Cc: Matthew Wilcox
    Cc: Geoff Levand
    Cc: Yehuda Sadeh
    Cc: Sage Weil
    Cc: Alex Elder
    Cc: ceph-devel@vger.kernel.org
    Cc: Joshua Morris
    Cc: Philip Kelleher
    Cc: Rusty Russell
    Cc: "Michael S. Tsirkin"
    Cc: Konrad Rzeszutek Wilk
    Cc: Jeremy Fitzhardinge
    Cc: Neil Brown
    Cc: Alasdair Kergon
    Cc: Mike Snitzer
    Cc: dm-devel@redhat.com
    Cc: Martin Schwidefsky
    Cc: Heiko Carstens
    Cc: linux390@de.ibm.com
    Cc: Boaz Harrosh
    Cc: Benny Halevy
    Cc: "James E.J. Bottomley"
    Cc: Greg Kroah-Hartman
    Cc: "Nicholas A. Bellinger"
    Cc: Alexander Viro
    Cc: Chris Mason
    Cc: "Theodore Ts'o"
    Cc: Andreas Dilger
    Cc: Jaegeuk Kim
    Cc: Steven Whitehouse
    Cc: Dave Kleikamp
    Cc: Joern Engel
    Cc: Prasad Joshi
    Cc: Trond Myklebust
    Cc: KONISHI Ryusuke
    Cc: Mark Fasheh
    Cc: Joel Becker
    Cc: Ben Myers
    Cc: xfs@oss.sgi.com
    Cc: Steven Rostedt
    Cc: Frederic Weisbecker
    Cc: Ingo Molnar
    Cc: Len Brown
    Cc: Pavel Machek
    Cc: "Rafael J. Wysocki"
    Cc: Herton Ronaldo Krzesinski
    Cc: Ben Hutchings
    Cc: Andrew Morton
    Cc: Guo Chao
    Cc: Tejun Heo
    Cc: Asai Thambi S P
    Cc: Selvan Mani
    Cc: Sam Bradshaw
    Cc: Wei Yongjun
    Cc: "Roger Pau Monné"
    Cc: Jan Beulich
    Cc: Stefano Stabellini
    Cc: Ian Campbell
    Cc: Sebastian Ott
    Cc: Christian Borntraeger
    Cc: Minchan Kim
    Cc: Jiang Liu
    Cc: Nitin Gupta
    Cc: Jerome Marchand
    Cc: Joe Perches
    Cc: Peng Tao
    Cc: Andy Adamson
    Cc: fanchaoting
    Cc: Jie Liu
    Cc: Sunil Mushran
    Cc: "Martin K. Petersen"
    Cc: Namjae Jeon
    Cc: Pankaj Kumar
    Cc: Dan Magenheimer
    Cc: Mel Gorman 6

    Kent Overstreet
     

09 Nov, 2013

2 commits

  • Add locking of q->sysfs_lock into elevator_change() (an exported function)
    to ensure it is held to protect q->elevator from elevator_init(), even if
    elevator_change() is called from non-sysfs paths.
    sysfs path (elv_iosched_store) uses __elevator_change(), non-locking
    version, as the lock is already taken by elv_iosched_store().

    Signed-off-by: Tomoki Sekiyama
    Signed-off-by: Jens Axboe

    Tomoki Sekiyama
     
  • The soft lockup below happens at the boot time of the system using dm
    multipath and the udev rules to switch scheduler.

    [ 356.127001] BUG: soft lockup - CPU#3 stuck for 22s! [sh:483]
    [ 356.127001] RIP: 0010:[] [] lock_timer_base.isra.35+0x1d/0x50
    ...
    [ 356.127001] Call Trace:
    [ 356.127001] [] try_to_del_timer_sync+0x20/0x70
    [ 356.127001] [] ? kmem_cache_alloc_node_trace+0x20a/0x230
    [ 356.127001] [] del_timer_sync+0x52/0x60
    [ 356.127001] [] cfq_exit_queue+0x32/0xf0
    [ 356.127001] [] elevator_exit+0x2f/0x50
    [ 356.127001] [] elevator_change+0xf1/0x1c0
    [ 356.127001] [] elv_iosched_store+0x20/0x50
    [ 356.127001] [] queue_attr_store+0x59/0xb0
    [ 356.127001] [] sysfs_write_file+0xc6/0x140
    [ 356.127001] [] vfs_write+0xbd/0x1e0
    [ 356.127001] [] SyS_write+0x49/0xa0
    [ 356.127001] [] system_call_fastpath+0x16/0x1b

    This is caused by a race between md device initialization by multipathd and
    shell script to switch the scheduler using sysfs.

    - multipathd:
    SyS_ioctl -> do_vfs_ioctl -> dm_ctl_ioctl -> ctl_ioctl -> table_load
    -> dm_setup_md_queue -> blk_init_allocated_queue -> elevator_init
    q->elevator = elevator_alloc(q, e); // not yet initialized

    - sh -c 'echo deadline > /sys/$DEVPATH/queue/scheduler':
    elevator_switch (in the call trace above)
    struct elevator_queue *old = q->elevator;
    q->elevator = elevator_alloc(q, new_e);
    elevator_exit(old); // lockup! (*)

    - multipathd: (cont.)
    err = e->ops.elevator_init_fn(q); // init fails; q->elevator is modified

    (*) When del_timer_sync() is called, lock_timer_base() will loop infinitely
    while timer->base == NULL. In this case, as timer will never initialized,
    it results in lockup.

    This patch introduces acquisition of q->sysfs_lock around elevator_init()
    into blk_init_allocated_queue(), to provide mutual exclusion between
    initialization of the q->scheduler and switching of the scheduler.

    This should fix this bugzilla:
    https://bugzilla.redhat.com/show_bug.cgi?id=902012

    Signed-off-by: Tomoki Sekiyama
    Signed-off-by: Jens Axboe

    Tomoki Sekiyama
     

12 Sep, 2013

1 commit


03 Jul, 2013

1 commit

  • There's a race between elevator switching and normal io operation.
    Because the allocation of struct elevator_queue and struct elevator_data
    don't in a atomic operation.So there are have chance to use NULL
    ->elevator_data.
    For example:
    Thread A: Thread B
    blk_queu_bio elevator_switch
    spin_lock_irq(q->queue_block) elevator_alloc
    elv_merge elevator_init_fn

    Because call elevator_alloc, it can't hold queue_lock and the
    ->elevator_data is NULL.So at the same time, threadA call elv_merge and
    nedd some info of elevator_data.So the crash happened.

    Move the elevator_alloc into func elevator_init_fn, it make the
    operations in a atomic operation.

    Using the follow method can easy reproduce this bug
    1:dd if=/dev/sdb of=/dev/null
    2:while true;do echo noop > scheduler;echo deadline > scheduler;done

    The test method also use this method.

    Signed-off-by: Jianpeng Ma
    Signed-off-by: Jens Axboe

    Jianpeng Ma
     

23 Mar, 2013

1 commit

  • When a request is added:
    If device is suspended or is suspending and the request is not a
    PM request, resume the device.

    When the last request finishes:
    Call pm_runtime_mark_last_busy().

    When pick a request:
    If device is resuming/suspending, then only PM request is allowed
    to go.

    The idea and API is designed by Alan Stern and described here:
    http://marc.info/?l=linux-scsi&m=133727953625963&w=2

    Signed-off-by: Lin Ming
    Signed-off-by: Aaron Lu
    Acked-by: Alan Stern
    Signed-off-by: Jens Axboe

    Lin Ming
     

01 Mar, 2013

1 commit

  • Pull block IO core bits from Jens Axboe:
    "Below are the core block IO bits for 3.9. It was delayed a few days
    since my workstation kept crashing every 2-8h after pulling it into
    current -git, but turns out it is a bug in the new pstate code (divide
    by zero, will report separately). In any case, it contains:

    - The big cfq/blkcg update from Tejun and and Vivek.

    - Additional block and writeback tracepoints from Tejun.

    - Improvement of the should sort (based on queues) logic in the plug
    flushing.

    - _io() variants of the wait_for_completion() interface, using
    io_schedule() instead of schedule() to contribute to io wait
    properly.

    - Various little fixes.

    You'll get two trivial merge conflicts, which should be easy enough to
    fix up"

    Fix up the trivial conflicts due to hlist traversal cleanups (commit
    b67bfe0d42ca: "hlist: drop the node parameter from iterators").

    * 'for-3.9/core' of git://git.kernel.dk/linux-block: (39 commits)
    block: remove redundant check to bd_openers()
    block: use i_size_write() in bd_set_size()
    cfq: fix lock imbalance with failed allocations
    drivers/block/swim3.c: fix null pointer dereference
    block: don't select PERCPU_RWSEM
    block: account iowait time when waiting for completion of IO request
    sched: add wait_for_completion_io[_timeout]
    writeback: add more tracepoints
    block: add block_{touch|dirty}_buffer tracepoint
    buffer: make touch_buffer() an exported function
    block: add @req to bio_{front|back}_merge tracepoints
    block: add missing block_bio_complete() tracepoint
    block: Remove should_sort judgement when flush blk_plug
    block,elevator: use new hashtable implementation
    cfq-iosched: add hierarchical cfq_group statistics
    cfq-iosched: collect stats from dead cfqgs
    cfq-iosched: separate out cfqg_stats_reset() from cfq_pd_reset_stats()
    blkcg: make blkcg_print_blkgs() grab q locks instead of blkcg lock
    block: RCU free request_queue
    blkcg: implement blkg_[rw]stat_recursive_sum() and blkg_[rw]stat_merge()
    ...

    Linus Torvalds
     

28 Feb, 2013

1 commit

  • I'm not sure why, but the hlist for each entry iterators were conceived

    list_for_each_entry(pos, head, member)

    The hlist ones were greedy and wanted an extra parameter:

    hlist_for_each_entry(tpos, pos, head, member)

    Why did they need an extra pos parameter? I'm not quite sure. Not only
    they don't really need it, it also prevents the iterator from looking
    exactly like the list iterator, which is unfortunate.

    Besides the semantic patch, there was some manual work required:

    - Fix up the actual hlist iterators in linux/list.h
    - Fix up the declaration of other iterators based on the hlist ones.
    - A very small amount of places were using the 'node' parameter, this
    was modified to use 'obj->member' instead.
    - Coccinelle didn't handle the hlist_for_each_entry_safe iterator
    properly, so those had to be fixed up manually.

    The semantic patch which is mostly the work of Peter Senna Tschudin is here:

    @@
    iterator name hlist_for_each_entry, hlist_for_each_entry_continue, hlist_for_each_entry_from, hlist_for_each_entry_rcu, hlist_for_each_entry_rcu_bh, hlist_for_each_entry_continue_rcu_bh, for_each_busy_worker, ax25_uid_for_each, ax25_for_each, inet_bind_bucket_for_each, sctp_for_each_hentry, sk_for_each, sk_for_each_rcu, sk_for_each_from, sk_for_each_safe, sk_for_each_bound, hlist_for_each_entry_safe, hlist_for_each_entry_continue_rcu, nr_neigh_for_each, nr_neigh_for_each_safe, nr_node_for_each, nr_node_for_each_safe, for_each_gfn_indirect_valid_sp, for_each_gfn_sp, for_each_host;

    type T;
    expression a,c,d,e;
    identifier b;
    statement S;
    @@

    -T b;

    [akpm@linux-foundation.org: drop bogus change from net/ipv4/raw.c]
    [akpm@linux-foundation.org: drop bogus hunk from net/ipv6/raw.c]
    [akpm@linux-foundation.org: checkpatch fixes]
    [akpm@linux-foundation.org: fix warnings]
    [akpm@linux-foudnation.org: redo intrusive kvm changes]
    Tested-by: Peter Senna Tschudin
    Acked-by: Paul E. McKenney
    Signed-off-by: Sasha Levin
    Cc: Wu Fengguang
    Cc: Marcelo Tosatti
    Cc: Gleb Natapov
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Sasha Levin
     

23 Jan, 2013

1 commit

  • Block layer allows selecting an elevator which is built as a module to
    be selected as system default via kernel param "elevator=". This is
    achieved by automatically invoking request_module() whenever a new
    block device is initialized and the elevator is not available.

    This led to an interesting deadlock problem involving async and module
    init. Block device probing running off an async job invokes
    request_module(). While the module is being loaded, it performs
    async_synchronize_full() which ends up waiting for the async job which
    is already waiting for request_module() to finish, leading to
    deadlock.

    Invoking request_module() from deep in block device init path is
    already nasty in itself. It seems best to avoid these situations from
    the beginning by moving on-demand module loading out of block init
    path.

    The previous patch made sure that the default elevator module is
    loaded early during boot if available. This patch removes on-demand
    loading of the default elevator from elevator init path. As the
    module would have been loaded during boot, userland-visible behavior
    difference should be minimal.

    For more details, please refer to the following thread.

    http://thread.gmane.org/gmane.linux.kernel/1420814

    v2: The bool parameter was named @request_module which conflicted with
    request_module(). This built okay w/ CONFIG_MODULES because
    request_module() was defined as a macro. W/o CONFIG_MODULES, it
    causes build breakage. Rename the parameter to @try_loading.
    Reported by Fengguang.

    Signed-off-by: Tejun Heo
    Cc: Jens Axboe
    Cc: Arjan van de Ven
    Cc: Linus Torvalds
    Cc: Alex Riesen
    Cc: Fengguang Wu

    Tejun Heo
     

19 Jan, 2013

1 commit

  • This patch adds default module loading and uses it to load the default
    block elevator. During boot, it's called right after initramfs or
    initrd is made available and right before control is passed to
    userland. This ensures that as long as the modules are available in
    the usual places in initramfs, initrd or the root filesystem, the
    default modules are loaded as soon as possible.

    This will replace the on-demand elevator module loading from elevator
    init path.

    v2: Fixed build breakage when !CONFIG_BLOCK. Reported by kbuild test
    robot.

    Signed-off-by: Tejun Heo
    Cc: Jens Axboe
    Cc: Arjan van de Ven
    Cc: Linus Torvalds
    Cc: Alex Riesen
    Cc: Fengguang We

    Tejun Heo
     

11 Jan, 2013

1 commit

  • Switch elevator to use the new hashtable implementation. This reduces the
    amount of generic unrelated code in the elevator.

    This also removes the dymanic allocation of the hash table. The size of the table is
    constant so there's no point in paying the price of an extra dereference when accessing
    it.

    This patch depends on d9b482c ("hashtable: introduce a small and naive
    hashtable") which was merged in v3.6.

    Signed-off-by: Sasha Levin
    Signed-off-by: Jens Axboe

    Sasha Levin
     

09 Nov, 2012

1 commit

  • In a workload, thread 1 accesses a, a+2, ..., thread 2 accesses a+1, a+3,....
    When the requests are flushed to queue, a and a+1 are merged to (a, a+1), a+2
    and a+3 too to (a+2, a+3), but (a, a+1) and (a+2, a+3) aren't merged.

    If we do recursive merge for such interleave access, some workloads throughput
    get improvement. A recent worload I'm checking on is swap, below change
    boostes the throughput around 5% ~ 10%.

    Signed-off-by: Shaohua Li
    Signed-off-by: Jens Axboe

    Shaohua Li
     

20 Sep, 2012

1 commit

  • Remove special-casing of non-rw fs style requests (discard). The nomerge
    flags are consolidated in blk_types.h, and rq_mergeable() and
    bio_mergeable() have been modified to use them.

    bio_is_rw() is used in place of bio_has_data() a few places. This is
    done to to distinguish true reads and writes from other fs type requests
    that carry a payload (e.g. write same).

    Signed-off-by: Martin K. Petersen
    Acked-by: Mike Snitzer
    Signed-off-by: Jens Axboe

    Martin K. Petersen
     

20 Apr, 2012

1 commit

  • All blkcg policies were assumed to be enabled on all request_queues.
    Due to various implementation obstacles, during the recent blkcg core
    updates, this was temporarily implemented as shooting down all !root
    blkgs on elevator switch and policy [de]registration combined with
    half-broken in-place root blkg updates. In addition to being buggy
    and racy, this meant losing all blkcg configurations across those
    events.

    Now that blkcg is cleaned up enough, this patch replaces the temporary
    implementation with proper per-queue policy activation. Each blkcg
    policy should call the new blkcg_[de]activate_policy() to enable and
    disable the policy on a specific queue. blkcg_activate_policy()
    allocates and installs policy data for the policy for all existing
    blkgs. blkcg_deactivate_policy() does the reverse. If a policy is
    not enabled for a given queue, blkg printing / config functions skip
    the respective blkg for the queue.

    blkcg_activate_policy() also takes care of root blkg creation, and
    cfq_init_queue() and blk_throtl_init() are updated accordingly.

    This replaces blkcg_bypass_{start|end}() and update_root_blkg_pd()
    unnecessary. Dropped.

    v2: cfq_init_queue() was returning uninitialized @ret on root_group
    alloc failure if !CONFIG_CFQ_GROUP_IOSCHED. Fixed.

    Signed-off-by: Tejun Heo
    Cc: Vivek Goyal
    Signed-off-by: Jens Axboe

    Tejun Heo
     

07 Mar, 2012

7 commits

  • IO scheduling and cgroup are tied to the issuing task via io_context
    and cgroup of %current. Unfortunately, there are cases where IOs need
    to be routed via a different task which makes scheduling and cgroup
    limit enforcement applied completely incorrectly.

    For example, all bios delayed by blk-throttle end up being issued by a
    delayed work item and get assigned the io_context of the worker task
    which happens to serve the work item and dumped to the default block
    cgroup. This is double confusing as bios which aren't delayed end up
    in the correct cgroup and makes using blk-throttle and cfq propio
    together impossible.

    Any code which punts IO issuing to another task is affected which is
    getting more and more common (e.g. btrfs). As both io_context and
    cgroup are firmly tied to task including userland visible APIs to
    manipulate them, it makes a lot of sense to match up tasks to bios.

    This patch implements bio_associate_current() which associates the
    specified bio with %current. The bio will record the associated ioc
    and blkcg at that point and block layer will use the recorded ones
    regardless of which task actually ends up issuing the bio. bio
    release puts the associated ioc and blkcg.

    It grabs and remembers ioc and blkcg instead of the task itself
    because task may already be dead by the time the bio is issued making
    ioc and blkcg inaccessible and those are all block layer cares about.

    elevator_set_req_fn() is updated such that the bio elvdata is being
    allocated for is available to the elevator.

    This doesn't update block cgroup policies yet. Further patches will
    implement the support.

    -v2: #ifdef CONFIG_BLK_CGROUP added around bio->bi_ioc dereference in
    rq_ioc() to fix build breakage.

    Signed-off-by: Tejun Heo
    Cc: Vivek Goyal
    Cc: Kent Overstreet
    Signed-off-by: Jens Axboe

    Tejun Heo
     
  • Currently, blkg is per cgroup-queue-policy combination. This is
    unnatural and leads to various convolutions in partially used
    duplicate fields in blkg, config / stat access, and general management
    of blkgs.

    This patch make blkg's per cgroup-queue and let them serve all
    policies. blkgs are now created and destroyed by blkcg core proper.
    This will allow further consolidation of common management logic into
    blkcg core and API with better defined semantics and layering.

    As a transitional step to untangle blkg management, elvswitch and
    policy [de]registration, all blkgs except the root blkg are being shot
    down during elvswitch and bypass. This patch adds blkg_root_update()
    to update root blkg in place on policy change. This is hacky and racy
    but should be good enough as interim step until we get locking
    simplified and switch over to proper in-place update for all blkgs.

    -v2: Root blkgs need to be updated on elvswitch too and blkg_alloc()
    comment wasn't updated according to the function change. Fixed.
    Both pointed out by Vivek.

    -v3: v2 updated blkg_destroy_all() to invoke update_root_blkg_pd() for
    all policies. This freed root pd during elvswitch before the
    last queue finished exiting and led to oops. Directly invoke
    update_root_blkg_pd() only on BLKIO_POLICY_PROP from
    cfq_exit_queue(). This also is closer to what will be done with
    proper in-place blkg update. Reported by Vivek.

    Signed-off-by: Tejun Heo
    Cc: Vivek Goyal
    Signed-off-by: Jens Axboe

    Tejun Heo
     
  • With the previous patch to move blkg list heads and counters to
    request_queue and blkg, logic to manage them in both policies are
    almost identical and can be moved to blkcg core.

    This patch moves blkg link logic into blkg_lookup_create(), implements
    common blkg unlink code in blkg_destroy(), and updates
    blkg_destory_all() so that it's policy specific and can skip root
    group. The updated blkg_destroy_all() is now used to both clear queue
    for bypassing and elv switching, and release all blkgs on q exit.

    This patch introduces a race window where policy [de]registration may
    race against queue blkg clearing. This can only be a problem on cfq
    unload and shouldn't be a real problem in practice (and we have many
    other places where this race already exists). Future patches will
    remove these unlikely races.

    Signed-off-by: Tejun Heo
    Cc: Vivek Goyal
    Signed-off-by: Jens Axboe

    Tejun Heo
     
  • Elevator switch may involve changes to blkcg policies. Implement
    shoot down of blkio_groups.

    Combined with the previous bypass updates, the end goal is updating
    blkcg core such that it can ensure that blkcg's being affected become
    quiescent and don't have any per-blkg data hanging around before
    commencing any policy updates. Until queues are made aware of the
    policies that applies to them, as an interim step, all per-policy blkg
    data will be shot down.

    * blk-throtl doesn't need this change as it can't be disabled for a
    live queue; however, update it anyway as the scheduled blkg
    unification requires this behavior change. This means that
    blk-throtl configuration will be unnecessarily lost over elevator
    switch. This oddity will be removed after blkcg learns to associate
    individual policies with request_queues.

    * blk-throtl dosen't shoot down root_tg. This is to ease transition.
    Unified blkg will always have persistent root group and not shooting
    down root_tg for now eases transition to that point by avoiding
    having to update td->root_tg and is safe as blk-throtl can never be
    disabled

    -v2: Vivek pointed out that group list is not guaranteed to be empty
    on return from clear function if it raced cgroup removal and
    lost. Fix it by waiting a bit and retrying. This kludge will
    soon be removed once locking is updated such that blkg is never
    in limbo state between blkcg and request_queue locks.

    blk-throtl no longer shoots down root_tg to avoid breaking
    td->root_tg.

    Also, Nest queue_lock inside blkio_list_lock not the other way
    around to avoid introduce possible deadlock via blkcg lock.

    -v3: blkcg_clear_queue() repositioned and renamed to
    blkg_destroy_all() to increase consistency with later changes.
    cfq_clear_queue() updated to check q->elevator before
    dereferencing it to avoid NULL dereference on not fully
    initialized queues (used by later change).

    Signed-off-by: Tejun Heo
    Cc: Vivek Goyal
    Signed-off-by: Jens Axboe

    Tejun Heo
     
  • Rename and extend elv_queisce_start/end() to
    blk_queue_bypass_start/end() which are exported and supports nesting
    via @q->bypass_depth. Also add blk_queue_bypass() to test bypass
    state.

    This will be further extended and used for blkio_group management.

    Signed-off-by: Tejun Heo
    Cc: Vivek Goyal
    Signed-off-by: Jens Axboe

    Tejun Heo
     
  • elevator_ops->elevator_init_fn() has a weird return value. It returns
    a void * which the caller should assign to q->elevator->elevator_data
    and %NULL return denotes init failure.

    Update such that it returns integer 0/-errno and sets elevator_data
    directly as necessary.

    This makes the interface more conventional and eases further cleanup.

    Signed-off-by: Tejun Heo
    Cc: Vivek Goyal
    Signed-off-by: Jens Axboe

    Tejun Heo
     
  • Elevator switch tries hard to keep as much as context until new
    elevator is ready so that it can revert to the original state if
    initializing the new elevator fails for some reason. Unfortunately,
    with more auxiliary contexts to manage, this makes elevator init and
    exit paths too complex and fragile.

    This patch makes elevator_switch() unregister the current elevator and
    flush icq's before start initializing the new one. As we still keep
    the old elevator itself, the only difference is that we lose icq's on
    rare occassions of switching failure, which isn't critical at all.

    Note that this makes explicit elevator parameter to
    elevator_init_queue() and __elv_register_queue() unnecessary as they
    always can use the current elevator.

    This patch enables block cgroup cleanups.

    -v2: blk_add_trace_msg() prints elevator name from @new_e instead of
    @e->type as the local variable no longer exists. This caused
    build failure on CONFIG_BLK_DEV_IO_TRACE.

    Signed-off-by: Tejun Heo
    Cc: Vivek Goyal
    Signed-off-by: Jens Axboe

    Tejun Heo
     

08 Feb, 2012

1 commit

  • blk_rq_merge_ok() is the elevator-neutral part of merge eligibility
    test. blk_try_merge() determines merge direction and expects the
    caller to have tested elv_rq_merge_ok() previously.

    elv_rq_merge_ok() now wraps blk_rq_merge_ok() and then calls
    elv_iosched_allow_merge(). elv_try_merge() is removed and the two
    callers are updated to call elv_rq_merge_ok() explicitly followed by
    blk_try_merge(). While at it, make rq_merge_ok() functions return
    bool.

    This is to prepare for plug merge update and doesn't introduce any
    behavior change.

    This is based on Jens' patch to skip elevator_allow_merge_fn() from
    plug merge.

    Signed-off-by: Tejun Heo
    LKML-Reference:
    Original-patch-by: Jens Axboe
    Signed-off-by: Jens Axboe

    Tejun Heo
     

15 Jan, 2012

1 commit

  • This reverts commit 274193224cdabd687d804a26e0150bb20f2dd52c.

    We have some problems related to selection of empty queues
    that need to be resolved, evidence so far points to the
    recursive merge logic making either being the cause or at
    least the accelerator for this. So revert it for now, until
    we figure this out.

    Signed-off-by: Jens Axboe

    Jens Axboe
     

16 Dec, 2011

1 commit

  • In my workload, thread 1 accesses a, a+2, ..., thread 2 accesses a+1,
    a+3,.... When the requests are flushed to queue, a and a+1 are merged
    to (a, a+1), a+2 and a+3 too to (a+2, a+3), but (a, a+1) and (a+2, a+3)
    aren't merged.
    With recursive merge below, the workload throughput gets improved 20%
    and context switch drops 60%.

    Signed-off-by: Shaohua Li
    Signed-off-by: Jens Axboe

    Shaohua Li
     

14 Dec, 2011

6 commits

  • With kmem_cache managed by blk-ioc, io_cq exit/release can be moved to
    blk-ioc too. The odd ->io_cq->exit/release() callbacks are replaced
    with elevator_ops->elevator_exit_icq_fn() with unlinking from both ioc
    and q, and freeing automatically handled by blk-ioc. The elevator
    operation only need to perform exit operation specific to the elevator
    - in cfq's case, exiting the cfqq's.

    Also, clearing of io_cq's on q detach is moved to block core and
    automatically performed on elevator switch and q release.

    Because the q io_cq points to might be freed before RCU callback for
    the io_cq runs, blk-ioc code should remember to which cache the io_cq
    needs to be freed when the io_cq is released. New field
    io_cq->__rcu_icq_cache is added for this purpose. As both the new
    field and rcu_head are used only after io_cq is released and the
    q/ioc_node fields aren't, they are put into unions.

    Signed-off-by: Tejun Heo
    Signed-off-by: Jens Axboe

    Tejun Heo
     
  • Let elevators set ->icq_size and ->icq_align in elevator_type and
    elv_register() and elv_unregister() respectively create and destroy
    kmem_cache for icq.

    * elv_register() now can return failure. All callers updated.

    * icq caches are automatically named "ELVNAME_io_cq".

    * cfq_slab_setup/kill() are collapsed into cfq_init/exit().

    * While at it, minor indentation change for iosched_cfq.elevator_name
    for consistency.

    This will help moving icq management to block core. This doesn't
    introduce any functional change.

    Signed-off-by: Tejun Heo
    Signed-off-by: Jens Axboe

    Tejun Heo
     
  • Most of icq management is about to be moved out of cfq into blk-ioc.
    This patch prepares for it.

    * Move cfqd->icq_list to request_queue->icq_list

    * Make request explicitly point to icq instead of through elevator
    private data. ->elevator_private[3] is replaced with sub struct elv
    which contains icq pointer and priv[2]. cfq is updated accordingly.

    * Meaningless clearing of ->elevator_private[0] removed from
    elv_set_request(). At that point in code, the field was guaranteed
    to be %NULL anyway.

    This patch doesn't introduce any functional change.

    Signed-off-by: Tejun Heo
    Signed-off-by: Jens Axboe

    Tejun Heo
     
  • elevator_queue->ops points to the same ops struct ->elevator_type.ops
    is pointing to. The only effect of caching it in elevator_queue is
    shorter notation - it doesn't save any indirect derefence.

    Relocate elevator_type->list which used only during module init/exit
    to the end of the structure, rename elevator_queue->elevator_type to
    ->type, and replace elevator_queue->ops with elevator_queue->type.ops.

    This doesn't introduce any functional difference.

    Signed-off-by: Tejun Heo
    Signed-off-by: Jens Axboe

    Tejun Heo
     
  • Elevator switch sequence first attached the new elevator, then tried
    registering it (sysfs) and if that failed attached back the old
    elevator. However, sysfs registration doesn't require the elevator to
    be attached, so there is no reason to do the "detach, attach new,
    register, maybe re-attach old" sequence. It can just do "register,
    detach, attach".

    * elevator_init_queue() is updated to set ->elevator_data directly and
    return 0 / -errno. This allows elevator_exit() on an unattached
    elevator.

    * __elv_unregister_queue() which was necessary to unregister
    unattached q is removed in favor of __elv_register_queue() which can
    register unattached q.

    * elevator_attach() becomes a single assignment and obscures more then
    it helps. Dropped.

    This will help cleaning up io_context handling across elevator switch.

    This patch doesn't introduce visible behavior change.

    Signed-off-by: Tejun Heo
    Signed-off-by: Jens Axboe

    Tejun Heo
     
  • Now that all cic's are immediately unlinked from both ioc and queue,
    lazy dropping from lookup path and trimming on elevator unregister are
    unnecessary. Kill them and remove now unused elevator_ops->trim().

    This also leaves call_for_each_cic() without any user. Removed.

    Signed-off-by: Tejun Heo
    Signed-off-by: Jens Axboe

    Tejun Heo
     

19 Oct, 2011

2 commits

  • request_queue is refcounted but actually depdends on lifetime
    management from the queue owner - on blk_cleanup_queue(), block layer
    expects that there's no request passing through request_queue and no
    new one will.

    This is fundamentally broken. The queue owner (e.g. SCSI layer)
    doesn't have a way to know whether there are other active users before
    calling blk_cleanup_queue() and other users (e.g. bsg) don't have any
    guarantee that the queue is and would stay valid while it's holding a
    reference.

    With delay added in blk_queue_bio() before queue_lock is grabbed, the
    following oops can be easily triggered when a device is removed with
    in-flight IOs.

    sd 0:0:1:0: [sdb] Stopping disk
    ata1.01: disabled
    general protection fault: 0000 [#1] PREEMPT SMP
    CPU 2
    Modules linked in:

    Pid: 648, comm: test_rawio Not tainted 3.1.0-rc3-work+ #56 Bochs Bochs
    RIP: 0010:[] [] elv_rqhash_find+0x61/0x100
    ...
    Process test_rawio (pid: 648, threadinfo ffff880019efa000, task ffff880019ef8a80)
    ...
    Call Trace:
    [] elv_merge+0x84/0xe0
    [] blk_queue_bio+0xf4/0x400
    [] generic_make_request+0xca/0x100
    [] submit_bio+0x74/0x100
    [] dio_bio_submit+0xbc/0xc0
    [] __blockdev_direct_IO+0x92e/0xb40
    [] blkdev_direct_IO+0x57/0x60
    [] generic_file_aio_read+0x6d5/0x760
    [] do_sync_read+0xda/0x120
    [] vfs_read+0xc5/0x180
    [] sys_pread64+0x9a/0xb0
    [] system_call_fastpath+0x16/0x1b

    This happens because blk_queue_cleanup() destroys the queue and
    elevator whether IOs are in progress or not and DEAD tests are
    sprinkled in the request processing path without proper
    synchronization.

    Similar problem exists for blk-throtl. On queue cleanup, blk-throtl
    is shutdown whether it has requests in it or not. Depending on
    timing, it either oopses or throttled bios are lost putting tasks
    which are waiting for bio completion into eternal D state.

    The way it should work is having the usual clear distinction between
    shutdown and release. Shutdown drains all currently pending requests,
    marks the queue dead, and performs partial teardown of the now
    unnecessary part of the queue. Even after shutdown is complete,
    reference holders are still allowed to issue requests to the queue
    although they will be immmediately failed. The rest of teardown
    happens on release.

    This patch makes the following changes to make blk_queue_cleanup()
    behave as proper shutdown.

    * QUEUE_FLAG_DEAD is now set while holding both q->exit_mutex and
    queue_lock.

    * Unsynchronized DEAD check in generic_make_request_checks() removed.
    This couldn't make any meaningful difference as the queue could die
    after the check.

    * blk_drain_queue() updated such that it can drain all requests and is
    now called during cleanup.

    * blk_throtl updated such that it checks DEAD on grabbing queue_lock,
    drains all throttled bios during cleanup and free td when queue is
    released.

    Signed-off-by: Tejun Heo
    Cc: Vivek Goyal
    Signed-off-by: Jens Axboe

    Tejun Heo
     
  • Reorganize queue draining related code in preparation of queue exit
    changes.

    * Factor out actual draining from elv_quiesce_start() to
    blk_drain_queue().

    * Make elv_quiesce_start/end() responsible for their own locking.

    * Replace open-coded ELVSWITCH clearing in elevator_switch() with
    elv_quiesce_end().

    This patch doesn't cause any visible functional difference.

    Signed-off-by: Tejun Heo
    Signed-off-by: Jens Axboe

    Tejun Heo
     

12 Sep, 2011

1 commit


03 Jun, 2011

1 commit

  • Hi, Jens,

    If you recall, I posted an RFC patch for this back in July of last year:
    http://lkml.org/lkml/2010/7/13/279

    The basic problem is that a process can issue a never-ending stream of
    async direct I/Os to the same sector on a device, thus starving out
    other I/O in the system (due to the way the alias handling works in both
    cfq and deadline). The solution I proposed back then was to start
    dispatching from the fifo after a certain number of aliases had been
    dispatched. Vivek asked why we had to treat aliases differently at all,
    and I never had a good answer. So, I put together a simple patch which
    allows aliases to be added to the rb tree (it adds them to the right,
    though that doesn't matter as the order isn't guaranteed anyway). I
    think this is the preferred solution, as it doesn't break up time slices
    in CFQ or batches in deadline. I've tested it, and it does solve the
    starvation issue. Let me know what you think.

    Cheers,
    Jeff

    Signed-off-by: Jeff Moyer
    Signed-off-by: Jens Axboe

    Jeff Moyer
     

21 May, 2011

2 commits

  • We don't need them anymore, so kill:

    - REQ_ON_PLUG checks in various places
    - !rq_mergeable() check in plug merging

    Signed-off-by: Jens Axboe

    Jens Axboe
     
  • Since for-2.6.40/core was forked off the 2.6.39 devel tree, we've
    had churn in the core area that makes it difficult to handle
    patches for eg cfq or blk-throttle. Instead of requiring that they
    be based in older versions with bugs that have been fixed later
    in the rc cycle, merge in 2.6.39 final.

    Also fixes up conflicts in the below files.

    Conflicts:
    drivers/block/paride/pcd.c
    drivers/cdrom/viocd.c
    drivers/ide/ide-cd.c

    Signed-off-by: Jens Axboe

    Jens Axboe
     

06 May, 2011

1 commit

  • After the anticipatory scheduler was dropped, there was no need to
    special-case the request_module string. As such, drop the redundant
    sprintf and stack variable.

    Signed-off-by: Kees Cook
    Signed-off-by: Jens Axboe

    Kees Cook
     

22 Apr, 2011

1 commit


18 Apr, 2011

1 commit

  • Instead of overloading __blk_run_queue to force an offload to kblockd
    add a new blk_run_queue_async helper to do it explicitly. I've kept
    the blk_queue_stopped check for now, but I suspect it's not needed
    as the check we do when the workqueue items runs should be enough.

    Signed-off-by: Christoph Hellwig
    Signed-off-by: Jens Axboe

    Christoph Hellwig