Re: [PATCH*2] mdadm works with uClibc from SVN
On 26 Jun 2006, Neil Brown said: On Tuesday June 20, [EMAIL PROTECTED] wrote: For some time, mdadm's been dumping core on me in my uClibc-built initramfs. As you might imagine this is somewhat frustrating, not least since my root filesystem's in LVM on RAID. Half an hour ago I got around to debugging this. Imagine my surprise when I found that it was effectively guaranteed to crash: map_dev() in util.c is stubbed out for uClibc builds, and returns -1 at all times. That means that code in, among other places, config.c:load_partitions() is guaranteed to segfault, which is a bit tough if you're using the (sane) default `DEVICE partitions' in your mdadm.conf. I'm confused. map_dev doesn't return -1, it returns NULL. Yes, that was a typo on my part. And the code in load_partitions() handles a NULL return. Er, what? The code in load_partitions() in mdadm 2.5.1 and below says ,[ config.c:load_partitions() ] | name = map_dev(major, minor, 1); | | d = malloc(sizeof(*d)); | d-devname = strdup(name); ` So if map_dev() returns NULL, we do a strdup(NULL) - crash inside uClibc. So I cannot see why it would be core dumping. If you have more details, I'd be really interested. Backtrace (against uClibc compiled with debugging symbols, tested against a temporary RAID array on a loopback device): (gdb) run Starting program: /usr/packages/mdadm/i686-loki/mdadm.uclibc --assemble --uuid=572aeae1:532641bf:49c9aec5:6036454d /dev/md127 Program received signal SIGSEGV, Segmentation fault. 0x080622f8 in strlen (s=0x0) at libc/string/i386/strlen.c:40 40 libc/string/i386/strlen.c: No such file or directory. in libc/string/i386/strlen.c (gdb) bt #0 0x080622f8 in strlen (s=0x0) at libc/string/i386/strlen.c:40 #1 0x08062d1d in strdup (s1=0x0) at libc/string/strdup.c:30 #2 0x0804b0df in load_partitions () at config.c:247 #3 0x0804b77c in conf_get_devs (conffile=0x0) at config.c:715 #4 0x0804d685 in Assemble (st=0x0, mddev=0xbf9c753e /dev/md127, mdfd=6, ident=0xbf9c630c, conffile=0x0, devlist=0x0, backup_file=0x0, readonly=0, runstop=0, update=0x0, homehost=0x0, verbose=0, force=0) at Assemble.c:184 #5 0x08049d62 in main (argc=4, argv=0xbf9c6514) at mdadm.c:958 (gdb) frame 2 #2 0x0804b0df in load_partitions () at config.c:247 247 d-devname = strdup(name); (gdb) info locals name = 0x0 mp = 0xbf9c5a34 09873360 hda\n f = (FILE *) 0x807b018 buf =3 09873360 hda\n, '\0' repeats 546 times, 1\000\000\000\000\000((ÿÿÿ\177\021, '\0' repeats 15 times, \021\000\000\000\231\231\231\031Z]\234¿\000\000(\005ÿÿÿ\177¼\\\234¿ÓU\006\bZ]\234¿\000\000\000\000\n, '\0' repeats 15 times, d^\234¿ÏÈ\004\bX]\234¿\000\000\000\000\n\000\000\000\000\000Linux, '\0' repeats 60 times, loki\000), '\0' repeats 43 times, \006, '\0' repeats 15 times, 2.6.17-s\000\000\000\000à]\234¿ßÉ\005\b|]\234¿ð]\234¿, '\0' repeats 12 times... rv = (mddev_dev_t) 0x0 I hope the bug's more obvious now :) That said: utils doesn't handle the case for 'ftw not available' as well as it could. I will fix that. The __UCLIBC_HAS_FTW__ macro in features.h is the right way to do that for uClibc, as Luca noted. -- `NB: Anyone suggesting that we should say Tibibytes instead of Terabytes there will be hunted down and brutally slain. That is all.' --- Matthew Wilcox - To unsubscribe from this list: send the line unsubscribe linux-raid in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH*2] mdadm works with uClibc from SVN
On Tue, 27 Jun 2006, Neil Brown prattled cheerily: On Tuesday June 27, [EMAIL PROTECTED] wrote: ,[ config.c:load_partitions() ] | name = map_dev(major, minor, 1); | | d = malloc(sizeof(*d)); | d-devname = strdup(name); ` Ahh.. uhmmm... Oh yes. I've fixed that since, but completely forgot about it and the change log didn't make it obvious. Yahoo. That's what I like about free software: the pre-emptive bugfixes! It seems that as soon as I find a bug, someone else will have already fixed it :) So: that's fixed for 2.5.2. Thanks for following it up. No, thank *you* for fixing my boot process :) -- `NB: Anyone suggesting that we should say Tibibytes instead of Terabytes there will be hunted down and brutally slain. That is all.' --- Matthew Wilcox - To unsubscribe from this list: send the line unsubscribe linux-raid in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH*2] mdadm works with uClibc from SVN
On Fri, Jun 23, 2006 at 08:45:47PM +0100, Nix wrote: On Fri, 23 Jun 2006, Neil Brown mused: Is there some #define in an include file which will allow me to tell if the current uclibc supports ftw or not? it is not only depending on the uClibc version, but also if ftw support was compiled in or not. I misspoke: ftw was split into multiple files in late 2005, but it was originally added in September 2003, in time for version 0.9.21. Obviously the #defines in ftw.h don't exist before that date, but that's a bit late to check, really. features.h provides the macros __UCLIBC_MAJOR__, __UCLIBC_MINOR__, and __UCLIBC_SUBLEVEL__: versions above 0.9.20 appear to support ftw() (at least, they have the function, in 32-bit form at least, which is certainly enough for this application!) the following would be the correct check. #include features.h #ifdef __UCLIBC_HAS_FTW__ . #else . #endif /* __UCLIBC_HAS_FTW__ */ -- Luca Berra -- [EMAIL PROTECTED] Communication Media Services S.r.l. /\ \ / ASCII RIBBON CAMPAIGN XAGAINST HTML MAIL / \ - To unsubscribe from this list: send the line unsubscribe linux-raid in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH*2] mdadm works with uClibc from SVN
On Fri, 23 Jun 2006, Neil Brown mused: On Friday June 23, [EMAIL PROTECTED] wrote: On 20 Jun 2006, [EMAIL PROTECTED] prattled cheerily: For some time, mdadm's been dumping core on me in my uClibc-built initramfs. As you might imagine this is somewhat frustrating, not least since my root filesystem's in LVM on RAID. Half an hour ago I got around to debugging this. Ping? No, but I do know someone by that name. Sorry, I'm pinging all the patches I've sent out in the last few weeks and this one was on my hit list :) Yeh, it's on my todo list. Agreed. I suspect I would rather make mdadm not dump core of ftw isn't available. It was... surprising. I'm still not sure how I ever made it work without this fxi; the first uClibc-based initramfs I ever rolled had mdadm 2.3.1 in it and a `DEVICE partitions', and there was no core dump. A mystery. Is there some #define in an include file which will allow me to tell if the current uclibc supports ftw or not? I misspoke: ftw was split into multiple files in late 2005, but it was originally added in September 2003, in time for version 0.9.21. Obviously the #defines in ftw.h don't exist before that date, but that's a bit late to check, really. features.h provides the macros __UCLIBC_MAJOR__, __UCLIBC_MINOR__, and __UCLIBC_SUBLEVEL__: versions above 0.9.20 appear to support ftw() (at least, they have the function, in 32-bit form at least, which is certainly enough for this application!) (and I'm more like to reply to mail if it the To or Cc line mentions me specifically - it's less likely to get lost that way). OK. -- `NB: Anyone suggesting that we should say Tibibytes instead of Terabytes there will be hunted down and brutally slain. That is all.' --- Matthew Wilcox - To unsubscribe from this list: send the line unsubscribe linux-raid in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH*2] mdadm works with uClibc from SVN
For some time, mdadm's been dumping core on me in my uClibc-built
initramfs. As you might imagine this is somewhat frustrating, not least
since my root filesystem's in LVM on RAID. Half an hour ago I got around
to debugging this.
Imagine my surprise when I found that it was effectively guaranteed to
crash: map_dev() in util.c is stubbed out for uClibc builds, and
returns -1 at all times. That means that code in, among other places,
config.c:load_partitions() is guaranteed to segfault, which is a bit
tough if you're using the (sane) default `DEVICE partitions' in your
mdadm.conf.
As far as I can tell this is entirely because ftw() support is not
implemented in uClibc. But as of November 2005, it *is* implemented in
uClibc 0.9.29-to-be in SVN, which rumour has it is soon to be officially
released. (The ftw() implementation is currently a copy of that from
glibc, but knowing the uClibc folks it will shrink dramatically as time
goes by. :) )
There are two options here. Either the trivial but rather unhelpful
approach of, well, *telling* people that what they're building
isn't going to work:
diff -durN 2.5.1-orig/Makefile 2.5.1-patched/Makefile
--- 2.5.1-orig/Makefile 2006-06-20 22:49:56.0 +0100
+++ 2.5.1-patched/Makefile 2006-06-20 22:50:34.0 +0100
@@ -97,6 +97,7 @@
mdadm.tcc : $(SRCS) mdadm.h
$(TCC) -o mdadm.tcc $(SRCS)
+# This doesn't work
mdadm.uclibc : $(SRCS) mdadm.h
$(UCLIBC_GCC) -DUCLIBC -DHAVE_STDINT_H -o mdadm.uclibc $(SRCS)
$(STATICSRC)
@@ -115,6 +116,7 @@
rm -f $(OBJS)
$(CC) $(LDFLAGS) $(ASSEMBLE_FLAGS) -static -DHAVE_STDINT_H -o
mdassemble.static $(ASSEMBLE_SRCS) $(STATICSRC)
+# This doesn't work
mdassemble.uclibc : $(ASSEMBLE_SRCS) mdadm.h
rm -f $(OJS)
$(UCLIBC_GCC) $(ASSEMBLE_FLAGS) -DUCLIBC -DHAVE_STDINT_H -static -o
mdassemble.uclibc $(ASSEMBLE_SRCS) $(STATICSRC)
diff -durN 2.5.1-orig/mdadm.h 2.5.1-patched/mdadm.h
--- 2.5.1-orig/mdadm.h 2006-06-02 06:35:22.0 +0100
+++ 2.5.1-patched/mdadm.h 2006-06-20 22:50:55.0 +0100
@@ -344,6 +344,7 @@
#endif
#ifdef UCLIBC
+#error This is known not to work.
struct FTW {};
# define FTW_PHYS 1
#else
Or the even-more-trivial but arguably far more useful approach of making
it work when possible (failing to compile with old uClibc because of the
absence of ftw.h, and working perfectly well with new uClibc):
diff -durN 2.5.1-orig/Makefile 2.5.1-patched/Makefile
--- 2.5.1-orig/Makefile 2006-06-20 22:49:56.0 +0100
+++ 2.5.1-patched/Makefile 2006-06-20 22:52:34.0 +0100
@@ -98,7 +98,7 @@
$(TCC) -o mdadm.tcc $(SRCS)
mdadm.uclibc : $(SRCS) mdadm.h
- $(UCLIBC_GCC) -DUCLIBC -DHAVE_STDINT_H -o mdadm.uclibc $(SRCS)
$(STATICSRC)
+ $(UCLIBC_GCC) -DHAVE_STDINT_H -o mdadm.uclibc $(SRCS) $(STATICSRC)
mdadm.klibc : $(SRCS) mdadm.h
rm -f $(OBJS)
@@ -117,7 +117,7 @@
mdassemble.uclibc : $(ASSEMBLE_SRCS) mdadm.h
rm -f $(OJS)
- $(UCLIBC_GCC) $(ASSEMBLE_FLAGS) -DUCLIBC -DHAVE_STDINT_H -static -o
mdassemble.uclibc $(ASSEMBLE_SRCS) $(STATICSRC)
+ $(UCLIBC_GCC) $(ASSEMBLE_FLAGS) -DHAVE_STDINT_H -static -o
mdassemble.uclibc $(ASSEMBLE_SRCS) $(STATICSRC)
# This doesn't work
mdassemble.klibc : $(ASSEMBLE_SRCS) mdadm.h
diff -durN 2.5.1-orig/mdadm.h 2.5.1-patched/mdadm.h
--- 2.5.1-orig/mdadm.h 2006-06-02 06:35:22.0 +0100
+++ 2.5.1-patched/mdadm.h 2006-06-20 22:53:02.0 +0100
@@ -343,14 +343,9 @@
struct stat64;
#endif
-#ifdef UCLIBC
- struct FTW {};
+#include ftw.h
+#ifdef __dietlibc__
# define FTW_PHYS 1
-#else
-# include ftw.h
-# ifdef __dietlibc__
-# define FTW_PHYS 1
-# endif
#endif
extern int add_dev(const char *name, const struct stat *stb, int flag, struct
FTW *s);
diff -durN 2.5.1-orig/util.c 2.5.1-patched/util.c
--- 2.5.1-orig/util.c 2006-06-16 01:25:44.0 +0100
+++ 2.5.1-patched/util.c2006-06-20 22:54:13.0 +0100
@@ -354,21 +354,6 @@
} *devlist = NULL;
int devlist_ready = 0;
-#ifdef UCLIBC
-int add_dev(const char *name, const struct stat *stb, int flag, struct FTW *s)
-{
- return 0;
-}
-char *map_dev(int major, int minor, int create)
-{
-#if 0
- fprintf(stderr, Warning - fail to map %d,%d to a device name\n,
- major, minor);
-#endif
- return NULL;
-}
-#else
-
#ifdef __dietlibc__
int add_dev_1(const char *name, const struct stat *stb, int flag)
{
@@ -467,8 +452,6 @@
return nonstd ? nonstd : std;
}
-#endif
-
unsigned long calc_csum(void *super, int bytes)
{
unsigned long long newcsum = 0;
With this latter patch, mdadm works flawlessly with my uClibc,
svnversion r15342 from 2006-06-08, and probably works just as well with
all SVN releases after r13017, 2005-12-30.
(One final request: please turn on world-readability in your generated
tarballs ;) right now util.c and some others are readable only by
user. Thanks.)
--
`NB: Anyone suggesting that we should say
