xfs
[Top] [All Lists]

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

To: Brian Foster <bfoster@xxxxxxxxxx>
Subject: Re: [PATCH v5 06/10] xfs: add XFS_IOC_FREE_EOFBLOCKS ioctl
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Thu, 25 Oct 2012 06:27:43 +1100
Cc: xfs@xxxxxxxxxxx
In-reply-to: <50881477.5050602@xxxxxxxxxx>
References: <1349446636-8611-1-git-send-email-bfoster@xxxxxxxxxx> <1349446636-8611-7-git-send-email-bfoster@xxxxxxxxxx> <20121023013104.GJ4291@dastard> <50881477.5050602@xxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
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@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>