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@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
