On 09/28/2012 02:59 AM, Dave Chinner wrote:
> On Thu, Sep 27, 2012 at 01:45:47PM -0400, Brian Foster wrote:
>> This check is used in multiple places to determine whether we
>> should check for (and potentially free) post EOF blocks on an
>> inode. Add a helper to consolidate the check.
>>
>> Note that when we remove an inode from the cache (xfs_inactive()),
>> we are required to trim post-EOF blocks even if the inode is marked
>> preallocated or append-only to maintain correct space accounting.
>> The 'force' parameter to xfs_can_free_eofblocks() specifies whether
>> we should ignore the prealloc/append-only status of the inode.
>>
>> Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx>
> ....
>> diff --git a/fs/xfs/xfs_vnodeops.h b/fs/xfs/xfs_vnodeops.h
>> index 447e146..d5701e3 100644
>> --- a/fs/xfs/xfs_vnodeops.h
>> +++ b/fs/xfs/xfs_vnodeops.h
>> @@ -1,6 +1,10 @@
>> #ifndef _XFS_VNODEOPS_H
>> #define _XFS_VNODEOPS_H 1
>>
>> +#include "xfs_bmap_btree.h"
>> +#include "xfs_dinode.h"
>> +#include "xfs_inode.h"
>> +
>> struct attrlist_cursor_kern;
>> struct file;
>> struct iattr;
>
> Generally we try to avoid including XFS header files in XFS
> header files, and instead order the header files in the .c files
> appropriately.
>
Ok, good to know. I'll try to clean that up.
>> @@ -9,8 +13,42 @@ struct iovec;
>> struct kiocb;
>> struct pipe_inode_info;
>> struct uio;
>> -struct xfs_inode;
>>
>> +/*
>> + * Test whether it is appropriate to check an inode for and free post EOF
>> + * blocks. The 'force' parameter determines whether we should also consider
>> + * regular files that are marked preallocated or append-only.
>> + */
>> +static inline bool
>> +xfs_can_free_eofblocks(struct xfs_inode *ip, bool force)
>> +{
>> + /* prealloc/delalloc exists only on regular files */
>> + if (!S_ISREG(ip->i_d.di_mode))
>> + return false;
>> +
>> + /*
>> + * Zero sized files with no cached pages and delalloc blocks will not
>> + * have speculative prealloc/delalloc blocks to remove.
>> + */
>> + if (VFS_I(ip)->i_size == 0 &&
>> + VN_CACHED(VFS_I(ip)) == 0 &&
>> + ip->i_delayed_blks == 0)
>> + return false;
>> +
>> + /* If we haven't read in the extent list, then don't do it now. */
>> + if (!(ip->i_df.if_flags & XFS_IFEXTENTS))
>> + return false;
>> +
>> + /*
>> + * Do not free real preallocated or append-only files unless the file
>> + * has delalloc blocks and we are forced to remove them.
>> + */
>> + if (ip->i_d.di_flags & (XFS_DIFLAG_PREALLOC | XFS_DIFLAG_APPEND))
>> + if (!force || ip->i_delayed_blks == 0)
>> + return false;
>> +
>> + return true;
>> +}
>
> And I'd say that function is too large for a static inline function
> in a header file, so moving it to xfs_inode.c is probably
> appropriate. I know, I asked you to write it like this, but
> not looking at it for a couple weeks brings new persepctive ;)
>
No problem. I guess that takes care of the include problem. :)
Brian
> Cheers,
>
> Dave.
>
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
