On 2018/5/4 11:17 AM, Michael Lyle wrote: > Hi Coly-- > > On Thu, May 3, 2018 at 7:49 PM, Coly Li <colyli@xxxxxxx> wrote: >> On 2018/5/4 3:37 AM, Andy Shevchenko wrote: >> I like the idea. So far a problem is endianness, bch_crc64() is for >> on-disk format, which means it is always little endian, and lib/crc64.c >> should not have such assumption. Further more, almost all the bcache >> code assumes the processor is little endian, including the checksum >> calculation.And the order to calculate and check checksum in bcache code >> should be fixed as well. The result is, so far bcache does not work on >> s390x. > Hi Micheal, > I don't see where this code has any endian implications, can you > please point it out to me? > So far cache_sb.csum is defined as __u64, and it should be le64 or __le64 as on-disk format. For on-disk format, we need something like crc64_le() to calculate 64bits crc. In my mind, first we need to fix the endianness issue in bcache. If we move the code into lib, and other sub system uses it, then after the endianness issue is fixed in bcache and we find we need to change the API of lib/crc64.c:crc64(), that will not be easier. So I suggest to wait for months. When this code goes into lib/crc64,c, there should also be crc64_le() and crc64_be(). Then bcache code may explicitly reference crc64_le() version for checksum. > I like the idea of factoring this to the library. Though one issue I > see is there's many different CRC64 polynomials. Calling this one > "CRC64" may not be appropriate. Yes, I also concern about this. Because this crc64 is used for on-disk storage, if its algorithm changes out side of bcache code, and bcache references it, this change may probably results bogus checksum failure. Another thing why I said 'wait' is, so far only bcache uses crc64(), it does not contribute too much benefit to move the code into lib/crc64.c temporarily. Because bcachefs also uses crc64, if bcache and bcache fs may both uses same crc64 code, it will make sense to make the code as lib, IMHO. Thanks. Coly Li -- To unsubscribe from this list: send the line "unsubscribe linux-bcache" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
