* [PATCH 1/2] xfs: initialize iomap->flags in xfs_bmbt_to_iomap
2019-07-30 1:18 [PATCH v4 0/2] xfs: use new list helpers in writeback code Darrick J. Wong
@ 2019-07-30 1:18 ` Darrick J. Wong
2019-07-30 1:18 ` [PATCH 2/2] xfs: refactor the ioend merging code Darrick J. Wong
1 sibling, 0 replies; 3+ messages in thread
From: Darrick J. Wong @ 2019-07-30 1:18 UTC (permalink / raw)
To: hch, darrick.wong
Cc: linux-xfs, linux-fsdevel, Damien.LeMoal, Christoph Hellwig, agruenba
From: Christoph Hellwig <hch@lst.de>
Currently we don't overwrite the flags field in the iomap in
xfs_bmbt_to_iomap. This works fine with 0-initialized iomaps on stack,
but is harmful once we want to be able to reuse an iomap in the
writeback code. Replace the shared paramter with a set of initial
flags an thus ensures the flags field is always reinitialized.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/xfs_iomap.c | 28 +++++++++++++++++-----------
fs/xfs/xfs_iomap.h | 2 +-
fs/xfs/xfs_pnfs.c | 2 +-
3 files changed, 19 insertions(+), 13 deletions(-)
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 434ff589f0fc..d340db666c06 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -54,7 +54,7 @@ xfs_bmbt_to_iomap(
struct xfs_inode *ip,
struct iomap *iomap,
struct xfs_bmbt_irec *imap,
- bool shared)
+ u16 flags)
{
struct xfs_mount *mp = ip->i_mount;
@@ -79,12 +79,11 @@ xfs_bmbt_to_iomap(
iomap->length = XFS_FSB_TO_B(mp, imap->br_blockcount);
iomap->bdev = xfs_find_bdev_for_inode(VFS_I(ip));
iomap->dax_dev = xfs_find_daxdev_for_inode(VFS_I(ip));
+ iomap->flags = flags;
if (xfs_ipincount(ip) &&
(ip->i_itemp->ili_fsync_fields & ~XFS_ILOG_TIMESTAMP))
iomap->flags |= IOMAP_F_DIRTY;
- if (shared)
- iomap->flags |= IOMAP_F_SHARED;
return 0;
}
@@ -540,6 +539,7 @@ xfs_file_iomap_begin_delay(
struct xfs_iext_cursor icur, ccur;
xfs_fsblock_t prealloc_blocks = 0;
bool eof = false, cow_eof = false, shared = false;
+ u16 iomap_flags = 0;
int whichfork = XFS_DATA_FORK;
int error = 0;
@@ -708,7 +708,7 @@ xfs_file_iomap_begin_delay(
* them out if the write happens to fail.
*/
if (whichfork == XFS_DATA_FORK) {
- iomap->flags |= IOMAP_F_NEW;
+ iomap_flags |= IOMAP_F_NEW;
trace_xfs_iomap_alloc(ip, offset, count, whichfork, &imap);
} else {
trace_xfs_iomap_alloc(ip, offset, count, whichfork, &cmap);
@@ -718,14 +718,17 @@ xfs_file_iomap_begin_delay(
if (imap.br_startoff > offset_fsb) {
xfs_trim_extent(&cmap, offset_fsb,
imap.br_startoff - offset_fsb);
- error = xfs_bmbt_to_iomap(ip, iomap, &cmap, true);
+ error = xfs_bmbt_to_iomap(ip, iomap, &cmap,
+ IOMAP_F_SHARED);
goto out_unlock;
}
/* ensure we only report blocks we have a reservation for */
xfs_trim_extent(&imap, cmap.br_startoff, cmap.br_blockcount);
shared = true;
}
- error = xfs_bmbt_to_iomap(ip, iomap, &imap, shared);
+ if (shared)
+ iomap_flags |= IOMAP_F_SHARED;
+ error = xfs_bmbt_to_iomap(ip, iomap, &imap, iomap_flags);
out_unlock:
xfs_iunlock(ip, XFS_ILOCK_EXCL);
return error;
@@ -933,6 +936,7 @@ xfs_file_iomap_begin(
xfs_fileoff_t offset_fsb, end_fsb;
int nimaps = 1, error = 0;
bool shared = false;
+ u16 iomap_flags = 0;
unsigned lockmode;
if (XFS_FORCED_SHUTDOWN(mp))
@@ -1048,11 +1052,13 @@ xfs_file_iomap_begin(
if (error)
return error;
- iomap->flags |= IOMAP_F_NEW;
+ iomap_flags |= IOMAP_F_NEW;
trace_xfs_iomap_alloc(ip, offset, length, XFS_DATA_FORK, &imap);
out_finish:
- return xfs_bmbt_to_iomap(ip, iomap, &imap, shared);
+ if (shared)
+ iomap_flags |= IOMAP_F_SHARED;
+ return xfs_bmbt_to_iomap(ip, iomap, &imap, iomap_flags);
out_found:
ASSERT(nimaps);
@@ -1196,7 +1202,7 @@ xfs_seek_iomap_begin(
if (data_fsb < cow_fsb + cmap.br_blockcount)
end_fsb = min(end_fsb, data_fsb);
xfs_trim_extent(&cmap, offset_fsb, end_fsb);
- error = xfs_bmbt_to_iomap(ip, iomap, &cmap, true);
+ error = xfs_bmbt_to_iomap(ip, iomap, &cmap, IOMAP_F_SHARED);
/*
* This is a COW extent, so we must probe the page cache
* because there could be dirty page cache being backed
@@ -1218,7 +1224,7 @@ xfs_seek_iomap_begin(
imap.br_state = XFS_EXT_NORM;
done:
xfs_trim_extent(&imap, offset_fsb, end_fsb);
- error = xfs_bmbt_to_iomap(ip, iomap, &imap, false);
+ error = xfs_bmbt_to_iomap(ip, iomap, &imap, 0);
out_unlock:
xfs_iunlock(ip, lockmode);
return error;
@@ -1264,7 +1270,7 @@ xfs_xattr_iomap_begin(
if (error)
return error;
ASSERT(nimaps);
- return xfs_bmbt_to_iomap(ip, iomap, &imap, false);
+ return xfs_bmbt_to_iomap(ip, iomap, &imap, 0);
}
const struct iomap_ops xfs_xattr_iomap_ops = {
diff --git a/fs/xfs/xfs_iomap.h b/fs/xfs/xfs_iomap.h
index 5c2f6aa6d78f..71d0ae460c44 100644
--- a/fs/xfs/xfs_iomap.h
+++ b/fs/xfs/xfs_iomap.h
@@ -16,7 +16,7 @@ int xfs_iomap_write_direct(struct xfs_inode *, xfs_off_t, size_t,
int xfs_iomap_write_unwritten(struct xfs_inode *, xfs_off_t, xfs_off_t, bool);
int xfs_bmbt_to_iomap(struct xfs_inode *, struct iomap *,
- struct xfs_bmbt_irec *, bool shared);
+ struct xfs_bmbt_irec *, u16);
xfs_extlen_t xfs_eof_alignment(struct xfs_inode *ip, xfs_extlen_t extsize);
static inline xfs_filblks_t
diff --git a/fs/xfs/xfs_pnfs.c b/fs/xfs/xfs_pnfs.c
index 0c954cad7449..d1e293b62902 100644
--- a/fs/xfs/xfs_pnfs.c
+++ b/fs/xfs/xfs_pnfs.c
@@ -178,7 +178,7 @@ xfs_fs_map_blocks(
}
xfs_iunlock(ip, XFS_IOLOCK_EXCL);
- error = xfs_bmbt_to_iomap(ip, iomap, &imap, false);
+ error = xfs_bmbt_to_iomap(ip, iomap, &imap, 0);
*device_generation = mp->m_generation;
return error;
out_unlock:
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH 2/2] xfs: refactor the ioend merging code
2019-07-30 1:18 [PATCH v4 0/2] xfs: use new list helpers in writeback code Darrick J. Wong
2019-07-30 1:18 ` [PATCH 1/2] xfs: initialize iomap->flags in xfs_bmbt_to_iomap Darrick J. Wong
@ 2019-07-30 1:18 ` Darrick J. Wong
1 sibling, 0 replies; 3+ messages in thread
From: Darrick J. Wong @ 2019-07-30 1:18 UTC (permalink / raw)
To: hch, darrick.wong
Cc: linux-xfs, linux-fsdevel, Damien.LeMoal, Christoph Hellwig, agruenba
From: Christoph Hellwig <hch@lst.de>
Introduce two nicely abstracted helper, which can be moved to the
iomap code later. Also use list_pop_entry and list_first_entry_or_null
to simplify the code a bit.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/xfs_aops.c | 70 +++++++++++++++++++++++++++++------------------------
1 file changed, 38 insertions(+), 32 deletions(-)
diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
index f16d5f196c6b..4e4a4d7df5ac 100644
--- a/fs/xfs/xfs_aops.c
+++ b/fs/xfs/xfs_aops.c
@@ -116,6 +116,19 @@ xfs_destroy_ioend(
}
}
+static void
+xfs_destroy_ioends(
+ struct xfs_ioend *ioend,
+ int error)
+{
+ struct list_head tmp;
+
+ list_replace_init(&ioend->io_list, &tmp);
+ xfs_destroy_ioend(ioend, error);
+ while ((ioend = list_pop_entry(&tmp, struct xfs_ioend, io_list)))
+ xfs_destroy_ioend(ioend, error);
+}
+
/*
* Fast and loose check if this write could update the on-disk inode size.
*/
@@ -230,7 +243,6 @@ STATIC void
xfs_end_ioend(
struct xfs_ioend *ioend)
{
- struct list_head ioend_list;
struct xfs_inode *ip = XFS_I(ioend->io_inode);
xfs_off_t offset = ioend->io_offset;
size_t size = ioend->io_size;
@@ -275,16 +287,7 @@ xfs_end_ioend(
done:
if (ioend->io_append_trans)
error = xfs_setfilesize_ioend(ioend, error);
- list_replace_init(&ioend->io_list, &ioend_list);
- xfs_destroy_ioend(ioend, error);
-
- while (!list_empty(&ioend_list)) {
- ioend = list_first_entry(&ioend_list, struct xfs_ioend,
- io_list);
- list_del_init(&ioend->io_list);
- xfs_destroy_ioend(ioend, error);
- }
-
+ xfs_destroy_ioends(ioend, error);
memalloc_nofs_restore(nofs_flag);
}
@@ -333,17 +336,18 @@ xfs_ioend_try_merge(
struct xfs_ioend *ioend,
struct list_head *more_ioends)
{
- struct xfs_ioend *next_ioend;
+ struct xfs_ioend *next;
- while (!list_empty(more_ioends)) {
- next_ioend = list_first_entry(more_ioends, struct xfs_ioend,
- io_list);
- if (!xfs_ioend_can_merge(ioend, next_ioend))
+ INIT_LIST_HEAD(&ioend->io_list);
+
+ while ((next = list_first_entry_or_null(more_ioends, struct xfs_ioend,
+ io_list))) {
+ if (!xfs_ioend_can_merge(ioend, next))
break;
- list_move_tail(&next_ioend->io_list, &ioend->io_list);
- ioend->io_size += next_ioend->io_size;
- if (next_ioend->io_append_trans)
- xfs_ioend_merge_append_transactions(ioend, next_ioend);
+ list_move_tail(&next->io_list, &ioend->io_list);
+ ioend->io_size += next->io_size;
+ if (next->io_append_trans)
+ xfs_ioend_merge_append_transactions(ioend, next);
}
}
@@ -366,29 +370,31 @@ xfs_ioend_compare(
return 0;
}
+static void
+xfs_sort_ioends(
+ struct list_head *ioend_list)
+{
+ list_sort(NULL, ioend_list, xfs_ioend_compare);
+}
+
/* Finish all pending io completions. */
void
xfs_end_io(
struct work_struct *work)
{
- struct xfs_inode *ip;
+ struct xfs_inode *ip =
+ container_of(work, struct xfs_inode, i_ioend_work);
struct xfs_ioend *ioend;
- struct list_head completion_list;
+ struct list_head tmp;
unsigned long flags;
- ip = container_of(work, struct xfs_inode, i_ioend_work);
-
spin_lock_irqsave(&ip->i_ioend_lock, flags);
- list_replace_init(&ip->i_ioend_list, &completion_list);
+ list_replace_init(&ip->i_ioend_list, &tmp);
spin_unlock_irqrestore(&ip->i_ioend_lock, flags);
- list_sort(NULL, &completion_list, xfs_ioend_compare);
-
- while (!list_empty(&completion_list)) {
- ioend = list_first_entry(&completion_list, struct xfs_ioend,
- io_list);
- list_del_init(&ioend->io_list);
- xfs_ioend_try_merge(ioend, &completion_list);
+ xfs_sort_ioends(&tmp);
+ while ((ioend = list_pop_entry(&tmp, struct xfs_ioend, io_list))) {
+ xfs_ioend_try_merge(ioend, &tmp);
xfs_end_ioend(ioend);
}
}
^ permalink raw reply related [flat|nested] 3+ messages in thread