On Mon, Jul 23, 2018 at 03:55:46PM -0700, Eric Sandeen wrote: > On 5/25/18 3:12 PM, Darrick J. Wong wrote: > > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx> > > > > XFS uses crc32c, not crc32. Remove the unnecessary crc32 code, which > > decreases binary size by the 8K crc32 table. > > Getting back to this ... > > The files have diverged a bit since the initial lift from the kernel. They've diverged a lot -- in the kernel there's code for some sort of software parallelized computation, and the self test code has been moved to a separate file. There's enough difference in the preprocessor code that I think we might be better off just removing all the cruft except for the bare minimum code we need + test code. The giant ugly table of self test code was created to check the fast crc32* computations against the slow bit-by-bit versions, so as long as we maintain the same input and output tables in userspace I don't think we have much to worry about wrt breakage. TBH this discussion has also convinced me that perhaps it would be useful to add a crc test command to xfs_db so that we can have xfstests run the crc32selftest code on any xfstests target instead of just the distro package build machines and developers' workstations. --D > Commenting out to keep things diffable is a bit pointless now, due > to that divergence (for example, the crc32test stuff has been moved > out to its own file, a big swath of code.) > > So, I think we should make a decision about whether we want to try > to keep this in sync with the kernel, or just let it drift. > > If the former, I'd go with an #if 0 but only after syncing things > up again. > > If the latter, just nuke the unused code, if we ever need the crc32c > variants we can re-lift them from the kernel. I'd still take a brief > look at whether there's anything that needs to be synced in an ad-hoc > fashion. > > I guess right now I'm feeling inclined to go with the former, and > even add the crc32 file(s) to the libxfs-diff and/or libxfs-apply > scripts, if we plan to keep them more in sync. > > I'd accept either plan, really - but there's no reason to comment > out or #ifdef code to keep it around if it doesn't even necessarily > match kernelspace anymore. > > -Eric > > > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx> > > --- > > include/libxfs.h | 3 --- > > libxfs/crc32.c | 16 ++++++++++------ > > libxfs/gen_crc32table.c | 8 +++++++- > > libxfs/libxfs_priv.h | 3 --- > > 4 files changed, 17 insertions(+), 13 deletions(-) > > > > > > diff --git a/include/libxfs.h b/include/libxfs.h > > index fbaae089..109866de 100644 > > --- a/include/libxfs.h > > +++ b/include/libxfs.h > > @@ -43,10 +43,7 @@ > > > > > > /* CRC stuff, buffer API dependent on it */ > > -extern uint32_t crc32_le(uint32_t crc, unsigned char const *p, size_t len); > > extern uint32_t crc32c_le(uint32_t crc, unsigned char const *p, size_t len); > > - > > -#define crc32(c,p,l) crc32_le((c),(unsigned char const *)(p),(l)) > > #define crc32c(c,p,l) crc32c_le((c),(unsigned char const *)(p),(l)) > > > > #include "xfs_cksum.h" > > diff --git a/libxfs/crc32.c b/libxfs/crc32.c > > index 783d62e9..8fe5c42c 100644 > > --- a/libxfs/crc32.c > > +++ b/libxfs/crc32.c > > @@ -176,20 +176,26 @@ static inline u32 __pure crc32_le_generic(u32 crc, unsigned char const *p, > > } > > > > #if CRC_LE_BITS == 1 > > +/* > > + * not used by xfs. > > u32 __pure crc32_le(u32 crc, unsigned char const *p, size_t len) > > { > > return crc32_le_generic(crc, p, len, NULL, CRCPOLY_LE); > > } > > + */ > > ... > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html