1. 30 Apr, 2019 4 commits
  2. 26 Apr, 2019 1 commit
    • Jens Axboe's avatar
      Merge branch 'nvme-5.2' of git://git.infradead.org/nvme into for-5.2/block · 41d7f2ed
      Jens Axboe authored
      Pull NVMe changes from Christoph.
      
      * 'nvme-5.2' of git://git.infradead.org/nvme:
        nvme: set 0 capacity if namespace block size exceeds PAGE_SIZE
        nvme-rdma: fix typo in struct comment
        nvme-loop: kill timeout handler
        nvme-tcp: rename function to have nvme_tcp prefix
        nvme-rdma: fix a NULL deref when an admin connect times out
        nvme-tcp: fix a NULL deref when an admin connect times out
        nvmet-tcp: don't fail maxr2t greater than 1
        nvmet-file: clamp-down file namespace lba_shift
        nvmet: include <linux/scatterlist.h>
        nvmet: return a specified error it subsys_alloc fails
        nvmet: rename nvme_completion instances from rsp to cqe
        nvmet-rdma: remove p2p_client initialization from fast-path
      41d7f2ed
  3. 25 Apr, 2019 13 commits
  4. 24 Apr, 2019 19 commits
    • Shenghui Wang's avatar
      bcache: avoid potential memleak of list of journal_replay(s) in the CACHE_SYNC... · 95f18c9d
      Shenghui Wang authored
      bcache: avoid potential memleak of list of journal_replay(s) in the CACHE_SYNC branch of run_cache_set
      
      In the CACHE_SYNC branch of run_cache_set(), LIST_HEAD(journal) is used
      to collect journal_replay(s) and filled by bch_journal_read().
      
      If all goes well, bch_journal_replay() will release the list of
      jounal_replay(s) at the end of the branch.
      
      If something goes wrong, code flow will jump to the label "err:" and leave
      the list unreleased.
      
      This patch will release the list of journal_replay(s) in the case of
      error detected.
      
      v1 -> v2:
      * Move the release code to the location after label 'err:' to
        simply the change.
      Signed-off-by: default avatarShenghui Wang <shhuiw@foxmail.com>
      Signed-off-by: default avatarColy Li <colyli@suse.de>
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      95f18c9d
    • Shenghui Wang's avatar
      bcache: fix wrong usage use-after-freed on keylist in out_nocoalesce branch of btree_gc_coalesce · f16277ca
      Shenghui Wang authored
      Elements of keylist should be accessed before the list is freed.
      Move bch_keylist_free() calling after the while loop to avoid wrong
      content accessed.
      Signed-off-by: default avatarShenghui Wang <shhuiw@foxmail.com>
      Signed-off-by: default avatarColy Li <colyli@suse.de>
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      f16277ca
    • Tang Junhui's avatar
      bcache: fix failure in journal relplay · 63120731
      Tang Junhui authored
      journal replay failed with messages:
      Sep 10 19:10:43 ceph kernel: bcache: error on
      bb379a64-e44e-4812-b91d-a5599871a3b1: bcache: journal entries
      2057493-2057567 missing! (replaying 2057493-20766016), disabling
      caching
      
      The reason is in journal_reclaim(), when discard is enabled, we send
      discard command and reclaim those journal buckets whose seq is old
      than the last_seq_now, but before we write a journal with last_seq_now,
      the machine is restarted, so the journal with the last_seq_now is not
      written to the journal bucket, and the last_seq_wrote in the newest
      journal is old than last_seq_now which we expect to be, so when we doing
      replay, journals from last_seq_wrote to last_seq_now are missing.
      
      It's hard to write a journal immediately after journal_reclaim(),
      and it harmless if those missed journal are caused by discarding
      since those journals are already wrote to btree node. So, if miss
      seqs are started from the beginning journal, we treat it as normal,
      and only print a message to show the miss journal, and point out
      it maybe caused by discarding.
      
      Patch v2 add a judgement condition to ignore the missed journal
      only when discard enabled as Coly suggested.
      
      (Coly Li: rebase the patch with other changes in bch_journal_replay())
      Signed-off-by: default avatarTang Junhui <tang.junhui.linux@gmail.com>
      Tested-by: default avatarDennis Schridde <devurandom@gmx.net>
      Signed-off-by: default avatarColy Li <colyli@suse.de>
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      63120731
    • Coly Li's avatar
      bcache: improve bcache_reboot() · eb8cbb6d
      Coly Li authored
      This patch tries to release mutex bch_register_lock early, to give
      chance to stop cache set and bcache device early.
      
      This patch also expends time out of stopping all bcache device from
      2 seconds to 10 seconds, because stopping writeback rate update worker
      may delay for 5 seconds, 2 seconds is not enough.
      
      After this patch applied, stopping bcache devices during system reboot
      or shutdown is very hard to be observed any more.
      Signed-off-by: default avatarColy Li <colyli@suse.de>
      Reviewed-by: default avatarHannes Reinecke <hare@suse.com>
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      eb8cbb6d
    • Coly Li's avatar
      bcache: add comments for closure_fn to be called in closure_queue() · 63d63b51
      Coly Li authored
      Add code comments to explain which call back function might be called
      for the closure_queue(). This is an effort to make code to be more
      understandable for readers.
      Signed-off-by: default avatarColy Li <colyli@suse.de>
      Reviewed-by: default avatarChaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
      Reviewed-by: default avatarHannes Reinecke <hare@suse.com>
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      63d63b51
    • Coly Li's avatar
      bcache: Add comments for blkdev_put() in registration code path · bb6d355c
      Coly Li authored
      Add comments to explain why in register_bcache() blkdev_put() won't
      be called in two location. Add comments to explain why blkdev_put()
      must be called in register_cache() when cache_alloc() failed.
      Signed-off-by: default avatarColy Li <colyli@suse.de>
      Reviewed-by: default avatarChaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
      Reviewed-by: default avatarHannes Reinecke <hare@suse.com>
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      bb6d355c
    • Coly Li's avatar
      bcache: add error check for calling register_bdev() · 88c12d42
      Coly Li authored
      This patch adds return value to register_bdev(). Then if failure happens
      inside register_bdev(), its caller register_bcache() may detect and
      handle the failure more properly.
      Signed-off-by: default avatarColy Li <colyli@suse.de>
      Reviewed-by: default avatarHannes Reinecke <hare@suse.com>
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      88c12d42
    • Coly Li's avatar
      bcache: return error immediately in bch_journal_replay() · 68d10e69
      Coly Li authored
      When failure happens inside bch_journal_replay(), calling
      cache_set_err_on() and handling the failure in async way is not a good
      idea. Because after bch_journal_replay() returns, registering code will
      continue to execute following steps, and unregistering code triggered
      by cache_set_err_on() is running in same time. First it is unnecessary
      to handle failure and unregister cache set in an async way, second there
      might be potential race condition to run register and unregister code
      for same cache set.
      
      So in this patch, if failure happens in bch_journal_replay(), we don't
      call cache_set_err_on(), and just print out the same error message to
      kernel message buffer, then return -EIO immediately caller. Then caller
      can detect such failure and handle it in synchrnozied way.
      Signed-off-by: default avatarColy Li <colyli@suse.de>
      Reviewed-by: default avatarHannes Reinecke <hare@suse.com>
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      68d10e69
    • Coly Li's avatar
      bcache: add comments for kobj release callback routine · 2d17456e
      Coly Li authored
      Bcache has several routines to release resources in implicit way, they
      are called when the associated kobj released. This patch adds code
      comments to notice when and which release callback will be called,
      - When dc->disk.kobj released:
        void bch_cached_dev_release(struct kobject *kobj)
      - When d->kobj released:
        void bch_flash_dev_release(struct kobject *kobj)
      - When c->kobj released:
        void bch_cache_set_release(struct kobject *kobj)
      - When ca->kobj released
        void bch_cache_release(struct kobject *kobj)
      Signed-off-by: default avatarColy Li <colyli@suse.de>
      Reviewed-by: default avatarChaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
      Reviewed-by: default avatarHannes Reinecke <hare@suse.com>
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      2d17456e
    • Coly Li's avatar
      bcache: add failure check to run_cache_set() for journal replay · ce3e4cfb
      Coly Li authored
      Currently run_cache_set() has no return value, if there is failure in
      bch_journal_replay(), the caller of run_cache_set() has no idea about
      such failure and just continue to execute following code after
      run_cache_set().  The internal failure is triggered inside
      bch_journal_replay() and being handled in async way. This behavior is
      inefficient, while failure handling inside bch_journal_replay(), cache
      register code is still running to start the cache set. Registering and
      unregistering code running as same time may introduce some rare race
      condition, and make the code to be more hard to be understood.
      
      This patch adds return value to run_cache_set(), and returns -EIO if
      bch_journal_rreplay() fails. Then caller of run_cache_set() may detect
      such failure and stop registering code flow immedidately inside
      register_cache_set().
      
      If journal replay fails, run_cache_set() can report error immediately
      to register_cache_set(). This patch makes the failure handling for
      bch_journal_replay() be in synchronized way, easier to understand and
      debug, and avoid poetential race condition for register-and-unregister
      in same time.
      Signed-off-by: default avatarColy Li <colyli@suse.de>
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      ce3e4cfb
    • Coly Li's avatar
      bcache: never set KEY_PTRS of journal key to 0 in journal_reclaim() · 1bee2add
      Coly Li authored
      In journal_reclaim() ja->cur_idx of each cache will be update to
      reclaim available journal buckets. Variable 'int n' is used to count how
      many cache is successfully reclaimed, then n is set to c->journal.key
      by SET_KEY_PTRS(). Later in journal_write_unlocked(), a for_each_cache()
      loop will write the jset data onto each cache.
      
      The problem is, if all jouranl buckets on each cache is full, the
      following code in journal_reclaim(),
      
      529 for_each_cache(ca, c, iter) {
      530       struct journal_device *ja = &ca->journal;
      531       unsigned int next = (ja->cur_idx + 1) % ca->sb.njournal_buckets;
      532
      533       /* No space available on this device */
      534       if (next == ja->discard_idx)
      535               continue;
      536
      537       ja->cur_idx = next;
      538       k->ptr[n++] = MAKE_PTR(0,
      539                         bucket_to_sector(c, ca->sb.d[ja->cur_idx]),
      540                         ca->sb.nr_this_dev);
      541 }
      542
      543 bkey_init(k);
      544 SET_KEY_PTRS(k, n);
      
      If there is no available bucket to reclaim, the if() condition at line
      534 will always true, and n remains 0. Then at line 544, SET_KEY_PTRS()
      will set KEY_PTRS field of c->journal.key to 0.
      
      Setting KEY_PTRS field of c->journal.key to 0 is wrong. Because in
      journal_write_unlocked() the journal data is written in following loop,
      
      649	for (i = 0; i < KEY_PTRS(k); i++) {
      650-671		submit journal data to cache device
      672	}
      
      If KEY_PTRS field is set to 0 in jouranl_reclaim(), the journal data
      won't be written to cache device here. If system crahed or rebooted
      before bkeys of the lost journal entries written into btree nodes, data
      corruption will be reported during bcache reload after rebooting the
      system.
      
      Indeed there is only one cache in a cache set, there is no need to set
      KEY_PTRS field in journal_reclaim() at all. But in order to keep the
      for_each_cache() logic consistent for now, this patch fixes the above
      problem by not setting 0 KEY_PTRS of journal key, if there is no bucket
      available to reclaim.
      Signed-off-by: default avatarColy Li <colyli@suse.de>
      Reviewed-by: default avatarHannes Reinecke <hare@suse.com>
      Cc: stable@vger.kernel.org
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      1bee2add
    • Coly Li's avatar
      bcache: move definition of 'int ret' out of macro read_bucket() · 14215ee0
      Coly Li authored
      'int ret' is defined as a local variable inside macro read_bucket().
      Since this macro is called multiple times, and following patches will
      use a 'int ret' variable in bch_journal_read(), this patch moves
      definition of 'int ret' from macro read_bucket() to range of function
      bch_journal_read().
      Signed-off-by: default avatarColy Li <colyli@suse.de>
      Reviewed-by: default avatarHannes Reinecke <hare@suse.com>
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      14215ee0
    • Liang Chen's avatar
      bcache: fix a race between cache register and cacheset unregister · a4b732a2
      Liang Chen authored
      There is a race between cache device register and cache set unregister.
      For an already registered cache device, register_bcache will call
      bch_is_open to iterate through all cachesets and check every cache
      there. The race occurs if cache_set_free executes at the same time and
      clears the caches right before ca is dereferenced in bch_is_open_cache.
      To close the race, let's make sure the clean up work is protected by
      the bch_register_lock as well.
      
      This issue can be reproduced as follows,
      while true; do echo /dev/XXX> /sys/fs/bcache/register ; done&
      while true; do echo 1> /sys/block/XXX/bcache/set/unregister ; done &
      
      and results in the following oops,
      
      [  +0.000053] BUG: unable to handle kernel NULL pointer dereference at 0000000000000998
      [  +0.000457] #PF error: [normal kernel read fault]
      [  +0.000464] PGD 800000003ca9d067 P4D 800000003ca9d067 PUD 3ca9c067 PMD 0
      [  +0.000388] Oops: 0000 [#1] SMP PTI
      [  +0.000269] CPU: 1 PID: 3266 Comm: bash Not tainted 5.0.0+ #6
      [  +0.000346] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-2.fc28 04/01/2014
      [  +0.000472] RIP: 0010:register_bcache+0x1829/0x1990 [bcache]
      [  +0.000344] Code: b0 48 83 e8 50 48 81 fa e0 e1 10 c0 0f 84 a9 00 00 00 48 89 c6 48 89 ca 0f b7 ba 54 04 00 00 4c 8b 82 60 0c 00 00 85 ff 74 2f <49> 3b a8 98 09 00 00 74 4e 44 8d 47 ff 31 ff 49 c1 e0 03 eb 0d
      [  +0.000839] RSP: 0018:ffff92ee804cbd88 EFLAGS: 00010202
      [  +0.000328] RAX: ffffffffc010e190 RBX: ffff918b5c6b5000 RCX: ffff918b7d8e0000
      [  +0.000399] RDX: ffff918b7d8e0000 RSI: ffffffffc010e190 RDI: 0000000000000001
      [  +0.000398] RBP: ffff918b7d318340 R08: 0000000000000000 R09: ffffffffb9bd2d7a
      [  +0.000385] R10: ffff918b7eb253c0 R11: ffffb95980f51200 R12: ffffffffc010e1a0
      [  +0.000411] R13: fffffffffffffff2 R14: 000000000000000b R15: ffff918b7e232620
      [  +0.000384] FS:  00007f955bec2740(0000) GS:ffff918b7eb00000(0000) knlGS:0000000000000000
      [  +0.000420] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
      [  +0.000801] CR2: 0000000000000998 CR3: 000000003cad6000 CR4: 00000000001406e0
      [  +0.000837] Call Trace:
      [  +0.000682]  ? _cond_resched+0x10/0x20
      [  +0.000691]  ? __kmalloc+0x131/0x1b0
      [  +0.000710]  kernfs_fop_write+0xfa/0x170
      [  +0.000733]  __vfs_write+0x2e/0x190
      [  +0.000688]  ? inode_security+0x10/0x30
      [  +0.000698]  ? selinux_file_permission+0xd2/0x120
      [  +0.000752]  ? security_file_permission+0x2b/0x100
      [  +0.000753]  vfs_write+0xa8/0x1a0
      [  +0.000676]  ksys_write+0x4d/0xb0
      [  +0.000699]  do_syscall_64+0x3a/0xf0
      [  +0.000692]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
      Signed-off-by: default avatarLiang Chen <liangchen.linux@gmail.com>
      Cc: stable@vger.kernel.org
      Signed-off-by: default avatarColy Li <colyli@suse.de>
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      a4b732a2
    • George Spelvin's avatar
      bcache: Clean up bch_get_congested() · 3a394727
      George Spelvin authored
      There are a few nits in this function.  They could in theory all
      be separate patches, but that's probably taking small commits
      too far.
      
      1) I added a brief comment saying what it does.
      
      2) I like to declare pointer parameters "const" where possible
         for documentation reasons.
      
      3) It uses bitmap_weight(&rand, BITS_PER_LONG) to compute the Hamming
      weight of a 32-bit random number (giving a random integer with
      mean 16 and variance 8).  Passing by reference in a 64-bit variable
      is silly; just use hweight32().
      
      4) Its helper function fract_exp_two is unnecessarily tangled.
      Gcc can optimize the multiply by (1 << x) to a shift, but it can
      be written in a much more straightforward way at the cost of one
      more bit of internal precision.  Some analysis reveals that this
      bit is always available.
      
      This shrinks the object code for fract_exp_two(x, 6) from 23 bytes:
      
      0000000000000000 <foo1>:
         0:   89 f9                   mov    %edi,%ecx
         2:   c1 e9 06                shr    $0x6,%ecx
         5:   b8 01 00 00 00          mov    $0x1,%eax
         a:   d3 e0                   shl    %cl,%eax
         c:   83 e7 3f                and    $0x3f,%edi
         f:   d3 e7                   shl    %cl,%edi
        11:   c1 ef 06                shr    $0x6,%edi
        14:   01 f8                   add    %edi,%eax
        16:   c3                      retq
      
      To 19:
      
      0000000000000017 <foo2>:
        17:   89 f8                   mov    %edi,%eax
        19:   83 e0 3f                and    $0x3f,%eax
        1c:   83 c0 40                add    $0x40,%eax
        1f:   89 f9                   mov    %edi,%ecx
        21:   c1 e9 06                shr    $0x6,%ecx
        24:   d3 e0                   shl    %cl,%eax
        26:   c1 e8 06                shr    $0x6,%eax
        29:   c3                      retq
      
      (Verified with 0 <= frac_bits <= 8, 0 <= x < 16<<frac_bits;
      both versions produce the same output.)
      
      5) And finally, the call to bch_get_congested() in check_should_bypass()
      is separated from the use of the value by multiple tests which
      could moot the need to compute it.  Move the computation down to
      where it's needed.  This also saves a local register to hold the
      computed value.
      Signed-off-by: default avatarGeorge Spelvin <lkml@sdf.org>
      Signed-off-by: default avatarColy Li <colyli@suse.de>
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      3a394727
    • Geliang Tang's avatar
      bcache: use kmemdup_nul for CACHED_LABEL buffer · 792732d9
      Geliang Tang authored
      This patch uses kmemdup_nul to create a NUL-terminated string from
      dc->sb.label. This is better than open coding it.
      
      With this, we can move env[2] initialization into env[] array to make
      code more elegant.
      Signed-off-by: default avatarGeliang Tang <geliangtang@gmail.com>
      Signed-off-by: default avatarColy Li <colyli@suse.de>
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      792732d9
    • Arnd Bergmann's avatar
      bcache: avoid clang -Wunintialized warning · 78d4eb8a
      Arnd Bergmann authored
      clang has identified a code path in which it thinks a
      variable may be unused:
      
      drivers/md/bcache/alloc.c:333:4: error: variable 'bucket' is used uninitialized whenever 'if' condition is false
            [-Werror,-Wsometimes-uninitialized]
                              fifo_pop(&ca->free_inc, bucket);
                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      drivers/md/bcache/util.h:219:27: note: expanded from macro 'fifo_pop'
       #define fifo_pop(fifo, i)       fifo_pop_front(fifo, (i))
                                      ^~~~~~~~~~~~~~~~~~~~~~~~~
      drivers/md/bcache/util.h:189:6: note: expanded from macro 'fifo_pop_front'
              if (_r) {                                                       \
                  ^~
      drivers/md/bcache/alloc.c:343:46: note: uninitialized use occurs here
                              allocator_wait(ca, bch_allocator_push(ca, bucket));
                                                                        ^~~~~~
      drivers/md/bcache/alloc.c:287:7: note: expanded from macro 'allocator_wait'
                      if (cond)                                               \
                          ^~~~
      drivers/md/bcache/alloc.c:333:4: note: remove the 'if' if its condition is always true
                              fifo_pop(&ca->free_inc, bucket);
                              ^
      drivers/md/bcache/util.h:219:27: note: expanded from macro 'fifo_pop'
       #define fifo_pop(fifo, i)       fifo_pop_front(fifo, (i))
                                      ^
      drivers/md/bcache/util.h:189:2: note: expanded from macro 'fifo_pop_front'
              if (_r) {                                                       \
              ^
      drivers/md/bcache/alloc.c:331:15: note: initialize the variable 'bucket' to silence this warning
                              long bucket;
                                         ^
      
      This cannot happen in practice because we only enter the loop
      if there is at least one element in the list.
      
      Slightly rearranging the code makes this clearer to both the
      reader and the compiler, which avoids the warning.
      Signed-off-by: default avatarArnd Bergmann <arnd@arndb.de>
      Reviewed-by: default avatarNathan Chancellor <natechancellor@gmail.com>
      Signed-off-by: default avatarColy Li <colyli@suse.de>
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      78d4eb8a
    • Guoju Fang's avatar
      bcache: fix inaccurate result of unused buckets · 4e0c04ec
      Guoju Fang authored
      To get the amount of unused buckets in sysfs_priority_stats, the code
      count the buckets which GC_SECTORS_USED is zero. It's correct and should
      not be overwritten by the count of buckets which prio is zero.
      Signed-off-by: default avatarGuoju Fang <fangguoju@gmail.com>
      Signed-off-by: default avatarColy Li <colyli@suse.de>
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      4e0c04ec
    • Guoju Fang's avatar
      bcache: fix crashes stopping bcache device before read miss done · 1568ee7e
      Guoju Fang authored
      The bio from upper layer is considered completed when bio_complete()
      returns. In most scenarios bio_complete() is called in search_free(),
      but when read miss happens, the bio_compete() is called when backing
      device reading completed, while the struct search is still in use until
      cache inserting finished.
      
      If someone stops the bcache device just then, the device may be closed
      and released, but after cache inserting finished the struct search will
      access a freed struct cached_dev.
      
      This patch add the reference of bcache device before bio_complete() when
      read miss happens, and put it after the search is not used.
      Signed-off-by: default avatarGuoju Fang <fangguoju@gmail.com>
      Signed-off-by: default avatarColy Li <colyli@suse.de>
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      1568ee7e
    • Ming Lei's avatar
      block: don't run get_page() on pages from non-bvec iov iter · 0257c0ed
      Ming Lei authored
      The refcount has been increased for pages retrieved from non-bvec iov iter
      via __bio_iov_iter_get_pages(), so don't need to do that again.
      
      Otherwise, IO pages are leaked easily.
      
      Cc: Christoph Hellwig <hch@lst.de>
      Reviewed-by: default avatarChaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
      Fixes: 7321ecbf ("block: change how we get page references in bio_iov_iter_get_pages")
      Signed-off-by: default avatarMing Lei <ming.lei@redhat.com>
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      0257c0ed
  5. 23 Apr, 2019 1 commit
  6. 22 Apr, 2019 2 commits