On 2018年03月02日 19:00, Filipe Manana wrote: > On Fri, Mar 2, 2018 at 10:54 AM, Qu Wenruo <quwenruo.bt...@gmx.com> wrote: >> >> >> On 2018年03月02日 18:46, Filipe Manana wrote: >>> On Fri, Mar 2, 2018 at 5:22 AM, Qu Wenruo <w...@suse.com> wrote: >>>> Normally when specifying max_inline, we should normally limit it by >>>> uncompressed extent size, as it's the only thing user can control. >>> >>> Why does it matter that users can control it? Will they write less (or >>> more) data to files because stuff won't get inlined? >>> Why do they care about stuff getting inlined or not? That's an >>> implementation detail of btrfs to speed up access to file data and >>> save some space. >> >> Then why we still have max_inline mount option? > > My comment was about deciding based on which size to make the decision > (compressed vs uncompressed).
The same thing, we have given user options to trigger the behavior, then we should give them *predictable* option to modify the behavior. Not something confusing like current max_inline. Either we give user max_inline and max_inline_compressed, or both follow max_inline. Thanks, Qu > >> Just do everything we *think* is best is good enough in that case. >> >> If we provide that mount option to allow *user* to specify the behavior, >> then allow then to do the same control. >> >> Thanks, >> Qu >> >>> >>>> (Control the algorithm and compressed data is almost impossible) >>>> >>>> Since btrfs is providing *TRANSPARENT* compression, max_inline should >>>> behave the same for both plain and compress data. >>> >>> Taking away the benefits of compression for. So now some cases that >>> ended up getting the benefits of inlining won't get them anymore. >>> >>> I don't agree with this change. >> >> >>> >>>> >>>> So this patch will use @inline_len instead of @data_len in >>>> cow_file_range_inline() so user will know their max_inline mount option >>>> works exactly the same for both plain and compressed data extent. >>>> >>>> Signed-off-by: Qu Wenruo <w...@suse.com> >>>> --- >>>> fs/btrfs/inode.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c >>>> index e1a7f3cb5be9..48472509239b 100644 >>>> --- a/fs/btrfs/inode.c >>>> +++ b/fs/btrfs/inode.c >>>> @@ -303,7 +303,7 @@ static noinline int cow_file_range_inline(struct >>>> btrfs_root *root, >>>> (!compressed_size && >>>> (actual_end & (fs_info->sectorsize - 1)) == 0) || >>>> end + 1 < isize || >>>> - data_len > fs_info->max_inline) { >>>> + inline_len > fs_info->max_inline) { >>>> return 1; >>>> } >>>> >>>> -- >>>> 2.16.2 >>>> >>>> -- >>>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in >>>> the body of a message to majord...@vger.kernel.org >>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> >>> >>> >> > > >
signature.asc
Description: OpenPGP digital signature