1. 28 6月, 2019 9 次提交
    • A
      bcache: use sysfs_match_string() instead of __sysfs_match_string() · 89e0341a
      Alexandru Ardelean 提交于
      The arrays (of strings) that are passed to __sysfs_match_string() are
      static, so use sysfs_match_string() which does an implicit ARRAY_SIZE()
      over these arrays.
      
      Functionally, this doesn't change anything.
      The change is more cosmetic.
      
      It only shrinks the static arrays by 1 byte each.
      Signed-off-by: NAlexandru Ardelean <alexandru.ardelean@analog.com>
      Signed-off-by: NColy Li <colyli@suse.de>
      Signed-off-by: NJens Axboe <axboe@kernel.dk>
      89e0341a
    • C
      bcache: remove unnecessary prefetch() in bset_search_tree() · f960facb
      Coly Li 提交于
      In function bset_search_tree(), when p >= t->size, t->tree[0] will be
      prefetched by the following code piece,
       974                 unsigned int p = n << 4;
       975
       976                 p &= ((int) (p - t->size)) >> 31;
       977
       978                 prefetch(&t->tree[p]);
      
      The purpose of the above code is to avoid a branch instruction, but
      when p >= t->size, prefetch(&t->tree[0]) has no positive performance
      contribution at all. This patch avoids the unncessary prefetch by only
      calling prefetch() when p < t->size.
      Signed-off-by: NColy Li <colyli@suse.de>
      Signed-off-by: NJens Axboe <axboe@kernel.dk>
      f960facb
    • C
      bcache: add io error counting in write_bdev_super_endio() · 08ec1e62
      Coly Li 提交于
      When backing device super block is written by bch_write_bdev_super(),
      the bio complete callback write_bdev_super_endio() simply ignores I/O
      status. Indeed such write request also contribute to backing device
      health status if the request failed.
      
      This patch checkes bio->bi_status in write_bdev_super_endio(), if there
      is error, bch_count_backing_io_errors() will be called to count an I/O
      error to dc->io_errors.
      Signed-off-by: NColy Li <colyli@suse.de>
      Signed-off-by: NJens Axboe <axboe@kernel.dk>
      08ec1e62
    • C
      bcache: ignore read-ahead request failure on backing device · 578df99b
      Coly Li 提交于
      When md raid device (e.g. raid456) is used as backing device, read-ahead
      requests on a degrading and recovering md raid device might be failured
      immediately by md raid code, but indeed this md raid array can still be
      read or write for normal I/O requests. Therefore such failed read-ahead
      request are not real hardware failure. Further more, after degrading and
      recovering accomplished, read-ahead requests will be handled by md raid
      array again.
      
      For such condition, I/O failures of read-ahead requests don't indicate
      real health status (because normal I/O still be served), they should not
      be counted into I/O error counter dc->io_errors.
      
      Since there is no simple way to detect whether the backing divice is a
      md raid device, this patch simply ignores I/O failures for read-ahead
      bios on backing device, to avoid bogus backing device failure on a
      degrading md raid array.
      Suggested-and-tested-by: NThorsten Knabe <linux@thorsten-knabe.de>
      Signed-off-by: NColy Li <colyli@suse.de>
      Cc: stable@vger.kernel.org
      Signed-off-by: NJens Axboe <axboe@kernel.dk>
      578df99b
    • C
      bcache: avoid flushing btree node in cache_set_flush() if io disabled · e6dcbd3e
      Coly Li 提交于
      When cache_set_flush() is called for too many I/O errors detected on
      cache device and the cache set is retiring, inside the function it
      doesn't make sense to flushing cached btree nodes from c->btree_cache
      because CACHE_SET_IO_DISABLE is set on c->flags already and all I/Os
      onto cache device will be rejected.
      
      This patch checks in cache_set_flush() that whether CACHE_SET_IO_DISABLE
      is set. If yes, then avoids to flush the cached btree nodes to reduce
      more time and make cache set retiring more faster.
      Signed-off-by: NColy Li <colyli@suse.de>
      Signed-off-by: NJens Axboe <axboe@kernel.dk>
      e6dcbd3e
    • C
      Revert "bcache: set CACHE_SET_IO_DISABLE in bch_cached_dev_error()" · 695277f1
      Coly Li 提交于
      This reverts commit 6147305c.
      
      Although this patch helps the failed bcache device to stop faster when
      too many I/O errors detected on corresponding cached device, setting
      CACHE_SET_IO_DISABLE bit to cache set c->flags was not a good idea. This
      operation will disable all I/Os on cache set, which means other attached
      bcache devices won't work neither.
      
      Without this patch, the failed bcache device can also be stopped
      eventually if internal I/O accomplished (e.g. writeback). Therefore here
      I revert it.
      
      Fixes: 6147305c ("bcache: set CACHE_SET_IO_DISABLE in bch_cached_dev_error()")
      Reported-by: NYong Li <mr.liyong@qq.com>
      Signed-off-by: NColy Li <colyli@suse.de>
      Cc: stable@vger.kernel.org
      Signed-off-by: NJens Axboe <axboe@kernel.dk>
      695277f1
    • C
      bcache: fix return value error in bch_journal_read() · 0ae49cb7
      Coly Li 提交于
      When everything is OK in bch_journal_read(), finally the return value
      is returned by,
      	return ret;
      which assumes ret will be 0 here. This assumption is wrong when all
      journal buckets as are full and filled with valid journal entries. In
      such cache the last location referencess read_bucket() sets 'ret' to
      1, which means new jset added into jset list. The jset list is list
      'journal' in caller run_cache_set().
      
      Return 1 to run_cache_set() means something wrong and the cache set
      won't start, but indeed everything is OK.
      
      This patch changes the line at end of bch_journal_read() to directly
      return 0 since everything if verything is good. Then a bogus error
      is fixed.
      Signed-off-by: NColy Li <colyli@suse.de>
      Signed-off-by: NJens Axboe <axboe@kernel.dk>
      0ae49cb7
    • C
      bcache: check c->gc_thread by IS_ERR_OR_NULL in cache_set_flush() · b387e9b5
      Coly Li 提交于
      When system memory is in heavy pressure, bch_gc_thread_start() from
      run_cache_set() may fail due to out of memory. In such condition,
      c->gc_thread is assigned to -ENOMEM, not NULL pointer. Then in following
      failure code path bch_cache_set_error(), when cache_set_flush() gets
      called, the code piece to stop c->gc_thread is broken,
               if (!IS_ERR_OR_NULL(c->gc_thread))
                       kthread_stop(c->gc_thread);
      
      And KASAN catches such NULL pointer deference problem, with the warning
      information:
      
      [  561.207881] ==================================================================
      [  561.207900] BUG: KASAN: null-ptr-deref in kthread_stop+0x3b/0x440
      [  561.207904] Write of size 4 at addr 000000000000001c by task kworker/15:1/313
      
      [  561.207913] CPU: 15 PID: 313 Comm: kworker/15:1 Tainted: G        W         5.0.0-vanilla+ #3
      [  561.207916] Hardware name: Lenovo ThinkSystem SR650 -[7X05CTO1WW]-/-[7X05CTO1WW]-, BIOS -[IVE136T-2.10]- 03/22/2019
      [  561.207935] Workqueue: events cache_set_flush [bcache]
      [  561.207940] Call Trace:
      [  561.207948]  dump_stack+0x9a/0xeb
      [  561.207955]  ? kthread_stop+0x3b/0x440
      [  561.207960]  ? kthread_stop+0x3b/0x440
      [  561.207965]  kasan_report+0x176/0x192
      [  561.207973]  ? kthread_stop+0x3b/0x440
      [  561.207981]  kthread_stop+0x3b/0x440
      [  561.207995]  cache_set_flush+0xd4/0x6d0 [bcache]
      [  561.208008]  process_one_work+0x856/0x1620
      [  561.208015]  ? find_held_lock+0x39/0x1d0
      [  561.208028]  ? drain_workqueue+0x380/0x380
      [  561.208048]  worker_thread+0x87/0xb80
      [  561.208058]  ? __kthread_parkme+0xb6/0x180
      [  561.208067]  ? process_one_work+0x1620/0x1620
      [  561.208072]  kthread+0x326/0x3e0
      [  561.208079]  ? kthread_create_worker_on_cpu+0xc0/0xc0
      [  561.208090]  ret_from_fork+0x3a/0x50
      [  561.208110] ==================================================================
      [  561.208113] Disabling lock debugging due to kernel taint
      [  561.208115] irq event stamp: 11800231
      [  561.208126] hardirqs last  enabled at (11800231): [<ffffffff83008538>] do_syscall_64+0x18/0x410
      [  561.208127] BUG: unable to handle kernel NULL pointer dereference at 000000000000001c
      [  561.208129] #PF error: [WRITE]
      [  561.312253] hardirqs last disabled at (11800230): [<ffffffff830052ff>] trace_hardirqs_off_thunk+0x1a/0x1c
      [  561.312259] softirqs last  enabled at (11799832): [<ffffffff850005c7>] __do_softirq+0x5c7/0x8c3
      [  561.405975] PGD 0 P4D 0
      [  561.442494] softirqs last disabled at (11799821): [<ffffffff831add2c>] irq_exit+0x1ac/0x1e0
      [  561.791359] Oops: 0002 [#1] SMP KASAN NOPTI
      [  561.791362] CPU: 15 PID: 313 Comm: kworker/15:1 Tainted: G    B   W         5.0.0-vanilla+ #3
      [  561.791363] Hardware name: Lenovo ThinkSystem SR650 -[7X05CTO1WW]-/-[7X05CTO1WW]-, BIOS -[IVE136T-2.10]- 03/22/2019
      [  561.791371] Workqueue: events cache_set_flush [bcache]
      [  561.791374] RIP: 0010:kthread_stop+0x3b/0x440
      [  561.791376] Code: 00 00 65 8b 05 26 d5 e0 7c 89 c0 48 0f a3 05 ec aa df 02 0f 82 dc 02 00 00 4c 8d 63 20 be 04 00 00 00 4c 89 e7 e8 65 c5 53 00 <f0> ff 43 20 48 8d 7b 24 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48
      [  561.791377] RSP: 0018:ffff88872fc8fd10 EFLAGS: 00010286
      [  561.838895] bcache: bch_count_io_errors() nvme0n1: IO error on writing btree.
      [  561.838916] bcache: bch_count_io_errors() nvme0n1: IO error on writing btree.
      [  561.838934] bcache: bch_count_io_errors() nvme0n1: IO error on writing btree.
      [  561.838948] bcache: bch_count_io_errors() nvme0n1: IO error on writing btree.
      [  561.838966] bcache: bch_count_io_errors() nvme0n1: IO error on writing btree.
      [  561.838979] bcache: bch_count_io_errors() nvme0n1: IO error on writing btree.
      [  561.838996] bcache: bch_count_io_errors() nvme0n1: IO error on writing btree.
      [  563.067028] RAX: 0000000000000000 RBX: fffffffffffffffc RCX: ffffffff832dd314
      [  563.067030] RDX: 0000000000000000 RSI: 0000000000000004 RDI: 0000000000000297
      [  563.067032] RBP: ffff88872fc8fe88 R08: fffffbfff0b8213d R09: fffffbfff0b8213d
      [  563.067034] R10: 0000000000000001 R11: fffffbfff0b8213c R12: 000000000000001c
      [  563.408618] R13: ffff88dc61cc0f68 R14: ffff888102b94900 R15: ffff88dc61cc0f68
      [  563.408620] FS:  0000000000000000(0000) GS:ffff888f7dc00000(0000) knlGS:0000000000000000
      [  563.408622] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
      [  563.408623] CR2: 000000000000001c CR3: 0000000f48a1a004 CR4: 00000000007606e0
      [  563.408625] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
      [  563.408627] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
      [  563.904795] bcache: bch_count_io_errors() nvme0n1: IO error on writing btree.
      [  563.915796] PKRU: 55555554
      [  563.915797] Call Trace:
      [  563.915807]  cache_set_flush+0xd4/0x6d0 [bcache]
      [  563.915812]  process_one_work+0x856/0x1620
      [  564.001226] bcache: bch_count_io_errors() nvme0n1: IO error on writing btree.
      [  564.033563]  ? find_held_lock+0x39/0x1d0
      [  564.033567]  ? drain_workqueue+0x380/0x380
      [  564.033574]  worker_thread+0x87/0xb80
      [  564.062823] bcache: bch_count_io_errors() nvme0n1: IO error on writing btree.
      [  564.118042]  ? __kthread_parkme+0xb6/0x180
      [  564.118046]  ? process_one_work+0x1620/0x1620
      [  564.118048]  kthread+0x326/0x3e0
      [  564.118050]  ? kthread_create_worker_on_cpu+0xc0/0xc0
      [  564.167066] bcache: bch_count_io_errors() nvme0n1: IO error on writing btree.
      [  564.252441]  ret_from_fork+0x3a/0x50
      [  564.252447] Modules linked in: msr rpcrdma sunrpc rdma_ucm ib_iser ib_umad rdma_cm ib_ipoib i40iw configfs iw_cm ib_cm libiscsi scsi_transport_iscsi mlx4_ib ib_uverbs mlx4_en ib_core nls_iso8859_1 nls_cp437 vfat fat intel_rapl skx_edac x86_pkg_temp_thermal coretemp iTCO_wdt iTCO_vendor_support crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel ses raid0 aesni_intel cdc_ether enclosure usbnet ipmi_ssif joydev aes_x86_64 i40e scsi_transport_sas mii bcache md_mod crypto_simd mei_me ioatdma crc64 ptp cryptd pcspkr i2c_i801 mlx4_core glue_helper pps_core mei lpc_ich dca wmi ipmi_si ipmi_devintf nd_pmem dax_pmem nd_btt ipmi_msghandler device_dax pcc_cpufreq button hid_generic usbhid mgag200 i2c_algo_bit drm_kms_helper syscopyarea sysfillrect xhci_pci sysimgblt fb_sys_fops xhci_hcd ttm megaraid_sas drm usbcore nfit libnvdimm sg dm_multipath dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua efivarfs
      [  564.299390] bcache: bch_count_io_errors() nvme0n1: IO error on writing btree.
      [  564.348360] CR2: 000000000000001c
      [  564.348362] ---[ end trace b7f0e5cc7b2103b0 ]---
      
      Therefore, it is not enough to only check whether c->gc_thread is NULL,
      we should use IS_ERR_OR_NULL() to check both NULL pointer and error
      value.
      
      This patch changes the above buggy code piece in this way,
               if (!IS_ERR_OR_NULL(c->gc_thread))
                       kthread_stop(c->gc_thread);
      Signed-off-by: NColy Li <colyli@suse.de>
      Signed-off-by: NJens Axboe <axboe@kernel.dk>
      b387e9b5
    • C
      bcache: don't set max writeback rate if gc is running · 141df8bb
      Coly Li 提交于
      When gc is running, user space I/O processes may wait inside
      bcache code, so no new I/O coming. Indeed this is not a real idle
      time, maximum writeback rate should not be set in such situation.
      Otherwise a faster writeback thread may compete locks with gc thread
      and makes garbage collection slower, which results a longer I/O
      freeze period.
      
      This patch checks c->gc_mark_valid in set_at_max_writeback_rate(). If
      c->gc_mark_valid is 0 (gc running), set_at_max_writeback_rate() returns
      false, then update_writeback_rate() will not set writeback rate to
      maximum value even c->idle_counter reaches an idle threshold.
      
      Now writeback thread won't interfere gc thread performance.
      Signed-off-by: NColy Li <colyli@suse.de>
      Signed-off-by: NJens Axboe <axboe@kernel.dk>
      141df8bb
  2. 13 6月, 2019 2 次提交
    • C
      bcache: only set BCACHE_DEV_WB_RUNNING when cached device attached · 1f0ffa67
      Coly Li 提交于
      When people set a writeback percent via sysfs file,
        /sys/block/bcache<N>/bcache/writeback_percent
      current code directly sets BCACHE_DEV_WB_RUNNING to dc->disk.flags
      and schedules kworker dc->writeback_rate_update.
      
      If there is no cache set attached to, the writeback kernel thread is
      not running indeed, running dc->writeback_rate_update does not make
      sense and may cause NULL pointer deference when reference cache set
      pointer inside update_writeback_rate().
      
      This patch checks whether the cache set point (dc->disk.c) is NULL in
      sysfs interface handler, and only set BCACHE_DEV_WB_RUNNING and
      schedule dc->writeback_rate_update when dc->disk.c is not NULL (it
      means the cache device is attached to a cache set).
      
      This problem might be introduced from initial bcache commit, but
      commit 3fd47bfe ("bcache: stop dc->writeback_rate_update properly")
      changes part of the original code piece, so I add 'Fixes: 3fd47bfe'
      to indicate from which commit this patch can be applied.
      
      Fixes: 3fd47bfe ("bcache: stop dc->writeback_rate_update properly")
      Reported-by: NBjørn Forsman <bjorn.forsman@gmail.com>
      Signed-off-by: NColy Li <colyli@suse.de>
      Reviewed-by: NBjørn Forsman <bjorn.forsman@gmail.com>
      Cc: stable@vger.kernel.org
      Signed-off-by: NJens Axboe <axboe@kernel.dk>
      1f0ffa67
    • C
      bcache: fix stack corruption by PRECEDING_KEY() · 31b90956
      Coly Li 提交于
      Recently people report bcache code compiled with gcc9 is broken, one of
      the buggy behavior I observe is that two adjacent 4KB I/Os should merge
      into one but they don't. Finally it turns out to be a stack corruption
      caused by macro PRECEDING_KEY().
      
      See how PRECEDING_KEY() is defined in bset.h,
      437 #define PRECEDING_KEY(_k)                                       \
      438 ({                                                              \
      439         struct bkey *_ret = NULL;                               \
      440                                                                 \
      441         if (KEY_INODE(_k) || KEY_OFFSET(_k)) {                  \
      442                 _ret = &KEY(KEY_INODE(_k), KEY_OFFSET(_k), 0);  \
      443                                                                 \
      444                 if (!_ret->low)                                 \
      445                         _ret->high--;                           \
      446                 _ret->low--;                                    \
      447         }                                                       \
      448                                                                 \
      449         _ret;                                                   \
      450 })
      
      At line 442, _ret points to address of a on-stack variable combined by
      KEY(), the life range of this on-stack variable is in line 442-446,
      once _ret is returned to bch_btree_insert_key(), the returned address
      points to an invalid stack address and this address is overwritten in
      the following called bch_btree_iter_init(). Then argument 'search' of
      bch_btree_iter_init() points to some address inside stackframe of
      bch_btree_iter_init(), exact address depends on how the compiler
      allocates stack space. Now the stack is corrupted.
      
      Fixes: 0eacac22 ("bcache: PRECEDING_KEY()")
      Signed-off-by: NColy Li <colyli@suse.de>
      Reviewed-by: NRolf Fokkens <rolf@rolffokkens.nl>
      Reviewed-by: NPierre JUHEN <pierre.juhen@orange.fr>
      Tested-by: NShenghui Wang <shhuiw@foxmail.com>
      Tested-by: NPierre JUHEN <pierre.juhen@orange.fr>
      Cc: Kent Overstreet <kent.overstreet@gmail.com>
      Cc: Nix <nix@esperi.org.uk>
      Cc: stable@vger.kernel.org
      Signed-off-by: NJens Axboe <axboe@kernel.dk>
      31b90956
  3. 21 5月, 2019 1 次提交
  4. 01 5月, 2019 1 次提交
  5. 30 4月, 2019 3 次提交
  6. 25 4月, 2019 18 次提交
    • S
      bcache: avoid potential memleak of list of journal_replay(s) in the CACHE_SYNC... · 95f18c9d
      Shenghui Wang 提交于
      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: NShenghui Wang <shhuiw@foxmail.com>
      Signed-off-by: NColy Li <colyli@suse.de>
      Signed-off-by: NJens Axboe <axboe@kernel.dk>
      95f18c9d
    • S
      bcache: fix wrong usage use-after-freed on keylist in out_nocoalesce branch of btree_gc_coalesce · f16277ca
      Shenghui Wang 提交于
      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: NShenghui Wang <shhuiw@foxmail.com>
      Signed-off-by: NColy Li <colyli@suse.de>
      Signed-off-by: NJens Axboe <axboe@kernel.dk>
      f16277ca
    • T
      bcache: fix failure in journal relplay · 63120731
      Tang Junhui 提交于
      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: NTang Junhui <tang.junhui.linux@gmail.com>
      Tested-by: NDennis Schridde <devurandom@gmx.net>
      Signed-off-by: NColy Li <colyli@suse.de>
      Signed-off-by: NJens Axboe <axboe@kernel.dk>
      63120731
    • C
      bcache: improve bcache_reboot() · eb8cbb6d
      Coly Li 提交于
      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: NColy Li <colyli@suse.de>
      Reviewed-by: NHannes Reinecke <hare@suse.com>
      Signed-off-by: NJens Axboe <axboe@kernel.dk>
      eb8cbb6d
    • C
      bcache: add comments for closure_fn to be called in closure_queue() · 63d63b51
      Coly Li 提交于
      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: NColy Li <colyli@suse.de>
      Reviewed-by: NChaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
      Reviewed-by: NHannes Reinecke <hare@suse.com>
      Signed-off-by: NJens Axboe <axboe@kernel.dk>
      63d63b51
    • C
      bcache: Add comments for blkdev_put() in registration code path · bb6d355c
      Coly Li 提交于
      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: NColy Li <colyli@suse.de>
      Reviewed-by: NChaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
      Reviewed-by: NHannes Reinecke <hare@suse.com>
      Signed-off-by: NJens Axboe <axboe@kernel.dk>
      bb6d355c
    • C
      bcache: add error check for calling register_bdev() · 88c12d42
      Coly Li 提交于
      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: NColy Li <colyli@suse.de>
      Reviewed-by: NHannes Reinecke <hare@suse.com>
      Signed-off-by: NJens Axboe <axboe@kernel.dk>
      88c12d42
    • C
      bcache: return error immediately in bch_journal_replay() · 68d10e69
      Coly Li 提交于
      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: NColy Li <colyli@suse.de>
      Reviewed-by: NHannes Reinecke <hare@suse.com>
      Signed-off-by: NJens Axboe <axboe@kernel.dk>
      68d10e69
    • C
      bcache: add comments for kobj release callback routine · 2d17456e
      Coly Li 提交于
      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: NColy Li <colyli@suse.de>
      Reviewed-by: NChaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
      Reviewed-by: NHannes Reinecke <hare@suse.com>
      Signed-off-by: NJens Axboe <axboe@kernel.dk>
      2d17456e
    • C
      bcache: add failure check to run_cache_set() for journal replay · ce3e4cfb
      Coly Li 提交于
      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: NColy Li <colyli@suse.de>
      Signed-off-by: NJens Axboe <axboe@kernel.dk>
      ce3e4cfb
    • C
      bcache: never set KEY_PTRS of journal key to 0 in journal_reclaim() · 1bee2add
      Coly Li 提交于
      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: NColy Li <colyli@suse.de>
      Reviewed-by: NHannes Reinecke <hare@suse.com>
      Cc: stable@vger.kernel.org
      Signed-off-by: NJens Axboe <axboe@kernel.dk>
      1bee2add
    • C
      bcache: move definition of 'int ret' out of macro read_bucket() · 14215ee0
      Coly Li 提交于
      '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: NColy Li <colyli@suse.de>
      Reviewed-by: NHannes Reinecke <hare@suse.com>
      Signed-off-by: NJens Axboe <axboe@kernel.dk>
      14215ee0
    • L
      bcache: fix a race between cache register and cacheset unregister · a4b732a2
      Liang Chen 提交于
      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: NLiang Chen <liangchen.linux@gmail.com>
      Cc: stable@vger.kernel.org
      Signed-off-by: NColy Li <colyli@suse.de>
      Signed-off-by: NJens Axboe <axboe@kernel.dk>
      a4b732a2
    • G
      bcache: Clean up bch_get_congested() · 3a394727
      George Spelvin 提交于
      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: NGeorge Spelvin <lkml@sdf.org>
      Signed-off-by: NColy Li <colyli@suse.de>
      Signed-off-by: NJens Axboe <axboe@kernel.dk>
      3a394727
    • G
      bcache: use kmemdup_nul for CACHED_LABEL buffer · 792732d9
      Geliang Tang 提交于
      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: NGeliang Tang <geliangtang@gmail.com>
      Signed-off-by: NColy Li <colyli@suse.de>
      Signed-off-by: NJens Axboe <axboe@kernel.dk>
      792732d9
    • A
      bcache: avoid clang -Wunintialized warning · 78d4eb8a
      Arnd Bergmann 提交于
      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: NArnd Bergmann <arnd@arndb.de>
      Reviewed-by: NNathan Chancellor <natechancellor@gmail.com>
      Signed-off-by: NColy Li <colyli@suse.de>
      Signed-off-by: NJens Axboe <axboe@kernel.dk>
      78d4eb8a
    • G
      bcache: fix inaccurate result of unused buckets · 4e0c04ec
      Guoju Fang 提交于
      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: NGuoju Fang <fangguoju@gmail.com>
      Signed-off-by: NColy Li <colyli@suse.de>
      Signed-off-by: NJens Axboe <axboe@kernel.dk>
      4e0c04ec
    • G
      bcache: fix crashes stopping bcache device before read miss done · 1568ee7e
      Guoju Fang 提交于
      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: NGuoju Fang <fangguoju@gmail.com>
      Signed-off-by: NColy Li <colyli@suse.de>
      Signed-off-by: NJens Axboe <axboe@kernel.dk>
      1568ee7e
  7. 15 2月, 2019 2 次提交
  8. 09 2月, 2019 4 次提交
    • C
      bcache: use (REQ_META|REQ_PRIO) to indicate bio for metadata · dc7292a5
      Coly Li 提交于
      In 'commit 752f66a7 ("bcache: use REQ_PRIO to indicate bio for
      metadata")' REQ_META is replaced by REQ_PRIO to indicate metadata bio.
      This assumption is not always correct, e.g. XFS uses REQ_META to mark
      metadata bio other than REQ_PRIO. This is why Nix noticed that bcache
      does not cache metadata for XFS after the above commit.
      
      Thanks to Dave Chinner, he explains the difference between REQ_META and
      REQ_PRIO from view of file system developer. Here I quote part of his
      explanation from mailing list,
         REQ_META is used for metadata. REQ_PRIO is used to communicate to
         the lower layers that the submitter considers this IO to be more
         important that non REQ_PRIO IO and so dispatch should be expedited.
      
         IOWs, if the filesystem considers metadata IO to be more important
         that user data IO, then it will use REQ_PRIO | REQ_META rather than
         just REQ_META.
      
      Then it seems bios with REQ_META or REQ_PRIO should both be cached for
      performance optimation, because they are all probably low I/O latency
      demand by upper layer (e.g. file system).
      
      So in this patch, when we want to decide whether to bypass the cache,
      REQ_META and REQ_PRIO are both checked. Then both metadata and
      high priority I/O requests will be handled properly.
      Reported-by: NNix <nix@esperi.org.uk>
      Signed-off-by: NColy Li <colyli@suse.de>
      Reviewed-by: NAndre Noll <maan@tuebingen.mpg.de>
      Tested-by: NNix <nix@esperi.org.uk>
      Cc: stable@vger.kernel.org
      Cc: Dave Chinner <david@fromorbit.com>
      Cc: Christoph Hellwig <hch@lst.de>
      Signed-off-by: NJens Axboe <axboe@kernel.dk>
      dc7292a5
    • C
      bcache: fix input overflow to cache set sysfs file io_error_halflife · a91fbda4
      Coly Li 提交于
      Cache set sysfs entry io_error_halflife is used to set c->error_decay.
      c->error_decay is in type unsigned int, and it is converted by
      strtoul_or_return(), therefore overflow to c->error_decay is possible
      for a large input value.
      
      This patch fixes the overflow by using strtoul_safe_clamp() to convert
      input string to an unsigned long value in range [0, UINT_MAX], then
      divides by 88 and set it to c->error_decay.
      Signed-off-by: NColy Li <colyli@suse.de>
      Signed-off-by: NJens Axboe <axboe@kernel.dk>
      a91fbda4
    • C
      bcache: fix input overflow to cache set io_error_limit · b1500840
      Coly Li 提交于
      c->error_limit is in type unsigned int, it is set via cache set sysfs
      file io_error_limit. Inside the bcache code, input string is converted
      by strtoul_or_return() and set the converted value to c->error_limit.
      
      Because the converted value is unsigned long, and c->error_limit is
      unsigned int, if the input is large enought, overflow will happen to
      c->error_limit.
      
      This patch uses sysfs_strtoul_clamp() to convert input string, and set
      the range in [0, UINT_MAX] to avoid the potential overflow.
      Signed-off-by: NColy Li <colyli@suse.de>
      Signed-off-by: NJens Axboe <axboe@kernel.dk>
      b1500840
    • C
      bcache: fix input overflow to journal_delay_ms · 453745fb
      Coly Li 提交于
      c->journal_delay_ms is in type unsigned short, it is set via sysfs
      interface and converted by sysfs_strtoul() from input string to
      unsigned short value. Therefore overflow to unsigned short might be
      happen when the converted value exceed USHRT_MAX. e.g. writing
      65536 into sysfs file journal_delay_ms, c->journal_delay_ms is set to
      0.
      
      This patch uses sysfs_strtoul_clamp() to convert the input string and
      limit value range in [0, USHRT_MAX], to avoid the input overflow.
      Signed-off-by: NColy Li <colyli@suse.de>
      Signed-off-by: NJens Axboe <axboe@kernel.dk>
      453745fb