Re: [PATCH v5 06/10] xfs: add XFS_IOC_FREE_EOFBLOCKS ioctl

Dave Chinner
2012-10-24T19:26:01Z

On Wed, Oct 24, 2012 at 12:16:55PM -0400, Brian Foster wrote:
> On 10/22/2012 09:31 PM, Dave Chinner wrote:
> > On Fri, Oct 05, 2012 at 10:17:12AM -0400, Brian Foster wrote:
> >> + flags = (eofb.eof_flags & XFS_EOF_FLAGS_SYNC) ? SYNC_WAIT : SYNC_TRYLOCK;
> >
> > Line if a bit too long. However, would it be better to place this
> > inside xfs_icache_free_eofblocks()?
>
> Are you suggesting to eliminate the flags parameter to
> xfs_icache_free_eofblocks? The reason for the current interface is that
> the background scan caller doesn't require the eofb parameter, so I
> decided to generalize the sync/wait parameter in the caller.

That's on possibility. I was thinking more of flags = 0, and parsing
the eofb structure for meaning inside xfs_icache_free_eofblocks().
i.e. it overrides flags.

> If we want to push it into xfs_icache_free_eofblocks(), I think it would
> be better at that point to eliminate the flags param and either infer
> SYNC_TRYLOCK on a NULL eofb or to require an eofb and pass one with
> a cleared XFS_EOF_FLAGS_SYNC from the background scan. Thoughts?

Sounds good - a NULL eofb meaning "default background scan" works
for me. If we want anything other than default scan parameters, use
the eofb to be precise....

Cheers,

Dave.
--
Dave Chinner
david@fromorbit.com