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

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

[Luoxi]

Hi, Jerry

On 2024/11/30 14:34:31 +0800，"Jerry" <jerrydeng079@163.com> wrote:

>Signed-off-by: Jerry <jerrydeng079@163.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

(no subject)

[Jerry]

Signed-off-by: Jerry <jerrydeng79@163.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 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 (0 == bwce.bdi_unregister)
+		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( 0 > balance_dirty_pages(wb, current->nr_dirtied))
+			errseq_set(&(mapping->wb_err), -EIO);
+
+	}
 
 	wb_put(wb);
 }
-- 
2.43.0