COW improvements and always_cow support V3
Hi all,
this series adds the always_cow mode support after improving our COW write support a little bit first.
The always_cow mode stresses the COW path a lot, but with a few xfstests fixups it generall looks good, except for a few tests that complain about fragmentation, which is rather inherent in this mode, and xfs/326 which inserts error tags into the COW path not getting the expected result.
Changes since v2:
- add a patch to remove xfs_trim_extent_eof
- add a patch to remove the separate io_type and rely on existing state in the writeback path
- rework the truncate race handling in the writeback path a little more
Changes since v1:
- make delalloc and unwritten extent conversions simpler and more robust
- add a few additional cleanups
- support all fallocate modes but actual preallocation
- rebase on top of a fix from Brian (which is included as first patch to make the patch set more usable)
PATCH 1/11 xfs: remove xfs_trim_extent_eof
Opencoding this function in the only caller makes it blindly obvious what is going on instead of having to look at two files for that.
--- fs/xfs/libxfs/xfs_bmap.c | 11 ----------- fs/xfs/libxfs/xfs_bmap.h | 1 - fs/xfs/xfs_aops.c | 2 +- 3 files changed, 1 insertion(+), 13 deletions(-)
fs/xfs/libxfs/xfs_bmap.c +0 -11
@@ -3685,17 +3685,6 @@ xfs_trim_extent( } } -/* trim extent to within eof */ -void -xfs_trim_extent_eof ( -struct xfs_bmbt_irec *irec, -struct xfs_inode *ip) - -{ -xfs_trim_extent (irec,0 ,XFS_B_TO_FSB (ip->i_mount, -i_size_read (VFS_I (ip)))); -} -/* * Trim the returned map to the required bounds */
fs/xfs/libxfs/xfs_bmap.h +0 -1
@@ -181,7 +181,6 @@ static inline bool xfs_bmap_is_real_extent(struct xfs_bmbt_irec *irec)void xfs_trim_extent (struct xfs_bmbt_irec *irec,xfs_fileoff_t bno,xfs_filblks_t len); -void xfs_trim_extent_eof (struct xfs_bmbt_irec *,struct xfs_inode *);int xfs_bmap_add_attrfork (struct xfs_inode *ip,int size,int rsvd);int xfs_bmap_set_attrforkoff (struct xfs_inode *ip,int size,int *version);void xfs_bmap_local_to_extents_empty (struct xfs_inode *ip,int whichfork);
fs/xfs/xfs_aops.c +1 -1
@@ -329,7 +329,7 @@ xfs_map_blocks(* mechanism to protect us from arbitrary extent modifying contexts, not * just eofblocks. */ -xfs_trim_extent_eof (&wpc->imap, ip); +xfs_trim_extent (&wpc->imap,0 ,XFS_B_TO_FSB (mp,i_size_read (inode)));/* * COW fork blocks can overlap data fork blocks even if the blocks
Signature · 1 line
2.19.1
PATCH 2/11 xfs: remove the io_type field from the writeback context and ioend
In reply to Christoph HellwigThe io_type field contains what is basically a summary of information from the inode fork and the imap. But we can just as easily use that information directly, simplifying a few bits here and there and improving the trace points.
--- fs/xfs/xfs_aops.c | 91 ++++++++++++++++++++------------------------ fs/xfs/xfs_aops.h | 21 +--------- fs/xfs/xfs_iomap.c | 8 ++-- fs/xfs/xfs_reflink.c | 2 +- fs/xfs/xfs_trace.h | 28 +++++++------- 5 files changed, 62 insertions(+), 88 deletions(-)
fs/xfs/xfs_aops.c +41 -50
@@ -28,7 +28,7 @@*/ struct xfs_writepage_ctx {struct xfs_bmbt_irec imap; -unsigned int io_type; +int fork;unsigned int cow_seq;struct xfs_ioend *ioend; };
@@ -255,30 +255,20 @@ xfs_end_io(*/ error =blk_status_to_errno (ioend->io_bio->bi_status);if (unlikely (error)) { -switch (ioend->io_type) { -case XFS_IO_COW : +if (ioend->io_fork ==XFS_COW_FORK )xfs_reflink_cancel_cow_range (ip, offset, size, true); -break ; - } - goto done; }/* -* Success: commit the COW or unwritten blocks if needed. +* Success: commit the COW or unwritten blocks if needed. */ -switch (ioend->io_type) { -case XFS_IO_COW : +if (ioend->io_fork ==XFS_COW_FORK ) error =xfs_reflink_end_cow (ip, offset, size); -break ; -case XFS_IO_UNWRITTEN : -/* writeback should never update isize */ +else if (ioend->io_state ==XFS_EXT_UNWRITTEN ) error =xfs_iomap_write_unwritten (ip, offset, size, false); -break ; -default : +else ASSERT (!xfs_ioend_is_append (ioend) || ioend->io_append_trans); -break ; - } done:if (ioend->io_append_trans)
@@ -293,7 +283,8 @@ xfs_end_bio(struct xfs_ioend *ioend = bio->bi_private;struct xfs_mount *mp =XFS_I (ioend->io_inode)->i_mount; -if (ioend->io_type ==XFS_IO_UNWRITTEN || ioend->io_type ==XFS_IO_COW ) +if (ioend->io_fork ==XFS_COW_FORK || + ioend->io_state ==XFS_EXT_UNWRITTEN )queue_work (mp->m_unwritten_workqueue, &ioend->io_work);else if (ioend->io_append_trans)queue_work (mp->m_data_workqueue, &ioend->io_work);
@@ -313,7 +304,6 @@ xfs_map_blocks(xfs_fileoff_t offset_fsb =XFS_B_TO_FSBT (mp, offset), end_fsb;xfs_fileoff_t cow_fsb =NULLFILEOFF ;struct xfs_bmbt_irec imap; -int whichfork =XFS_DATA_FORK ;struct xfs_iext_cursor icur;bool imap_valid;int error =0 ;
@@ -350,7 +340,7 @@ xfs_map_blocks( offset_fsb < wpc->imap.br_startoff + wpc->imap.br_blockcount;if (imap_valid && (!xfs_inode_has_cow_data (ip) || - wpc->io_type ==XFS_IO_COW || + wpc->fork ==XFS_COW_FORK || wpc->cow_seq ==READ_ONCE (ip->i_cowfp->if_seq)))return 0 ;
@@ -382,6 +372,9 @@ xfs_map_blocks(if (cow_fsb !=NULLFILEOFF && cow_fsb <= offset_fsb) { wpc->cow_seq =READ_ONCE (ip->i_cowfp->if_seq);xfs_iunlock (ip,XFS_ILOCK_SHARED ); + + wpc->fork =XFS_COW_FORK ; +/* * Truncate can race with writeback since writeback doesn't * take the iolock and truncate decreases the file size before
@@ -394,11 +387,13 @@ xfs_map_blocks(* will kill the contents anyway. */ if (offset >i_size_read (inode)) { - wpc->io_type =XFS_IO_HOLE ; + wpc->imap.br_blockcount = end_fsb - offset_fsb; + wpc->imap.br_startoff = offset_fsb; + wpc->imap.br_startblock =HOLESTARTBLOCK ; + wpc->imap.br_state =XFS_EXT_NORM ;return 0 ; } - whichfork =XFS_COW_FORK ; - wpc->io_type =XFS_IO_COW ; + goto allocate_blocks; }
@@ -419,12 +414,14 @@ xfs_map_blocks( imap.br_startoff = end_fsb;/* fake a hole past EOF */ xfs_iunlock (ip,XFS_ILOCK_SHARED ); + wpc->fork =XFS_DATA_FORK ; +if (imap.br_startoff > offset_fsb) {/* landed in a hole or beyond EOF */ imap.br_blockcount = imap.br_startoff - offset_fsb; imap.br_startoff = offset_fsb; imap.br_startblock =HOLESTARTBLOCK ; - wpc->io_type =XFS_IO_HOLE ; + imap.br_state =XFS_EXT_NORM ; }else {/* * Truncate to the next COW extent if there is one. This is the
@@ -436,30 +433,23 @@ xfs_map_blocks( cow_fsb < imap.br_startoff + imap.br_blockcount) imap.br_blockcount = cow_fsb - imap.br_startoff; -if (isnullstartblock (imap.br_startblock)) { -/* got a delalloc extent */ - wpc->io_type =XFS_IO_DELALLOC ; +/* got a delalloc extent? */ +if (isnullstartblock (imap.br_startblock)) goto allocate_blocks; - } - -if (imap.br_state ==XFS_EXT_UNWRITTEN ) - wpc->io_type =XFS_IO_UNWRITTEN ; -else - wpc->io_type =XFS_IO_OVERWRITE ; } wpc->imap = imap; -trace_xfs_map_blocks_found (ip, offset, count, wpc->io_type, &imap); +trace_xfs_map_blocks_found (ip, offset, count, wpc->fork, &imap);return 0 ; allocate_blocks: - error =xfs_iomap_write_allocate (ip, whichfork, offset, &imap, + error =xfs_iomap_write_allocate (ip, wpc->fork, offset, &imap, &wpc->cow_seq);if (error)return error; -ASSERT (whichfork ==XFS_COW_FORK || cow_fsb ==NULLFILEOFF || +ASSERT (wpc->fork ==XFS_COW_FORK || cow_fsb ==NULLFILEOFF || imap.br_startoff + imap.br_blockcount <= cow_fsb); wpc->imap = imap; -trace_xfs_map_blocks_alloc (ip, offset, count, wpc->io_type, &imap); +trace_xfs_map_blocks_alloc (ip, offset, count, wpc->fork, &imap);return 0 ; }
@@ -484,7 +474,7 @@ xfs_submit_ioend( int status) {/* Convert CoW extents to regular */ -if (!status && ioend->io_type ==XFS_IO_COW ) { +if (!status && ioend->io_fork ==XFS_COW_FORK ) {/* * Yuk. This can do memory allocation, but is not a * transactional operation so everything is done in GFP_KERNEL
@@ -502,7 +492,8 @@ xfs_submit_ioend(/* Reserve log space if we might write beyond the on-disk inode size. */ if (!status && - ioend->io_type !=XFS_IO_UNWRITTEN && + (ioend->io_fork ==XFS_COW_FORK || + ioend->io_state !=XFS_EXT_UNWRITTEN ) &&xfs_ioend_is_append (ioend) && !ioend->io_append_trans) status =xfs_setfilesize_trans_alloc (ioend);
@@ -531,7 +522,8 @@ xfs_submit_ioend(static struct xfs_ioend *xfs_alloc_ioend (struct inode *inode, -unsigned int type, +int fork, +xfs_exntst_t state,xfs_off_t offset,struct block_device *bdev,sector_t sector)
@@ -545,7 +537,8 @@ xfs_alloc_ioend( ioend = container_of(bio, struct xfs_ioend, io_inline_bio);INIT_LIST_HEAD (&ioend->io_list); - ioend->io_type = type; + ioend->io_fork = fork; + ioend->io_state = state; ioend->io_inode = inode; ioend->io_size =0 ; ioend->io_offset = offset;
@@ -606,13 +599,15 @@ xfs_add_to_ioend( sector =xfs_fsb_to_db (ip, wpc->imap.br_startblock) + ((offset -XFS_FSB_TO_B (mp, wpc->imap.br_startoff)) >>9 ); -if (!wpc->ioend || wpc->io_type != wpc->ioend->io_type || +if (!wpc->ioend || + wpc->fork != wpc->ioend->io_fork || + wpc->imap.br_state != wpc->ioend->io_state || sector !=bio_end_sector (wpc->ioend->io_bio) || offset != wpc->ioend->io_offset + wpc->ioend->io_size) {if (wpc->ioend)list_add (&wpc->ioend->io_list, iolist); - wpc->ioend =xfs_alloc_ioend (inode, wpc->io_type, offset, - bdev, sector); + wpc->ioend =xfs_alloc_ioend (inode, wpc->fork, + wpc->imap.br_state, offset, bdev, sector); }if (!__bio_try_merge_page (wpc->ioend->io_bio, page, len, poff)) {
@@ -721,7 +716,7 @@ xfs_writepage_map( error =xfs_map_blocks (wpc, inode, file_offset);if (error)break ; -if (wpc->io_type ==XFS_IO_HOLE ) +if (wpc->imap.br_startblock ==HOLESTARTBLOCK )continue ;xfs_add_to_ioend (inode, file_offset, page, iop, wpc, wbc, &submit_list);
@@ -916,9 +911,7 @@ xfs_vm_writepage(struct page *page, struct writeback_control *wbc) { - struct xfs_writepage_ctx wpc = { - .io_type =XFS_IO_HOLE , - }; + struct xfs_writepage_ctx wpc = { };int ret; ret =xfs_do_writepage (page, wbc, &wpc);
@@ -932,9 +925,7 @@ xfs_vm_writepages(struct address_space *mapping, struct writeback_control *wbc) { - struct xfs_writepage_ctx wpc = { - .io_type =XFS_IO_HOLE , - }; + struct xfs_writepage_ctx wpc = { };int ret;xfs_iflags_clear (XFS_I (mapping->host),XFS_ITRUNCATED );
fs/xfs/xfs_aops.h +2 -19
@@ -8,30 +8,13 @@extern struct bio_set xfs_ioend_bioset; -/* -* Types of I/O for bmap clustering and I/O completion tracking. -*/ -enum { -XFS_IO_HOLE ,/* covers region without any block allocation */ -XFS_IO_DELALLOC ,/* covers delalloc region */ -XFS_IO_UNWRITTEN ,/* covers allocated but uninitialized data */ -XFS_IO_OVERWRITE ,/* covers already allocated extent */ -XFS_IO_COW ,/* covers copy-on-write extent */ -}; - -#define XFS_IO_TYPES \ - { XFS_IO_HOLE, "hole" }, \ - { XFS_IO_DELALLOC, "delalloc" }, \ - { XFS_IO_UNWRITTEN, "unwritten" }, \ - { XFS_IO_OVERWRITE, "overwrite" }, \ - { XFS_IO_COW, "CoW" } -/* * Structure for buffered I/O completions. */ struct xfs_ioend {struct list_head io_list;/* next ioend in chain */ -unsigned int io_type;/* delalloc / unwritten */ +int io_fork;/* inode fork written back */ +xfs_exntst_t io_state;/* extent state */ struct inode *io_inode;/* file being written to */ size_t io_size;/* size of the extent */ xfs_off_t io_offset;/* offset in the file */
fs/xfs/xfs_iomap.c +4 -4
@@ -575,7 +575,7 @@ xfs_file_iomap_begin_delay( goto out_unlock; } -trace_xfs_iomap_found (ip, offset, count,0 , &got); +trace_xfs_iomap_found (ip, offset, count,XFS_DATA_FORK , &got); goto done; }
@@ -647,7 +647,7 @@ xfs_file_iomap_begin_delay(* them out if the write happens to fail. */ iomap->flags |=IOMAP_F_NEW ; -trace_xfs_iomap_alloc (ip, offset, count,0 , &got); +trace_xfs_iomap_alloc (ip, offset, count,XFS_DATA_FORK , &got); done:if (isnullstartblock (got.br_startblock)) got.br_startblock =DELAYSTARTBLOCK ;
@@ -1139,7 +1139,7 @@ xfs_file_iomap_begin(return error; iomap->flags |=IOMAP_F_NEW ; -trace_xfs_iomap_alloc (ip, offset, length,0 , &imap); +trace_xfs_iomap_alloc (ip, offset, length,XFS_DATA_FORK , &imap); out_finish: if (xfs_ipincount(ip) && (ip->i_itemp->ili_fsync_fields
@@ -1155,7 +1155,7 @@ xfs_file_iomap_begin( out_found:ASSERT (nimaps);xfs_iunlock (ip, lockmode); -trace_xfs_iomap_found (ip, offset, length,0 , &imap); +trace_xfs_iomap_found (ip, offset, length,XFS_DATA_FORK , &imap); goto out_finish; out_unlock:
fs/xfs/xfs_reflink.c +1 -1
@@ -1148,7 +1148,7 @@ xfs_reflink_remap_blocks(break ;ASSERT (nimaps ==1 ); -trace_xfs_reflink_remap_imap (src, srcoff, len,XFS_IO_OVERWRITE , +trace_xfs_reflink_remap_imap (src, srcoff, len,XFS_DATA_FORK , &imap);/* Translate imap into the destination file. */
fs/xfs/xfs_trace.h +14 -14
@@ -1210,15 +1210,15 @@ DEFINE_READPAGE_EVENT(xfs_vm_readpages); DECLARE_EVENT_CLASS(xfs_imap_class, TP_PROTO(struct xfs_inode *ip, xfs_off_t offset, ssize_t count, - int type, struct xfs_bmbt_irec *irec), - TP_ARGS(ip, offset, count, type, irec), + int whichfork, struct xfs_bmbt_irec *irec), + TP_ARGS(ip, offset, count, whichfork, irec), TP_STRUCT__entry( __field(dev_t, dev) __field(xfs_ino_t, ino) __field(loff_t, size) __field(loff_t, offset) __field(size_t, count) - __field(int, type) + __field(int, whichfork) __field(xfs_fileoff_t, startoff) __field(xfs_fsblock_t, startblock) __field(xfs_filblks_t, blockcount)
@@ -1229,33 +1229,33 @@ DECLARE_EVENT_CLASS(xfs_imap_class, __entry->size = ip->i_d.di_size; __entry->offset = offset; __entry->count = count; - __entry->type = type; + __entry->whichfork = whichfork; __entry->startoff = irec ? irec->br_startoff :0 ; __entry->startblock = irec ? irec->br_startblock :0 ; __entry->blockcount = irec ? irec->br_blockcount :0 ; ), TP_printk("dev %d:%d ino 0x%llx size 0x%llx offset 0x%llx count %zd " - "type %s startoff 0x%llx startblock %lld blockcount 0x%llx", + "fork %s startoff 0x%llx startblock %lld blockcount 0x%llx", MAJOR(__entry->dev), MINOR(__entry->dev), __entry->ino, __entry->size, __entry->offset, __entry->count, - __print_symbolic(__entry->type, XFS_IO_TYPES), + __entry->whichfork == XFS_COW_FORK ? "cow" : "data", __entry->startoff, (int64_t)__entry->startblock, __entry->blockcount) ) -#define DEFINE_IOMAP_EVENT (name) \ +#define DEFINE_IMAP_EVENT (name) \ DEFINE_EVENT(xfs_imap_class, name, \ TP_PROTO(struct xfs_inode *ip, xfs_off_t offset, ssize_t count, \ - int type, struct xfs_bmbt_irec *irec), \ - TP_ARGS(ip, offset, count, type, irec)) -DEFINE_IOMAP_EVENT (xfs_map_blocks_found); -DEFINE_IOMAP_EVENT (xfs_map_blocks_alloc); -DEFINE_IOMAP_EVENT (xfs_iomap_alloc); -DEFINE_IOMAP_EVENT (xfs_iomap_found); + int whichfork, struct xfs_bmbt_irec *irec), \ + TP_ARGS(ip, offset, count, whichfork, irec)) +DEFINE_IMAP_EVENT (xfs_map_blocks_found); +DEFINE_IMAP_EVENT (xfs_map_blocks_alloc); +DEFINE_IMAP_EVENT (xfs_iomap_alloc); +DEFINE_IMAP_EVENT (xfs_iomap_found); DECLARE_EVENT_CLASS(xfs_simple_io_class, TP_PROTO(struct xfs_inode *ip, xfs_off_t offset, ssize_t count),
@@ -3055,7 +3055,7 @@ DEFINE_EVENT(xfs_inode_irec_class, name, \DEFINE_INODE_EVENT (xfs_reflink_set_inode_flag);DEFINE_INODE_EVENT (xfs_reflink_unset_inode_flag);DEFINE_ITRUNC_EVENT (xfs_reflink_update_inode_size); -DEFINE_IOMAP_EVENT (xfs_reflink_remap_imap); +DEFINE_IMAP_EVENT (xfs_reflink_remap_imap); TRACE_EVENT(xfs_reflink_remap_blocks_loop, TP_PROTO(struct xfs_inode *src, xfs_fileoff_t soffset, xfs_filblks_t len, struct xfs_inode *dest,
Signature · 1 line
2.19.1
PATCH 3/11 xfs: remove the s_maxbytes checks in xfs_map_blocks
In reply to Christoph HellwigWe already ensure all data fits into s_maxbytes in the write / fault path. The only reason we have them here is that they were copy and pasted from xfs_bmapi_read when we stopped using that function.
--- fs/xfs/xfs_aops.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
fs/xfs/xfs_aops.c +2 -6
@@ -301,7 +301,8 @@ xfs_map_blocks(struct xfs_inode *ip =XFS_I (inode);struct xfs_mount *mp = ip->i_mount;ssize_t count =i_blocksize (inode); -xfs_fileoff_t offset_fsb =XFS_B_TO_FSBT (mp, offset), end_fsb; +xfs_fileoff_t offset_fsb =XFS_B_TO_FSBT (mp, offset); +xfs_fileoff_t end_fsb =XFS_B_TO_FSB (mp, offset + count);xfs_fileoff_t cow_fsb =NULLFILEOFF ;struct xfs_bmbt_irec imap;struct xfs_iext_cursor icur;
@@ -356,11 +357,6 @@ xfs_map_blocks(xfs_ilock (ip,XFS_ILOCK_SHARED );ASSERT (ip->i_d.di_format !=XFS_DINODE_FMT_BTREE || (ip->i_df.if_flags &XFS_IFEXTENTS )); -ASSERT (offset <= mp->m_super->s_maxbytes); - -if (offset > mp->m_super->s_maxbytes - count) - count = mp->m_super->s_maxbytes - offset; - end_fsb =XFS_B_TO_FSB (mp, (xfs_ufsize_t )offset + count);/* * Check if this is offset is covered by a COW extents, and if yes use
Signature · 1 line
2.19.1
Please discard this for now - the inflight wifi is to unstable to send a patch series..