| Message ID | 20191025150336.19411-4-hch@lst.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/8] xfs: simplify xfs_iomap_eof_align_last_fsb | expand |
On Fri, Oct 25, 2019 at 05:03:31PM +0200, Christoph Hellwig wrote: > And move the code dependent on it to the one caller that cares > instead. Hmm, so if I'm understanding this correctly, now xfs_eof_alignment rounds alignment up to the stripe width (or dalign) for files on the data device? And the alignment number it produces is further rounded up to the extent hint size which is then used to round up a space allocation (directio writes) or used to round up the speculative preallocation window (buffered writes)? Why does it make more sense to do the inode extsize roundup only for direct writes and not as an intermediate step of determining the speculative preallocation size than what the code does now? --D > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > fs/xfs/xfs_iomap.c | 28 +++++++++++++--------------- > 1 file changed, 13 insertions(+), 15 deletions(-) > > diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c > index c803a8efa8ff..e3b11cda447e 100644 > --- a/fs/xfs/xfs_iomap.c > +++ b/fs/xfs/xfs_iomap.c > @@ -118,8 +118,7 @@ xfs_iomap_end_fsb( > > static xfs_extlen_t > xfs_eof_alignment( > - struct xfs_inode *ip, > - xfs_extlen_t extsize) > + struct xfs_inode *ip) > { > struct xfs_mount *mp = ip->i_mount; > xfs_extlen_t align = 0; > @@ -142,17 +141,6 @@ xfs_eof_alignment( > align = 0; > } > > - /* > - * Always round up the allocation request to an extent boundary > - * (when file on a real-time subvolume or has di_extsize hint). > - */ > - if (extsize) { > - if (align) > - align = roundup_64(align, extsize); > - else > - align = extsize; > - } > - > return align; > } > > @@ -167,12 +155,22 @@ xfs_iomap_eof_align_last_fsb( > { > struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK); > xfs_extlen_t extsz = xfs_get_extsz_hint(ip); > - xfs_extlen_t align = xfs_eof_alignment(ip, extsz); > + xfs_extlen_t align = xfs_eof_alignment(ip); > struct xfs_bmbt_irec irec; > struct xfs_iext_cursor icur; > > ASSERT(ifp->if_flags & XFS_IFEXTENTS); > > + /* > + * Always round up the allocation request to the extent hint boundary. > + */ > + if (extsz) { > + if (align) > + align = roundup_64(align, extsz); > + else > + align = extsz; > + } > + > if (align) { > xfs_fileoff_t aligned_end_fsb = roundup_64(end_fsb, align); > > @@ -992,7 +990,7 @@ xfs_buffered_write_iomap_begin( > p_end_fsb = XFS_B_TO_FSBT(mp, end_offset) + > prealloc_blocks; > > - align = xfs_eof_alignment(ip, 0); > + align = xfs_eof_alignment(ip); > if (align) > p_end_fsb = roundup_64(p_end_fsb, align); > > -- > 2.20.1 >
On Mon, Oct 28, 2019 at 09:06:38AM -0700, Darrick J. Wong wrote: > Why does it make more sense to do the inode extsize roundup only for > direct writes and not as an intermediate step of determining the > speculative preallocation size than what the code does now? No behavior change in this patch. xfs_buffered_write_iomap_begin already passes a 0 extsize to xfs_eof_alignment, making the code moved from xfs_eof_alignment to xfs_iomap_eof_align_last_fsb a no-op for the buffered write path.
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c index c803a8efa8ff..e3b11cda447e 100644 --- a/fs/xfs/xfs_iomap.c +++ b/fs/xfs/xfs_iomap.c @@ -118,8 +118,7 @@ xfs_iomap_end_fsb( static xfs_extlen_t xfs_eof_alignment( - struct xfs_inode *ip, - xfs_extlen_t extsize) + struct xfs_inode *ip) { struct xfs_mount *mp = ip->i_mount; xfs_extlen_t align = 0; @@ -142,17 +141,6 @@ xfs_eof_alignment( align = 0; } - /* - * Always round up the allocation request to an extent boundary - * (when file on a real-time subvolume or has di_extsize hint). - */ - if (extsize) { - if (align) - align = roundup_64(align, extsize); - else - align = extsize; - } - return align; } @@ -167,12 +155,22 @@ xfs_iomap_eof_align_last_fsb( { struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK); xfs_extlen_t extsz = xfs_get_extsz_hint(ip); - xfs_extlen_t align = xfs_eof_alignment(ip, extsz); + xfs_extlen_t align = xfs_eof_alignment(ip); struct xfs_bmbt_irec irec; struct xfs_iext_cursor icur; ASSERT(ifp->if_flags & XFS_IFEXTENTS); + /* + * Always round up the allocation request to the extent hint boundary. + */ + if (extsz) { + if (align) + align = roundup_64(align, extsz); + else + align = extsz; + } + if (align) { xfs_fileoff_t aligned_end_fsb = roundup_64(end_fsb, align); @@ -992,7 +990,7 @@ xfs_buffered_write_iomap_begin( p_end_fsb = XFS_B_TO_FSBT(mp, end_offset) + prealloc_blocks; - align = xfs_eof_alignment(ip, 0); + align = xfs_eof_alignment(ip); if (align) p_end_fsb = roundup_64(p_end_fsb, align);
And move the code dependent on it to the one caller that cares instead. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/xfs/xfs_iomap.c | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-)