writeback: fix memory leak in wb_queue_work()
PATCH 1/1 writeback: fix memory leak in wb_queue_work()
When WB_registered flag is not set, wb_queue_work() skips queuing the work, but does not perform the necessary clean up. In particular, if work->auto_free is true, it should free the memory.
The leak condition can be reprouced by following these steps:
mount /dev/sdb /mnt/sdb
/* In qemu console: device_del sdb */
umount /dev/sdb
Above will result in a wb_queue_work() call on an unregistered wb and thus leak memory.
--- fs/fs-writeback.c | 35 +++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 14 deletions(-)
fs/fs-writeback.c +21 -14
@@ -173,19 +173,33 @@ static void wb_wakeup(struct bdi_writeback *wb)spin_unlock_bh (&wb->work_lock); } +static void finish_writeback_work (struct bdi_writeback *wb, +struct wb_writeback_work *work) +{ +struct wb_completion *done = work->done; + +if (work->auto_free) +kfree (work); +if (done &&atomic_dec_and_test (&done->cnt)) +wake_up_all (&wb->bdi->wb_waitq); +} +static void wb_queue_work (struct bdi_writeback *wb,struct wb_writeback_work *work) {trace_writeback_queue (wb, work); -spin_lock_bh (&wb->work_lock); -if (!test_bit (WB_registered, &wb->state)) - goto out_unlock;if (work->done)atomic_inc (&work->done->cnt); -list_add_tail (&work->list, &wb->work_list); -mod_delayed_work (bdi_wq, &wb->dwork,0 ); -out_unlock: + +spin_lock_bh (&wb->work_lock); + +if (test_bit (WB_registered, &wb->state)) { +list_add_tail (&work->list, &wb->work_list); +mod_delayed_work (bdi_wq, &wb->dwork,0 ); + }else +finish_writeback_work (wb, work); +spin_unlock_bh (&wb->work_lock); }
@@ -1873,16 +1887,9 @@ static long wb_do_writeback(struct bdi_writeback *wb)set_bit (WB_writeback_running, &wb->state);while ((work =get_next_work_item (wb)) !=NULL ) { -struct wb_completion *done = work->done; -trace_writeback_exec (wb, work); - wrote +=wb_writeback (wb, work); - -if (work->auto_free) -kfree (work); -if (done &&atomic_dec_and_test (&done->cnt)) -wake_up_all (&wb->bdi->wb_waitq); +finish_writeback_work (wb, work); }/*
Signature · 1 line
2.12.0.246.ga2ecc84866-goog
Quoted 10 lines · Tahsin Erdogan
When
WB_registeredflag is not set,wb_queue_work()skips queuing the work, but does not perform the necessary clean up. In particular, ifwork->auto_freeis true, it should free the memory.The leak condition can be reprouced by following these steps:
mount
/dev/sdb/mnt/sdb
/* In qemu console:device_delsdb */
umount/dev/sdb
Quoted 5 lines
Above will result in a
wb_queue_work()call on an unregistered wb and thus leak memory.Reported-by John Sperbeck <[email protected]>Signed-off-by Tahsin Erdogan <[email protected]>
The patch looks good. You can add:
Honza
Quoted 69 lines
--- fs/fs-writeback.c | 35 +++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 14 deletions(-)fs/fs-writeback.c +21
-14@@ -173,19 +173,33 @@ static void wb_wakeup(struct bdi_writeback *wb)spin_unlock_bh (&wb->work_lock); } +static void finish_writeback_work (struct bdi_writeback *wb, +struct wb_writeback_work *work) +{ +struct wb_completion *done = work->done; + +if (work->auto_free) +kfree (work); +if (done &&atomic_dec_and_test (&done->cnt)) +wake_up_all (&wb->bdi->wb_waitq); +} +static void wb_queue_work (struct bdi_writeback *wb,struct wb_writeback_work *work) {trace_writeback_queue (wb, work); -spin_lock_bh (&wb->work_lock); -if (!test_bit (WB_registered, &wb->state)) - goto out_unlock;if (work->done)atomic_inc (&work->done->cnt); -list_add_tail (&work->list, &wb->work_list); -mod_delayed_work (bdi_wq, &wb->dwork,0 ); -out_unlock: + +spin_lock_bh (&wb->work_lock); + +if (test_bit (WB_registered, &wb->state)) { +list_add_tail (&work->list, &wb->work_list); +mod_delayed_work (bdi_wq, &wb->dwork,0 ); + }else +finish_writeback_work (wb, work); +spin_unlock_bh (&wb->work_lock); }@@ -1873,16 +1887,9 @@ static long wb_do_writeback(struct bdi_writeback *wb)set_bit (WB_writeback_running, &wb->state);while ((work =get_next_work_item (wb)) !=NULL ) { -struct wb_completion *done = work->done; -trace_writeback_exec (wb, work); - wrote +=wb_writeback (wb, work); - -if (work->auto_free) -kfree (work); -if (done &&atomic_dec_and_test (&done->cnt)) -wake_up_all (&wb->bdi->wb_waitq); +finish_writeback_work (wb, work); }/* Signature · 1 line
2.12.0.246.ga2ecc84866-goog
Signature · 2 lines
Jan Kara <[email protected]> SUSE Labs, CR
Quoted 4 lines · Tahsin Erdogan
When
WB_registeredflag is not set,wb_queue_work()skips queuing the work, but does not perform the necessary clean up. In particular, ifwork->auto_freeis true, it should free the memory.
Quoted 8 lines
The leak condition can be reprouced by following these steps:
mount
/dev/sdb/mnt/sdb
/* In qemu console:device_delsdb */
umount/dev/sdbAbove will result in a
wb_queue_work()call on an unregistered wb and thus leak memory.
Added for 4.11, thanks Tahsin.
Signature · 1 line
Jens Axboe