linux-stable.git/mm/backing-dev.c, branch v4.14.166

writeback: synchronize sync(2) against cgroup writeback membership switches

2019-03-05T16:58:01+00:00

[ Upstream commit 7fc5854f8c6efae9e7624970ab49a1eac2faefb1 ]

sync_inodes_sb() can race against cgwb (cgroup writeback) membership
switches and fail to writeback some inodes.  For example, if an inode
switches to another wb while sync_inodes_sb() is in progress, the new
wb might not be visible to bdi_split_work_to_wbs() at all or the inode
might jump from a wb which hasn't issued writebacks yet to one which
already has.

This patch adds backing_dev_info->wb_switch_rwsem to synchronize cgwb
switch path against sync_inodes_sb() so that sync_inodes_sb() is
guaranteed to see all the target wbs and inodes can't jump wbs to
escape syncing.

v2: Fixed misplaced rwsem init.  Spotted by Jiufei.

Signed-off-by: Tejun Heo 
Reported-by: Jiufei Xue 
Link: http://lkml.kernel.org/r/dc694ae2-f07f-61e1-7097-7c8411cee12d@gmail.com
Acked-by: Jan Kara 
Signed-off-by: Jens Axboe 
Signed-off-by: Sasha Levin

bdi: Fix another oops in wb_workfn()

2018-07-22T12:28:49+00:00

commit 3ee7e8697d5860b173132606d80a9cd35e7113ee upstream.

syzbot is reporting NULL pointer dereference at wb_workfn() [1] due to
wb->bdi->dev being NULL. And Dmitry confirmed that wb->state was
WB_shutting_down after wb->bdi->dev became NULL. This indicates that
unregister_bdi() failed to call wb_shutdown() on one of wb objects.

The problem is in cgwb_bdi_unregister() which does cgwb_kill() and thus
drops bdi's reference to wb structures before going through the list of
wbs again and calling wb_shutdown() on each of them. This way the loop
iterating through all wbs can easily miss a wb if that wb has already
passed through cgwb_remove_from_bdi_list() called from wb_shutdown()
from cgwb_release_workfn() and as a result fully shutdown bdi although
wb_workfn() for this wb structure is still running. In fact there are
also other ways cgwb_bdi_unregister() can race with
cgwb_release_workfn() leading e.g. to use-after-free issues:

CPU1                            CPU2
                                cgwb_bdi_unregister()
                                  cgwb_kill(*slot);

cgwb_release()
  queue_work(cgwb_release_wq, &wb->release_work);
cgwb_release_workfn()
                                  wb = list_first_entry(&bdi->wb_list, ...)
                                  spin_unlock_irq(&cgwb_lock);
  wb_shutdown(wb);
  ...
  kfree_rcu(wb, rcu);
                                  wb_shutdown(wb); -> oops use-after-free

We solve these issues by synchronizing writeback structure shutdown from
cgwb_bdi_unregister() with cgwb_release_workfn() using a new mutex. That
way we also no longer need synchronization using WB_shutting_down as the
mutex provides it for CONFIG_CGROUP_WRITEBACK case and without
CONFIG_CGROUP_WRITEBACK wb_shutdown() can be called only once from
bdi_unregister().

Reported-by: syzbot 
Acked-by: Tejun Heo 
Signed-off-by: Jan Kara 
Signed-off-by: Jens Axboe 
Signed-off-by: Greg Kroah-Hartman

bdi: Move cgroup bdi_writeback to a dedicated low concurrency workqueue

2018-06-26T00:06:32+00:00

commit f183464684190bacbfb14623bd3e4e51b7575b4c upstream.

From 0aa2e9b921d6db71150633ff290199554f0842a8 Mon Sep 17 00:00:00 2001
From: Tejun Heo 
Date: Wed, 23 May 2018 10:29:00 -0700

cgwb_release() punts the actual release to cgwb_release_workfn() on
system_wq.  Depending on the number of cgroups or block devices, there
can be a lot of cgwb_release_workfn() in flight at the same time.

We're periodically seeing close to 256 kworkers getting stuck with the
following stack trace and overtime the entire system gets stuck.

  [] _synchronize_rcu_expedited.constprop.72+0x2fc/0x330
  [] synchronize_rcu_expedited+0x24/0x30
  [] bdi_unregister+0x53/0x290
  [] release_bdi+0x89/0xc0
  [] wb_exit+0x85/0xa0
  [] cgwb_release_workfn+0x54/0xb0
  [] process_one_work+0x150/0x410
  [] worker_thread+0x6d/0x520
  [] kthread+0x12c/0x160
  [] ret_from_fork+0x29/0x40
  [] 0xffffffffffffffff

The events leading to the lockup are...

1. A lot of cgwb_release_workfn() is queued at the same time and all
   system_wq kworkers are assigned to execute them.

2. They all end up calling synchronize_rcu_expedited().  One of them
   wins and tries to perform the expedited synchronization.

3. However, that invovles queueing rcu_exp_work to system_wq and
   waiting for it.  Because #1 is holding all available kworkers on
   system_wq, rcu_exp_work can't be executed.  cgwb_release_workfn()
   is waiting for synchronize_rcu_expedited() which in turn is waiting
   for cgwb_release_workfn() to free up some of the kworkers.

We shouldn't be scheduling hundreds of cgwb_release_workfn() at the
same time.  There's nothing to be gained from that.  This patch
updates cgwb release path to use a dedicated percpu workqueue with
@max_active of 1.

While this resolves the problem at hand, it might be a good idea to
isolate rcu_exp_work to its own workqueue too as it can be used from
various paths and is prone to this sort of indirect A-A deadlocks.

Signed-off-by: Tejun Heo 
Cc: "Paul E. McKenney" 
Cc: stable@vger.kernel.org
Signed-off-by: Jens Axboe 
Signed-off-by: Greg Kroah-Hartman

bdi: wake up concurrent wb_shutdown() callers.

2018-05-16T08:10:25+00:00

commit 8236b0ae31c837d2b3a2565c5f8d77f637e824cc upstream.

syzbot is reporting hung tasks at wait_on_bit(WB_shutting_down) in
wb_shutdown() [1]. This seems to be because commit 5318ce7d46866e1d ("bdi:
Shutdown writeback on all cgwbs in cgwb_bdi_destroy()") forgot to call
wake_up_bit(WB_shutting_down) after clear_bit(WB_shutting_down).

Introduce a helper function clear_and_wake_up_bit() and use it, in order
to avoid similar errors in future.

[1] https://syzkaller.appspot.com/bug?id=b297474817af98d5796bc544e1bb806fc3da0e5e

Signed-off-by: Tetsuo Handa 
Reported-by: syzbot 
Fixes: 5318ce7d46866e1d ("bdi: Shutdown writeback on all cgwbs in cgwb_bdi_destroy()")
Cc: Tejun Heo 
Reviewed-by: Jan Kara 
Suggested-by: Linus Torvalds 
Signed-off-by: Jens Axboe 
Signed-off-by: Greg Kroah-Hartman

mm/backing-dev.c: fix an error handling path in 'cgwb_create()'

2017-09-11T20:16:44+00:00

If the 'kmalloc' fails, we must go through the existing error handling
path.

Signed-off-by: Christophe JAILLET 
Fixes: 52ebea749aae ("writeback: make backing_dev_info host cgroup-specific bdi_writebacks")
Reviewed-by: Jan Kara 
Signed-off-by: Jens Axboe

bdi: Drop 'parent' argument from bdi_register[_va]()

2017-04-20T18:09:55+00:00

Drop 'parent' argument of bdi_register() and bdi_register_va().  It is
always NULL.

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

block: Remove unused functions

2017-04-20T18:09:55+00:00

Now that all backing_dev_info structure are allocated separately, we can
drop some unused functions.

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

bdi: Export bdi_alloc_node() and bdi_put()

2017-04-20T18:09:55+00:00

MTD will want to call bdi_alloc_node() and bdi_put() directly. Export
these functions.

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

block: Unregister bdi on last reference drop

2017-04-20T18:09:55+00:00

Most users will want to unregister bdi when dropping last reference to a
bdi. Only a few users (like block devices) want to play more complex
tricks with bdi registration and unregistration. So unregister bdi when
the last reference to bdi is dropped and just make sure we don't
unregister the bdi the second time if it is already unregistered.

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

bdi: Provide bdi_register_va() and bdi_alloc()

2017-04-20T18:09:55+00:00

Add function that registers bdi and takes va_list instead of variable
number of arguments.

Add bdi_alloc() as simple wrapper for NUMA-unaware users allocating BDI.

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