| Message ID | 1391029564-24160-1-git-send-email-fdmanana@gmail.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
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, Ever since this patch was committed (git ref 0b947aff1599afbbd2ec07ada87b05af0f94cf10), the btrfs module (presumably intentionally) no longer depends on the crc32c module. 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)". 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. 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. :( 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. 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 -- 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
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. 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
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
On 27 February 2014 12:43, Filipe David Manana <fdmanana@gmail.com> wrote: > On Wed, Feb 26, 2014 at 11:26 PM, WorMzy Tykashi > <wormzy.tykashi@gmail.com> wrote: >> >> Hi, > > Hi > Hi again, Sorry for the delay in replying, I've not had the time to gather my thoughts and write a coherent reply. >> >> 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). > Thanks for clarifying this, I assumed that libcrc32c and crc32c were aliases of the same module. I can see from the modinfo input that they're not. >> 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)". I've found that mkinitcpio automatically adds crc32c to the initrd if lib32crc is needed, so before, even though libcrc32c wasn't actually needed, btrfs' dependency on it caused mkinitcpio to include the actual dependency (crc32c) in the image. https://projects.archlinux.org/mkinitcpio.git/tree/functions?id=v16#n398 Now that the dependency is gone, and nothing else necessary to mount my root filesystem depends on libcrc32c, crc32c is no longer being added to the initrd. > > 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). > Debian uses a different tool, but has a similar workaround to the problem. I found this in the comments... /usr/share/initramfs-tools/hook-functions: line 507: --- # 'depmod' only looks at symbol dependencies; there is no way for # modules to declare explicit dependencies through module information, # so dependencies on e.g. crypto providers are hidden. Until this is # fixed, we need to handle those hidden dependencies. hidden_dep_add_modules() { local modules= for dep in "lib/libcrc32c crc32c" "fs/ubifs/ubifs deflate zlib lzo"; do set -- $dep if [ -f "${DESTDIR}/lib/modules/${version}/kernel/$1.ko" ]; then shift modules="$modules $@" fi done manual_add_modules $modules } --- This makes sure that, if libcrc32c is needed, crc32c is added too. Presumably, one or more of the modules being added to your initrd still needs libcrc32c, which is why you don't experience this problem. > 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 > Right, so the problem is actually that btrfs module doesn't depend on the crc32c module. The crc32c module doesn't export any symbols, so it's impossible (?) to explicitly depend on it, so initrd creation tools have had to hack around the problem to make sure that the crc32c module is still added to initrds when it's needed. >> >> 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. > > Hope it helps, thanks. > Thanks. I think I've learnt a lot in the past few days. And I've typed crc32c so many times that I see it when I close my eyes. It's clear that the removal of the libcrc32c module wasn't the direct cause of this bug, but it did remove the crutch that was stopping things falling over. I guess that initrd creation tools have to adapt to this change, and include crc32c when btrfs is necessary. Unless you can see any flaws in my logic, I'll write a patch for mkinitcpio and see if the maintainer will include it before 3.14 proper goes stable, so that not too many Arch users get bitten by this problem. I guess someone should do the same for debian's initramfs-tools. >> 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." Cheers again, WorMzy -- 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
On Sat, Mar 1, 2014 at 9:20 PM, WorMzy Tykashi <wormzy.tykashi@gmail.com> wrote: > On 27 February 2014 12:43, Filipe David Manana <fdmanana@gmail.com> wrote: >> On Wed, Feb 26, 2014 at 11:26 PM, WorMzy Tykashi >> <wormzy.tykashi@gmail.com> wrote: >>> >>> Hi, >> >> Hi >> > > Hi again, > > Sorry for the delay in replying, I've not had the time to gather my > thoughts and write a coherent reply. >>> >>> 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). >> > > Thanks for clarifying this, I assumed that libcrc32c and crc32c were > aliases of the same module. I can see from the modinfo input that > they're not. > >>> 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)". > > I've found that mkinitcpio automatically adds crc32c to the initrd if > lib32crc is needed, so before, even though libcrc32c wasn't actually > needed, btrfs' dependency on it caused mkinitcpio to include the > actual dependency (crc32c) in the image. > > https://projects.archlinux.org/mkinitcpio.git/tree/functions?id=v16#n398 > > Now that the dependency is gone, and nothing else necessary to mount > my root filesystem depends on libcrc32c, crc32c is no longer being > added to the initrd. > >> >> 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). >> > > Debian uses a different tool, but has a similar workaround to the > problem. I found this in the comments... > > /usr/share/initramfs-tools/hook-functions: line 507: > --- > # 'depmod' only looks at symbol dependencies; there is no way for > # modules to declare explicit dependencies through module information, > # so dependencies on e.g. crypto providers are hidden. Until this is > # fixed, we need to handle those hidden dependencies. > hidden_dep_add_modules() > { > local modules= > for dep in "lib/libcrc32c crc32c" "fs/ubifs/ubifs deflate zlib lzo"; do > set -- $dep > if [ -f "${DESTDIR}/lib/modules/${version}/kernel/$1.ko" ]; then > shift > modules="$modules $@" > fi > done > manual_add_modules $modules > } > --- > > This makes sure that, if libcrc32c is needed, crc32c is added too. You've made great progress finding this. > > Presumably, one or more of the modules being added to your initrd > still needs libcrc32c, which is why you don't experience this problem. Possibly, since libcrc32c is used by xfs and ceph, both of which I have built as modules. Just remembered now, that ext4 for example depends on crypto's crc32c as well but not on libcrc32c (just like btrfs in 3.14) - https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/fs/ext4/Kconfig So it sounds like you could get into the same issue if you build crypto's crc32c as a module and ext4 as a module too. > > >> 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 >> > > Right, so the problem is actually that btrfs module doesn't depend on > the crc32c module. The crc32c module doesn't export any symbols, so > it's impossible (?) to explicitly depend on it, so initrd creation > tools have had to hack around the problem to make sure that the crc32c > module is still added to initrds when it's needed. > >>> >>> 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. >> >> Hope it helps, thanks. >> > > Thanks. I think I've learnt a lot in the past few days. And I've typed > crc32c so many times that I see it when I close my eyes. > > It's clear that the removal of the libcrc32c module wasn't the direct > cause of this bug, but it did remove the crutch that was stopping > things falling over. > > I guess that initrd creation tools have to adapt to this change, and > include crc32c when btrfs is necessary. > > Unless you can see any flaws in my logic, I'll write a patch for > mkinitcpio and see if the maintainer will include it before 3.14 > proper goes stable, so that not too many Arch users get bitten by this > problem. I guess someone should do the same for debian's > initramfs-tools. Seems ok, but I don't have much expertise in using mkinitcpio and similar tools directly, as I always rely on the kernel's "make install" to figure out everything for me (on debian and ubuntu). Let me know how it goes. Thanks. > >>> 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." > > Cheers again, > > > WorMzy
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,
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(-)