| Message ID | 20180917205354.15401-4-hch@lst.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [01/10] xfs: fix transaction leak in xfs_reflink_allocate_cow() | expand |
On Mon, Sep 17, 2018 at 10:53:47PM +0200, Christoph Hellwig wrote: > The invalid state isn't any different from a hole, so merge the two > states. Use the more descriptive hole name, but keep it as the first > value of the enum to catch uninitialized fields. > > Signed-off-by: Christoph Hellwig <hch@lst.de> It's probably worth mentioning that I'm just reading patches *without* having previously shoved all the day's patches into a branch, built it, and started xfstests.... :) > --- > fs/xfs/xfs_aops.c | 4 ++-- > fs/xfs/xfs_aops.h | 14 ++++++-------- > 2 files changed, 8 insertions(+), 10 deletions(-) > > diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c > index 49f5f5896a43..338b9d9984e0 100644 > --- a/fs/xfs/xfs_aops.c > +++ b/fs/xfs/xfs_aops.c > @@ -917,7 +917,7 @@ xfs_vm_writepage( > struct writeback_control *wbc) > { > struct xfs_writepage_ctx wpc = { > - .io_type = XFS_IO_INVALID, > + .io_type = XFS_IO_HOLE, Hm. So I guess if we ever saw XFS_IO_INVALID that meant "we never did find any extents and so never set io_type", right? Just checking assumptions here, having let everything page out of my brain these past 6 weeks... --D > }; > int ret; > > @@ -933,7 +933,7 @@ xfs_vm_writepages( > struct writeback_control *wbc) > { > struct xfs_writepage_ctx wpc = { > - .io_type = XFS_IO_INVALID, > + .io_type = XFS_IO_HOLE, > }; > int ret; > > diff --git a/fs/xfs/xfs_aops.h b/fs/xfs/xfs_aops.h > index 9af867951a10..494b4338446e 100644 > --- a/fs/xfs/xfs_aops.h > +++ b/fs/xfs/xfs_aops.h > @@ -12,21 +12,19 @@ extern struct bio_set xfs_ioend_bioset; > * Types of I/O for bmap clustering and I/O completion tracking. > */ > enum { > - XFS_IO_INVALID, /* initial state */ > + 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 */ > - XFS_IO_HOLE, /* covers region without any block allocation */ > }; > > #define XFS_IO_TYPES \ > - { XFS_IO_INVALID, "invalid" }, \ > - { XFS_IO_DELALLOC, "delalloc" }, \ > - { XFS_IO_UNWRITTEN, "unwritten" }, \ > - { XFS_IO_OVERWRITE, "overwrite" }, \ > - { XFS_IO_COW, "CoW" }, \ > - { XFS_IO_HOLE, "hole" } > + { 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. > -- > 2.18.0 >
On Thu, Sep 20, 2018 at 01:31:27PM -0700, Darrick J. Wong wrote: > > { > > struct xfs_writepage_ctx wpc = { > > - .io_type = XFS_IO_INVALID, > > + .io_type = XFS_IO_HOLE, > > Hm. So I guess if we ever saw XFS_IO_INVALID that meant "we never did > find any extents and so never set io_type", right? Yes.
diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c index 49f5f5896a43..338b9d9984e0 100644 --- a/fs/xfs/xfs_aops.c +++ b/fs/xfs/xfs_aops.c @@ -917,7 +917,7 @@ xfs_vm_writepage( struct writeback_control *wbc) { struct xfs_writepage_ctx wpc = { - .io_type = XFS_IO_INVALID, + .io_type = XFS_IO_HOLE, }; int ret; @@ -933,7 +933,7 @@ xfs_vm_writepages( struct writeback_control *wbc) { struct xfs_writepage_ctx wpc = { - .io_type = XFS_IO_INVALID, + .io_type = XFS_IO_HOLE, }; int ret; diff --git a/fs/xfs/xfs_aops.h b/fs/xfs/xfs_aops.h index 9af867951a10..494b4338446e 100644 --- a/fs/xfs/xfs_aops.h +++ b/fs/xfs/xfs_aops.h @@ -12,21 +12,19 @@ extern struct bio_set xfs_ioend_bioset; * Types of I/O for bmap clustering and I/O completion tracking. */ enum { - XFS_IO_INVALID, /* initial state */ + 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 */ - XFS_IO_HOLE, /* covers region without any block allocation */ }; #define XFS_IO_TYPES \ - { XFS_IO_INVALID, "invalid" }, \ - { XFS_IO_DELALLOC, "delalloc" }, \ - { XFS_IO_UNWRITTEN, "unwritten" }, \ - { XFS_IO_OVERWRITE, "overwrite" }, \ - { XFS_IO_COW, "CoW" }, \ - { XFS_IO_HOLE, "hole" } + { 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.
The invalid state isn't any different from a hole, so merge the two states. Use the more descriptive hole name, but keep it as the first value of the enum to catch uninitialized fields. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/xfs/xfs_aops.c | 4 ++-- fs/xfs/xfs_aops.h | 14 ++++++-------- 2 files changed, 8 insertions(+), 10 deletions(-)