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
