[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <em69028b68-d2c5-485d-a2ec-463c4673e723@f742d22a.com>
Date: Sat, 30 Nov 2024 07:02:20 +0000
From: Luoxi <kaixa@...oview.com>
To: Jerry <jerrydeng079@....com>
Cc: linux-mm@...ck.org, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, Jerry <jerrydeng079@....com>
Subject: Re: [PATCH] Fix issue: Writing a block devices case dead loop.
generic_perform_write()-> balance_dirty_pages_ratelimited()->
balance_dirty_pages() At this point, if the block device removed, the process
may trapped in a dead loop. and the memory of the bdi device hass also been
released. I found this issue through SCSI devices
Hi, Jerry
On 2024/11/30 14:34:31 +0800,"Jerry" <jerrydeng079@....com> wrote:
>Signed-off-by: Jerry <jerrydeng079@....com>
>---
> mm/backing-dev.c | 1 +
> mm/filemap.c | 6 ++++-
> mm/page-writeback.c | 61 +++++++++++++++++++++++++++++++++++++++++----
> 3 files changed, 62 insertions(+), 6 deletions(-)
>
>diff --git a/mm/backing-dev.c b/mm/backing-dev.c
>index dd08ab928..0b86bd980 100755
>--- a/mm/backing-dev.c
>+++ b/mm/backing-dev.c
>@@ -878,6 +878,7 @@ void bdi_unregister(struct backing_dev_info *bdi)
> /* make sure nobody finds us on the bdi_list anymore */
> bdi_remove_from_list(bdi);
> wb_shutdown(&bdi->wb);
>+ wake_up(&(bdi->wb_waitq));
> cgwb_bdi_unregister(bdi);
>
> /*
>diff --git a/mm/filemap.c b/mm/filemap.c
>index 3b0d8c6dd..3282840f0 100755
>--- a/mm/filemap.c
>+++ b/mm/filemap.c
>@@ -3300,6 +3300,7 @@ ssize_t generic_perform_write(struct file *file,
> long status = 0;
> ssize_t written = 0;
> unsigned int flags = 0;
>+ errseq_t err = 0;
>
> do {
> struct page *page;
>@@ -3368,8 +3369,11 @@ ssize_t generic_perform_write(struct file *file,
> }
> pos += copied;
> written += copied;
>-
> balance_dirty_pages_ratelimited(mapping);
>+ err = errseq_check(&mapping->wb_err, 0);
>+ if (err)
>+ return err;
>+
> } while (iov_iter_count(i));
>
> return written ? written : status;
>diff --git a/mm/page-writeback.c b/mm/page-writeback.c
>index b2c916474..e013a6d01 100755
>--- a/mm/page-writeback.c
>+++ b/mm/page-writeback.c
>@@ -146,6 +146,16 @@ struct dirty_throttle_control {
> unsigned long pos_ratio;
> };
>
>+
>+
>+struct bdi_wq_callback_entry {
>+
>+ struct task_struct *tsk;
>+ struct wait_queue_entry wq_entry;
>+ int bdi_unregister;
>+};
>+
>+
> /*
> * Length of period for aging writeout fractions of bdis. This is an
> * arbitrarily chosen number. The longer the period, the slower fractions will
>@@ -1567,6 +1577,22 @@ static inline void wb_dirty_limits(struct dirty_throttle_control *dtc)
> }
> }
>
>+
>+static int wake_up_bdi_waitq(wait_queue_entry_t *wait, unsigned int mode,
>+ int sync, void *key)
>+{
>+
>+ struct bdi_wq_callback_entry *bwce =
>+ container_of(wait, struct bdi_wq_callback_entry, wq_entry);
>+
>+ bwce->bdi_unregister = 1;
>+ if (bwce->tsk)
>+ wake_up_process(bwce->tsk);
>+
>+ return 0;
>+}
>+
>+
> /*
> * balance_dirty_pages() must be called by processes which are generating dirty
> * data. It looks at the number of dirty pages in the machine and will force
>@@ -1574,7 +1600,7 @@ static inline void wb_dirty_limits(struct dirty_throttle_control *dtc)
> * If we're over `background_thresh' then the writeback threads are woken to
> * perform some writeout.
> */
>-static void balance_dirty_pages(struct bdi_writeback *wb,
>+static int balance_dirty_pages(struct bdi_writeback *wb,
> unsigned long pages_dirtied)
> {
> struct dirty_throttle_control gdtc_stor = { GDTC_INIT(wb) };
>@@ -1595,6 +1621,16 @@ static void balance_dirty_pages(struct bdi_writeback *wb,
> struct backing_dev_info *bdi = wb->bdi;
> bool strictlimit = bdi->capabilities & BDI_CAP_STRICTLIMIT;
> unsigned long start_time = jiffies;
>+ struct bdi_wq_callback_entry bwce = {NULL};
>+ int ret = 0;
>+
>+
>+ if (!test_bit(WB_registered, &wb->state))
>+ return -EIO;
>+
>+ init_waitqueue_func_entry(&(bwce.wq_entry), wake_up_bdi_waitq);
>+ bwce.tsk = current;
>+ add_wait_queue(&(bdi->wb_waitq), &(bwce.wq_entry));
>
> for (;;) {
> unsigned long now = jiffies;
>@@ -1816,6 +1852,12 @@ static void balance_dirty_pages(struct bdi_writeback *wb,
> wb->dirty_sleep = now;
> io_schedule_timeout(pause);
>
>+ /* bid is unregister NULL, all bdi memory is illegal */
>+ if (bwce.bdi_unregister) {
>+ ret = -EIO;
>+ break;
>+ }
>+
> current->dirty_paused_when = now + pause;
> current->nr_dirtied = 0;
> current->nr_dirtied_pause = nr_dirtied_pause;
>@@ -1843,12 +1885,15 @@ static void balance_dirty_pages(struct bdi_writeback *wb,
> if (fatal_signal_pending(current))
> break;
> }
>+
>+ if (bwce.bdi_unregister == 0)
>+ remove_wait_queue(&(bdi->wb_waitq), &(bwce.wq_entry));
>
> if (!dirty_exceeded && wb->dirty_exceeded)
> wb->dirty_exceeded = 0;
>
> if (writeback_in_progress(wb))
>- return;
>+ return ret;
>
> /*
> * In laptop mode, we wait until hitting the higher threshold before
>@@ -1859,10 +1904,12 @@ static void balance_dirty_pages(struct bdi_writeback *wb,
> * background_thresh, to keep the amount of dirty memory low.
> */
> if (laptop_mode)
>- return;
>+ return ret;
>
> if (nr_reclaimable > gdtc->bg_thresh)
> wb_start_background_writeback(wb);
>+
>+ return ret;
> }
>
> static DEFINE_PER_CPU(int, bdp_ratelimits);
>@@ -1944,8 +1991,12 @@ void balance_dirty_pages_ratelimited(struct address_space *mapping)
> }
> preempt_enable();
>
>- if (unlikely(current->nr_dirtied >= ratelimit))
>- balance_dirty_pages(wb, current->nr_dirtied);
>+ if (unlikely(current->nr_dirtied >= ratelimit)) {
>+
>+ if (balance_dirty_pages(wb, current->nr_dirtied) < 0)
>+ errseq_set(&(mapping->wb_err), -EIO);
>+
>+ }
>
> wb_put(wb);
> }
>--
>2.43.0
>
>
I think the title may be misplaced and needs to be described in concise
sentences. :)
For detailed process, please refer to this link:
https://docs.kernel.org/process/submitting-patches.html
Regards,
Luoxi
Powered by blists - more mailing lists