[prev in list] [next in list] [prev in thread] [next in thread]
List: linux-btrfs
Subject: Re: [PATCH] Btrfs: use btrfs_crc32c everywhere instead of libcrc32c
From: Philipp Klein <philipptheklein () gmail ! com>
Date: 2014-02-27 13:10:58
Message-ID: CAAr2hixwf824ok2JGP9M3U0M=mTHUUDatXF=a5sAvhfuTn4EWw () mail ! gmail ! com
[Download RAW message or body]
Hi,
I am the Arch user who initially reported this problem to the AUR (
https://aur.archlinux.org/packages/linux-mainline/).
2014-02-27 13:43 GMT+01:00 Filipe David Manana <fdmanana@gmail.com>:
> On Wed, Feb 26, 2014 at 11:26 PM, WorMzy Tykashi
> <wormzy.tykashi@gmail.com> wrote:
> > On 29 January 2014 21:06, Filipe David Borba Manana <fdmanana@gmail.com>
> wrote:
> > > After the commit titled "Btrfs: fix btrfs boot when compiled as
> built-in",
> > > LIBCRC32C requirement was removed from btrfs' Kconfig. This made it not
> > > possible to build a kernel with btrfs enabled (either as module or
> built-in)
> > > if libcrc32c is not enabled as well. So just replace all uses of
> libcrc32c
> > > with the equivalent function in btrfs hash.h - btrfs_crc32c.
> > >
> > > Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
> > > ---
> > > fs/btrfs/check-integrity.c | 4 ++--
> > > fs/btrfs/disk-io.c | 4 ++--
> > > fs/btrfs/send.c | 4 ++--
> > > 3 files changed, 6 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/fs/btrfs/check-integrity.c b/fs/btrfs/check-integrity.c
> > > index 160fb50..39bfd56 100644
> > > --- a/fs/btrfs/check-integrity.c
> > > +++ b/fs/btrfs/check-integrity.c
> > > @@ -92,11 +92,11 @@
> > > #include <linux/slab.h>
> > > #include <linux/buffer_head.h>
> > > #include <linux/mutex.h>
> > > -#include <linux/crc32c.h>
> > > #include <linux/genhd.h>
> > > #include <linux/blkdev.h>
> > > #include "ctree.h"
> > > #include "disk-io.h"
> > > +#include "hash.h"
> > > #include "transaction.h"
> > > #include "extent_io.h"
> > > #include "volumes.h"
> > > @@ -1823,7 +1823,7 @@ static int btrfsic_test_for_metadata(struct
> btrfsic_state *state,
> > > size_t sublen = i ? PAGE_CACHE_SIZE :
> > > (PAGE_CACHE_SIZE - BTRFS_CSUM_SIZE);
> > >
> > > - crc = crc32c(crc, data, sublen);
> > > + crc = btrfs_crc32c(crc, data, sublen);
> > > }
> > > btrfs_csum_final(crc, csum);
> > > if (memcmp(csum, h->csum, state->csum_size))
> > > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> > > index 7619147..3903bd3 100644
> > > --- a/fs/btrfs/disk-io.c
> > > +++ b/fs/btrfs/disk-io.c
> > > @@ -26,7 +26,6 @@
> > > #include <linux/workqueue.h>
> > > #include <linux/kthread.h>
> > > #include <linux/freezer.h>
> > > -#include <linux/crc32c.h>
> > > #include <linux/slab.h>
> > > #include <linux/migrate.h>
> > > #include <linux/ratelimit.h>
> > > @@ -35,6 +34,7 @@
> > > #include <asm/unaligned.h>
> > > #include "ctree.h"
> > > #include "disk-io.h"
> > > +#include "hash.h"
> > > #include "transaction.h"
> > > #include "btrfs_inode.h"
> > > #include "volumes.h"
> > > @@ -244,7 +244,7 @@ out:
> > >
> > > u32 btrfs_csum_data(char *data, u32 seed, size_t len)
> > > {
> > > - return crc32c(seed, data, len);
> > > + return btrfs_crc32c(seed, data, len);
> > > }
> > >
> > > void btrfs_csum_final(u32 crc, char *result)
> > > diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> > > index 04c07ed..31b76d0 100644
> > > --- a/fs/btrfs/send.c
> > > +++ b/fs/btrfs/send.c
> > > @@ -24,12 +24,12 @@
> > > #include <linux/xattr.h>
> > > #include <linux/posix_acl_xattr.h>
> > > #include <linux/radix-tree.h>
> > > -#include <linux/crc32c.h>
> > > #include <linux/vmalloc.h>
> > > #include <linux/string.h>
> > >
> > > #include "send.h"
> > > #include "backref.h"
> > > +#include "hash.h"
> > > #include "locking.h"
> > > #include "disk-io.h"
> > > #include "btrfs_inode.h"
> > > @@ -620,7 +620,7 @@ static int send_cmd(struct send_ctx *sctx)
> > > hdr->len = cpu_to_le32(sctx->send_size - sizeof(*hdr));
> > > hdr->crc = 0;
> > >
> > > - crc = crc32c(0, (unsigned char *)sctx->send_buf,
> sctx->send_size);
> > > + crc = btrfs_crc32c(0, (unsigned char *)sctx->send_buf,
> sctx->send_size);
> > > hdr->crc = cpu_to_le32(crc);
> > >
> > > ret = write_buf(sctx->send_filp, sctx->send_buf,
> sctx->send_size,
> > > --
> > > 1.7.9.5
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
> in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
> > Hi,
>
> Hi
>
> >
> > Ever since this patch was committed (git ref
> > 0b947aff1599afbbd2ec07ada87b05af0f94cf10), the btrfs module
> > (presumably intentionally) no longer depends on the crc32c module.
>
> To be more clear, it no longer depends on LIBCRC32C (which is just a
> convenience library to access crypto's crc32c).
> It still depends on CRYPTO and CRYPTO_CRC32C (which is what LIBCRC32C
> uses).
>
> > However, this means that this module is not pulled in during initrd
> > creation (at least using mkinitcpio on Arch Linux), and as a result,
> > the btrfs module cannot be loaded. Instead modprobe complains with:
> > "Unknown symbol in module, or unknown parameter (see dmesg)".
>
> That is weird. On debian creating the initrd via kernel's makefile
> (make modules_install && make install) works for me (don't know if it
> uses mkinitcpio or something else).
>
> >
> > Unfortunately there is no accompanying message in dmesg, so I can't
> > provide much more information. However, I have bisected the commit to
> > confirm that this problem was introduced by this patch. The following
> > is a grep of btrfs module's dependencies before and after this was
> > committed:
> >
> > $ grep btrfs pkg/lib/modules/3.13.0-ARCH-00150-g8101c8d/modules.dep
> > kernel/fs/btrfs/btrfs.ko: kernel/lib/raid6/raid6_pq.ko
> > kernel/lib/libcrc32c.ko kernel/crypto/xor.ko
> >
> > $ grep btrfs pkg/lib/modules/3.13.0-ARCH-00151-g0b947af/modules.dep
> > kernel/fs/btrfs/btrfs.ko: kernel/lib/raid6/raid6_pq.ko
> kernel/crypto/xor.ko
> >
> > As you can see, the dependency on kernel/lib/libcrc32c.ko was removed.
>
> Yep, it is intentional.
>
> >
> > However, if crc32c.ko is manually added to the initrd, the btrfs
> > module loads fine, which suggests that it should still be a
> > dependency. If it shouldn't be a dependency any more, then something
> > is wrong somewhere, but I don't understand enough about kernel modules
> > to find out why right now. :(
>
> So crc32c.ko isn't libcrc32c (which is libcrc32c.ko). It's crypto's
> crc32c, and that's a correct module dependency:
>
>
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/fs/btrfs/Kconfig?id=refs/tags/v3.14-rc4
>
> >
> > I hope that this information is enough to help you discover the
> > problem. I've tried adding back in the linux/crc32c.h include
> > statements in the affected files, but this hasn't changed the
> > situation, so I guess a full revert would be necessary. Unfortunately
> > I have to sleep now, so I can't do any further investigation tonight.
> > I'll try to find time tomorrow to investigate further.
> >
> > Please let me know if any parts of this message don't make sense, or
> > require further investigation.
>
> So the intention of this patch was to remove calls to libcrc32c, which
> isn't a dependency anymore. Without this patch, you would get linker
> errors when building btrfs unless libcrc32c is built-in (i.e. only
> compiled if CONFIG_LIBCRC32C=y). Several people reported this issue
> and this patch fixed the problem for them, e.g.
> https://lkml.org/lkml/2014/2/1/165
>
> Another issue that got fixed in 3.14-rc2, but possibly unrelated to
> your issue, is http://www.spinics.net/lists/linux-btrfs/msg31276.html
>
> Can you try 3.14-rc4? Maybe it's some arch specific issue, but
> crc32c.ko (or crc32c-intel.ko) isn't definitely the same as
> libcrc32c.ko.
>
I am runnning 3.14-rc4 right now and had to use the workaround WorMzy
described.
I cannot say if it is a Arch Linux specific problem because I have no
knowledge how mkinitcpio works.
>
> Hope it helps, thanks.
>
> >
> > Also, apologies for the late reporting of this problem, I was
> > previously explicitly including the crc32c module it my initrd, I was
> > only made aware of the problem after another mainline kernel Arch user
> > pointed the problem out here:
> > https://aur.archlinux.org/packages/linux-mainline/
> >
> > Also, apologies if this has already been reported. I searched the
> > mailing list, but couldn't find anything.
> >
> > Cheers,
> >
> >
> > WorMzy
>
>
>
> --
> Filipe David Manana,
>
> "Reasonable men adapt themselves to the world.
> Unreasonable men adapt the world to themselves.
> That's why all progress depends on unreasonable men."
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Thanks
Philipp
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic