| Message ID | 20161206044309.3450-4-anand.jain@oracle.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Tue, Dec 06, 2016 at 12:43:09PM +0800, Anand Jain wrote: > As of now writes smaller than 64k for non compressed extents and 16k > for compressed extents inside eof are considered as candidate > for auto defrag, put them together at a place. Missing sign-off. > --- > fs/btrfs/inode.c | 27 +++++++++++++++++++-------- > 1 file changed, 19 insertions(+), 8 deletions(-) > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > index 79f073e94f2d..b157575166c6 100644 > --- a/fs/btrfs/inode.c > +++ b/fs/btrfs/inode.c > @@ -388,6 +388,22 @@ static inline int inode_need_compress(struct inode *inode) > return 0; > } > > +static inline void inode_should_defrag(struct inode *inode, > + u64 start, u64 end, u64 num_bytes, int comp_type) > +{ > + u64 small_write = SZ_64K; > + if (comp_type) > + small_write = SZ_16K; I think the small_write value should be passed directly, not the compression type. > + > + if (!num_bytes) > + num_bytes = end - start + 1; And the callers should pass the range length. Calculating inside the function makes it less clear. One of the callers passes an blocksize aligned value and the other doest not, so both know what's the right value. Otherwise the cleanup is good. I'll merge the other two patches, please update and resend this one. Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
(sorry for the delay due to my vacation). On 12/12/16 22:12, David Sterba wrote: > On Tue, Dec 06, 2016 at 12:43:09PM +0800, Anand Jain wrote: >> As of now writes smaller than 64k for non compressed extents and 16k >> for compressed extents inside eof are considered as candidate >> for auto defrag, put them together at a place. > > Missing sign-off. sorry will fix. >> --- >> fs/btrfs/inode.c | 27 +++++++++++++++++++-------- >> 1 file changed, 19 insertions(+), 8 deletions(-) >> >> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c >> index 79f073e94f2d..b157575166c6 100644 >> --- a/fs/btrfs/inode.c >> +++ b/fs/btrfs/inode.c >> @@ -388,6 +388,22 @@ static inline int inode_need_compress(struct inode *inode) >> return 0; >> } >> >> +static inline void inode_should_defrag(struct inode *inode, >> + u64 start, u64 end, u64 num_bytes, int comp_type) >> +{ >> + u64 small_write = SZ_64K; >> + if (comp_type) >> + small_write = SZ_16K; > > I think the small_write value should be passed directly, not the > compression type. Will do. >> + >> + if (!num_bytes) >> + num_bytes = end - start + 1; > > And the callers should pass the range length. Calculating inside the > function makes it less clear. One of the callers passes an blocksize > aligned value and the other doest not, so both know what's the right > value. agreed. I have sent out v2. Kindly find it. Thanks, Anand > Otherwise the cleanup is good. I'll merge the other two patches, please > update and resend this one. Thanks. > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 79f073e94f2d..b157575166c6 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -388,6 +388,22 @@ static inline int inode_need_compress(struct inode *inode) return 0; } +static inline void inode_should_defrag(struct inode *inode, + u64 start, u64 end, u64 num_bytes, int comp_type) +{ + u64 small_write = SZ_64K; + if (comp_type) + small_write = SZ_16K; + + if (!num_bytes) + num_bytes = end - start + 1; + + /* If this is a small write inside eof, kick off a defrag */ + if (num_bytes < small_write && + (start > 0 || end + 1 < BTRFS_I(inode)->disk_i_size)) + btrfs_add_inode_defrag(NULL, inode); +} + /* * we create compressed extents in two phases. The first * phase compresses a range of pages that have already been @@ -429,10 +445,7 @@ static noinline void compress_file_range(struct inode *inode, int compress_type = root->fs_info->compress_type; int redirty = 0; - /* if this is a small write inside eof, kick off a defrag */ - if ((end - start + 1) < SZ_16K && - (start > 0 || end + 1 < BTRFS_I(inode)->disk_i_size)) - btrfs_add_inode_defrag(NULL, inode); + inode_should_defrag(inode, start, end, 0, compress_type); actual_end = min_t(u64, isize, end + 1); again: @@ -960,10 +973,8 @@ static noinline int cow_file_range(struct inode *inode, num_bytes = ALIGN(end - start + 1, blocksize); num_bytes = max(blocksize, num_bytes); - /* if this is a small write inside eof, kick off defrag */ - if (num_bytes < SZ_64K && - (start > 0 || end + 1 < BTRFS_I(inode)->disk_i_size)) - btrfs_add_inode_defrag(NULL, inode); + inode_should_defrag(inode, start, end, num_bytes, + BTRFS_COMPRESS_NONE); if (start == 0) { /* lets try to make an inline extent */