1. 19 Mar, 2018 1 commit
    • Coly Li's avatar
      bcache: fix cached_dev->count usage for bch_cache_set_error() · 804f3c69
      Coly Li authored
      When bcache metadata I/O fails, bcache will call bch_cache_set_error()
      to retire the whole cache set. The expected behavior to retire a cache
      set is to unregister the cache set, and unregister all backing device
      attached to this cache set, then remove sysfs entries of the cache set
      and all attached backing devices, finally release memory of structs
      cache_set, cache, cached_dev and bcache_device.
      
      In my testing when journal I/O failure triggered by disconnected cache
      device, sometimes the cache set cannot be retired, and its sysfs
      entry /sys/fs/bcache/<uuid> still exits and the backing device also
      references it. This is not expected behavior.
      
      When metadata I/O failes, the call senquence to retire whole cache set is,
              bch_cache_set_error()
              bch_cache_set_unregister()
              bch_cache_set_stop()
              __cache_set_unregister()     <- called as callback by calling
                                              clousre_queue(&c->caching)
              cache_set_flush()            <- called as a callback when refcount
                                              of cache_set->caching is 0
              cache_set_free()             <- called as a callback when refcount
                                              of catch_set->cl is 0
              bch_cache_set_release()      <- called as a callback when refcount
                                              of catch_set->kobj is 0
      
      I find if kernel thread bch_writeback_thread() quits while-loop when
      kthread_should_stop() is true and searched_full_index is false, clousre
      callback cache_set_flush() set by continue_at() will never be called. The
      result is, bcache fails to retire whole cache set.
      
      cache_set_flush() will be called when refcount of closure c->caching is 0,
      and in function bcache_device_detach() refcount of closure c->caching is
      released to 0 by clousre_put(). In metadata error code path, function
      bcache_device_detach() is called by cached_dev_detach_finish(). This is a
      callback routine being called when cached_dev->count is 0. This refcount
      is decreased by cached_dev_put().
      
      The above dependence indicates, cache_set_flush() will be called when
      refcount of cache_set->cl is 0, and refcount of cache_set->cl to be 0
      when refcount of cache_dev->count is 0.
      
      The reason why sometimes cache_dev->count is not 0 (when metadata I/O fails
      and bch_cache_set_error() called) is, in bch_writeback_thread(), refcount
      of cache_dev is not decreased properly.
      
      In bch_writeback_thread(), cached_dev_put() is called only when
      searched_full_index is true and cached_dev->writeback_keys is empty, a.k.a
      there is no dirty data on cache. In most of run time it is correct, but
      when bch_writeback_thread() quits the while-loop while cache is still
      dirty, current code forget to call cached_dev_put() before this kernel
      thread exits. This is why sometimes cache_set_flush() is not executed and
      cache set fails to be retired.
      
      The reason to call cached_dev_put() in bch_writeback_rate() is, when the
      cache device changes from clean to dirty, cached_dev_get() is called, to
      make sure during writeback operatiions both backing and cache devices
      won't be released.
      
      Adding following code in bch_writeback_thread() does not work,
         static int bch_writeback_thread(void *arg)
              }
      
      +       if (atomic_read(&dc->has_dirty))
      +               cached_dev_put()
      +
              return 0;
       }
      because writeback kernel thread can be waken up and start via sysfs entry:
              echo 1 > /sys/block/bcache<N>/bcache/writeback_running
      It is difficult to check whether backing device is dirty without race and
      extra lock. So the above modification will introduce potential refcount
      underflow in some conditions.
      
      The correct fix is, to take cached dev refcount when creating the kernel
      thread, and put it before the kernel thread exits. Then bcache does not
      need to take a cached dev refcount when cache turns from clean to dirty,
      or to put a cached dev refcount when cache turns from ditry to clean. The
      writeback kernel thread is alwasy safe to reference data structure from
      cache set, cache and cached device (because a refcount of cache device is
      taken for it already), and no matter the kernel thread is stopped by I/O
      errors or system reboot, cached_dev->count can always be used correctly.
      
      The patch is simple, but understanding how it works is quite complicated.
      
      Changelog:
      v2: set dc->writeback_thread to NULL in this patch, as suggested by Hannes.
      v1: initial version for review.
      Signed-off-by: default avatarColy Li <colyli@suse.de>
      Reviewed-by: default avatarHannes Reinecke <hare@suse.com>
      Reviewed-by: default avatarMichael Lyle <mlyle@lyle.org>
      Cc: Michael Lyle <mlyle@lyle.org>
      Cc: Junhui Tang <tang.junhui@zte.com.cn>
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      804f3c69
  2. 17 Mar, 2018 3 commits
  3. 16 Mar, 2018 2 commits
    • Joseph Qi's avatar
      blk-throttle: fix race between blkcg_bio_issue_check() and cgroup_rmdir() · 4c699480
      Joseph Qi authored
      We've triggered a WARNING in blk_throtl_bio() when throttling writeback
      io, which complains blkg->refcnt is already 0 when calling blkg_get(),
      and then kernel crashes with invalid page request.
      After investigating this issue, we've found it is caused by a race
      between blkcg_bio_issue_check() and cgroup_rmdir(), which is described
      below:
      
      writeback kworker               cgroup_rmdir
                                        cgroup_destroy_locked
                                          kill_css
                                            css_killed_ref_fn
                                              css_killed_work_fn
                                                offline_css
                                                  blkcg_css_offline
        blkcg_bio_issue_check
          rcu_read_lock
          blkg_lookup
                                                    spin_trylock(q->queue_lock)
                                                    blkg_destroy
                                                    spin_unlock(q->queue_lock)
          blk_throtl_bio
          spin_lock_irq(q->queue_lock)
          ...
          spin_unlock_irq(q->queue_lock)
        rcu_read_unlock
      
      Since rcu can only prevent blkg from releasing when it is being used,
      the blkg->refcnt can be decreased to 0 during blkg_destroy() and schedule
      blkg release.
      Then trying to blkg_get() in blk_throtl_bio() will complains the WARNING.
      And then the corresponding blkg_put() will schedule blkg release again,
      which result in double free.
      This race is introduced by commit ae118896 ("blkcg: consolidate blkg
      creation in blkcg_bio_issue_check()"). Before this commit, it will
      lookup first and then try to lookup/create again with queue_lock. Since
      revive this logic is a bit drastic, so fix it by only offlining pd during
      blkcg_css_offline(), and move the rest destruction (especially
      blkg_put()) into blkcg_css_free(), which should be the right way as
      discussed.
      
      Fixes: ae118896 ("blkcg: consolidate blkg creation in blkcg_bio_issue_check()")
      Reported-by: default avatarJiufei Xue <jiufei.xue@linux.alibaba.com>
      Signed-off-by: default avatarJoseph Qi <joseph.qi@linux.alibaba.com>
      Acked-by: default avatarTejun Heo <tj@kernel.org>
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      4c699480
    • Jonas Rabenstein's avatar
      block: sed-opal: fix u64 short atom length · 5f990d31
      Jonas Rabenstein authored
      The length must be given as bytes and not as 4 bit tuples.
      Reviewed-by: default avatarScott Bauer <scott.bauer@intel.com>
      Signed-off-by: default avatarJonas Rabenstein <jonas.rabenstein@studium.uni-erlangen.de>
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      5f990d31
  4. 13 Mar, 2018 3 commits
  5. 12 Mar, 2018 2 commits
  6. 09 Mar, 2018 5 commits
    • Ross Zwisler's avatar
      MAINTAINERS: add coverage for drivers/block · fc9de9a5
      Ross Zwisler authored
      To help folks like me that use scripts/get_maintainer.pl.
      Signed-off-by: default avatarRoss Zwisler <ross.zwisler@linux.intel.com>
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      fc9de9a5
    • Bart Van Assche's avatar
      block: Suppress kernel-doc warnings triggered by blk-zoned.c · 56c4bddb
      Bart Van Assche authored
      Avoid that building with W=1 causes the kernel-doc tool to complain
      about undocumented function arguments for the blk-zoned.c source file.
      Signed-off-by: default avatarBart Van Assche <bart.vanassche@wdc.com>
      Cc: Christoph Hellwig <hch@lst.de>
      Cc: Damien Le Moal <damien.lemoal@wdc.com>
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      56c4bddb
    • Maurizio Lombardi's avatar
      cdrom: do not call check_disk_change() inside cdrom_open() · 2bbea6e1
      Maurizio Lombardi authored
      when mounting an ISO filesystem sometimes (very rarely)
      the system hangs because of a race condition between two tasks.
      
      PID: 6766   TASK: ffff88007b2a6dd0  CPU: 0   COMMAND: "mount"
       #0 [ffff880078447ae0] __schedule at ffffffff8168d605
       #1 [ffff880078447b48] schedule_preempt_disabled at ffffffff8168ed49
       #2 [ffff880078447b58] __mutex_lock_slowpath at ffffffff8168c995
       #3 [ffff880078447bb8] mutex_lock at ffffffff8168bdef
       #4 [ffff880078447bd0] sr_block_ioctl at ffffffffa00b6818 [sr_mod]
       #5 [ffff880078447c10] blkdev_ioctl at ffffffff812fea50
       #6 [ffff880078447c70] ioctl_by_bdev at ffffffff8123a8b3
       #7 [ffff880078447c90] isofs_fill_super at ffffffffa04fb1e1 [isofs]
       #8 [ffff880078447da8] mount_bdev at ffffffff81202570
       #9 [ffff880078447e18] isofs_mount at ffffffffa04f9828 [isofs]
      #10 [ffff880078447e28] mount_fs at ffffffff81202d09
      #11 [ffff880078447e70] vfs_kern_mount at ffffffff8121ea8f
      #12 [ffff880078447ea8] do_mount at ffffffff81220fee
      #13 [ffff880078447f28] sys_mount at ffffffff812218d6
      #14 [ffff880078447f80] system_call_fastpath at ffffffff81698c49
          RIP: 00007fd9ea914e9a  RSP: 00007ffd5d9bf648  RFLAGS: 00010246
          RAX: 00000000000000a5  RBX: ffffffff81698c49  RCX: 0000000000000010
          RDX: 00007fd9ec2bc210  RSI: 00007fd9ec2bc290  RDI: 00007fd9ec2bcf30
          RBP: 0000000000000000   R8: 0000000000000000   R9: 0000000000000010
          R10: 00000000c0ed0001  R11: 0000000000000206  R12: 00007fd9ec2bc040
          R13: 00007fd9eb6b2380  R14: 00007fd9ec2bc210  R15: 00007fd9ec2bcf30
          ORIG_RAX: 00000000000000a5  CS: 0033  SS: 002b
      
      This task was trying to mount the cdrom.  It allocated and configured a
      super_block struct and owned the write-lock for the super_block->s_umount
      rwsem. While exclusively owning the s_umount lock, it called
      sr_block_ioctl and waited to acquire the global sr_mutex lock.
      
      PID: 6785   TASK: ffff880078720fb0  CPU: 0   COMMAND: "systemd-udevd"
       #0 [ffff880078417898] __schedule at ffffffff8168d605
       #1 [ffff880078417900] schedule at ffffffff8168dc59
       #2 [ffff880078417910] rwsem_down_read_failed at ffffffff8168f605
       #3 [ffff880078417980] call_rwsem_down_read_failed at ffffffff81328838
       #4 [ffff8800784179d0] down_read at ffffffff8168cde0
       #5 [ffff8800784179e8] get_super at ffffffff81201cc7
       #6 [ffff880078417a10] __invalidate_device at ffffffff8123a8de
       #7 [ffff880078417a40] flush_disk at ffffffff8123a94b
       #8 [ffff880078417a88] check_disk_change at ffffffff8123ab50
       #9 [ffff880078417ab0] cdrom_open at ffffffffa00a29e1 [cdrom]
      #10 [ffff880078417b68] sr_block_open at ffffffffa00b6f9b [sr_mod]
      #11 [ffff880078417b98] __blkdev_get at ffffffff8123ba86
      #12 [ffff880078417bf0] blkdev_get at ffffffff8123bd65
      #13 [ffff880078417c78] blkdev_open at ffffffff8123bf9b
      #14 [ffff880078417c90] do_dentry_open at ffffffff811fc7f7
      #15 [ffff880078417cd8] vfs_open at ffffffff811fc9cf
      #16 [ffff880078417d00] do_last at ffffffff8120d53d
      #17 [ffff880078417db0] path_openat at ffffffff8120e6b2
      #18 [ffff880078417e48] do_filp_open at ffffffff8121082b
      #19 [ffff880078417f18] do_sys_open at ffffffff811fdd33
      #20 [ffff880078417f70] sys_open at ffffffff811fde4e
      #21 [ffff880078417f80] system_call_fastpath at ffffffff81698c49
          RIP: 00007f29438b0c20  RSP: 00007ffc76624b78  RFLAGS: 00010246
          RAX: 0000000000000002  RBX: ffffffff81698c49  RCX: 0000000000000000
          RDX: 00007f2944a5fa70  RSI: 00000000000a0800  RDI: 00007f2944a5fa70
          RBP: 00007f2944a5f540   R8: 0000000000000000   R9: 0000000000000020
          R10: 00007f2943614c40  R11: 0000000000000246  R12: ffffffff811fde4e
          R13: ffff880078417f78  R14: 000000000000000c  R15: 00007f2944a4b010
          ORIG_RAX: 0000000000000002  CS: 0033  SS: 002b
      
      This task tried to open the cdrom device, the sr_block_open function
      acquired the global sr_mutex lock. The call to check_disk_change()
      then saw an event flag indicating a possible media change and tried
      to flush any cached data for the device.
      As part of the flush, it tried to acquire the super_block->s_umount
      lock associated with the cdrom device.
      This was the same super_block as created and locked by the previous task.
      
      The first task acquires the s_umount lock and then the sr_mutex_lock;
      the second task acquires the sr_mutex_lock and then the s_umount lock.
      
      This patch fixes the issue by moving check_disk_change() out of
      cdrom_open() and let the caller take care of it.
      Signed-off-by: default avatarMaurizio Lombardi <mlombard@redhat.com>
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      2bbea6e1
    • Randy Dunlap's avatar
      Documentation/cdrom: fix German sharp s in LaTex · da5ff37c
      Randy Dunlap authored
      Apparently the LaTex abbreviation for the German "sharp s" (ß)
      (Unicode U+00DF) has changed from {\sz} to {\ss}.  With {\sz},
      I get this error at line 1016 (line number after another patch):
      
      ! Undefined control sequence.
      l.1016 ...nel~2.0.  Further thanks to Heiko Ei{\sz
                                                        }feldt,
      
      This is fixed by changing the {\sz} to {\ss}.
      Signed-off-by: default avatarRandy Dunlap <rdunlap@infradead.org>
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      da5ff37c
    • Randy Dunlap's avatar
      Documentation/cdrom: update cdrom-standard.tex for kernel changes · 0a703c1f
      Randy Dunlap authored
      Documentation updates for Documentation/cdrom/cdrom-standard.tex:
      
      cdrom_device_ops:
      - add check_events() and generic_packet()
      
      cdrom_device_info:
      - add one 'const' modifier
      - correct some field descriptions
      - add some missing fields
      - drop 'kdev_t dev;' field
      
      Also drop <n_discs> sentence from documentation because it is not
      referenced anywhere in the kernel header or C files.
      Signed-off-by: default avatarRandy Dunlap <rdunlap@infradead.org>
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      0a703c1f
  7. 08 Mar, 2018 11 commits
  8. 07 Mar, 2018 1 commit
  9. 06 Mar, 2018 1 commit
  10. 01 Mar, 2018 2 commits
    • Arnd Bergmann's avatar
      staging: rts5208: rename SG_END macro · b5d013bc
      Arnd Bergmann authored
      A change to the generic scatterlist code caused a conflict with
      the rtsx card reader driver:
      
      In file included from drivers/staging/rts5208/rtsx.h:180,
                       from drivers/staging/rts5208/rtsx.c:28:
      drivers/staging/rts5208/rtsx_chip.h:343: error: "SG_END" redefined [-Werror]
      
      This changes one instance of the driver to prefix SG_END and
      related constants.
      
      Fixes: 723fbf56 ("lib/scatterlist: Add SG_CHAIN and SG_END macros for LSB encodings")
      Cc: Anshuman Khandual <khandual@linux.vnet.ibm.com>
      Reviewed-by: default avatarAndy Shevchenko <andy.shevchenko@gmail.com>
      Signed-off-by: default avatarArnd Bergmann <arnd@arndb.de>
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      b5d013bc
    • Arnd Bergmann's avatar
      misc: rtsx: rename SG_END macro · f16ee7c7
      Arnd Bergmann authored
      A change to the generic scatterlist code caused a conflict with
      the rtsx card reader driver:
      
      In file included from drivers/misc/cardreader/rtsx_pcr.c:32:
      include/linux/rtsx_pci.h:40: error: "SG_END" redefined [-Werror]
      
      This changes one instance of the driver to prefix SG_END and
      related constants.
      
      Fixes: 723fbf56 ("lib/scatterlist: Add SG_CHAIN and SG_END macros for LSB encodings")
      Cc: Anshuman Khandual <khandual@linux.vnet.ibm.com>
      Signed-off-by: default avatarArnd Bergmann <arnd@arndb.de>
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      f16ee7c7
  11. 28 Feb, 2018 9 commits