| Message ID | 20160826160753.GD17728@bfoster.bfoster (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Fri, Aug 26, 2016 at 12:07:53PM -0400, Brian Foster wrote: > > Not quite sure I follow the last bit, but I don't necessarily think the > > whole thing has to be boxed into a helper to clean it up. E.g., I'd do > > something like the appended diff (compile tested only). > > > > ... and if the function signature is really an issue, trade off idx & > prev for a conditional base preallocation size (applies on top of the > previous diff): These two patches together look pretty reasonable. I'll retest with it include and will pick it up for the next version if it works.
On Fri, Aug 26, 2016 at 12:07:53PM -0400, Brian Foster wrote: > On Fri, Aug 26, 2016 at 12:03:39PM -0400, Brian Foster wrote: > > On Fri, Aug 26, 2016 at 04:33:44PM +0200, Christoph Hellwig wrote: > > > On Thu, Aug 25, 2016 at 10:37:09AM -0400, Brian Foster wrote: > ... > > > (we could potentially re-derive offset_fsb from offset if we don't > > > mind the inefficieny and recalculate maxbytes_fsb. This already > > > assumes mp is trivially derived from ip) > > > > > > and return > > > > > > alloc_blocks, end_fsb > > > > > > so the function would be quite a monster in terms of its calling > > > convention. Additionally we'd have the related by not qute the > > > same if blocks around XFS_MOUNT_DFLT_IOSIZE and the isize split > > > over two functions, which doesn't exactly help understanding > > > the flow. > > > > > > > Not quite sure I follow the last bit, but I don't necessarily think the > > whole thing has to be boxed into a helper to clean it up. E.g., I'd do > > something like the appended diff (compile tested only). > > > > ... and if the function signature is really an issue, trade off idx & > prev for a conditional base preallocation size (applies on top of the > previous diff): > > diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c > index 45e5268..c748429 100644 > --- a/fs/xfs/xfs_iomap.c > +++ b/fs/xfs/xfs_iomap.c > @@ -381,7 +381,7 @@ STATIC xfs_fsblock_t > xfs_iomap_prealloc_size( > struct xfs_inode *ip, > xfs_off_t offset, > - struct xfs_bmbt_irec *prev) > + xfs_extlen_t base) > { > struct xfs_mount *mp = ip->i_mount; > int shift = 0; > @@ -406,8 +406,8 @@ xfs_iomap_prealloc_size( > * always extends to MAXEXTLEN rather than falling short due to things > * like stripe unit/width alignment of real extents. > */ > - if (prev->br_blockcount <= (MAXEXTLEN >> 1)) > - alloc_blocks = prev->br_blockcount << 1; > + if (base <= (MAXEXTLEN >> 1)) > + alloc_blocks = base << 1; > else > alloc_blocks = XFS_B_TO_FSB(mp, offset); > if (!alloc_blocks) > @@ -506,12 +506,10 @@ xfs_iomap_prealloc( > struct xfs_inode *ip, > loff_t offset, > loff_t count, > - xfs_extnum_t idx, > - struct xfs_bmbt_irec *prev) > + xfs_extlen_t base) > { > struct xfs_mount *mp = ip->i_mount; > xfs_fsblock_t alloc_blocks; > - xfs_fileoff_t offset_fsb = XFS_B_TO_FSBT(mp, offset); > > if (offset + count <= XFS_ISIZE(ip)) > return 0; > @@ -526,12 +524,11 @@ xfs_iomap_prealloc( > */ > if ((mp->m_flags & XFS_MOUNT_DFLT_IOSIZE) || > XFS_ISIZE(ip) < XFS_FSB_TO_B(mp, mp->m_dalign) || > - idx == 0 || > - prev->br_startoff + prev->br_blockcount < offset_fsb) > + !base) > alloc_blocks = mp->m_writeio_blocks; > else > alloc_blocks = > - xfs_iomap_prealloc_size(ip, offset, prev); > + xfs_iomap_prealloc_size(ip, offset, base); > > return alloc_blocks; > } > @@ -603,8 +600,15 @@ xfs_file_iomap_begin_delay( > end_fsb = orig_end_fsb = > min(XFS_B_TO_FSB(mp, offset + count), maxbytes_fsb); > > - if (eof) > - prealloc = xfs_iomap_prealloc(ip, offset, count, idx, &prev); > + if (eof) { > + xfs_extlen_t base = 0; > + > + /* use prev extent as base if contiguous */ > + if (idx > 0 && > + prev.br_startoff + prev.br_blockcount < offset_fsb) This should probably be: startoff + blockcount == offset_fsb Brian > + base = prev.br_blockcount; > + prealloc = xfs_iomap_prealloc(ip, offset, count, base); > + } > if (prealloc) { > xfs_off_t aligned_offset; > xfs_extlen_t align; > > _______________________________________________ > xfs mailing list > xfs@oss.sgi.com > http://oss.sgi.com/mailman/listinfo/xfs
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c index 45e5268..c748429 100644 --- a/fs/xfs/xfs_iomap.c +++ b/fs/xfs/xfs_iomap.c @@ -381,7 +381,7 @@ STATIC xfs_fsblock_t xfs_iomap_prealloc_size( struct xfs_inode *ip, xfs_off_t offset, - struct xfs_bmbt_irec *prev) + xfs_extlen_t base) { struct xfs_mount *mp = ip->i_mount; int shift = 0; @@ -406,8 +406,8 @@ xfs_iomap_prealloc_size( * always extends to MAXEXTLEN rather than falling short due to things * like stripe unit/width alignment of real extents. */ - if (prev->br_blockcount <= (MAXEXTLEN >> 1)) - alloc_blocks = prev->br_blockcount << 1; + if (base <= (MAXEXTLEN >> 1)) + alloc_blocks = base << 1; else alloc_blocks = XFS_B_TO_FSB(mp, offset); if (!alloc_blocks) @@ -506,12 +506,10 @@ xfs_iomap_prealloc( struct xfs_inode *ip, loff_t offset, loff_t count, - xfs_extnum_t idx, - struct xfs_bmbt_irec *prev) + xfs_extlen_t base) { struct xfs_mount *mp = ip->i_mount; xfs_fsblock_t alloc_blocks; - xfs_fileoff_t offset_fsb = XFS_B_TO_FSBT(mp, offset); if (offset + count <= XFS_ISIZE(ip)) return 0; @@ -526,12 +524,11 @@ xfs_iomap_prealloc( */ if ((mp->m_flags & XFS_MOUNT_DFLT_IOSIZE) || XFS_ISIZE(ip) < XFS_FSB_TO_B(mp, mp->m_dalign) || - idx == 0 || - prev->br_startoff + prev->br_blockcount < offset_fsb) + !base) alloc_blocks = mp->m_writeio_blocks; else alloc_blocks = - xfs_iomap_prealloc_size(ip, offset, prev); + xfs_iomap_prealloc_size(ip, offset, base); return alloc_blocks; } @@ -603,8 +600,15 @@ xfs_file_iomap_begin_delay( end_fsb = orig_end_fsb = min(XFS_B_TO_FSB(mp, offset + count), maxbytes_fsb); - if (eof) - prealloc = xfs_iomap_prealloc(ip, offset, count, idx, &prev); + if (eof) { + xfs_extlen_t base = 0; + + /* use prev extent as base if contiguous */ + if (idx > 0 && + prev.br_startoff + prev.br_blockcount < offset_fsb) + base = prev.br_blockcount; + prealloc = xfs_iomap_prealloc(ip, offset, count, base); + } if (prealloc) { xfs_off_t aligned_offset; xfs_extlen_t align;