On 2019/6/28 下午7:34, David Sterba wrote: > On Fri, Jun 28, 2019 at 09:26:53AM +0800, Qu Wenruo wrote: >> >> >> On 2019/6/27 下午10:58, David Sterba wrote: >>> On Tue, Jun 25, 2019 at 04:24:57PM +0800, Qu Wenruo wrote: >>>> Ping? >>>> >>>> This patch should fix the problem of compressed extent even when >>>> nodatasum is set. >>>> >>>> It has been one year but we still didn't get a conclusion on where >>>> force_compress should behave. >>> >>> Note that pings to patches sent year ago will get lost, I noticed only >>> because you resent it and I remembered that we had some discussions, >>> without conclusions. >>> >>>> But at least to me, NODATASUM is a strong exclusion for compress, no >>>> matter whatever option we use, we should NEVER compress data without >>>> datasum/datacow. >>> >>> That's correct, but the way you fix it is IMO not right. This was also >>> noticed by Nikolay, that there are 2 locations that call >>> inode_need_compress but with different semantics. >>> >>> One is the decision if compression applies at all, >> >>> and the second one >>> when that's certain it's compression, to do it or not based on the >>> status decision of eg. heuristics. >> >> The second call is in compress_file_extent(), with inode_need_compress() >> return 0 for NODATACOW/NODATASUM inodes, we will not go into >> cow_file_range_async() branch at all. >> >> So would you please explain how this could cause problem? >> To me, prevent the problem in inode_need_compress() is the safest location. > > Let me repeat: two places with different semantics. So this means that > we need two functions that reflect the differences. That it's in one > function that works both contexts is ok from functionality point of > view, but if we care about clarity of design and code we want two > functions. >
OK, so in next version I'll split the inode_need_compress() into two functions for different semantics: - inode_can_compress() The hard requirement for compress code. E.g. COW and checksum checks. - inode_need_compress() The soft requirement, for things like ratio, force_compress checks. Will this modification be fine? Thanks, Qu
signature.asc
Description: OpenPGP digital signature
