io-uring.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v9 00/14] io-uring/xfs: support async buffered writes
@ 2022-06-16 21:22 Stefan Roesch
  2022-06-16 21:22 ` [PATCH v9 01/14] mm: Move starting of background writeback into the main balancing loop Stefan Roesch
                   ` (10 more replies)
  0 siblings, 11 replies; 19+ messages in thread
From: Stefan Roesch @ 2022-06-16 21:22 UTC (permalink / raw)
  To: io-uring, kernel-team, linux-mm, linux-xfs, linux-fsdevel
  Cc: shr, david, jack, hch, axboe, willy

This patch series adds support for async buffered writes when using both
xfs and io-uring. Currently io-uring only supports buffered writes in the
slow path, by processing them in the io workers. With this patch series it is
now possible to support buffered writes in the fast path. To be able to use
the fast path the required pages must be in the page cache, the required locks
in xfs can be granted immediately and no additional blocks need to be read
form disk.

Updating the inode can take time. An optimization has been implemented for
the time update. Time updates will be processed in the slow path. While there
is already a time update in process, other write requests for the same file,
can skip the update of the modification time.
  

Performance results:
  For fio the following results have been obtained with a queue depth of
  1 and 4k block size (runtime 600 secs):

                 sequential writes:
                 without patch           with patch      libaio     psync
  iops:              77k                    209k          195K       233K
  bw:               314MB/s                 854MB/s       790MB/s    953MB/s
  clat:            9600ns                   120ns         540ns     3000ns


For an io depth of 1, the new patch improves throughput by over three times
(compared to the exiting behavior, where buffered writes are processed by an
io-worker process) and also the latency is considerably reduced. To achieve the
same or better performance with the exisiting code an io depth of 4 is required.
Increasing the iodepth further does not lead to improvements.

In addition the latency of buffered write operations is reduced considerably.



Support for async buffered writes:

  To support async buffered writes the flag FMODE_BUF_WASYNC is introduced. In
  addition the check in generic_write_checks is modified to allow for async
  buffered writes that have this flag set.

  Changes to the iomap page create function to allow the caller to specify
  the gfp flags. Sets the IOMAP_NOWAIT flag in iomap if IOCB_NOWAIT has been set
  and specifies the requested gfp flags.

  Adds the iomap async buffered write support to the xfs iomap layer.
  Adds async buffered write support to the xfs iomap layer.

Support for async buffered write support and inode time modification

  Splits the functions for checking if the file privileges need to be removed in
  two functions: check function and a function for the removal of file privileges.
  The same split is also done for the function to update the file modification time.

  Implement an optimization that while a file modification time is pending other
  requests for the same file don't need to wait for the file modification update. 
  This avoids that a considerable number of buffered async write requests get
  punted.

  Take the ilock in nowait mode if async buffered writes are enabled and enable
  the async buffered writes optimization in io_uring.

Support for write throttling of async buffered writes:

  Add a no_wait parameter to the exisiting balance_dirty_pages() function. The
  function will return -EAGAIN if the parameter is true and write throttling is
  required.

  Add a new function called balance_dirty_pages_ratelimited_async() that will be
  invoked from iomap_write_iter() if an async buffered write is requested.
  
Enable async buffered write support in xfs
   This enables async buffered writes for xfs.


Testing:
  This patch has been tested with xfstests, fsx, fio and individual test programs.


Changes:
  V9:
  - Added comment for function balance_dirty_pages_ratelimited_flags()
  - checking return code for iop allocation in iomap_page_create()
  
  V8:
  - Reverted back changes to iomap_write_iter and used Mathew Wilcox code review
    recommendation with an additional change to revert the iterator.
  - Removed patch "fs: Optimization for concurrent file time updates" 
  - Setting flag value in file_modified_flags()
  - Removed additional spaces in comment in file_update_time()
  - Run fsx with 1 billion ops against the changes (Run passed)

  V7:
  - Change definition and if clause in " iomap: Add flags parameter to
    iomap_page_create()"
  - Added patch "iomap: Return error code from iomap_write_iter()" to address
    the problem Dave Chinner brought up: retrying memory allocation a second
    time when we are under memory pressure. 
  - Removed patch "xfs: Change function signature of xfs_ilock_iocb()"
  - Merged patch "xfs: Enable async buffered write support" with previous
    patch

  V6:
  - Pass in iter->flags to calls in iomap_page_create()
  
  V5:
  - Refreshed to 5.19-rc1
  - Merged patch 3 and patch 4
    "mm: Prepare balance_dirty_pages() for async buffered writes" and
    "mm: Add balance_dirty_pages_ratelimited_flags() function"
  - Reformatting long file in iomap_page_create()
  - Replacing gfp parameter with flags parameter in iomap_page_create()
    This makes sure that the gfp setting is done in one location.
  - Moved variable definition outside of loop in iomap_write_iter()
  - Merged patch 7 with patch 6.
  - Introduced __file_remove_privs() that get the iocb_flags passed in
    as an additional parameter
  - Removed file_needs_remove_privs() function
  - Renamed file_needs_update_time() inode_needs_update_time()
  - inode_needs_update_time() no longer passes the file pointer
  - Renamed file_modified_async() to file_modified_flags()
  - Made file_modified_flags() an internal function
  - Removed extern keyword in file_modified_async definition
  - Added kiocb_modified function.
  - Separate patch for changes to xfs_ilock_for_iomap()
  - Separate patch for changes to xfs_ilock_inode()
  - Renamed xfs_ilock_xfs_inode()n back to xfs_ilock_iocb()
  - Renamed flags parameter to iocb_flags in function xfs_ilock_iocb()
  - Used inode_set_flags() to manipulate inode flags in the function
    file_modified_flags()

  V4:
  - Reformat new code in generic_write_checks_count().
  - Removed patch that introduced new function iomap_page_create_gfp().
  - Add gfp parameter to iomap_page_create() and change all callers
    All users will enforce the number of blocks check
  - Removed confusing statement in iomap async buffer support patch
  - Replace no_wait variable in __iomap_write_begin with check of
    IOMAP_NOWAIT for easier readability.
  - Moved else if clause in __iomap_write_begin into else clause for
    easier readability
  - Removed the balance_dirty_pages_ratelimited_async() function and
    reverted back to the earlier version that used the function
    balance_dirty_pages_ratelimited_flags()
  - Introduced the flag BDP_ASYNC.
  - Renamed variable in iomap_write_iter from i_mapping to mapping.
  - Directly call balance_dirty_pages_ratelimited_flags() in the function
    iomap_write_iter().
  - Re-ordered the patches.
  
  V3:
  - Reformat new code in generic_write_checks_count() to line lengthof 80.
  - Remove if condition in __iomap_write_begin to maintain current behavior.
  - use GFP_NOWAIT flag in __iomap_write_begin
  - rename need_file_remove_privs() function to file_needs_remove_privs()
  - rename do_file_remove_privs to __file_remove_privs()
  - add kernel documentation to file_remove_privs() function
  - rework else if branch in file_remove_privs() function
  - add kernel documentation to file_modified() function
  - add kernel documentation to file_modified_async() function
  - rename err variable in file_update_time to ret
  - rename function need_file_update_time() to file_needs_update_time()
  - rename function do_file_update_time() to __file_update_time()
  - don't move check for FMODE_NOCMTIME in generic helper
  - reformat __file_update_time for easier reading
  - add kernel documentation to file_update_time() function
  - fix if in file_update_time from < to <=
  - move modification of inode flags from do_file_update_time to file_modified()
    When this function is called, the caller must hold the inode lock.
  - 3 new patches from Jan to add new no_wait flag to balance_dirty_pages(),
    remove patch 12 from previous series
  - Make balance_dirty_pages_ratelimited_flags() a static function
  - Add new balance_dirty_pages_ratelimited_async() function
  
  V2:
  - Remove atomic allocation
  - Use direct write in xfs_buffered_write_iomap_begin()
  - Use xfs_ilock_for_iomap() in xfs_buffered_write_iomap_begin()
  - Remove no_wait check at the end of xfs_buffered_write_iomap_begin() for
    the COW path.
  - Pass xfs_inode pointer to xfs_ilock_iocb and rename function to
    xfs_lock_xfs_inode
  - Replace existing uses of xfs_ilock_iocb with xfs_ilock_xfs_inode
  - Use xfs_ilock_xfs_inode in xfs_file_buffered_write()
  - Callers of xfs_ilock_for_iomap need to initialize lock mode. This is
    required so writes use an exclusive lock
  - Split of _balance_dirty_pages() from balance_dirty_pages() and return
    sleep time
  - Call _balance_dirty_pages() in balance_dirty_pages_ratelimited_flags()
  - Move call to balance_dirty_pages_ratelimited_flags() in iomap_write_iter()
    to the beginning of the loop



Jan Kara (3):
  mm: Move starting of background writeback into the main balancing loop
  mm: Move updates of dirty_exceeded into one place
  mm: Add balance_dirty_pages_ratelimited_flags() function

Stefan Roesch (11):
  iomap: Add flags parameter to iomap_page_create()
  iomap: Add async buffered write support
  iomap: Return -EAGAIN from iomap_write_iter()
  fs: Add check for async buffered writes to generic_write_checks
  fs: add __remove_file_privs() with flags parameter
  fs: Split off inode_needs_update_time and __file_update_time
  fs: Add async write file modification handling.
  io_uring: Add support for async buffered writes
  io_uring: Add tracepoint for short writes
  xfs: Specify lockmode when calling xfs_ilock_for_iomap()
  xfs: Add async buffered write support

 fs/inode.c                      | 168 +++++++++++++++++++++++---------
 fs/io_uring.c                   |  32 +++++-
 fs/iomap/buffered-io.c          |  71 +++++++++++---
 fs/read_write.c                 |   4 +-
 fs/xfs/xfs_file.c               |  11 +--
 fs/xfs/xfs_iomap.c              |  11 ++-
 include/linux/fs.h              |   4 +
 include/linux/writeback.h       |   7 ++
 include/trace/events/io_uring.h |  25 +++++
 mm/page-writeback.c             |  89 +++++++++++------
 10 files changed, 314 insertions(+), 108 deletions(-)


base-commit: b13baccc3850ca8b8cccbf8ed9912dbaa0fdf7f3
-- 
2.30.2


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v9 01/14] mm: Move starting of background writeback into the main balancing loop
  2022-06-16 21:22 [PATCH v9 00/14] io-uring/xfs: support async buffered writes Stefan Roesch
@ 2022-06-16 21:22 ` Stefan Roesch
  2022-06-16 21:22 ` [PATCH v9 02/14] mm: Move updates of dirty_exceeded into one place Stefan Roesch
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Stefan Roesch @ 2022-06-16 21:22 UTC (permalink / raw)
  To: io-uring, kernel-team, linux-mm, linux-xfs, linux-fsdevel
  Cc: shr, david, jack, hch, axboe, willy

From: Jan Kara <jack@suse.cz>

We start background writeback if we are over background threshold after
exiting the main loop in balance_dirty_pages(). This may result in
basing the decision on already stale values (we may have slept for
significant amount of time) and it is also inconvenient for refactoring
needed for async dirty throttling. Move the check into the main waiting
loop.

Signed-off-by: Jan Kara <jack@suse.cz>
Signed-off-by: Stefan Roesch <shr@fb.com>
---
 mm/page-writeback.c | 31 ++++++++++++++-----------------
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index 55c2776ae699..e59c523aed1a 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -1627,6 +1627,19 @@ static void balance_dirty_pages(struct bdi_writeback *wb,
 			}
 		}
 
+		/*
+		 * In laptop mode, we wait until hitting the higher threshold
+		 * before starting background writeout, and then write out all
+		 * the way down to the lower threshold.  So slow writers cause
+		 * minimal disk activity.
+		 *
+		 * In normal mode, we start background writeout at the lower
+		 * background_thresh, to keep the amount of dirty memory low.
+		 */
+		if (!laptop_mode && nr_reclaimable > gdtc->bg_thresh &&
+		    !writeback_in_progress(wb))
+			wb_start_background_writeback(wb);
+
 		/*
 		 * Throttle it only when the background writeback cannot
 		 * catch-up. This avoids (excessively) small writeouts
@@ -1657,6 +1670,7 @@ static void balance_dirty_pages(struct bdi_writeback *wb,
 			break;
 		}
 
+		/* Start writeback even when in laptop mode */
 		if (unlikely(!writeback_in_progress(wb)))
 			wb_start_background_writeback(wb);
 
@@ -1823,23 +1837,6 @@ static void balance_dirty_pages(struct bdi_writeback *wb,
 
 	if (!dirty_exceeded && wb->dirty_exceeded)
 		wb->dirty_exceeded = 0;
-
-	if (writeback_in_progress(wb))
-		return;
-
-	/*
-	 * In laptop mode, we wait until hitting the higher threshold before
-	 * starting background writeout, and then write out all the way down
-	 * to the lower threshold.  So slow writers cause minimal disk activity.
-	 *
-	 * In normal mode, we start background writeout at the lower
-	 * background_thresh, to keep the amount of dirty memory low.
-	 */
-	if (laptop_mode)
-		return;
-
-	if (nr_reclaimable > gdtc->bg_thresh)
-		wb_start_background_writeback(wb);
 }
 
 static DEFINE_PER_CPU(int, bdp_ratelimits);
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v9 02/14] mm: Move updates of dirty_exceeded into one place
  2022-06-16 21:22 [PATCH v9 00/14] io-uring/xfs: support async buffered writes Stefan Roesch
  2022-06-16 21:22 ` [PATCH v9 01/14] mm: Move starting of background writeback into the main balancing loop Stefan Roesch
@ 2022-06-16 21:22 ` Stefan Roesch
  2022-06-16 21:22 ` [PATCH v9 03/14] mm: Add balance_dirty_pages_ratelimited_flags() function Stefan Roesch
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Stefan Roesch @ 2022-06-16 21:22 UTC (permalink / raw)
  To: io-uring, kernel-team, linux-mm, linux-xfs, linux-fsdevel
  Cc: shr, david, jack, hch, axboe, willy

From: Jan Kara <jack@suse.cz>

Transition of wb->dirty_exceeded from 0 to 1 happens before we go to
sleep in balance_dirty_pages() while transition from 1 to 0 happens when
exiting from balance_dirty_pages(), possibly based on old values. This
does not make a lot of sense since wb->dirty_exceeded should simply
reflect whether wb is over dirty limit and so we should ratelimit
entering to balance_dirty_pages() less. Move the two updates together.

Signed-off-by: Jan Kara <jack@suse.cz>
Signed-off-by: Stefan Roesch <shr@fb.com>
---
 mm/page-writeback.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index e59c523aed1a..90b1998c16a1 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -1729,8 +1729,8 @@ static void balance_dirty_pages(struct bdi_writeback *wb,
 				sdtc = mdtc;
 		}
 
-		if (dirty_exceeded && !wb->dirty_exceeded)
-			wb->dirty_exceeded = 1;
+		if (dirty_exceeded != wb->dirty_exceeded)
+			wb->dirty_exceeded = dirty_exceeded;
 
 		if (time_is_before_jiffies(READ_ONCE(wb->bw_time_stamp) +
 					   BANDWIDTH_INTERVAL))
@@ -1834,9 +1834,6 @@ static void balance_dirty_pages(struct bdi_writeback *wb,
 		if (fatal_signal_pending(current))
 			break;
 	}
-
-	if (!dirty_exceeded && wb->dirty_exceeded)
-		wb->dirty_exceeded = 0;
 }
 
 static DEFINE_PER_CPU(int, bdp_ratelimits);
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v9 03/14] mm: Add balance_dirty_pages_ratelimited_flags() function
  2022-06-16 21:22 [PATCH v9 00/14] io-uring/xfs: support async buffered writes Stefan Roesch
  2022-06-16 21:22 ` [PATCH v9 01/14] mm: Move starting of background writeback into the main balancing loop Stefan Roesch
  2022-06-16 21:22 ` [PATCH v9 02/14] mm: Move updates of dirty_exceeded into one place Stefan Roesch
@ 2022-06-16 21:22 ` Stefan Roesch
  2022-06-20  0:53   ` kernel test robot
  2022-06-16 21:22 ` [PATCH v9 05/14] iomap: Add async buffered write support Stefan Roesch
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 19+ messages in thread
From: Stefan Roesch @ 2022-06-16 21:22 UTC (permalink / raw)
  To: io-uring, kernel-team, linux-mm, linux-xfs, linux-fsdevel
  Cc: shr, david, jack, hch, axboe, willy, Christoph Hellwig

From: Jan Kara <jack@suse.cz>

This adds the helper function balance_dirty_pages_ratelimited_flags().
It adds the parameter flags to balance_dirty_pages_ratelimited().
The flags parameter is passed to balance_dirty_pages(). For async
buffered writes the flag value will be BDP_ASYNC.

If balance_dirty_pages() gets called for async buffered write, we don't
want to wait. Instead we need to indicate to the caller that throttling
is needed so that it can stop writing and offload the rest of the write
to a context that can block.

The new helper function is also used by balance_dirty_pages_ratelimited().

Signed-off-by: Jan Kara <jack@suse.cz>
Signed-off-by: Stefan Roesch <shr@fb.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 include/linux/writeback.h |  7 ++++++
 mm/page-writeback.c       | 51 +++++++++++++++++++++++++++++++--------
 2 files changed, 48 insertions(+), 10 deletions(-)

diff --git a/include/linux/writeback.h b/include/linux/writeback.h
index da21d63f70e2..b8c9610c2313 100644
--- a/include/linux/writeback.h
+++ b/include/linux/writeback.h
@@ -364,7 +364,14 @@ void global_dirty_limits(unsigned long *pbackground, unsigned long *pdirty);
 unsigned long wb_calc_thresh(struct bdi_writeback *wb, unsigned long thresh);
 
 void wb_update_bandwidth(struct bdi_writeback *wb);
+
+/* Invoke balance dirty pages in async mode. */
+#define BDP_ASYNC 0x0001
+
 void balance_dirty_pages_ratelimited(struct address_space *mapping);
+int balance_dirty_pages_ratelimited_flags(struct address_space *mapping,
+		unsigned int flags);
+
 bool wb_over_bg_thresh(struct bdi_writeback *wb);
 
 typedef int (*writepage_t)(struct page *page, struct writeback_control *wbc,
diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index 90b1998c16a1..bfca433640fc 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -1554,8 +1554,8 @@ 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,
-				unsigned long pages_dirtied)
+static int balance_dirty_pages(struct bdi_writeback *wb,
+			       unsigned long pages_dirtied, unsigned int flags)
 {
 	struct dirty_throttle_control gdtc_stor = { GDTC_INIT(wb) };
 	struct dirty_throttle_control mdtc_stor = { MDTC_INIT(wb, &gdtc_stor) };
@@ -1575,6 +1575,7 @@ 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;
+	int ret = 0;
 
 	for (;;) {
 		unsigned long now = jiffies;
@@ -1803,6 +1804,10 @@ static void balance_dirty_pages(struct bdi_writeback *wb,
 					  period,
 					  pause,
 					  start_time);
+		if (flags & BDP_ASYNC) {
+			ret = -EAGAIN;
+			break;
+		}
 		__set_current_state(TASK_KILLABLE);
 		wb->dirty_sleep = now;
 		io_schedule_timeout(pause);
@@ -1834,6 +1839,7 @@ static void balance_dirty_pages(struct bdi_writeback *wb,
 		if (fatal_signal_pending(current))
 			break;
 	}
+	return ret;
 }
 
 static DEFINE_PER_CPU(int, bdp_ratelimits);
@@ -1855,27 +1861,34 @@ static DEFINE_PER_CPU(int, bdp_ratelimits);
 DEFINE_PER_CPU(int, dirty_throttle_leaks) = 0;
 
 /**
- * balance_dirty_pages_ratelimited - balance dirty memory state
- * @mapping: address_space which was dirtied
+ * balance_dirty_pages_ratelimited_flags - Balance dirty memory state.
+ * @mapping: address_space which was dirtied.
+ * @flags: BDP flags.
  *
  * Processes which are dirtying memory should call in here once for each page
  * which was newly dirtied.  The function will periodically check the system's
  * dirty state and will initiate writeback if needed.
  *
- * Once we're over the dirty memory limit we decrease the ratelimiting
- * by a lot, to prevent individual processes from overshooting the limit
- * by (ratelimit_pages) each.
+ * See balance_dirty_pages_ratelimited() for details.
+ *
+ * Return: If @flags contains BDP_ASYNC, it may return -EAGAIN to
+ * indicate that memory is out of balance and the caller must wait
+ * for I/O to complete.  Otherwise, it will return 0 to indicate
+ * that either memory was already in balance, or it was able to sleep
+ * until the amount of dirty memory returned to balance.
  */
-void balance_dirty_pages_ratelimited(struct address_space *mapping)
+int balance_dirty_pages_ratelimited_flags(struct address_space *mapping,
+					unsigned int flags)
 {
 	struct inode *inode = mapping->host;
 	struct backing_dev_info *bdi = inode_to_bdi(inode);
 	struct bdi_writeback *wb = NULL;
 	int ratelimit;
+	int ret = 0;
 	int *p;
 
 	if (!(bdi->capabilities & BDI_CAP_WRITEBACK))
-		return;
+		return ret;
 
 	if (inode_cgwb_enabled(inode))
 		wb = wb_get_create_current(bdi, GFP_KERNEL);
@@ -1915,9 +1928,27 @@ void balance_dirty_pages_ratelimited(struct address_space *mapping)
 	preempt_enable();
 
 	if (unlikely(current->nr_dirtied >= ratelimit))
-		balance_dirty_pages(wb, current->nr_dirtied);
+		balance_dirty_pages(wb, current->nr_dirtied, flags);
 
 	wb_put(wb);
+	return ret;
+}
+
+/**
+ * balance_dirty_pages_ratelimited - balance dirty memory state.
+ * @mapping: address_space which was dirtied.
+ *
+ * Processes which are dirtying memory should call in here once for each page
+ * which was newly dirtied.  The function will periodically check the system's
+ * dirty state and will initiate writeback if needed.
+ *
+ * Once we're over the dirty memory limit we decrease the ratelimiting
+ * by a lot, to prevent individual processes from overshooting the limit
+ * by (ratelimit_pages) each.
+ */
+void balance_dirty_pages_ratelimited(struct address_space *mapping)
+{
+	balance_dirty_pages_ratelimited_flags(mapping, 0);
 }
 EXPORT_SYMBOL(balance_dirty_pages_ratelimited);
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v9 05/14] iomap: Add async buffered write support
  2022-06-16 21:22 [PATCH v9 00/14] io-uring/xfs: support async buffered writes Stefan Roesch
                   ` (2 preceding siblings ...)
  2022-06-16 21:22 ` [PATCH v9 03/14] mm: Add balance_dirty_pages_ratelimited_flags() function Stefan Roesch
@ 2022-06-16 21:22 ` Stefan Roesch
  2022-06-16 21:22 ` [PATCH v9 06/14] iomap: Return -EAGAIN from iomap_write_iter() Stefan Roesch
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Stefan Roesch @ 2022-06-16 21:22 UTC (permalink / raw)
  To: io-uring, kernel-team, linux-mm, linux-xfs, linux-fsdevel
  Cc: shr, david, jack, hch, axboe, willy, Christoph Hellwig

This adds async buffered write support to iomap.

This replaces the call to balance_dirty_pages_ratelimited() with the
call to balance_dirty_pages_ratelimited_flags. This allows to specify if
the write request is async or not.

In addition this also moves the above function call to the beginning of
the function. If the function call is at the end of the function and the
decision is made to throttle writes, then there is no request that
io-uring can wait on. By moving it to the beginning of the function, the
write request is not issued, but returns -EAGAIN instead. io-uring will
punt the request and process it in the io-worker.

By moving the function call to the beginning of the function, the write
throttling will happen one page later.

Signed-off-by: Stefan Roesch <shr@fb.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/iomap/buffered-io.c | 33 ++++++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index 3c97b713f831..83cf093fcb92 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -559,6 +559,7 @@ static int __iomap_write_begin(const struct iomap_iter *iter, loff_t pos,
 	loff_t block_size = i_blocksize(iter->inode);
 	loff_t block_start = round_down(pos, block_size);
 	loff_t block_end = round_up(pos + len, block_size);
+	unsigned int nr_blocks = i_blocks_per_folio(iter->inode, folio);
 	size_t from = offset_in_folio(folio, pos), to = from + len;
 	size_t poff, plen;
 
@@ -567,6 +568,8 @@ static int __iomap_write_begin(const struct iomap_iter *iter, loff_t pos,
 	folio_clear_error(folio);
 
 	iop = iomap_page_create(iter->inode, folio, iter->flags);
+	if ((iter->flags & IOMAP_NOWAIT) && !iop && nr_blocks > 1)
+		return -EAGAIN;
 
 	do {
 		iomap_adjust_read_range(iter->inode, folio, &block_start,
@@ -584,7 +587,12 @@ static int __iomap_write_begin(const struct iomap_iter *iter, loff_t pos,
 				return -EIO;
 			folio_zero_segments(folio, poff, from, to, poff + plen);
 		} else {
-			int status = iomap_read_folio_sync(block_start, folio,
+			int status;
+
+			if (iter->flags & IOMAP_NOWAIT)
+				return -EAGAIN;
+
+			status = iomap_read_folio_sync(block_start, folio,
 					poff, plen, srcmap);
 			if (status)
 				return status;
@@ -613,6 +621,9 @@ static int iomap_write_begin(const struct iomap_iter *iter, loff_t pos,
 	unsigned fgp = FGP_LOCK | FGP_WRITE | FGP_CREAT | FGP_STABLE | FGP_NOFS;
 	int status = 0;
 
+	if (iter->flags & IOMAP_NOWAIT)
+		fgp |= FGP_NOWAIT;
+
 	BUG_ON(pos + len > iter->iomap.offset + iter->iomap.length);
 	if (srcmap != &iter->iomap)
 		BUG_ON(pos + len > srcmap->offset + srcmap->length);
@@ -632,7 +643,7 @@ static int iomap_write_begin(const struct iomap_iter *iter, loff_t pos,
 	folio = __filemap_get_folio(iter->inode->i_mapping, pos >> PAGE_SHIFT,
 			fgp, mapping_gfp_mask(iter->inode->i_mapping));
 	if (!folio) {
-		status = -ENOMEM;
+		status = (iter->flags & IOMAP_NOWAIT) ? -EAGAIN : -ENOMEM;
 		goto out_no_page;
 	}
 	if (pos + len > folio_pos(folio) + folio_size(folio))
@@ -750,6 +761,8 @@ static loff_t iomap_write_iter(struct iomap_iter *iter, struct iov_iter *i)
 	loff_t pos = iter->pos;
 	ssize_t written = 0;
 	long status = 0;
+	struct address_space *mapping = iter->inode->i_mapping;
+	unsigned int bdp_flags = (iter->flags & IOMAP_NOWAIT) ? BDP_ASYNC : 0;
 
 	do {
 		struct folio *folio;
@@ -762,6 +775,11 @@ static loff_t iomap_write_iter(struct iomap_iter *iter, struct iov_iter *i)
 		bytes = min_t(unsigned long, PAGE_SIZE - offset,
 						iov_iter_count(i));
 again:
+		status = balance_dirty_pages_ratelimited_flags(mapping,
+							       bdp_flags);
+		if (unlikely(status))
+			break;
+
 		if (bytes > length)
 			bytes = length;
 
@@ -770,6 +788,10 @@ static loff_t iomap_write_iter(struct iomap_iter *iter, struct iov_iter *i)
 		 * Otherwise there's a nasty deadlock on copying from the
 		 * same page as we're writing to, without it being marked
 		 * up-to-date.
+		 *
+		 * For async buffered writes the assumption is that the user
+		 * page has already been faulted in. This can be optimized by
+		 * faulting the user page.
 		 */
 		if (unlikely(fault_in_iov_iter_readable(i, bytes) == bytes)) {
 			status = -EFAULT;
@@ -781,7 +803,7 @@ static loff_t iomap_write_iter(struct iomap_iter *iter, struct iov_iter *i)
 			break;
 
 		page = folio_file_page(folio, pos >> PAGE_SHIFT);
-		if (mapping_writably_mapped(iter->inode->i_mapping))
+		if (mapping_writably_mapped(mapping))
 			flush_dcache_page(page);
 
 		copied = copy_page_from_iter_atomic(page, offset, bytes, i);
@@ -806,8 +828,6 @@ static loff_t iomap_write_iter(struct iomap_iter *iter, struct iov_iter *i)
 		pos += status;
 		written += status;
 		length -= status;
-
-		balance_dirty_pages_ratelimited(iter->inode->i_mapping);
 	} while (iov_iter_count(i) && length);
 
 	return written ? written : status;
@@ -825,6 +845,9 @@ iomap_file_buffered_write(struct kiocb *iocb, struct iov_iter *i,
 	};
 	int ret;
 
+	if (iocb->ki_flags & IOCB_NOWAIT)
+		iter.flags |= IOMAP_NOWAIT;
+
 	while ((ret = iomap_iter(&iter, ops)) > 0)
 		iter.processed = iomap_write_iter(&iter, i);
 	if (iter.pos == iocb->ki_pos)
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v9 06/14] iomap: Return -EAGAIN from iomap_write_iter()
  2022-06-16 21:22 [PATCH v9 00/14] io-uring/xfs: support async buffered writes Stefan Roesch
                   ` (3 preceding siblings ...)
  2022-06-16 21:22 ` [PATCH v9 05/14] iomap: Add async buffered write support Stefan Roesch
@ 2022-06-16 21:22 ` Stefan Roesch
  2022-06-16 21:22 ` [PATCH v9 09/14] fs: Split off inode_needs_update_time and __file_update_time Stefan Roesch
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Stefan Roesch @ 2022-06-16 21:22 UTC (permalink / raw)
  To: io-uring, kernel-team, linux-mm, linux-xfs, linux-fsdevel
  Cc: shr, david, jack, hch, axboe, willy

If iomap_write_iter() encounters -EAGAIN, return -EAGAIN to the caller.

Signed-off-by: Stefan Roesch <shr@fb.com>
---
 fs/iomap/buffered-io.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index 83cf093fcb92..f2e36240079f 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -830,7 +830,13 @@ static loff_t iomap_write_iter(struct iomap_iter *iter, struct iov_iter *i)
 		length -= status;
 	} while (iov_iter_count(i) && length);
 
-	return written ? written : status;
+	if (status == -EAGAIN) {
+		iov_iter_revert(i, written);
+		return -EAGAIN;
+	}
+	if (written)
+		return written;
+	return status;
 }
 
 ssize_t
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v9 09/14] fs: Split off inode_needs_update_time and __file_update_time
  2022-06-16 21:22 [PATCH v9 00/14] io-uring/xfs: support async buffered writes Stefan Roesch
                   ` (4 preceding siblings ...)
  2022-06-16 21:22 ` [PATCH v9 06/14] iomap: Return -EAGAIN from iomap_write_iter() Stefan Roesch
@ 2022-06-16 21:22 ` Stefan Roesch
  2022-06-16 21:22 ` [PATCH v9 11/14] io_uring: Add support for async buffered writes Stefan Roesch
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Stefan Roesch @ 2022-06-16 21:22 UTC (permalink / raw)
  To: io-uring, kernel-team, linux-mm, linux-xfs, linux-fsdevel
  Cc: shr, david, jack, hch, axboe, willy, Christian Brauner

This splits off the functions inode_needs_update_time() and
__file_update_time() from the function file_update_time().

This is required to support async buffered writes.
No intended functional changes in this patch.

Signed-off-by: Stefan Roesch <shr@fb.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Reviewed-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
 fs/inode.c | 76 +++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 50 insertions(+), 26 deletions(-)

diff --git a/fs/inode.c b/fs/inode.c
index a2e18379c8a6..ff726d99ecc7 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -2049,35 +2049,18 @@ int file_remove_privs(struct file *file)
 }
 EXPORT_SYMBOL(file_remove_privs);
 
-/**
- *	file_update_time	-	update mtime and ctime time
- *	@file: file accessed
- *
- *	Update the mtime and ctime members of an inode and mark the inode
- *	for writeback.  Note that this function is meant exclusively for
- *	usage in the file write path of filesystems, and filesystems may
- *	choose to explicitly ignore update via this function with the
- *	S_NOCMTIME inode flag, e.g. for network filesystem where these
- *	timestamps are handled by the server.  This can return an error for
- *	file systems who need to allocate space in order to update an inode.
- */
-
-int file_update_time(struct file *file)
+static int inode_needs_update_time(struct inode *inode, struct timespec64 *now)
 {
-	struct inode *inode = file_inode(file);
-	struct timespec64 now;
 	int sync_it = 0;
-	int ret;
 
 	/* First try to exhaust all avenues to not sync */
 	if (IS_NOCMTIME(inode))
 		return 0;
 
-	now = current_time(inode);
-	if (!timespec64_equal(&inode->i_mtime, &now))
+	if (!timespec64_equal(&inode->i_mtime, now))
 		sync_it = S_MTIME;
 
-	if (!timespec64_equal(&inode->i_ctime, &now))
+	if (!timespec64_equal(&inode->i_ctime, now))
 		sync_it |= S_CTIME;
 
 	if (IS_I_VERSION(inode) && inode_iversion_need_inc(inode))
@@ -2086,15 +2069,50 @@ int file_update_time(struct file *file)
 	if (!sync_it)
 		return 0;
 
-	/* Finally allowed to write? Takes lock. */
-	if (__mnt_want_write_file(file))
-		return 0;
+	return sync_it;
+}
+
+static int __file_update_time(struct file *file, struct timespec64 *now,
+			int sync_mode)
+{
+	int ret = 0;
+	struct inode *inode = file_inode(file);
 
-	ret = inode_update_time(inode, &now, sync_it);
-	__mnt_drop_write_file(file);
+	/* try to update time settings */
+	if (!__mnt_want_write_file(file)) {
+		ret = inode_update_time(inode, now, sync_mode);
+		__mnt_drop_write_file(file);
+	}
 
 	return ret;
 }
+
+/**
+ * file_update_time - update mtime and ctime time
+ * @file: file accessed
+ *
+ * Update the mtime and ctime members of an inode and mark the inode for
+ * writeback. Note that this function is meant exclusively for usage in
+ * the file write path of filesystems, and filesystems may choose to
+ * explicitly ignore updates via this function with the _NOCMTIME inode
+ * flag, e.g. for network filesystem where these imestamps are handled
+ * by the server. This can return an error for file systems who need to
+ * allocate space in order to update an inode.
+ *
+ * Return: 0 on success, negative errno on failure.
+ */
+int file_update_time(struct file *file)
+{
+	int ret;
+	struct inode *inode = file_inode(file);
+	struct timespec64 now = current_time(inode);
+
+	ret = inode_needs_update_time(inode, &now);
+	if (ret <= 0)
+		return ret;
+
+	return __file_update_time(file, &now, ret);
+}
 EXPORT_SYMBOL(file_update_time);
 
 /**
@@ -2111,6 +2129,8 @@ EXPORT_SYMBOL(file_update_time);
 int file_modified(struct file *file)
 {
 	int ret;
+	struct inode *inode = file_inode(file);
+	struct timespec64 now = current_time(inode);
 
 	/*
 	 * Clear the security bits if the process is not being run by root.
@@ -2123,7 +2143,11 @@ int file_modified(struct file *file)
 	if (unlikely(file->f_mode & FMODE_NOCMTIME))
 		return 0;
 
-	return file_update_time(file);
+	ret = inode_needs_update_time(inode, &now);
+	if (ret <= 0)
+		return ret;
+
+	return __file_update_time(file, &now, ret);
 }
 EXPORT_SYMBOL(file_modified);
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v9 11/14] io_uring: Add support for async buffered writes
  2022-06-16 21:22 [PATCH v9 00/14] io-uring/xfs: support async buffered writes Stefan Roesch
                   ` (5 preceding siblings ...)
  2022-06-16 21:22 ` [PATCH v9 09/14] fs: Split off inode_needs_update_time and __file_update_time Stefan Roesch
@ 2022-06-16 21:22 ` Stefan Roesch
  2022-06-16 21:22 ` [PATCH v9 12/14] io_uring: Add tracepoint for short writes Stefan Roesch
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Stefan Roesch @ 2022-06-16 21:22 UTC (permalink / raw)
  To: io-uring, kernel-team, linux-mm, linux-xfs, linux-fsdevel
  Cc: shr, david, jack, hch, axboe, willy

This enables the async buffered writes for the filesystems that support
async buffered writes in io-uring. Buffered writes are enabled for
blocks that are already in the page cache or can be acquired with noio.

Signed-off-by: Stefan Roesch <shr@fb.com>
---
 fs/io_uring.c | 29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 3aab4182fd89..22a0bb8c5fe5 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -4311,7 +4311,7 @@ static inline int io_iter_do_read(struct io_kiocb *req, struct iov_iter *iter)
 		return -EINVAL;
 }
 
-static bool need_read_all(struct io_kiocb *req)
+static bool need_complete_io(struct io_kiocb *req)
 {
 	return req->flags & REQ_F_ISREG ||
 		S_ISBLK(file_inode(req->file)->i_mode);
@@ -4440,7 +4440,7 @@ static int io_read(struct io_kiocb *req, unsigned int issue_flags)
 	} else if (ret == -EIOCBQUEUED) {
 		goto out_free;
 	} else if (ret == req->cqe.res || ret <= 0 || !force_nonblock ||
-		   (req->flags & REQ_F_NOWAIT) || !need_read_all(req)) {
+		   (req->flags & REQ_F_NOWAIT) || !need_complete_io(req)) {
 		/* read all, failed, already did sync or don't want to retry */
 		goto done;
 	}
@@ -4536,9 +4536,10 @@ static int io_write(struct io_kiocb *req, unsigned int issue_flags)
 		if (unlikely(!io_file_supports_nowait(req)))
 			goto copy_iov;
 
-		/* file path doesn't support NOWAIT for non-direct_IO */
-		if (force_nonblock && !(kiocb->ki_flags & IOCB_DIRECT) &&
-		    (req->flags & REQ_F_ISREG))
+		/* File path supports NOWAIT for non-direct_IO only for block devices. */
+		if (!(kiocb->ki_flags & IOCB_DIRECT) &&
+			!(kiocb->ki_filp->f_mode & FMODE_BUF_WASYNC) &&
+			(req->flags & REQ_F_ISREG))
 			goto copy_iov;
 
 		kiocb->ki_flags |= IOCB_NOWAIT;
@@ -4592,6 +4593,24 @@ static int io_write(struct io_kiocb *req, unsigned int issue_flags)
 		/* IOPOLL retry should happen for io-wq threads */
 		if (ret2 == -EAGAIN && (req->ctx->flags & IORING_SETUP_IOPOLL))
 			goto copy_iov;
+
+		if (ret2 != req->cqe.res && ret2 >= 0 && need_complete_io(req)) {
+			struct io_async_rw *rw;
+
+			/* This is a partial write. The file pos has already been
+			 * updated, setup the async struct to complete the request
+			 * in the worker. Also update bytes_done to account for
+			 * the bytes already written.
+			 */
+			iov_iter_save_state(&s->iter, &s->iter_state);
+			ret = io_setup_async_rw(req, iovec, s, true);
+
+			rw = req->async_data;
+			if (rw)
+				rw->bytes_done += ret2;
+
+			return ret ? ret : -EAGAIN;
+		}
 done:
 		kiocb_done(req, ret2, issue_flags);
 	} else {
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v9 12/14] io_uring: Add tracepoint for short writes
  2022-06-16 21:22 [PATCH v9 00/14] io-uring/xfs: support async buffered writes Stefan Roesch
                   ` (6 preceding siblings ...)
  2022-06-16 21:22 ` [PATCH v9 11/14] io_uring: Add support for async buffered writes Stefan Roesch
@ 2022-06-16 21:22 ` Stefan Roesch
  2022-06-16 21:22 ` [PATCH v9 14/14] xfs: Add async buffered write support Stefan Roesch
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Stefan Roesch @ 2022-06-16 21:22 UTC (permalink / raw)
  To: io-uring, kernel-team, linux-mm, linux-xfs, linux-fsdevel
  Cc: shr, david, jack, hch, axboe, willy

This adds the io_uring_short_write tracepoint to io_uring. A short write
is issued if not all pages that are required for a write are in the page
cache and the async buffered writes have to return EAGAIN.

Signed-off-by: Stefan Roesch <shr@fb.com>
---
 fs/io_uring.c                   |  3 +++
 include/trace/events/io_uring.h | 25 +++++++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 22a0bb8c5fe5..510c09192832 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -4597,6 +4597,9 @@ static int io_write(struct io_kiocb *req, unsigned int issue_flags)
 		if (ret2 != req->cqe.res && ret2 >= 0 && need_complete_io(req)) {
 			struct io_async_rw *rw;
 
+			trace_io_uring_short_write(req->ctx, kiocb->ki_pos - ret2,
+						req->cqe.res, ret2);
+
 			/* This is a partial write. The file pos has already been
 			 * updated, setup the async struct to complete the request
 			 * in the worker. Also update bytes_done to account for
diff --git a/include/trace/events/io_uring.h b/include/trace/events/io_uring.h
index 66fcc5a1a5b1..25df513660cc 100644
--- a/include/trace/events/io_uring.h
+++ b/include/trace/events/io_uring.h
@@ -600,6 +600,31 @@ TRACE_EVENT(io_uring_cqe_overflow,
 		  __entry->cflags, __entry->ocqe)
 );
 
+TRACE_EVENT(io_uring_short_write,
+
+	TP_PROTO(void *ctx, u64 fpos, u64 wanted, u64 got),
+
+	TP_ARGS(ctx, fpos, wanted, got),
+
+	TP_STRUCT__entry(
+		__field(void *,	ctx)
+		__field(u64,	fpos)
+		__field(u64,	wanted)
+		__field(u64,	got)
+	),
+
+	TP_fast_assign(
+		__entry->ctx	= ctx;
+		__entry->fpos	= fpos;
+		__entry->wanted	= wanted;
+		__entry->got	= got;
+	),
+
+	TP_printk("ring %p, fpos %lld, wanted %lld, got %lld",
+			  __entry->ctx, __entry->fpos,
+			  __entry->wanted, __entry->got)
+);
+
 #endif /* _TRACE_IO_URING_H */
 
 /* This part must be outside protection */
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v9 14/14] xfs: Add async buffered write support
  2022-06-16 21:22 [PATCH v9 00/14] io-uring/xfs: support async buffered writes Stefan Roesch
                   ` (7 preceding siblings ...)
  2022-06-16 21:22 ` [PATCH v9 12/14] io_uring: Add tracepoint for short writes Stefan Roesch
@ 2022-06-16 21:22 ` Stefan Roesch
  2022-06-22 17:41 ` [PATCH v9 00/14] io-uring/xfs: support async buffered writes Jens Axboe
  2022-06-22 22:27 ` Jens Axboe
  10 siblings, 0 replies; 19+ messages in thread
From: Stefan Roesch @ 2022-06-16 21:22 UTC (permalink / raw)
  To: io-uring, kernel-team, linux-mm, linux-xfs, linux-fsdevel
  Cc: shr, david, jack, hch, axboe, willy, Christoph Hellwig

This adds the async buffered write support to XFS. For async buffered
write requests, the request will return -EAGAIN if the ilock cannot be
obtained immediately.

Signed-off-by: Stefan Roesch <shr@fb.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_file.c  | 11 +++++------
 fs/xfs/xfs_iomap.c |  5 ++++-
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 5a171c0b244b..8d9b14d2b912 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -410,7 +410,7 @@ xfs_file_write_checks(
 		spin_unlock(&ip->i_flags_lock);
 
 out:
-	return file_modified(file);
+	return kiocb_modified(iocb);
 }
 
 static int
@@ -700,12 +700,11 @@ xfs_file_buffered_write(
 	bool			cleared_space = false;
 	unsigned int		iolock;
 
-	if (iocb->ki_flags & IOCB_NOWAIT)
-		return -EOPNOTSUPP;
-
 write_retry:
 	iolock = XFS_IOLOCK_EXCL;
-	xfs_ilock(ip, iolock);
+	ret = xfs_ilock_iocb(iocb, iolock);
+	if (ret)
+		return ret;
 
 	ret = xfs_file_write_checks(iocb, from, &iolock);
 	if (ret)
@@ -1165,7 +1164,7 @@ xfs_file_open(
 {
 	if (xfs_is_shutdown(XFS_M(inode->i_sb)))
 		return -EIO;
-	file->f_mode |= FMODE_NOWAIT | FMODE_BUF_RASYNC;
+	file->f_mode |= FMODE_NOWAIT | FMODE_BUF_RASYNC | FMODE_BUF_WASYNC;
 	return generic_file_open(inode, file);
 }
 
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index bcf7c3694290..5d50fed291b4 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -886,6 +886,7 @@ xfs_buffered_write_iomap_begin(
 	bool			eof = false, cow_eof = false, shared = false;
 	int			allocfork = XFS_DATA_FORK;
 	int			error = 0;
+	unsigned int		lockmode = XFS_ILOCK_EXCL;
 
 	if (xfs_is_shutdown(mp))
 		return -EIO;
@@ -897,7 +898,9 @@ xfs_buffered_write_iomap_begin(
 
 	ASSERT(!XFS_IS_REALTIME_INODE(ip));
 
-	xfs_ilock(ip, XFS_ILOCK_EXCL);
+	error = xfs_ilock_for_iomap(ip, flags, &lockmode);
+	if (error)
+		return error;
 
 	if (XFS_IS_CORRUPT(mp, !xfs_ifork_has_extents(&ip->i_df)) ||
 	    XFS_TEST_ERROR(false, mp, XFS_ERRTAG_BMAPIFORMAT)) {
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v9 03/14] mm: Add balance_dirty_pages_ratelimited_flags() function
  2022-06-16 21:22 ` [PATCH v9 03/14] mm: Add balance_dirty_pages_ratelimited_flags() function Stefan Roesch
@ 2022-06-20  0:53   ` kernel test robot
  0 siblings, 0 replies; 19+ messages in thread
From: kernel test robot @ 2022-06-20  0:53 UTC (permalink / raw)
  To: Stefan Roesch, io-uring, kernel-team, linux-mm, linux-xfs, linux-fsdevel
  Cc: kbuild-all, shr, david, jack, hch, axboe, willy, Christoph Hellwig

Hi Stefan,

I love your patch! Perhaps something to improve:

[auto build test WARNING on b13baccc3850ca8b8cccbf8ed9912dbaa0fdf7f3]

url:    https://github.com/intel-lab-lkp/linux/commits/Stefan-Roesch/io-uring-xfs-support-async-buffered-writes/20220617-060910
base:   b13baccc3850ca8b8cccbf8ed9912dbaa0fdf7f3
config: i386-randconfig-c021 (https://download.01.org/0day-ci/archive/20220620/202206200844.loRI4cvL-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-3) 11.3.0

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>


cocci warnings: (new ones prefixed by >>)
>> mm/page-writeback.c:1887:5-8: Unneeded variable: "ret". Return "0" on line 1891

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v9 00/14] io-uring/xfs: support async buffered writes
  2022-06-16 21:22 [PATCH v9 00/14] io-uring/xfs: support async buffered writes Stefan Roesch
                   ` (8 preceding siblings ...)
  2022-06-16 21:22 ` [PATCH v9 14/14] xfs: Add async buffered write support Stefan Roesch
@ 2022-06-22 17:41 ` Jens Axboe
  2022-06-22 19:35   ` Matthew Wilcox
  2022-06-22 21:24   ` Jan Kara
  2022-06-22 22:27 ` Jens Axboe
  10 siblings, 2 replies; 19+ messages in thread
From: Jens Axboe @ 2022-06-22 17:41 UTC (permalink / raw)
  To: Stefan Roesch, io-uring, kernel-team, linux-mm, linux-xfs, linux-fsdevel
  Cc: david, jack, hch, willy

Top posting - are people fine with queueing this up at this point? Will
need a bit of massaging for io_uring as certain things moved to another
file, but it's really minor. I'd do a separate topic branch for this.


On 6/16/22 3:22 PM, Stefan Roesch wrote:
> This patch series adds support for async buffered writes when using both
> xfs and io-uring. Currently io-uring only supports buffered writes in the
> slow path, by processing them in the io workers. With this patch series it is
> now possible to support buffered writes in the fast path. To be able to use
> the fast path the required pages must be in the page cache, the required locks
> in xfs can be granted immediately and no additional blocks need to be read
> form disk.
> 
> Updating the inode can take time. An optimization has been implemented for
> the time update. Time updates will be processed in the slow path. While there
> is already a time update in process, other write requests for the same file,
> can skip the update of the modification time.
>   
> 
> Performance results:
>   For fio the following results have been obtained with a queue depth of
>   1 and 4k block size (runtime 600 secs):
> 
>                  sequential writes:
>                  without patch           with patch      libaio     psync
>   iops:              77k                    209k          195K       233K
>   bw:               314MB/s                 854MB/s       790MB/s    953MB/s
>   clat:            9600ns                   120ns         540ns     3000ns
> 
> 
> For an io depth of 1, the new patch improves throughput by over three times
> (compared to the exiting behavior, where buffered writes are processed by an
> io-worker process) and also the latency is considerably reduced. To achieve the
> same or better performance with the exisiting code an io depth of 4 is required.
> Increasing the iodepth further does not lead to improvements.
> 
> In addition the latency of buffered write operations is reduced considerably.
> 
> 
> 
> Support for async buffered writes:
> 
>   To support async buffered writes the flag FMODE_BUF_WASYNC is introduced. In
>   addition the check in generic_write_checks is modified to allow for async
>   buffered writes that have this flag set.
> 
>   Changes to the iomap page create function to allow the caller to specify
>   the gfp flags. Sets the IOMAP_NOWAIT flag in iomap if IOCB_NOWAIT has been set
>   and specifies the requested gfp flags.
> 
>   Adds the iomap async buffered write support to the xfs iomap layer.
>   Adds async buffered write support to the xfs iomap layer.
> 
> Support for async buffered write support and inode time modification
> 
>   Splits the functions for checking if the file privileges need to be removed in
>   two functions: check function and a function for the removal of file privileges.
>   The same split is also done for the function to update the file modification time.
> 
>   Implement an optimization that while a file modification time is pending other
>   requests for the same file don't need to wait for the file modification update. 
>   This avoids that a considerable number of buffered async write requests get
>   punted.
> 
>   Take the ilock in nowait mode if async buffered writes are enabled and enable
>   the async buffered writes optimization in io_uring.
> 
> Support for write throttling of async buffered writes:
> 
>   Add a no_wait parameter to the exisiting balance_dirty_pages() function. The
>   function will return -EAGAIN if the parameter is true and write throttling is
>   required.
> 
>   Add a new function called balance_dirty_pages_ratelimited_async() that will be
>   invoked from iomap_write_iter() if an async buffered write is requested.
>   
> Enable async buffered write support in xfs
>    This enables async buffered writes for xfs.
> 
> 
> Testing:
>   This patch has been tested with xfstests, fsx, fio and individual test programs.
> 
> 
> Changes:
>   V9:
>   - Added comment for function balance_dirty_pages_ratelimited_flags()
>   - checking return code for iop allocation in iomap_page_create()
>   
>   V8:
>   - Reverted back changes to iomap_write_iter and used Mathew Wilcox code review
>     recommendation with an additional change to revert the iterator.
>   - Removed patch "fs: Optimization for concurrent file time updates" 
>   - Setting flag value in file_modified_flags()
>   - Removed additional spaces in comment in file_update_time()
>   - Run fsx with 1 billion ops against the changes (Run passed)
> 
>   V7:
>   - Change definition and if clause in " iomap: Add flags parameter to
>     iomap_page_create()"
>   - Added patch "iomap: Return error code from iomap_write_iter()" to address
>     the problem Dave Chinner brought up: retrying memory allocation a second
>     time when we are under memory pressure. 
>   - Removed patch "xfs: Change function signature of xfs_ilock_iocb()"
>   - Merged patch "xfs: Enable async buffered write support" with previous
>     patch
> 
>   V6:
>   - Pass in iter->flags to calls in iomap_page_create()
>   
>   V5:
>   - Refreshed to 5.19-rc1
>   - Merged patch 3 and patch 4
>     "mm: Prepare balance_dirty_pages() for async buffered writes" and
>     "mm: Add balance_dirty_pages_ratelimited_flags() function"
>   - Reformatting long file in iomap_page_create()
>   - Replacing gfp parameter with flags parameter in iomap_page_create()
>     This makes sure that the gfp setting is done in one location.
>   - Moved variable definition outside of loop in iomap_write_iter()
>   - Merged patch 7 with patch 6.
>   - Introduced __file_remove_privs() that get the iocb_flags passed in
>     as an additional parameter
>   - Removed file_needs_remove_privs() function
>   - Renamed file_needs_update_time() inode_needs_update_time()
>   - inode_needs_update_time() no longer passes the file pointer
>   - Renamed file_modified_async() to file_modified_flags()
>   - Made file_modified_flags() an internal function
>   - Removed extern keyword in file_modified_async definition
>   - Added kiocb_modified function.
>   - Separate patch for changes to xfs_ilock_for_iomap()
>   - Separate patch for changes to xfs_ilock_inode()
>   - Renamed xfs_ilock_xfs_inode()n back to xfs_ilock_iocb()
>   - Renamed flags parameter to iocb_flags in function xfs_ilock_iocb()
>   - Used inode_set_flags() to manipulate inode flags in the function
>     file_modified_flags()
> 
>   V4:
>   - Reformat new code in generic_write_checks_count().
>   - Removed patch that introduced new function iomap_page_create_gfp().
>   - Add gfp parameter to iomap_page_create() and change all callers
>     All users will enforce the number of blocks check
>   - Removed confusing statement in iomap async buffer support patch
>   - Replace no_wait variable in __iomap_write_begin with check of
>     IOMAP_NOWAIT for easier readability.
>   - Moved else if clause in __iomap_write_begin into else clause for
>     easier readability
>   - Removed the balance_dirty_pages_ratelimited_async() function and
>     reverted back to the earlier version that used the function
>     balance_dirty_pages_ratelimited_flags()
>   - Introduced the flag BDP_ASYNC.
>   - Renamed variable in iomap_write_iter from i_mapping to mapping.
>   - Directly call balance_dirty_pages_ratelimited_flags() in the function
>     iomap_write_iter().
>   - Re-ordered the patches.
>   
>   V3:
>   - Reformat new code in generic_write_checks_count() to line lengthof 80.
>   - Remove if condition in __iomap_write_begin to maintain current behavior.
>   - use GFP_NOWAIT flag in __iomap_write_begin
>   - rename need_file_remove_privs() function to file_needs_remove_privs()
>   - rename do_file_remove_privs to __file_remove_privs()
>   - add kernel documentation to file_remove_privs() function
>   - rework else if branch in file_remove_privs() function
>   - add kernel documentation to file_modified() function
>   - add kernel documentation to file_modified_async() function
>   - rename err variable in file_update_time to ret
>   - rename function need_file_update_time() to file_needs_update_time()
>   - rename function do_file_update_time() to __file_update_time()
>   - don't move check for FMODE_NOCMTIME in generic helper
>   - reformat __file_update_time for easier reading
>   - add kernel documentation to file_update_time() function
>   - fix if in file_update_time from < to <=
>   - move modification of inode flags from do_file_update_time to file_modified()
>     When this function is called, the caller must hold the inode lock.
>   - 3 new patches from Jan to add new no_wait flag to balance_dirty_pages(),
>     remove patch 12 from previous series
>   - Make balance_dirty_pages_ratelimited_flags() a static function
>   - Add new balance_dirty_pages_ratelimited_async() function
>   
>   V2:
>   - Remove atomic allocation
>   - Use direct write in xfs_buffered_write_iomap_begin()
>   - Use xfs_ilock_for_iomap() in xfs_buffered_write_iomap_begin()
>   - Remove no_wait check at the end of xfs_buffered_write_iomap_begin() for
>     the COW path.
>   - Pass xfs_inode pointer to xfs_ilock_iocb and rename function to
>     xfs_lock_xfs_inode
>   - Replace existing uses of xfs_ilock_iocb with xfs_ilock_xfs_inode
>   - Use xfs_ilock_xfs_inode in xfs_file_buffered_write()
>   - Callers of xfs_ilock_for_iomap need to initialize lock mode. This is
>     required so writes use an exclusive lock
>   - Split of _balance_dirty_pages() from balance_dirty_pages() and return
>     sleep time
>   - Call _balance_dirty_pages() in balance_dirty_pages_ratelimited_flags()
>   - Move call to balance_dirty_pages_ratelimited_flags() in iomap_write_iter()
>     to the beginning of the loop
> 
> 
> 
> Jan Kara (3):
>   mm: Move starting of background writeback into the main balancing loop
>   mm: Move updates of dirty_exceeded into one place
>   mm: Add balance_dirty_pages_ratelimited_flags() function
> 
> Stefan Roesch (11):
>   iomap: Add flags parameter to iomap_page_create()
>   iomap: Add async buffered write support
>   iomap: Return -EAGAIN from iomap_write_iter()
>   fs: Add check for async buffered writes to generic_write_checks
>   fs: add __remove_file_privs() with flags parameter
>   fs: Split off inode_needs_update_time and __file_update_time
>   fs: Add async write file modification handling.
>   io_uring: Add support for async buffered writes
>   io_uring: Add tracepoint for short writes
>   xfs: Specify lockmode when calling xfs_ilock_for_iomap()
>   xfs: Add async buffered write support
> 
>  fs/inode.c                      | 168 +++++++++++++++++++++++---------
>  fs/io_uring.c                   |  32 +++++-
>  fs/iomap/buffered-io.c          |  71 +++++++++++---
>  fs/read_write.c                 |   4 +-
>  fs/xfs/xfs_file.c               |  11 +--
>  fs/xfs/xfs_iomap.c              |  11 ++-
>  include/linux/fs.h              |   4 +
>  include/linux/writeback.h       |   7 ++
>  include/trace/events/io_uring.h |  25 +++++
>  mm/page-writeback.c             |  89 +++++++++++------
>  10 files changed, 314 insertions(+), 108 deletions(-)
> 
> 
> base-commit: b13baccc3850ca8b8cccbf8ed9912dbaa0fdf7f3


-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v9 00/14] io-uring/xfs: support async buffered writes
  2022-06-22 17:41 ` [PATCH v9 00/14] io-uring/xfs: support async buffered writes Jens Axboe
@ 2022-06-22 19:35   ` Matthew Wilcox
  2022-06-22 19:37     ` Jens Axboe
  2022-06-22 21:24   ` Jan Kara
  1 sibling, 1 reply; 19+ messages in thread
From: Matthew Wilcox @ 2022-06-22 19:35 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Stefan Roesch, io-uring, kernel-team, linux-mm, linux-xfs,
	linux-fsdevel, david, jack, hch

On Wed, Jun 22, 2022 at 11:41:14AM -0600, Jens Axboe wrote:
> Top posting - are people fine with queueing this up at this point? Will
> need a bit of massaging for io_uring as certain things moved to another
> file, but it's really minor. I'd do a separate topic branch for this.

I haven't had time to review this version, and I'm not likely to have
time before July 4th.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v9 00/14] io-uring/xfs: support async buffered writes
  2022-06-22 19:35   ` Matthew Wilcox
@ 2022-06-22 19:37     ` Jens Axboe
  0 siblings, 0 replies; 19+ messages in thread
From: Jens Axboe @ 2022-06-22 19:37 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Stefan Roesch, io-uring, kernel-team, linux-mm, linux-xfs,
	linux-fsdevel, david, jack, hch

On 6/22/22 1:35 PM, Matthew Wilcox wrote:
> On Wed, Jun 22, 2022 at 11:41:14AM -0600, Jens Axboe wrote:
>> Top posting - are people fine with queueing this up at this point? Will
>> need a bit of massaging for io_uring as certain things moved to another
>> file, but it's really minor. I'd do a separate topic branch for this.
> 
> I haven't had time to review this version, and I'm not likely to have
> time before July 4th.

I think Stefan addressed your previous concerns. But it's not like the
merge window is around the corner, but would be nice to get some -next
coverage in the mean time. So I don't think you being away should hold
up that part, and there will still be time to take a look once you're
back.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v9 00/14] io-uring/xfs: support async buffered writes
  2022-06-22 17:41 ` [PATCH v9 00/14] io-uring/xfs: support async buffered writes Jens Axboe
  2022-06-22 19:35   ` Matthew Wilcox
@ 2022-06-22 21:24   ` Jan Kara
  1 sibling, 0 replies; 19+ messages in thread
From: Jan Kara @ 2022-06-22 21:24 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Stefan Roesch, io-uring, kernel-team, linux-mm, linux-xfs,
	linux-fsdevel, david, jack, hch, willy

On Wed 22-06-22 11:41:14, Jens Axboe wrote:
> Top posting - are people fine with queueing this up at this point? Will
> need a bit of massaging for io_uring as certain things moved to another
> file, but it's really minor. I'd do a separate topic branch for this.

I have no objections to merging this. The parts I felt confident about
enough look OK to me (and have my reviewed-by tag).

								Honza

> On 6/16/22 3:22 PM, Stefan Roesch wrote:
> > This patch series adds support for async buffered writes when using both
> > xfs and io-uring. Currently io-uring only supports buffered writes in the
> > slow path, by processing them in the io workers. With this patch series it is
> > now possible to support buffered writes in the fast path. To be able to use
> > the fast path the required pages must be in the page cache, the required locks
> > in xfs can be granted immediately and no additional blocks need to be read
> > form disk.
> > 
> > Updating the inode can take time. An optimization has been implemented for
> > the time update. Time updates will be processed in the slow path. While there
> > is already a time update in process, other write requests for the same file,
> > can skip the update of the modification time.
> >   
> > 
> > Performance results:
> >   For fio the following results have been obtained with a queue depth of
> >   1 and 4k block size (runtime 600 secs):
> > 
> >                  sequential writes:
> >                  without patch           with patch      libaio     psync
> >   iops:              77k                    209k          195K       233K
> >   bw:               314MB/s                 854MB/s       790MB/s    953MB/s
> >   clat:            9600ns                   120ns         540ns     3000ns
> > 
> > 
> > For an io depth of 1, the new patch improves throughput by over three times
> > (compared to the exiting behavior, where buffered writes are processed by an
> > io-worker process) and also the latency is considerably reduced. To achieve the
> > same or better performance with the exisiting code an io depth of 4 is required.
> > Increasing the iodepth further does not lead to improvements.
> > 
> > In addition the latency of buffered write operations is reduced considerably.
> > 
> > 
> > 
> > Support for async buffered writes:
> > 
> >   To support async buffered writes the flag FMODE_BUF_WASYNC is introduced. In
> >   addition the check in generic_write_checks is modified to allow for async
> >   buffered writes that have this flag set.
> > 
> >   Changes to the iomap page create function to allow the caller to specify
> >   the gfp flags. Sets the IOMAP_NOWAIT flag in iomap if IOCB_NOWAIT has been set
> >   and specifies the requested gfp flags.
> > 
> >   Adds the iomap async buffered write support to the xfs iomap layer.
> >   Adds async buffered write support to the xfs iomap layer.
> > 
> > Support for async buffered write support and inode time modification
> > 
> >   Splits the functions for checking if the file privileges need to be removed in
> >   two functions: check function and a function for the removal of file privileges.
> >   The same split is also done for the function to update the file modification time.
> > 
> >   Implement an optimization that while a file modification time is pending other
> >   requests for the same file don't need to wait for the file modification update. 
> >   This avoids that a considerable number of buffered async write requests get
> >   punted.
> > 
> >   Take the ilock in nowait mode if async buffered writes are enabled and enable
> >   the async buffered writes optimization in io_uring.
> > 
> > Support for write throttling of async buffered writes:
> > 
> >   Add a no_wait parameter to the exisiting balance_dirty_pages() function. The
> >   function will return -EAGAIN if the parameter is true and write throttling is
> >   required.
> > 
> >   Add a new function called balance_dirty_pages_ratelimited_async() that will be
> >   invoked from iomap_write_iter() if an async buffered write is requested.
> >   
> > Enable async buffered write support in xfs
> >    This enables async buffered writes for xfs.
> > 
> > 
> > Testing:
> >   This patch has been tested with xfstests, fsx, fio and individual test programs.
> > 
> > 
> > Changes:
> >   V9:
> >   - Added comment for function balance_dirty_pages_ratelimited_flags()
> >   - checking return code for iop allocation in iomap_page_create()
> >   
> >   V8:
> >   - Reverted back changes to iomap_write_iter and used Mathew Wilcox code review
> >     recommendation with an additional change to revert the iterator.
> >   - Removed patch "fs: Optimization for concurrent file time updates" 
> >   - Setting flag value in file_modified_flags()
> >   - Removed additional spaces in comment in file_update_time()
> >   - Run fsx with 1 billion ops against the changes (Run passed)
> > 
> >   V7:
> >   - Change definition and if clause in " iomap: Add flags parameter to
> >     iomap_page_create()"
> >   - Added patch "iomap: Return error code from iomap_write_iter()" to address
> >     the problem Dave Chinner brought up: retrying memory allocation a second
> >     time when we are under memory pressure. 
> >   - Removed patch "xfs: Change function signature of xfs_ilock_iocb()"
> >   - Merged patch "xfs: Enable async buffered write support" with previous
> >     patch
> > 
> >   V6:
> >   - Pass in iter->flags to calls in iomap_page_create()
> >   
> >   V5:
> >   - Refreshed to 5.19-rc1
> >   - Merged patch 3 and patch 4
> >     "mm: Prepare balance_dirty_pages() for async buffered writes" and
> >     "mm: Add balance_dirty_pages_ratelimited_flags() function"
> >   - Reformatting long file in iomap_page_create()
> >   - Replacing gfp parameter with flags parameter in iomap_page_create()
> >     This makes sure that the gfp setting is done in one location.
> >   - Moved variable definition outside of loop in iomap_write_iter()
> >   - Merged patch 7 with patch 6.
> >   - Introduced __file_remove_privs() that get the iocb_flags passed in
> >     as an additional parameter
> >   - Removed file_needs_remove_privs() function
> >   - Renamed file_needs_update_time() inode_needs_update_time()
> >   - inode_needs_update_time() no longer passes the file pointer
> >   - Renamed file_modified_async() to file_modified_flags()
> >   - Made file_modified_flags() an internal function
> >   - Removed extern keyword in file_modified_async definition
> >   - Added kiocb_modified function.
> >   - Separate patch for changes to xfs_ilock_for_iomap()
> >   - Separate patch for changes to xfs_ilock_inode()
> >   - Renamed xfs_ilock_xfs_inode()n back to xfs_ilock_iocb()
> >   - Renamed flags parameter to iocb_flags in function xfs_ilock_iocb()
> >   - Used inode_set_flags() to manipulate inode flags in the function
> >     file_modified_flags()
> > 
> >   V4:
> >   - Reformat new code in generic_write_checks_count().
> >   - Removed patch that introduced new function iomap_page_create_gfp().
> >   - Add gfp parameter to iomap_page_create() and change all callers
> >     All users will enforce the number of blocks check
> >   - Removed confusing statement in iomap async buffer support patch
> >   - Replace no_wait variable in __iomap_write_begin with check of
> >     IOMAP_NOWAIT for easier readability.
> >   - Moved else if clause in __iomap_write_begin into else clause for
> >     easier readability
> >   - Removed the balance_dirty_pages_ratelimited_async() function and
> >     reverted back to the earlier version that used the function
> >     balance_dirty_pages_ratelimited_flags()
> >   - Introduced the flag BDP_ASYNC.
> >   - Renamed variable in iomap_write_iter from i_mapping to mapping.
> >   - Directly call balance_dirty_pages_ratelimited_flags() in the function
> >     iomap_write_iter().
> >   - Re-ordered the patches.
> >   
> >   V3:
> >   - Reformat new code in generic_write_checks_count() to line lengthof 80.
> >   - Remove if condition in __iomap_write_begin to maintain current behavior.
> >   - use GFP_NOWAIT flag in __iomap_write_begin
> >   - rename need_file_remove_privs() function to file_needs_remove_privs()
> >   - rename do_file_remove_privs to __file_remove_privs()
> >   - add kernel documentation to file_remove_privs() function
> >   - rework else if branch in file_remove_privs() function
> >   - add kernel documentation to file_modified() function
> >   - add kernel documentation to file_modified_async() function
> >   - rename err variable in file_update_time to ret
> >   - rename function need_file_update_time() to file_needs_update_time()
> >   - rename function do_file_update_time() to __file_update_time()
> >   - don't move check for FMODE_NOCMTIME in generic helper
> >   - reformat __file_update_time for easier reading
> >   - add kernel documentation to file_update_time() function
> >   - fix if in file_update_time from < to <=
> >   - move modification of inode flags from do_file_update_time to file_modified()
> >     When this function is called, the caller must hold the inode lock.
> >   - 3 new patches from Jan to add new no_wait flag to balance_dirty_pages(),
> >     remove patch 12 from previous series
> >   - Make balance_dirty_pages_ratelimited_flags() a static function
> >   - Add new balance_dirty_pages_ratelimited_async() function
> >   
> >   V2:
> >   - Remove atomic allocation
> >   - Use direct write in xfs_buffered_write_iomap_begin()
> >   - Use xfs_ilock_for_iomap() in xfs_buffered_write_iomap_begin()
> >   - Remove no_wait check at the end of xfs_buffered_write_iomap_begin() for
> >     the COW path.
> >   - Pass xfs_inode pointer to xfs_ilock_iocb and rename function to
> >     xfs_lock_xfs_inode
> >   - Replace existing uses of xfs_ilock_iocb with xfs_ilock_xfs_inode
> >   - Use xfs_ilock_xfs_inode in xfs_file_buffered_write()
> >   - Callers of xfs_ilock_for_iomap need to initialize lock mode. This is
> >     required so writes use an exclusive lock
> >   - Split of _balance_dirty_pages() from balance_dirty_pages() and return
> >     sleep time
> >   - Call _balance_dirty_pages() in balance_dirty_pages_ratelimited_flags()
> >   - Move call to balance_dirty_pages_ratelimited_flags() in iomap_write_iter()
> >     to the beginning of the loop
> > 
> > 
> > 
> > Jan Kara (3):
> >   mm: Move starting of background writeback into the main balancing loop
> >   mm: Move updates of dirty_exceeded into one place
> >   mm: Add balance_dirty_pages_ratelimited_flags() function
> > 
> > Stefan Roesch (11):
> >   iomap: Add flags parameter to iomap_page_create()
> >   iomap: Add async buffered write support
> >   iomap: Return -EAGAIN from iomap_write_iter()
> >   fs: Add check for async buffered writes to generic_write_checks
> >   fs: add __remove_file_privs() with flags parameter
> >   fs: Split off inode_needs_update_time and __file_update_time
> >   fs: Add async write file modification handling.
> >   io_uring: Add support for async buffered writes
> >   io_uring: Add tracepoint for short writes
> >   xfs: Specify lockmode when calling xfs_ilock_for_iomap()
> >   xfs: Add async buffered write support
> > 
> >  fs/inode.c                      | 168 +++++++++++++++++++++++---------
> >  fs/io_uring.c                   |  32 +++++-
> >  fs/iomap/buffered-io.c          |  71 +++++++++++---
> >  fs/read_write.c                 |   4 +-
> >  fs/xfs/xfs_file.c               |  11 +--
> >  fs/xfs/xfs_iomap.c              |  11 ++-
> >  include/linux/fs.h              |   4 +
> >  include/linux/writeback.h       |   7 ++
> >  include/trace/events/io_uring.h |  25 +++++
> >  mm/page-writeback.c             |  89 +++++++++++------
> >  10 files changed, 314 insertions(+), 108 deletions(-)
> > 
> > 
> > base-commit: b13baccc3850ca8b8cccbf8ed9912dbaa0fdf7f3
> 
> 
> -- 
> Jens Axboe
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v9 00/14] io-uring/xfs: support async buffered writes
  2022-06-16 21:22 [PATCH v9 00/14] io-uring/xfs: support async buffered writes Stefan Roesch
                   ` (9 preceding siblings ...)
  2022-06-22 17:41 ` [PATCH v9 00/14] io-uring/xfs: support async buffered writes Jens Axboe
@ 2022-06-22 22:27 ` Jens Axboe
  2022-06-23  0:29   ` Darrick J. Wong
  10 siblings, 1 reply; 19+ messages in thread
From: Jens Axboe @ 2022-06-22 22:27 UTC (permalink / raw)
  To: linux-mm, kernel-team, linux-xfs, io-uring, shr, linux-fsdevel
  Cc: david, hch, jack, willy

On Thu, 16 Jun 2022 14:22:07 -0700, Stefan Roesch wrote:
> This patch series adds support for async buffered writes when using both
> xfs and io-uring. Currently io-uring only supports buffered writes in the
> slow path, by processing them in the io workers. With this patch series it is
> now possible to support buffered writes in the fast path. To be able to use
> the fast path the required pages must be in the page cache, the required locks
> in xfs can be granted immediately and no additional blocks need to be read
> form disk.
> 
> [...]

Applied, thanks!

[01/14] mm: Move starting of background writeback into the main balancing loop
        commit: 29c36351d61fd08a2ed50a8028a7f752401dc88a
[02/14] mm: Move updates of dirty_exceeded into one place
        commit: a3fa4409eec3c094ad632ac1029094e061daf152
[03/14] mm: Add balance_dirty_pages_ratelimited_flags() function
        commit: 407619d2cef3b4d74565999a255a17cf5d559fa4
[04/14] iomap: Add flags parameter to iomap_page_create()
        commit: 49b5cd0830c1e9aa0f9a3717ac11a74ef23b9d4e
[05/14] iomap: Add async buffered write support
        commit: ccb885b4392143cea1bdbd8a0f35f0e6d909b114
[06/14] iomap: Return -EAGAIN from iomap_write_iter()
        commit: f0f9828d64393ea2ce87bd97f033051c8d7a337f
[07/14] fs: Add check for async buffered writes to generic_write_checks
        commit: cba06e23bc664ef419d389f1ed4cee523f468f8f
[08/14] fs: add __remove_file_privs() with flags parameter
        commit: 79d8ac83d6305fd8e996f720f955191e0d8c63b9
[09/14] fs: Split off inode_needs_update_time and __file_update_time
        commit: 1899b196859bac61ad71c3b3916e06de4b65246c
[10/14] fs: Add async write file modification handling.
        commit: 4705f225a56f216a59e09f7c2df16daabb7b4f76
[11/14] io_uring: Add support for async buffered writes
        commit: 6c8bbd82a43a0c7937e3e8e38cf46fcd90e15e68
[12/14] io_uring: Add tracepoint for short writes
        commit: 6c33dae4526ad079af6432aaf76827d0a27a9690
[13/14] xfs: Specify lockmode when calling xfs_ilock_for_iomap()
        commit: ddda2d473df70607bb456c515d984d05bf689790
[14/14] xfs: Add async buffered write support
        commit: e9cfc64a27f7a581b8c5d14da4efccfeae9c63bd

Best regards,
-- 
Jens Axboe



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v9 00/14] io-uring/xfs: support async buffered writes
  2022-06-22 22:27 ` Jens Axboe
@ 2022-06-23  0:29   ` Darrick J. Wong
  2022-06-23  0:50     ` Jens Axboe
  0 siblings, 1 reply; 19+ messages in thread
From: Darrick J. Wong @ 2022-06-23  0:29 UTC (permalink / raw)
  To: Jens Axboe
  Cc: linux-mm, kernel-team, linux-xfs, io-uring, shr, linux-fsdevel,
	david, hch, jack, willy

On Wed, Jun 22, 2022 at 04:27:07PM -0600, Jens Axboe wrote:
> On Thu, 16 Jun 2022 14:22:07 -0700, Stefan Roesch wrote:
> > This patch series adds support for async buffered writes when using both
> > xfs and io-uring. Currently io-uring only supports buffered writes in the
> > slow path, by processing them in the io workers. With this patch series it is
> > now possible to support buffered writes in the fast path. To be able to use
> > the fast path the required pages must be in the page cache, the required locks
> > in xfs can be granted immediately and no additional blocks need to be read
> > form disk.
> > 
> > [...]
> 
> Applied, thanks!
> 
> [01/14] mm: Move starting of background writeback into the main balancing loop
>         commit: 29c36351d61fd08a2ed50a8028a7f752401dc88a
> [02/14] mm: Move updates of dirty_exceeded into one place
>         commit: a3fa4409eec3c094ad632ac1029094e061daf152
> [03/14] mm: Add balance_dirty_pages_ratelimited_flags() function
>         commit: 407619d2cef3b4d74565999a255a17cf5d559fa4
> [04/14] iomap: Add flags parameter to iomap_page_create()
>         commit: 49b5cd0830c1e9aa0f9a3717ac11a74ef23b9d4e
> [05/14] iomap: Add async buffered write support
>         commit: ccb885b4392143cea1bdbd8a0f35f0e6d909b114
> [06/14] iomap: Return -EAGAIN from iomap_write_iter()
>         commit: f0f9828d64393ea2ce87bd97f033051c8d7a337f

I'm not sure /what/ happened here, but I never received the full V9
series, and neither did lore:

https://lore.kernel.org/linux-fsdevel/165593682792.161026.12974983413174964699.b4-ty@kernel.dk/T/#t

As it is, I already have my hands full trying to figure out why
generic/522 reports file corruption after 20 minutes of running on
vanilla 5.19-rc3, so I don't think I'm going to get to this for a while
either.

The v8 series looked all right to me, but ********* I hate how our
development process relies on such unreliable **** tooling.  I don't
think it's a /great/ idea to be pushing new code into -next when both
the xfs and pagecache maintainers are too busy to read the whole thing
through... but did hch actually RVB the whole thing prior to v9?

--D

> [07/14] fs: Add check for async buffered writes to generic_write_checks
>         commit: cba06e23bc664ef419d389f1ed4cee523f468f8f
> [08/14] fs: add __remove_file_privs() with flags parameter
>         commit: 79d8ac83d6305fd8e996f720f955191e0d8c63b9
> [09/14] fs: Split off inode_needs_update_time and __file_update_time
>         commit: 1899b196859bac61ad71c3b3916e06de4b65246c
> [10/14] fs: Add async write file modification handling.
>         commit: 4705f225a56f216a59e09f7c2df16daabb7b4f76
> [11/14] io_uring: Add support for async buffered writes
>         commit: 6c8bbd82a43a0c7937e3e8e38cf46fcd90e15e68
> [12/14] io_uring: Add tracepoint for short writes
>         commit: 6c33dae4526ad079af6432aaf76827d0a27a9690
> [13/14] xfs: Specify lockmode when calling xfs_ilock_for_iomap()
>         commit: ddda2d473df70607bb456c515d984d05bf689790
> [14/14] xfs: Add async buffered write support
>         commit: e9cfc64a27f7a581b8c5d14da4efccfeae9c63bd
> 
> Best regards,
> -- 
> Jens Axboe
> 
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v9 00/14] io-uring/xfs: support async buffered writes
  2022-06-23  0:29   ` Darrick J. Wong
@ 2022-06-23  0:50     ` Jens Axboe
  2022-06-23  6:29       ` Darrick J. Wong
  0 siblings, 1 reply; 19+ messages in thread
From: Jens Axboe @ 2022-06-23  0:50 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: linux-mm, kernel-team, linux-xfs, io-uring, shr, linux-fsdevel,
	david, hch, jack, willy

On 6/22/22 6:29 PM, Darrick J. Wong wrote:
> On Wed, Jun 22, 2022 at 04:27:07PM -0600, Jens Axboe wrote:
>> On Thu, 16 Jun 2022 14:22:07 -0700, Stefan Roesch wrote:
>>> This patch series adds support for async buffered writes when using both
>>> xfs and io-uring. Currently io-uring only supports buffered writes in the
>>> slow path, by processing them in the io workers. With this patch series it is
>>> now possible to support buffered writes in the fast path. To be able to use
>>> the fast path the required pages must be in the page cache, the required locks
>>> in xfs can be granted immediately and no additional blocks need to be read
>>> form disk.
>>>
>>> [...]
>>
>> Applied, thanks!
>>
>> [01/14] mm: Move starting of background writeback into the main balancing loop
>>         commit: 29c36351d61fd08a2ed50a8028a7f752401dc88a
>> [02/14] mm: Move updates of dirty_exceeded into one place
>>         commit: a3fa4409eec3c094ad632ac1029094e061daf152
>> [03/14] mm: Add balance_dirty_pages_ratelimited_flags() function
>>         commit: 407619d2cef3b4d74565999a255a17cf5d559fa4
>> [04/14] iomap: Add flags parameter to iomap_page_create()
>>         commit: 49b5cd0830c1e9aa0f9a3717ac11a74ef23b9d4e
>> [05/14] iomap: Add async buffered write support
>>         commit: ccb885b4392143cea1bdbd8a0f35f0e6d909b114
>> [06/14] iomap: Return -EAGAIN from iomap_write_iter()
>>         commit: f0f9828d64393ea2ce87bd97f033051c8d7a337f
> 
> I'm not sure /what/ happened here, but I never received the full V9
> series, and neither did lore:
> 
> https://lore.kernel.org/linux-fsdevel/165593682792.161026.12974983413174964699.b4-ty@kernel.dk/T/#t

Huh yes, didn't even notice that it's missing a few.

> As it is, I already have my hands full trying to figure out why
> generic/522 reports file corruption after 20 minutes of running on
> vanilla 5.19-rc3, so I don't think I'm going to get to this for a while
> either.
> 
> The v8 series looked all right to me, but ********* I hate how our
> development process relies on such unreliable **** tooling.  I don't

Me too, and the fact that email is getting worse and worse is not making
things any better...

> think it's a /great/ idea to be pushing new code into -next when both
> the xfs and pagecache maintainers are too busy to read the whole thing
> through... but did hch actually RVB the whole thing prior to v9?

Yes, hch did review the whole thing prior to v9. v9 has been pretty
quiet, but even v8 didn't have a whole lot. Which is to be expected for
a v9, this thing has been going for months.

We're only at -rc3 right now, so I think it's fine getting it some -next
exposure. It's not like it's getting pushed tomorrow, and if actual
concerns arise, let's just deal with them if that's the case. I'll check
in with folks before anything gets pushed certainly, I just don't think
it's fair to keep stalling when there are no real objections. Nothing
gets pushed unless the vested parties agree, obviously.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v9 00/14] io-uring/xfs: support async buffered writes
  2022-06-23  0:50     ` Jens Axboe
@ 2022-06-23  6:29       ` Darrick J. Wong
  0 siblings, 0 replies; 19+ messages in thread
From: Darrick J. Wong @ 2022-06-23  6:29 UTC (permalink / raw)
  To: Jens Axboe
  Cc: linux-mm, kernel-team, linux-xfs, io-uring, shr, linux-fsdevel,
	david, hch, jack, willy

On Wed, Jun 22, 2022 at 06:50:29PM -0600, Jens Axboe wrote:
> On 6/22/22 6:29 PM, Darrick J. Wong wrote:
> > On Wed, Jun 22, 2022 at 04:27:07PM -0600, Jens Axboe wrote:
> >> On Thu, 16 Jun 2022 14:22:07 -0700, Stefan Roesch wrote:
> >>> This patch series adds support for async buffered writes when using both
> >>> xfs and io-uring. Currently io-uring only supports buffered writes in the
> >>> slow path, by processing them in the io workers. With this patch series it is
> >>> now possible to support buffered writes in the fast path. To be able to use
> >>> the fast path the required pages must be in the page cache, the required locks
> >>> in xfs can be granted immediately and no additional blocks need to be read
> >>> form disk.
> >>>
> >>> [...]
> >>
> >> Applied, thanks!
> >>
> >> [01/14] mm: Move starting of background writeback into the main balancing loop
> >>         commit: 29c36351d61fd08a2ed50a8028a7f752401dc88a
> >> [02/14] mm: Move updates of dirty_exceeded into one place
> >>         commit: a3fa4409eec3c094ad632ac1029094e061daf152
> >> [03/14] mm: Add balance_dirty_pages_ratelimited_flags() function
> >>         commit: 407619d2cef3b4d74565999a255a17cf5d559fa4
> >> [04/14] iomap: Add flags parameter to iomap_page_create()
> >>         commit: 49b5cd0830c1e9aa0f9a3717ac11a74ef23b9d4e
> >> [05/14] iomap: Add async buffered write support
> >>         commit: ccb885b4392143cea1bdbd8a0f35f0e6d909b114
> >> [06/14] iomap: Return -EAGAIN from iomap_write_iter()
> >>         commit: f0f9828d64393ea2ce87bd97f033051c8d7a337f
> > 
> > I'm not sure /what/ happened here, but I never received the full V9
> > series, and neither did lore:
> > 
> > https://lore.kernel.org/linux-fsdevel/165593682792.161026.12974983413174964699.b4-ty@kernel.dk/T/#t
> 
> Huh yes, didn't even notice that it's missing a few.
> 
> > As it is, I already have my hands full trying to figure out why
> > generic/522 reports file corruption after 20 minutes of running on
> > vanilla 5.19-rc3, so I don't think I'm going to get to this for a while
> > either.
> > 
> > The v8 series looked all right to me, but ********* I hate how our
> > development process relies on such unreliable **** tooling.  I don't
> 
> Me too, and the fact that email is getting worse and worse is not making
> things any better...
> 
> > think it's a /great/ idea to be pushing new code into -next when both
> > the xfs and pagecache maintainers are too busy to read the whole thing
> > through... but did hch actually RVB the whole thing prior to v9?
> 
> Yes, hch did review the whole thing prior to v9. v9 has been pretty
> quiet, but even v8 didn't have a whole lot. Which is to be expected for
> a v9, this thing has been going for months.

<nod>

> We're only at -rc3 right now, so I think it's fine getting it some -next
> exposure. It's not like it's getting pushed tomorrow, and if actual
> concerns arise, let's just deal with them if that's the case. I'll check
> in with folks before anything gets pushed certainly, I just don't think
> it's fair to keep stalling when there are no real objections. Nothing
> gets pushed unless the vested parties agree, obviously.

Ok.  Would you or Stefan mind sending the whole v9 series again, so I
can have one more look?  Hopefully vger won't just eat the series a
third time... :(

Huh.  Ok.  LWN seems to have gotten the whole thing:
https://lwn.net/ml/linux-mm/20220616212221.2024518-1-shr@fb.com/

I'll go read that in the meantime.  $DEITY I hate email.

--D

> -- 
> Jens Axboe
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2022-06-23  6:29 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-16 21:22 [PATCH v9 00/14] io-uring/xfs: support async buffered writes Stefan Roesch
2022-06-16 21:22 ` [PATCH v9 01/14] mm: Move starting of background writeback into the main balancing loop Stefan Roesch
2022-06-16 21:22 ` [PATCH v9 02/14] mm: Move updates of dirty_exceeded into one place Stefan Roesch
2022-06-16 21:22 ` [PATCH v9 03/14] mm: Add balance_dirty_pages_ratelimited_flags() function Stefan Roesch
2022-06-20  0:53   ` kernel test robot
2022-06-16 21:22 ` [PATCH v9 05/14] iomap: Add async buffered write support Stefan Roesch
2022-06-16 21:22 ` [PATCH v9 06/14] iomap: Return -EAGAIN from iomap_write_iter() Stefan Roesch
2022-06-16 21:22 ` [PATCH v9 09/14] fs: Split off inode_needs_update_time and __file_update_time Stefan Roesch
2022-06-16 21:22 ` [PATCH v9 11/14] io_uring: Add support for async buffered writes Stefan Roesch
2022-06-16 21:22 ` [PATCH v9 12/14] io_uring: Add tracepoint for short writes Stefan Roesch
2022-06-16 21:22 ` [PATCH v9 14/14] xfs: Add async buffered write support Stefan Roesch
2022-06-22 17:41 ` [PATCH v9 00/14] io-uring/xfs: support async buffered writes Jens Axboe
2022-06-22 19:35   ` Matthew Wilcox
2022-06-22 19:37     ` Jens Axboe
2022-06-22 21:24   ` Jan Kara
2022-06-22 22:27 ` Jens Axboe
2022-06-23  0:29   ` Darrick J. Wong
2022-06-23  0:50     ` Jens Axboe
2022-06-23  6:29       ` Darrick J. Wong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).