06 Sep, 2019

1 commit

  • When elevator_init_mq() is called from blk_mq_init_allocated_queue(),
    the only information known about the device is the number of hardware
    queues as the block device scan by the device driver is not completed
    yet for most drivers. The device type and elevator required features
    are not set yet, preventing to correctly select the default elevator
    most suitable for the device.

    This currently affects all multi-queue zoned block devices which default
    to the "none" elevator instead of the required "mq-deadline" elevator.
    These drives currently include host-managed SMR disks connected to a
    smartpqi HBA and null_blk block devices with zoned mode enabled.
    Upcoming NVMe Zoned Namespace devices will also be affected.

    Fix this by adding the boolean elevator_init argument to
    blk_mq_init_allocated_queue() to control the execution of
    elevator_init_mq(). Two cases exist:
    1) elevator_init = false is used for calls to
    blk_mq_init_allocated_queue() within blk_mq_init_queue(). In this
    case, a call to elevator_init_mq() is added to __device_add_disk(),
    resulting in the delayed initialization of the queue elevator
    after the device driver finished probing the device information. This
    effectively allows elevator_init_mq() access to more information
    about the device.
    2) elevator_init = true preserves the current behavior of initializing
    the elevator directly from blk_mq_init_allocated_queue(). This case
    is used for the special request based DM devices where the device
    gendisk is created before the queue initialization and device
    information (e.g. queue limits) is already known when the queue
    initialization is executed.

    Additionally, to make sure that the elevator initialization is never
    done while requests are in-flight (there should be none when the device
    driver calls device_add_disk()), freeze and quiesce the device request
    queue before calling blk_mq_init_sched() in elevator_init_mq().

    Reviewed-by: Ming Lei
    Signed-off-by: Damien Le Moal
    Signed-off-by: Jens Axboe

    Damien Le Moal
     

17 Jul, 2019

1 commit


15 Jun, 2019

1 commit

  • Make use of the struct_size() helper instead of an open-coded version
    in order to avoid any potential type mistakes, in particular in the
    context in which this code is being used.

    So, replace the following form:

    sizeof(*new_ptbl) + target * sizeof(new_ptbl->part[0])

    with:

    struct_size(new_ptbl, part, target)

    Also, notice that variable size is unnecessary, hence it is removed.

    This code was detected with the help of Coccinelle.

    Signed-off-by: Gustavo A. R. Silva
    Signed-off-by: Jens Axboe

    Gustavo A. R. Silva
     

01 Jun, 2019

1 commit


01 May, 2019

1 commit


22 Apr, 2019

1 commit

  • commit 2da78092dda "block: Fix dev_t minor allocation lifetime"
    specifically moved blk_free_devt(dev->devt) call to part_release()
    to avoid reallocating device number before the device is fully
    shutdown.

    However, it can cause use-after-free on gendisk in get_gendisk().
    We use md device as example to show the race scenes:

    Process1 Worker Process2
    md_free
    blkdev_open
    del_gendisk
    add delete_partition_work_fn() to wq
    __blkdev_get
    get_gendisk
    put_disk
    disk_release
    kfree(disk)
    find part from ext_devt_idr
    get_disk_and_module(disk)
    cause use after free

    delete_partition_work_fn
    put_device(part)
    part_release
    remove part from ext_devt_idr

    Before is removed from ext_devt_idr by
    delete_partition_work_fn(), we can find the devt and then access
    gendisk by hd_struct pointer. But, if we access the gendisk after
    it have been freed, it can cause in use-after-freeon gendisk in
    get_gendisk().

    We fix this by adding a new helper blk_invalidate_devt() in
    delete_partition() and del_gendisk(). It replaces hd_struct
    pointer in idr with value 'NULL', and deletes the entry from
    idr in part_release() as we do now.

    Thanks to Jan Kara for providing the solution and more clear comments
    for the code.

    Fixes: 2da78092dda1 ("block: Fix dev_t minor allocation lifetime")
    Cc: Al Viro
    Reviewed-by: Bart Van Assche
    Reviewed-by: Keith Busch
    Reviewed-by: Jan Kara
    Suggested-by: Jan Kara
    Signed-off-by: Yufen Yu
    Signed-off-by: Jens Axboe

    Yufen Yu
     

13 Apr, 2019

3 commits

  • Drivers now report to the block layer if they support media change
    events. If this is not the case, there's no need to allocate the event
    structure, and all event handling code can effectively be skipped. This
    simplifies code flow in particular for non-removable sd devices.

    This effectively reverts commit 75e3f3ee3c64 ("block: always allocate
    genhd->ev if check_events is implemented").

    The sysfs files for the events are kept in place even if no events are
    supported, as user space may rely on them being present. The only
    difference is that an error code is now returned if the user tries to
    set poll_msecs.

    Reviewed-by: Hannes Reinecke
    Reviewed-by: Christoph Hellwig
    Signed-off-by: Martin Wilck
    Signed-off-by: Jens Axboe

    Martin Wilck
     
  • Currently, an empty disk->events field tells the block layer not to
    forward media change events to user space. This was done in commit
    7c88a168da80 ("block: don't propagate unlisted DISK_EVENTs to userland")
    in order to avoid events from "fringe" drivers to be forwarded to user
    space. By doing so, the block layer lost the information which events
    were supported by a particular block device, and most importantly,
    whether or not a given device supports media change events at all.

    Prepare for not interpreting the "events" field this way in the future
    any more. This is done by adding an additional field "event_flags" to
    struct gendisk, and two flag bits that can be set to have the device
    treated like one that had the "events" field set to a non-zero value
    before. This applies only to the sd and sr drivers, which are changed to
    set the new flags.

    The new flags are DISK_EVENT_FLAG_POLL to enforce polling of the device
    for synchronous events, and DISK_EVENT_FLAG_UEVENT to tell the
    blocklayer to generate udev events from kernel events.

    In order to add the event_flags field to struct gendisk, the events
    field is converted to an "unsigned short"; it doesn't need to hold
    values bigger than 2 anyway.

    This patch doesn't change behavior.

    Reviewed-by: Christoph Hellwig
    Signed-off-by: Martin Wilck
    Signed-off-by: Jens Axboe

    Martin Wilck
     
  • The async_events field, intended to be used for drivers that support
    asynchronous notifications about disk events (aka media change events),
    isn't currently used by any driver, and apparently that has been that
    way for a long time (if not forever). Remove it.

    Reviewed-by: Hannes Reinecke
    Reviewed-by: Christoph Hellwig
    Signed-off-by: Martin Wilck
    Signed-off-by: Jens Axboe

    Martin Wilck
     

01 Mar, 2019

2 commits


10 Dec, 2018

4 commits

  • The previous patches deleted all the code that needed the second value
    returned from part_in_flight - now the kernel only uses the first value.

    Consequently, part_in_flight (and blk_mq_in_flight) may be changed so that
    it only returns one value.

    This patch just refactors the code, there's no functional change.

    Signed-off-by: Mikulas Patocka
    Signed-off-by: Mike Snitzer
    Signed-off-by: Jens Axboe

    Mikulas Patocka
     
  • Now when part_round_stats is gone, we can switch to per-cpu in-flight
    counters.

    We use the local-atomic type local_t, so that if part_inc_in_flight or
    part_dec_in_flight is reentrantly called from an interrupt, the value will
    be correct.

    The other counters could be corrupted due to reentrant interrupt, but the
    corruption only results in slight counter skew - the in_flight counter
    must be exact, so it needs local_t.

    Signed-off-by: Mikulas Patocka
    Signed-off-by: Mike Snitzer
    Signed-off-by: Jens Axboe

    Mikulas Patocka
     
  • We want to convert to per-cpu in_flight counters.

    The function part_round_stats needs the in_flight counter every jiffy, it
    would be too costly to sum all the percpu variables every jiffy, so it
    must be deleted. part_round_stats is used to calculate two counters -
    time_in_queue and io_ticks.

    time_in_queue can be calculated without part_round_stats, by adding the
    duration of the I/O when the I/O ends (the value is almost as exact as the
    previously calculated value, except that time for in-progress I/Os is not
    counted).

    io_ticks can be approximated by increasing the value when I/O is started
    or ended and the jiffies value has changed. If the I/Os take less than a
    jiffy, the value is as exact as the previously calculated value. If the
    I/Os take more than a jiffy, io_ticks can drift behind the previously
    calculated value.

    Signed-off-by: Mikulas Patocka
    Signed-off-by: Mike Snitzer
    Signed-off-by: Jens Axboe

    Mikulas Patocka
     
  • All of part_stat_* and related methods are used with preempt disabled,
    so there is no need to pass cpu around to allow of them. Just call
    smp_processor_id() as needed.

    Suggested-by: Jens Axboe
    Signed-off-by: Mike Snitzer
    Signed-off-by: Jens Axboe

    Mike Snitzer
     

16 Nov, 2018

1 commit

  • Various spots check for q->mq_ops being non-NULL, but provide
    a helper to do this instead.

    Where the ->mq_ops != NULL check is redundant, remove it.

    Since mq == rq-based now that legacy is gone, get rid of the
    queue_is_rq_based() and just use queue_is_mq() everywhere.

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

    Jens Axboe
     

01 Oct, 2018

1 commit

  • Merge -rc6 in, for two reasons:

    1) Resolve a trivial conflict in the blk-mq-tag.c documentation
    2) A few important regression fixes went into upstream directly, so
    they aren't in the 4.20 branch.

    Signed-off-by: Jens Axboe

    * tag 'v4.19-rc6': (780 commits)
    Linux 4.19-rc6
    MAINTAINERS: fix reference to moved drivers/{misc => auxdisplay}/panel.c
    cpufreq: qcom-kryo: Fix section annotations
    perf/core: Add sanity check to deal with pinned event failure
    xen/blkfront: correct purging of persistent grants
    Revert "xen/blkfront: When purging persistent grants, keep them in the buffer"
    selftests/powerpc: Fix Makefiles for headers_install change
    blk-mq: I/O and timer unplugs are inverted in blktrace
    dax: Fix deadlock in dax_lock_mapping_entry()
    x86/boot: Fix kexec booting failure in the SEV bit detection code
    bcache: add separate workqueue for journal_write to avoid deadlock
    drm/amd/display: Fix Edid emulation for linux
    drm/amd/display: Fix Vega10 lightup on S3 resume
    drm/amdgpu: Fix vce work queue was not cancelled when suspend
    Revert "drm/panel: Add device_link from panel device to DRM device"
    xen/blkfront: When purging persistent grants, keep them in the buffer
    clocksource/drivers/timer-atmel-pit: Properly handle error cases
    block: fix deadline elevator drain for zoned block devices
    ACPI / hotplug / PCI: Don't scan for non-hotplug bridges if slot is not bridge
    drm/syncobj: Don't leak fences when WAIT_FOR_SUBMIT is set
    ...

    Signed-off-by: Jens Axboe

    Jens Axboe
     

28 Sep, 2018

1 commit

  • Update device_add_disk() to take an 'groups' argument so that
    individual drivers can register a device with additional sysfs
    attributes.
    This avoids race condition the driver would otherwise have if these
    groups were to be created with sysfs_add_groups().

    Signed-off-by: Martin Wilck
    Signed-off-by: Hannes Reinecke
    Reviewed-by: Christoph Hellwig
    Reviewed-by: Bart Van Assche
    Signed-off-by: Jens Axboe

    Hannes Reinecke
     

22 Sep, 2018

1 commit

  • Klaus Kusche reported that the I/O busy time in /proc/diskstats was not
    updating properly on 4.18. This is because we started using ktime to
    track elapsed time, and we convert nanoseconds to jiffies when we update
    the partition counter. However, this gets rounded down, so any I/Os that
    take less than a jiffy are not accounted for. Previously in this case,
    the value of jiffies would sometimes increment while we were doing I/O,
    so at least some I/Os were accounted for.

    Let's convert the stats to use nanoseconds internally. We still report
    milliseconds as before, now more accurately than ever. The value is
    still truncated to 32 bits for backwards compatibility.

    Fixes: 522a777566f5 ("block: consolidate struct request timestamp fields")
    Cc: stable@vger.kernel.org
    Reported-by: Klaus Kusche
    Signed-off-by: Omar Sandoval
    Signed-off-by: Jens Axboe

    Omar Sandoval
     

18 Jul, 2018

2 commits

  • Add tracking of REQ_OP_DISCARD ios to the partition statistics and
    append them to the various stat files in /sys as well as
    /proc/diskstats. These are tracked with the same four stats as reads
    and writes:

    Number of discard ios completed.
    Number of discard ios merged
    Number of discard sectors completed
    Milliseconds spent on discard requests

    This is done via adding a new STAT_DISCARD define to genhd.h and then
    using it to index that stat field for discard requests.

    tj: Refreshed on top of v4.17 and other previous updates.

    Signed-off-by: Michael Callahan
    Signed-off-by: Tejun Heo
    Cc: Andy Newell
    Signed-off-by: Jens Axboe

    Michael Callahan
     
  • Add defines for STAT_READ and STAT_WRITE for indexing the partition
    stat entries. This clarifies some fs/ code which has hardcoded 1 for
    STAT_WRITE and will make it easier to extend the stats with additional
    fields.

    tj: Refreshed on top of v4.17.

    Signed-off-by: Michael Callahan
    Signed-off-by: Tejun Heo
    Cc: "Theodore Ts'o"
    Cc: Jaegeuk Kim
    Signed-off-by: Jens Axboe

    Michael Callahan
     

05 Jun, 2018

1 commit

  • Pull procfs updates from Al Viro:
    "Christoph's proc_create_... cleanups series"

    * 'hch.procfs' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs: (44 commits)
    xfs, proc: hide unused xfs procfs helpers
    isdn/gigaset: add back gigaset_procinfo assignment
    proc: update SIZEOF_PDE_INLINE_NAME for the new pde fields
    tty: replace ->proc_fops with ->proc_show
    ide: replace ->proc_fops with ->proc_show
    ide: remove ide_driver_proc_write
    isdn: replace ->proc_fops with ->proc_show
    atm: switch to proc_create_seq_private
    atm: simplify procfs code
    bluetooth: switch to proc_create_seq_data
    netfilter/x_tables: switch to proc_create_seq_private
    netfilter/xt_hashlimit: switch to proc_create_{seq,single}_data
    neigh: switch to proc_create_seq_data
    hostap: switch to proc_create_{seq,single}_data
    bonding: switch to proc_create_seq_data
    rtc/proc: switch to proc_create_single_data
    drbd: switch to proc_create_single
    resource: switch to proc_create_seq_data
    staging/rtl8192u: simplify procfs code
    jfs: simplify procfs code
    ...

    Linus Torvalds
     

25 May, 2018

1 commit

  • Convert the S_ symbolic permissions to their octal equivalents as
    using octal and not symbolic permissions is preferred by many as more
    readable.

    see: https://lkml.org/lkml/2016/8/2/1945

    Done with automated conversion via:
    $ ./scripts/checkpatch.pl -f --types=SYMBOLIC_PERMS --fix-inplace

    Miscellanea:

    o Wrapped modified multi-line calls to a single line where appropriate
    o Realign modified multi-line calls to open parenthesis

    Signed-off-by: Joe Perches
    Signed-off-by: Jens Axboe

    Joe Perches
     

16 May, 2018

1 commit


26 Apr, 2018

1 commit

  • When the blk-mq inflight implementation was added, /proc/diskstats was
    converted to use it, but /sys/block/$dev/inflight was not. Fix it by
    adding another helper to count in-flight requests by data direction.

    Fixes: f299b7c7a9de ("blk-mq: provide internal in-flight variant")
    Signed-off-by: Omar Sandoval
    Signed-off-by: Jens Axboe

    Omar Sandoval
     

16 Mar, 2018

1 commit

  • register_blkdev() and __register_chrdev_region() treat the major
    number as an unsigned int. So print it the same way to avoid
    absurd error statements such as:
    "... major requested (-1) is greater than the maximum (511) ..."
    (and also fix off-by-one bugs in the error prints).

    While at it, also update the comment describing register_blkdev().

    Signed-off-by: Srivatsa S. Bhat
    Reviewed-by: Logan Gunthorpe
    Signed-off-by: Greg Kroah-Hartman

    Srivatsa S. Bhat
     

27 Feb, 2018

4 commits

  • When two blkdev_open() calls for a partition race with device removal
    and recreation, we can hit BUG_ON(!bd_may_claim(bdev, whole, holder)) in
    blkdev_open(). The race can happen as follows:

    CPU0 CPU1 CPU2
    del_gendisk()
    bdev_unhash_inode(part1);

    blkdev_open(part1, O_EXCL) blkdev_open(part1, O_EXCL)
    bdev = bd_acquire() bdev = bd_acquire()
    blkdev_get(bdev)
    bd_start_claiming(bdev)
    - finds old inode 'whole'
    bd_prepare_to_claim() -> 0
    bdev_unhash_inode(whole);


    blkdev_get(bdev);
    bd_start_claiming(bdev)
    - finds new inode 'whole'
    bd_prepare_to_claim()
    - this also succeeds as we have
    different 'whole' here...
    - bad things happen now as we
    have two exclusive openers of
    the same bdev

    The problem here is that block device opens can see various intermediate
    states while gendisk is shutting down and then being recreated.

    We fix the problem by introducing new lookup_sem in gendisk that
    synchronizes gendisk deletion with get_gendisk() and furthermore by
    making sure that get_gendisk() does not return gendisk that is being (or
    has been) deleted. This makes sure that once we ever manage to look up
    newly created bdev inode, we are also guaranteed that following
    get_gendisk() will either return failure (and we fail open) or it
    returns gendisk for the new device and following bdget_disk() will
    return new bdev inode (i.e., blkdev_open() follows the path as if it is
    completely run after new device is created).

    Reported-and-analyzed-by: Hou Tao
    Tested-by: Hou Tao
    Signed-off-by: Jan Kara
    Signed-off-by: Jens Axboe

    Jan Kara
     
  • Add a proper counterpart to get_disk_and_module() -
    put_disk_and_module(). Currently it is opencoded in several places.

    Signed-off-by: Jan Kara
    Signed-off-by: Jens Axboe

    Jan Kara
     
  • Rename get_disk() to get_disk_and_module() to make sure what the
    function does. It's not a great name but at least it is now clear that
    put_disk() is not it's counterpart.

    Signed-off-by: Jan Kara
    Signed-off-by: Jens Axboe

    Jan Kara
     
  • Commit 8ddcd653257c "block: introduce GENHD_FL_HIDDEN" added handling of
    hidden devices to get_gendisk() but forgot to drop module reference
    which is also acquired by get_disk(). Drop the reference as necessary.

    Arguably the function naming here is misleading as put_disk() is *not*
    the counterpart of get_disk() but let's fix that in the follow up
    commit since that will be more intrusive.

    Fixes: 8ddcd653257c18a669fcb75ee42c37054908e0d6
    CC: Christoph Hellwig
    Signed-off-by: Jan Kara
    Signed-off-by: Jens Axboe

    Jan Kara
     

15 Jan, 2018

2 commits

  • Since I can remember DM has forced the block layer to allow the
    allocation and initialization of the request_queue to be distinct
    operations. Reason for this is block/genhd.c:add_disk() has requires
    that the request_queue (and associated bdi) be tied to the gendisk
    before add_disk() is called -- because add_disk() also deals with
    exposing the request_queue via blk_register_queue().

    DM's dynamic creation of arbitrary device types (and associated
    request_queue types) requires the DM device's gendisk be available so
    that DM table loads can establish a master/slave relationship with
    subordinate devices that are referenced by loaded DM tables -- using
    bd_link_disk_holder(). But until these DM tables, and their associated
    subordinate devices, are known DM cannot know what type of request_queue
    it needs -- nor what its queue_limits should be.

    This chicken and egg scenario has created all manner of problems for DM
    and, at times, the block layer.

    Summary of changes:

    - Add device_add_disk_no_queue_reg() and add_disk_no_queue_reg() variant
    that drivers may use to add a disk without also calling
    blk_register_queue(). Driver must call blk_register_queue() once its
    request_queue is fully initialized.

    - Return early from blk_unregister_queue() if QUEUE_FLAG_REGISTERED
    is not set. It won't be set if driver used add_disk_no_queue_reg()
    but driver encounters an error and must del_gendisk() before calling
    blk_register_queue().

    - Export blk_register_queue().

    These changes allow DM to use add_disk_no_queue_reg() to anchor its
    gendisk as the "master" for master/slave relationships DM must establish
    with subordinate devices referenced in DM tables that get loaded. Once
    all "slave" devices for a DM device are known its request_queue can be
    properly initialized and then advertised via sysfs -- important
    improvement being that no request_queue resource initialization
    performed by blk_register_queue() is missed for DM devices anymore.

    Signed-off-by: Mike Snitzer
    Reviewed-by: Ming Lei
    Signed-off-by: Jens Axboe

    Mike Snitzer
     
  • device_add_disk() will only call bdi_register_owner() if
    !GENHD_FL_HIDDEN, so it follows that del_gendisk() should only call
    bdi_unregister() if !GENHD_FL_HIDDEN.

    Found with code inspection. bdi_unregister() won't do any harm if
    bdi_register_owner() wasn't used but best to avoid the unnecessary
    call to bdi_unregister().

    Fixes: 8ddcd65325 ("block: introduce GENHD_FL_HIDDEN")
    Signed-off-by: Mike Snitzer
    Reviewed-by: Ming Lei
    Reviewed-by: Hannes Reinecke
    Signed-off-by: Jens Axboe

    Mike Snitzer
     

20 Nov, 2017

2 commits


15 Nov, 2017

1 commit

  • Pull core block layer updates from Jens Axboe:
    "This is the main pull request for block storage for 4.15-rc1.

    Nothing out of the ordinary in here, and no API changes or anything
    like that. Just various new features for drivers, core changes, etc.
    In particular, this pull request contains:

    - A patch series from Bart, closing the whole on blk/scsi-mq queue
    quescing.

    - A series from Christoph, building towards hidden gendisks (for
    multipath) and ability to move bio chains around.

    - NVMe
    - Support for native multipath for NVMe (Christoph).
    - Userspace notifications for AENs (Keith).
    - Command side-effects support (Keith).
    - SGL support (Chaitanya Kulkarni)
    - FC fixes and improvements (James Smart)
    - Lots of fixes and tweaks (Various)

    - bcache
    - New maintainer (Michael Lyle)
    - Writeback control improvements (Michael)
    - Various fixes (Coly, Elena, Eric, Liang, et al)

    - lightnvm updates, mostly centered around the pblk interface
    (Javier, Hans, and Rakesh).

    - Removal of unused bio/bvec kmap atomic interfaces (me, Christoph)

    - Writeback series that fix the much discussed hundreds of millions
    of sync-all units. This goes all the way, as discussed previously
    (me).

    - Fix for missing wakeup on writeback timer adjustments (Yafang
    Shao).

    - Fix laptop mode on blk-mq (me).

    - {mq,name} tupple lookup for IO schedulers, allowing us to have
    alias names. This means you can use 'deadline' on both !mq and on
    mq (where it's called mq-deadline). (me).

    - blktrace race fix, oopsing on sg load (me).

    - blk-mq optimizations (me).

    - Obscure waitqueue race fix for kyber (Omar).

    - NBD fixes (Josef).

    - Disable writeback throttling by default on bfq, like we do on cfq
    (Luca Miccio).

    - Series from Ming that enable us to treat flush requests on blk-mq
    like any other request. This is a really nice cleanup.

    - Series from Ming that improves merging on blk-mq with schedulers,
    getting us closer to flipping the switch on scsi-mq again.

    - BFQ updates (Paolo).

    - blk-mq atomic flags memory ordering fixes (Peter Z).

    - Loop cgroup support (Shaohua).

    - Lots of minor fixes from lots of different folks, both for core and
    driver code"

    * 'for-4.15/block' of git://git.kernel.dk/linux-block: (294 commits)
    nvme: fix visibility of "uuid" ns attribute
    blk-mq: fixup some comment typos and lengths
    ide: ide-atapi: fix compile error with defining macro DEBUG
    blk-mq: improve tag waiting setup for non-shared tags
    brd: remove unused brd_mutex
    blk-mq: only run the hardware queue if IO is pending
    block: avoid null pointer dereference on null disk
    fs: guard_bio_eod() needs to consider partitions
    xtensa/simdisk: fix compile error
    nvme: expose subsys attribute to sysfs
    nvme: create 'slaves' and 'holders' entries for hidden controllers
    block: create 'slaves' and 'holders' entries for hidden gendisks
    nvme: also expose the namespace identification sysfs files for mpath nodes
    nvme: implement multipath access to nvme subsystems
    nvme: track shared namespaces
    nvme: introduce a nvme_ns_ids structure
    nvme: track subsystems
    block, nvme: Introduce blk_mq_req_flags_t
    block, scsi: Make SCSI quiesce and resume work reliably
    block: Add the QUEUE_FLAG_PREEMPT_ONLY request queue flag
    ...

    Linus Torvalds
     

11 Nov, 2017

2 commits

  • It is possible that the pointer disk can be null and hence
    we can get a null pointer deference when accessing disk->flags.
    Add a null pointer check to avoid the dereference.

    Detected by CoverityScan, CID#1461133 ("Explicit null dereferenced")

    Fixes: 8ddcd653257c ("block: introduce GENHD_FL_HIDDEN")
    Reviewed-by: Christoph Hellwig
    Signed-off-by: Colin Ian King
    Signed-off-by: Jens Axboe

    Colin Ian King
     
  • When creating nvme multipath devices we should populate the 'slaves' and
    'holders' directorys properly to aid userspace topology detection.

    Signed-off-by: Hannes Reinecke
    [hch: split from a larger patch]
    Reviewed-by: Keith Busch
    Reviewed-by: Martin K. Petersen
    Signed-off-by: Christoph Hellwig
    Signed-off-by: Jens Axboe

    Hannes Reinecke
     

04 Nov, 2017

2 commits

  • With this flag a driver can create a gendisk that can be used for I/O
    submission inside the kernel, but which is not registered as user
    facing block device. This will be useful for the NVMe multipath
    implementation.

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

    Christoph Hellwig
     
  • The hidden gendisks introduced in the next patch need to keep the dev
    field in their struct device empty so that udev won't try to create
    block device nodes for them. To support that rewrite disk_devt to
    look at the major and first_minor fields in the gendisk itself instead
    of looking into the struct device.

    Signed-off-by: Christoph Hellwig
    Reviewed-by: Johannes Thumshirn
    Reviewed-by: Hannes Reinecke
    Signed-off-by: Jens Axboe

    Christoph Hellwig
     

26 Oct, 2017

1 commit

  • Darrick posted the following warning and Dave Chinner analyzed it:

    > ======================================================
    > WARNING: possible circular locking dependency detected
    > 4.14.0-rc1-fixes #1 Tainted: G W
    > ------------------------------------------------------
    > loop0/31693 is trying to acquire lock:
    > (&(&ip->i_mmaplock)->mr_lock){++++}, at: [] xfs_ilock+0x23c/0x330 [xfs]
    >
    > but now in release context of a crosslock acquired at the following:
    > ((complete)&ret.event){+.+.}, at: [] submit_bio_wait+0x7f/0xb0
    >
    > which lock already depends on the new lock.
    >
    > the existing dependency chain (in reverse order) is:
    >
    > -> #2 ((complete)&ret.event){+.+.}:
    > lock_acquire+0xab/0x200
    > wait_for_completion_io+0x4e/0x1a0
    > submit_bio_wait+0x7f/0xb0
    > blkdev_issue_zeroout+0x71/0xa0
    > xfs_bmapi_convert_unwritten+0x11f/0x1d0 [xfs]
    > xfs_bmapi_write+0x374/0x11f0 [xfs]
    > xfs_iomap_write_direct+0x2ac/0x430 [xfs]
    > xfs_file_iomap_begin+0x20d/0xd50 [xfs]
    > iomap_apply+0x43/0xe0
    > dax_iomap_rw+0x89/0xf0
    > xfs_file_dax_write+0xcc/0x220 [xfs]
    > xfs_file_write_iter+0xf0/0x130 [xfs]
    > __vfs_write+0xd9/0x150
    > vfs_write+0xc8/0x1c0
    > SyS_write+0x45/0xa0
    > entry_SYSCALL_64_fastpath+0x1f/0xbe
    >
    > -> #1 (&xfs_nondir_ilock_class){++++}:
    > lock_acquire+0xab/0x200
    > down_write_nested+0x4a/0xb0
    > xfs_ilock+0x263/0x330 [xfs]
    > xfs_setattr_size+0x152/0x370 [xfs]
    > xfs_vn_setattr+0x6b/0x90 [xfs]
    > notify_change+0x27d/0x3f0
    > do_truncate+0x5b/0x90
    > path_openat+0x237/0xa90
    > do_filp_open+0x8a/0xf0
    > do_sys_open+0x11c/0x1f0
    > entry_SYSCALL_64_fastpath+0x1f/0xbe
    >
    > -> #0 (&(&ip->i_mmaplock)->mr_lock){++++}:
    > up_write+0x1c/0x40
    > xfs_iunlock+0x1d0/0x310 [xfs]
    > xfs_file_fallocate+0x8a/0x310 [xfs]
    > loop_queue_work+0xb7/0x8d0
    > kthread_worker_fn+0xb9/0x1f0
    >
    > Chain exists of:
    > &(&ip->i_mmaplock)->mr_lock --> &xfs_nondir_ilock_class --> (complete)&ret.event
    >
    > Possible unsafe locking scenario by crosslock:
    >
    > CPU0 CPU1
    > ---- ----
    > lock(&xfs_nondir_ilock_class);
    > lock((complete)&ret.event);
    > lock(&(&ip->i_mmaplock)->mr_lock);
    > unlock((complete)&ret.event);
    >
    > *** DEADLOCK ***

    The warning is a false positive, caused by the fact that all
    wait_for_completion()s in submit_bio_wait() are waiting with the same
    lock class.

    However, some bios have nothing to do with others, for example in the case
    of loop devices, there's no direct connection between the bios of an upper
    device and the bios of a lower device(=loop device).

    The safest way to assign different lock classes to different devices is
    to do it for each gendisk. In other words, this patch assigns a
    lockdep_map per gendisk and uses it when initializing completion in
    submit_bio_wait().

    Analyzed-by: Dave Chinner
    Reported-by: Darrick J. Wong
    Signed-off-by: Byungchul Park
    Reviewed-by: Jens Axboe
    Cc: Linus Torvalds
    Cc: Peter Zijlstra
    Cc: Thomas Gleixner
    Cc: amir73il@gmail.com
    Cc: axboe@kernel.dk
    Cc: david@fromorbit.com
    Cc: hch@infradead.org
    Cc: idryomov@gmail.com
    Cc: johan@kernel.org
    Cc: johannes.berg@intel.com
    Cc: kernel-team@lge.com
    Cc: linux-block@vger.kernel.org
    Cc: linux-fsdevel@vger.kernel.org
    Cc: linux-mm@kvack.org
    Cc: linux-xfs@vger.kernel.org
    Cc: oleg@redhat.com
    Cc: tj@kernel.org
    Link: http://lkml.kernel.org/r/1508921765-15396-10-git-send-email-byungchul.park@lge.com
    Signed-off-by: Ingo Molnar

    Byungchul Park