Re: very poor ext3 write performance on big filesystems?
Mark Lord wrote: Theodore Tso wrote: .. The following ld_preload can help in some cases. Mutt has this hack encoded in for maildir directories, which helps. .. Oddly enough, that same spd_readdir() preload craps out here too when used with "rm -r" on largish directories. From looking at the code, I think I've found at least one bug in opendir: ... dnew = realloc(dirstruct->dp, dirstruct->max * sizeof(struct dir_s)); ... Shouldn't this be: "...*sizeof(struct dirent_s));"? -- Paulo Marques - www.grupopie.com "Nostalgia isn't what it used to be." -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Disk schedulers
Lukas Hejtmanek wrote: [...] If I'm scping file (about 500MB) from the network (which is faster than the local disk), any process is totally unable to read anything from the local disk till the scp finishes. It is not caused by low free memory, while scping I have 500MB of free memory (not cached or buffered). If you want to take advantage of all that memory to buffer disk writes, so that the reads can proceed better, you might want to tweak your /proc/sys/vm/dirty_ratio amd /proc/sys/vm/dirty_background_ratio to more appropriate values. (maybe also dirty_writeback_centisecs and dirty_expire_centisecs) You can read all about those tunables in Documentation/filesystems/proc.txt. Just my 2 cents, -- Paulo Marques - www.grupopie.com "Very funny Scotty. Now beam up my clothes." -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: managing kallsyms_addresses
Robin Getz wrote: When the kernel needs to find out what symbol is at a specific address, it uses kallsyms_lookup() This seems to work pretty well - almost. The problem is today, we don't to remove the symbols from the init section when the init section is freed. There is invalid data in kallsyms_addresses. [...] There would be two solutions: - when freeing the init section, remove all the init labels from the kallsyms_addresses, and resort/pack it. This should be doable, but to be worth it, we would need to use different structures for the init symbols, that would be stored in __initdata. Then, ideally we would have separate functions in the __init section to look up symbols in those structures that would only be called until we released the init sections. On the plus side, this would avoid the "repacking" the kallsyms structures to remove the init labels. - if the init section is unloaded, have is_kernel_inittext always return 0. This is by far the simplest solution. I even STR a patch floating around to do this, by I can't seem to locate it now... :( I assume that similar things need to be handled for module init too, but I have not run into that yet. It seems that at least the last solution should be easy enough to implement there. Thoughts? I think that the simplest solution (the second one) is probably the best for now. One thing that did cross my mind though, is stuff like lockdep. If we run a locking sequence that is called from init code, and later a different locking sequence is used when we already freed init data, how can the debug information show the names of the functions that generated the previous locking sequence? It seems that the only "correct" approach would be to store a "before freeing init sections" flag together with the function pointer, and then have a kallsyms interface that could receive this flag to know where to look. In that case, the first solution can not be used at all (because we need to keep the init symbols anyway) and the second solution could be simply implemented by having a default value for the flag that is the "current state" for that flag... -- Paulo Marques - www.grupopie.com "There cannot be a crisis today; my schedule is already full." -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] [Kallsyms] Blackfin: Allow kernel symbols in L1 to be found
Bryan Wu wrote: From: Robin Getz <[EMAIL PROTECTED]> [...] --- a/scripts/kallsyms.c +++ b/scripts/kallsyms.c @@ -12,6 +12,8 @@ * (25/Aug/2004) Paulo Marques <[EMAIL PROTECTED]> * Changed the compression method from stem compression to "table lookup" * compression + * (10/Jul/2006) Robin Getz <[EMAIL PROTECTED]> + * Add _stext_l1, _etext_l1 for the L1 memory section in Blackfin. When I wrote this initially, it was a mistake to add a Changelog in the first place, but I didn't know better at the time. If you're going to make changes to this file, please remove all the Changelog, instead of adding more entries to it. The "Changelog" should be kept by the version control system, and not the source code itself. The rest of the patch looks ok, though. -- Paulo Marques - www.grupopie.com "Very funny Scotty. Now beam up my clothes." -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/6] POWERPC: use KSYM_NAME_LEN
Cyrill Gorcunov wrote:
[Paulo Marques - Wed, Jan 23, 2008 at 06:26:28PM +]
Cyrill Gorcunov wrote:
[...]
case 's':
- getstring(tmp, 64);
+ getstring(tmp, sizeof(tmp));
if (setjmp(bus_error_jmp) == 0) {
catch_memory_errors = 1;
sync();
just after that poin in the original code a call to kallsyms_lookup_name
is done - so i think it could be an overflow (of course it depends
on what *exactly* the name is being searched, and Paulo - I didn't
managed to get *the whole picture* of what is going on in this
code - so the thoughs were like: kallsyms_lookup_name could find
a quite long name restricted by KSYM_NAME_LEN (dunno how it could
happens - due to buggy code or due to memory corruption outside,
it does not matter - the only matter - it *could* find that long
name).
Ah, now I understand your confusion: kallsyms_lookup_name doesn't fill
the name. It searches the name and returns the address. It is the
_caller_ that fills the name, not kallsyms_lookup_name.
It is used for stuff like: "give me the address of function foo":
addr = kallsyms_lookup_name("foo");
Anyway - it's just an attempt ;) we always could drop it far-far away ;)
I think that using KSYM_NAME_LEN would be a nice cleanup for xmon, but
it is for the powerpc guys to decide if they want to do it. I just
wanted to point the change in behavior so that it wouldn't go unnoticed.
For all we know, the stack may at that point be close to full and an
extra 64 bytes may tip it over the edge.
This also introduces a change in behavior. It is still a nice cleanup,
though. So, if the powerpc people feel they can spare an extra 64 bytes of
stack here, I guess it's ok.
Thanks a lot for review Paulo!
No problem. I always keep an eye out for kallsyms related stuff.
--
Paulo Marques - www.grupopie.com
"There cannot be a crisis today; my schedule is already full."
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/6] POWERPC: use KSYM_NAME_LEN
Cyrill Gorcunov wrote:
Use KSYM_NAME_LEN instead of numeric value.
The patch series looks like a nice cleanup, except for a few things in
this patch.
Actually because of too small 'tmp' there is
a potential buffer overflow.
I don't think there is. "tmp" is not being passed to kallsyms to be
filled with a symbol name, but it's being used to hold a name written by
the user to lookup an address.
If the powerpc/xmon people feel that 63 characters is enough to hold a
symbol name, it's their problem, but there is no buffer overflow.
Signed-off-by: Cyrill Gorcunov <[EMAIL PROTECTED]>
---
Index: linux-2.6.git/arch/powerpc/xmon/xmon.c
===
--- linux-2.6.git.orig/arch/powerpc/xmon/xmon.c 2008-01-23 19:04:42.0
+0300
+++ linux-2.6.git/arch/powerpc/xmon/xmon.c 2008-01-23 19:12:45.0
+0300
@@ -69,7 +69,7 @@ static unsigned long ndump = 64;
static unsigned long nidump = 16;
static unsigned long ncsum = 4096;
static int termch;
-static char tmpstr[128];
+static char tmpstr[KSYM_NAME_LEN];
This one seems ok, since "tmpstr" is used everywhere to hold symbol names.
#define JMP_BUF_LEN23
static long bus_error_jmp[JMP_BUF_LEN];
@@ -2354,7 +2354,7 @@ scanhex(unsigned long *vp)
}
} else if (c == '$') {
int i;
- for (i=0; i<63; i++) {
+ for (i = 0; i < sizeof(tmpstr) / 2; i++) {
This one is completely out of the blue. Why "sizeof(tmpstr) / 2"?
It would make more sense to use "sizeof(tmpstr) - 1", but either way it
is a change in behavior.
c = inchar();
if (isspace(c)) {
termch = c;
@@ -2467,7 +2467,7 @@ symbol_lookup(void)
{
int type = inchar();
unsigned long addr;
- static char tmp[64];
+ static char tmp[KSYM_NAME_LEN];
switch (type) {
case 'a':
@@ -2476,7 +2476,7 @@ symbol_lookup(void)
termch = 0;
break;
case 's':
- getstring(tmp, 64);
+ getstring(tmp, sizeof(tmp));
if (setjmp(bus_error_jmp) == 0) {
catch_memory_errors = 1;
sync();
This also introduces a change in behavior. It is still a nice cleanup,
though. So, if the powerpc people feel they can spare an extra 64 bytes
of stack here, I guess it's ok.
--
Paulo Marques - www.grupopie.com
"As far as we know, our computer has never had an undetected error."
Weisert
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] mmaped copy too slow?
KOSAKI Motohiro wrote: Hi at one point, I found the large file copy speed was different depending on the copy method. I compared below method - read(2) and write(2). - mmap(2) x2 and memcpy. - mmap(2) and write(2). in addition, effect of fadvice(2) and madvice(2) is checked. to a strange thing, - most faster method is read + write + fadvice. - worst method is mmap + memcpy. One thing you could also try is to pass MAP_POPULATE to mmap so that the page tables are filled in at the time of the mmap, avoiding a lot of page faults later. Just my 2 cents, -- Paulo Marques - www.grupopie.com "All I ask is a chance to prove that money can't make me happy." -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] call sysrq_timer_list_show from a workqueue
Rusty Russell wrote: On Wednesday 09 January 2008 11:21:59 Andrew Morton wrote: [...] And the fact that incoming arg `namebuf' MUST point at a KSYM_NAME_LEN-sized buffer could be better communicated by using a dedicated struct for this, or by giving the arg a type of `char namebuf[KSYM_NAME_LEN]'. Or by adding a comment. Or by just ignoring me and doing something more useful. Or better, rework all the name lookup interfaces, rather than having: Yes, there is some rework we can do here struct module *module_text_address(unsigned long addr); struct module *__module_text_address(unsigned long addr); int is_module_address(unsigned long addr); int module_get_kallsym(unsigned int symnum, unsigned long *value, char *type, char *name, char *module_name, int *exported); char *module_address_lookup(unsigned long addr, unsigned long *symbolsize, unsigned long *offset, char **modname, char *namebuf); int lookup_module_symbol_name(unsigned long addr, char *symname); int lookup_module_symbol_attrs(unsigned long addr, unsigned long *size, unsigned long *offset, char *modname, char *name); unsigned long module_kallsyms_lookup_name(const char *name); All of these are somewhat less important, because most users call the kallsyms generic functions which will in turn call these functions if the symbol isn't global. So, they should suffer basically the same transformations as the kallsyms_ counterparts and can be considered almost "internal" to the kallsyms infrastructure. unsigned long kallsyms_lookup_name(const char *name); This one look fine, as there is no duplication in any other function. extern int kallsyms_lookup_size_offset(unsigned long addr, unsigned long *symbolsize, unsigned long *offset); const char *kallsyms_lookup(unsigned long addr, unsigned long *symbolsize, unsigned long *offset, char **modname, char *namebuf); int lookup_symbol_attrs(unsigned long addr, unsigned long *size, unsigned long *offset, char *modname, char *name); int lookup_symbol_name(unsigned long addr, char *symname); These 4 functions can probably be condensed into just one, by allowing NULL pointer arguments to mean "don't need this result": kallsyms_lookup_size_offset(a,s,o) <=> kallsyms_lookup(a,s,o,NULL,NULL) lookup_symbol_attrs(a,s,o,m,n) <=> kallsyms_lookup(a,s,o,m,n) lookup_symbol_name(a,n) <=> kallsyms_lookup(a,NULL,NULL,NULL,n) extern int sprint_symbol(char *buffer, unsigned long address); extern void __print_symbol(const char *fmt, unsigned long address); These 2 are probably fine. There is a difference in the way the module name is passed, because kallsyms_lookup assumes it can return just a pointer to the module name. However, we should probably change that interface so that the caller provides the buffer to hold the module name, to avoid races. The stop_machine should help already, but returning a pointer that can be stale just a little bit later isn't pretty anyway. I can do a patch for this, but this will touch a few subsystems that use these interfaces (there are not a lot of them, though). The major change would probably be the allocation of a small buffer (56~60 bytes) in some of the callers to hold the module name. -- Paulo Marques - www.grupopie.com "There cannot be a crisis today; my schedule is already full." -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] libusb / in-kernel usb driver criteria (was: USB driver for talking to the Microchip PIC18 boot loader)
Xiaofan Chen wrote: On Dec 30, 2007 11:53 AM, mgross <[EMAIL PROTECTED]> wrote: [...] What is the linux-usb policies on new drivers that could be implemented in user space? When does a kernel driver make sense over a libusb one? That would be interesting to know. I myself have been faced with this question before, and I think we should try to clarify this by adding a document with some guidelines to Documentation/usb. So, to get the ball rolling, here are some factors that IMHO help decide in which side to implement a driver: - if the driver ties a hardware device to an existing in-kernel interface (network, block, serial, bluetooth, video4linux, etc.), it should probably be implemented in-kernel. - on the other hand, if the driver doesn't use an existing kernel interface and creates a new user-visible interface that is going to be used by a single userspace application, it should probably be done in userspace. - if it is going to be used by several applications it could still be implemented as a library, but it starts moving into the gray area. - performance might be a reason to move to kernel space, but I don't think it matters for transfer rates below 10Mbytes/sec or so. Anyway, this is just MHO, so feel free to discuss this further. I'm simply volunteering to sum up this thread into a patch to add a Documentation/usb/userspace_drivers.txt (or something like that), so that we can help future developers decide where to write their drivers. -- Paulo Marques - www.grupopie.com "Very funny Scotty. Now beam up my clothes." -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm PATCH] kallsyms should prefer non weak symbols
Mathieu Desnoyers wrote: * Paulo Marques ([EMAIL PROTECTED]) wrote: When resolving symbol names from addresses with aliased symbol names, kallsyms_lookup always returns the first symbol, even if it is a weak symbol. [...] From: Paulo Marques <[EMAIL PROTECTED]> Signed-off-by: Mathieu Desnoyers <[EMAIL PROTECTED]> Please wait for me to accept the changes before adding signed-off-by. My mistake was that I didn't realized you'd already corrected the original patch when you posted this: http://lkml.org/lkml/2007/11/13/292 Since I thought it was the same patch, it already had your "Signed-off-by". Sorry for the confusion... :( -- Paulo Marques - www.grupopie.com "I haven't lost my mind -- it's backed up on tape somewhere." -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[-mm PATCH] kallsyms should prefer non weak symbols
When resolving symbol names from addresses with aliased symbol names,
kallsyms_lookup always returns the first symbol, even if it is a weak
symbol.
This patch changes this by sorting the symbols with the weak symbols
last before feeding them to the kernel. This way the kernel runtime
isn't changed at all, only the kallsyms build system is changed.
Another side effect is that the symbols get sorted by address, too. So,
even if future binutils version have some bug in "nm" that makes it fail
to correctly sort symbols by address, the kernel won't be affected by this.
From: Paulo Marques <[EMAIL PROTECTED]>
Signed-off-by: Mathieu Desnoyers <[EMAIL PROTECTED]>
--
Paulo Marques - www.grupopie.com
"There cannot be a crisis today; my schedule is already full."
--- ./scripts/kallsyms.c.orig 2007-10-30 18:51:28.0 +
+++ ./scripts/kallsyms.c2007-10-30 19:07:58.0 +
@@ -34,7 +34,7 @@
struct sym_entry {
unsigned long long addr;
- unsigned int len;
+ unsigned int len, start_pos;
unsigned char *sym;
};
@@ -202,8 +202,10 @@ static void read_map(FILE *in)
exit (1);
}
}
- if (read_symbol(in, &table[table_cnt]) == 0)
+ if (read_symbol(in, &table[table_cnt]) == 0) {
+ table[table_cnt].start_pos = table_cnt;
table_cnt++;
+ }
}
}
@@ -507,6 +509,35 @@ static void optimize_token_table(void)
}
+static int compare_symbols(const void *a, const void *b)
+{
+ struct sym_entry *sa, *sb;
+ int wa, wb;
+
+ sa = (struct sym_entry *) a;
+ sb = (struct sym_entry *) b;
+
+ // sort by address first
+ if (sa->addr > sb->addr)
+ return 1;
+ if (sa->addr < sb->addr)
+ return -1;
+
+ // sort by "weakness" type
+ wa = (sa->sym[0] == 'w') || (sa->sym[0] == 'W');
+ wb = (sb->sym[0] == 'w') || (sb->sym[0] == 'W');
+ if (wa != wb)
+ return wa - wb;
+
+ // sort by initial order, so that other symbols are left undisturbed
+ return sa->start_pos - sb->start_pos;
+}
+
+static void sort_symbols(void)
+{
+ qsort(table, table_cnt, sizeof(struct sym_entry), compare_symbols);
+}
+
int main(int argc, char **argv)
{
if (argc >= 2) {
@@ -527,6 +558,7 @@ int main(int argc, char **argv)
usage();
read_map(stdin);
+ sort_symbols();
optimize_token_table();
write_src();
Re: [PATCH] Kallsyms Should Prefer Non Weak Symbols
Mathieu Desnoyers wrote: * Mathieu Desnoyers ([EMAIL PROTECTED]) wrote: [...] kallsyms returns the first symbol encountered, even though it is weak, when it should in fact return sys_ni_syscall. Is it a concern for anyone else out there ? Would it make sense to fix it ? I don't know if it is a concern, but if we're going to fix it, we should probably do it in "scripts/kallsyms" by providing a list that is already sorted according to "address, weakness". This way the run-time kernel keeps the current behavior, without any overhead. Something along the lines of the attached patch (just compile tested). However, this is an area where we've had problems in the past with some architectures giving different results between passes, and then any change to the symbol order might make the problem worse and make the build process fail with a "Inconsistent kallsyms data" error message. So, if someone wants to use this, it should go through -mm for a while, first. It applies on top of 2.6.24-rc2-git3. Please use this reply with correct CC list for further discussion. I've been wanting to send this as a proper patch request email, but I just hadn't found the time to do it, and then our mail server just went berserk and I lost 5 days of LKML :P I think the patch is ok as it is, but a nice message explaining what it does and why would be nice for the changelog. So, I'll post a new message with a nice description for inclusion in -mm today. Sorry for the delay, -- Paulo Marques - www.grupopie.com "Very funny Scotty. Now beam up my clothes." - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kallsyms __print_symbol prints first weak symbol encountered
Mathieu Desnoyers wrote:
Hi,
Hi,
[...]
kallsyms returns the first symbol encountered, even though it is weak,
when it should in fact return sys_ni_syscall.
Is it a concern for anyone else out there ? Would it make sense to fix
it ?
I don't know if it is a concern, but if we're going to fix it, we should
probably do it in "scripts/kallsyms" by providing a list that is already
sorted according to "address, weakness".
This way the run-time kernel keeps the current behavior, without any
overhead. Something along the lines of the attached patch (just compile
tested).
However, this is an area where we've had problems in the past with some
architectures giving different results between passes, and then any
change to the symbol order might make the problem worse and make the
build process fail with a "Inconsistent kallsyms data" error message.
So, if someone wants to use this, it should go through -mm for a while,
first.
--
Paulo Marques - www.grupopie.com
"All I ask is a chance to prove that money can't make me happy."
--- ./scripts/kallsyms.c.orig 2007-10-30 18:51:28.0 +
+++ ./scripts/kallsyms.c2007-10-30 19:07:58.0 +
@@ -34,7 +34,7 @@
struct sym_entry {
unsigned long long addr;
- unsigned int len;
+ unsigned int len, start_pos;
unsigned char *sym;
};
@@ -202,8 +202,10 @@ static void read_map(FILE *in)
exit (1);
}
}
- if (read_symbol(in, &table[table_cnt]) == 0)
+ if (read_symbol(in, &table[table_cnt]) == 0) {
+ table[table_cnt].start_pos = table_cnt;
table_cnt++;
+ }
}
}
@@ -507,6 +509,35 @@ static void optimize_token_table(void)
}
+static int compare_symbols(const void *a, const void *b)
+{
+ struct sym_entry *sa, *sb;
+ int wa, wb;
+
+ sa = (struct sym_entry *) a;
+ sb = (struct sym_entry *) b;
+
+ // sort by address first
+ if (sa->addr > sb->addr)
+ return 1;
+ if (sa->addr < sb->addr)
+ return -1;
+
+ // sort by "weakness" type
+ wa = (sa->sym[0] == 'w') || (sa->sym[0] == 'W');
+ wb = (sb->sym[0] == 'w') || (sb->sym[0] == 'W');
+ if (wa != wb)
+ return wa - wb;
+
+ // sort by initial order, so that other symbols are left undisturbed
+ return sa->start_pos - sb->start_pos;
+}
+
+static void sort_symbols(void)
+{
+ qsort(table, table_cnt, sizeof(struct sym_entry), compare_symbols);
+}
+
int main(int argc, char **argv)
{
if (argc >= 2) {
@@ -527,6 +558,7 @@ int main(int argc, char **argv)
usage();
read_map(stdin);
+ sort_symbols();
optimize_token_table();
write_src();
Re: /proc/kallsyms and symbol size
Stephane Eranian wrote: Hello, Hi, Stephane Many monitoring tools use /proc/kallsyms to build a symbol table for the kernel. This technique has the advantage that it does not require root privileges, nor an up-to-date /boot/System.map, nor a decompressed kernel in /boot. The problem is that /proc/kallsyms does not report the size of the symbols. Yet, the information is available in the kernel as it is used by functions such as __print_symbol(). Having the size is useful to correlate the address obtained is a sample with a symbol name. Most tools use an approximation which assumes symbols are contiguous to estimate the size. That is actually what the kernel does internally, too. It does not keep the size of the symbol, but tries to guess it from the address of the next non-aliased symbol. Since the addresses are sorted, this works fine most of the time. This is done to reduce the size used by the symbol table in the running kernel. Just take a look at "get_symbol_pos" in kernel/kallsyms.c and "get_ksymbol" in kernel/module.c to see exactly how this is done -- Paulo Marques - www.grupopie.com "There cannot be a crisis today; my schedule is already full." - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Reduce __print_symbol/sprint_symbol stack usage. (v3)
Gilboa Davara wrote: Hello all, Hi, Gilboa (1) Problem: I. When CONFIG_4KSTACKS and CONFIG_DEBUG_STACKOVERFLOW are enabled on i386 kernels, do_IRQ calls dump_stack which, down the path, uses print_symbol (display) and sprint_symbol (format) to format and display the function name/address/module. Both function use stack based char array (~350 bytes) that, given the initial state (<512 bytes of stack space) may overrun the stack. II. (Comments - previous patches) Using spinlock protected static storage within these functions might block or even deadlock dump_stack (E.g. Crash within dump_stack itself) (2) Solution: I. Break sprint_symbol into sprint_symbol (API functions; keeps the current interface) and sprint_symbol_helper (helper function with minimal local storage). II. Replace the char array in __print_symbol with two spinlock protected static char arrays; call the __sprint_symbol helper function instead of sprint_symbol. III. Ignore the spinlock if oops_in_progress is set. This is getting more and more convoluted :( The problem with the spinlock isn't just that during a panic, we can not trust the kernel structures enough to use spinlocks. It might well happen that lockdep code might want to use print_symbol (and I think it does, so this is not just theoretical) to dump the stack when someone calls spin_lock_irqsave. But now, because print_symbol itself uses spin_lock_irqsave, we might get into a recursive situation and a produce a deadlock. On the other hand, if you take the other approach of reducing the stack usage by creating a printk_symbol interface, the stack usage would drop from 350 bytes to 128 bytes and your problem would go away entirely. -- Paulo Marques - www.grupopie.com "All I ask is a chance to prove that money can't make me happy." - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Reduce __print_symbol/sprint_symbol stack usage.
Steven Rostedt wrote:
On Wed, Sep 19, 2007 at 03:25:15PM +0100, Paulo Marques wrote:
if we change the interface from "print_symbol(fmt, addr)" to
"print_symbol(prefix, addr, int newline)" we can simply do:
printk(prefix);
printk_symbol(addr);
if (newline)
printk("\n");
NACK
I just wrote something that does "print_symbol(" %s)\n", addr);"
Notice the ")" in the output.
We can just change that to "print_symbol(prefix, addr, suffix)" instead.
The concept is basically the same (and I wasn't very fond of that
newline argument either).
As an added bonus we stop needing an extra layer to check that the
string passed is in the right format with a single "%s" in it.
--
Paulo Marques - www.grupopie.com
"Very funny Scotty. Now beam up my clothes."
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Reduce __print_symbol/sprint_symbol stack usage.
Gilboa Davara wrote:
Hello Paulo,
Hi, Gilboa
[snip]
[...]
if we change the interface from "print_symbol(fmt, addr)" to
"print_symbol(prefix, addr, int newline)" we can simply do:
printk(prefix);
printk_symbol(addr);
if (newline)
printk("\n");
where "printk_symbol" is a new function that does the same as
sprint_symbol, but does "printk" instead of "sprintf".
This should reduce immensely the stack usage of print_symbol without the
need for locking.
I fully agree.
... Further more, multiple printk_symbols should be combined into a
single, multi-line printk transaction. (To prevent debug printk's from
trashing a BUG() dump_stack).
Usually the developer can separate the output by hand in the unlikely
case of simultaneous concurrent users of printk, so I don't think this
is really a big problem.
Of course this requires changing _all_ callers of print_symbol to use
the new interface, but these are less than 100 ;)
This is my first contribution to the Linux kernel. As such I rather
start small, and work my way up slowly. (Read: solve the immediate stack
over-run now, think about changing the symbol_display interface later)
Yes, but this is a sensitive area, so you can not just implement
something now that can cause regressions, and just fix it later.
Kernel panics are quite rare and the information provided by the stack
dump is _extremely_ precious.
Even more, risking deadlocks caused by something that should only be
used to produce debug information is even worse.
Comments?
I do agree that the current interface needs work.
... But as I said, I rather start slowly and on small scale. (Though I
did find a rather problematic place to start at... ;))
Well, if we agree that this is the way to go, then the way to start
slowly would be to submit a patch that makes both interfaces available
for a while and changes the most stack-critical callers to the new
interface.
Then slowly keep submitting patches to change other callers
progressively until there are no more callers of the old interface. At
that point we can just drop it entirely.
--
Paulo Marques - www.grupopie.com
"God is love. Love is blind. Ray Charles is blind. Ray Charles is God."
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Reduce __print_symbol/sprint_symbol stack usage.
Satyam Sharma wrote:
On Sat, 15 Sep 2007, Gilboa Davara wrote:
printk(fmt, buffer);
+
+ spin_unlock_irqrestore(&symbol_lock, flags);
But I still don't much like this :-(
I must say I agree with Satyam here.
Locking in the panic path might leave us without some critical debug
information, which is much more important than all this.
Maybe it would be better to change the print_symbol interface to avoid
having a "char buffer[KSYM_SYMBOL_LEN];" at all.
Most print_symbol callers use something like "yada yada %s" as the
format string, with an optional "\n" in the end.
if we change the interface from "print_symbol(fmt, addr)" to
"print_symbol(prefix, addr, int newline)" we can simply do:
printk(prefix);
printk_symbol(addr);
if (newline)
printk("\n");
where "printk_symbol" is a new function that does the same as
sprint_symbol, but does "printk" instead of "sprintf".
This should reduce immensely the stack usage of print_symbol without the
need for locking.
Of course this requires changing _all_ callers of print_symbol to use
the new interface, but these are less than 100 ;)
Comments?
--
Paulo Marques - www.grupopie.com
"You're just jealous because the voices only talk to me."
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm] video: uvesafb: Add X86 dependency.
Paul Mundt wrote: On Tue, Sep 11, 2007 at 12:41:49PM +0100, Paulo Marques wrote: [...] Why do you say that it's x86 specific? Am I missing something? The emulator it uses only runs on x86 and x86_64. Thus, it's x86 specific. The v86d and uvesafb pages seem to be in disagremeent, unless by 'non-x86' it's only implying x86_64. Additionally, it needs the vga I/O routines, as per vgacon. Most platforms don't define these. I just saw the v86d emulator code, and you're right. It mmaps /dev/mem and assumes the video BIOS is mapped there. We should try to change that to mmap something like "/sys/bus/pci/devices/:01:00.0/rom" instead so that it can work on any system with a video card plugged on a PCI bus. It should also be possible for the emulator to translate in/out instructions to appropriate memory mapped I/O equivalents, no? Anyway, I think it is up to Michal to decide if we should remove the kernel support for other archs, or let it stay a bit more while working on solving the v86d side of things. So I'll just step aside now -- Paulo Marques - www.grupopie.com "667: The neighbor of the beast." - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm] video: uvesafb: Add X86 dependency.
Paul Mundt wrote: uvesafb is x86-specific, reflect that in the Kconfig. Hummm... uvesafb _shouldn't_ be x86 specific. At least according to their page [1] where it says: "works on non-x86 systems". Uvesafb uses a x86 emulator in userspace to run code from the video card ROM, so it should work on any PCI system where we can access the video card ROM and can emulate the hardware used by the ROM code. Why do you say that it's x86 specific? Am I missing something? -- Paulo Marques - www.grupopie.com "You're just jealous because the voices only talk to me." [1] http://dev.gentoo.org/~spock/projects/uvesafb/ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Kernel panic with 2.6.23-rc5
Tilman Schmidt wrote: Paulo Marques schrieb: I just tried booting a brand new 2.6.23-rc5 and after a few minutes it just panicked: machine totally frozen, blinking keyboard leds. [...] Maybe someone out there has a good suggestion that I could try before bisecting... A probable candidate would be: http://lkml.org/lkml/2007/9/2/219 I've been running with that patch applied for a few hours now and everything seems to be working fine. Without the patch the kernel would hang in a few minutes, so I guess this fixed it. Thanks for the help, -- Paulo Marques - www.grupopie.com "The Computer made me do it." - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] make kobject dynamic allocation check use kallsyms_lookup()
Dave Hansen wrote:
One of the top ten sysfs problems is that users use statically
allocated kobjects. This patch reminds them that this is a
naughty thing.
One _really_ nice thing this patch does, is us the kallsyms
mechanism to print out exactly which symbol is being complained
about:
The kobject at, or inside 'statickobj.2'@(0xc040d020) is not
dynamically allocated.
This patch replaces the previous implementation's use of a
_sdata symbol in favor of using kallsyms_lookup(). If a
kobject's address is a resolvable symbol, then it isn't
dynamically allocated.
Just a few concerns that I'm not sure of having been addressed:
- doing a kallsyms_lookup() is more expensive then just a simple range
test. This might be a concern if this is called very often. In this case
you could keep the range check and only do the lookup for symbols that
fail that check
- kallsyms_lookup() never finds a symbol if CONFIG_KALLSYMS is not
selected
- more comments below
The one exception to this is init symbols. The patch also
checks to see whether __init memory has been freed and if
it has will allow kobjects in those sections.
Signed-off-by: Dave Hansen <[EMAIL PROTECTED]>
---
lxc-dave/arch/i386/kernel/vmlinux.lds.S |2 --
lxc-dave/include/linux/init.h |1 +
lxc-dave/init/main.c|9 +
lxc-dave/lib/kobject.c | 31 ++-
4 files changed, 32 insertions(+), 11 deletions(-)
diff -puN
lib/kobject.c~make-kobject-allocation-debugging-check-use-kallsyms_lookup
lib/kobject.c
---
lxc/lib/kobject.c~make-kobject-allocation-debugging-check-use-kallsyms_lookup
2007-08-22 14:51:50.0 -0700
+++ lxc-dave/lib/kobject.c 2007-08-22 14:51:50.0 -0700
@@ -139,23 +139,36 @@ static int ptr_in_range(void *ptr, void
return 0;
}
-static void verify_dynamic_kobject_allocation(struct kobject *kobj)
+void verify_dynamic_kobject_allocation(struct kobject *kobj)
{
- if (ptr_in_range(kobj, &_sdata[0], &_edata[0]))
- goto warn;
- if (ptr_in_range(kobj, &__bss_start[0], &__bss_stop[0]))
- goto warn;
- return;
-warn:
+ char *namebuf;
+ const char *ret;
+
+ namebuf = kzalloc(KSYM_NAME_LEN, GFP_KERNEL);
You don't need kzalloc here. kmalloc will do just fine.
+ ret = kallsyms_lookup((unsigned long)kobj, NULL, NULL, NULL,
+ namebuf);
+ /*
+* This is the X86_32-only part of this function.
+* This is here because it is valid to have a kobject
+* in an __init section, but only after those
+* sections have been freed back to the dynamic pool.
+*/
+ if (!initmem_now_dynamic &&
+ ptr_in_range(kobj, __init_begin, __init_end))
+ goto out;
+ if (!ret || !strlen(ret))
The "!strlen(ret)" is not only weird (why not write as "!ret[0] or
!*ret) but is also unnecessary. When kallsyms_lookup fails to find a
symbol it should always return NULL.
+ goto out;
pr_debug(" begin silly warning \n");
pr_debug("This is a janitorial warning, not a kernel bug.\n");
#ifdef CONFIG_DEBUG_KOBJECT
- print_symbol("The kobject at, or inside %s is not dynamically
allocated.\n",
- (unsigned long)kobj);
+ pr_debug("The kobject at, or inside '%s'@(0x%p) is not dynamically
allocated.\n",
+ namebuf, kobj);
#endif
pr_debug("kobjects must be dynamically allocated, not static\n");
/* dump_stack(); */
pr_debug(" end silly warning \n");
+out:
+ kfree(namebuf);
}
#else
[...]
--
Paulo Marques - www.grupopie.com
"You're just jealous because the voices only talk to me."
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2.6.22-rc6-mm1] kallsyms: make KSYM_NAME_LEN include space for trailing '\0'
Tejun Heo wrote: KSYM_NAME_LEN is peculiar in that it does not include the space for the trailing '\0', forcing all users to use KSYM_NAME_LEN + 1 when allocating buffer. This is nonsense and error-prone. Moreover, when the caller forgets that it's very likely to subtly bite back by corrupting the stack because the last position of the buffer is always cleared to zero. This patch increments KSYM_NAME_LEN by one and updates code accordingly. Nice work. I've been wanting to do that cleanup myself for a long time ;) Acked-by: Paulo Marques <[EMAIL PROTECTED]> -- Paulo Marques - www.grupopie.com "You're just jealous because the voices only talk to me." - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] remove usage of memmem from scripts/kallsyms.c
Segher Boessenkool wrote: So we could remove the "#define _GNU_SOURCE" at the top of scripts/kallsyms.c too, presumably? If not (i.e. if there are more GNUisms left in that file anyway), then I'm not sure if we really gain by the change. yes, i believe this is true I only tried in on x86 with my toolchain and it works, but I don't know if it is worth the risk of breaking someone's setup for virtually no gain... With the memmem() removed, the code builds (and works) fine on several non-GNU systems. It should be perfectly safe to remove the _GNU_SOURCE. You're right. I went back in history and it was me who introduced the _GNU_SOURCE when I added the memmem too (shame on me). So, if it worked fine before, there is no reason to not work now that memmem is removed. So I can: - send an incremental patch with just that line removed - send a replacement patch - just leave it for now and wait until I work on kallsyms again to silently remove that line together with other changes Andrew, what would you prefer? -- Paulo Marques - www.grupopie.com "All I ask is a chance to prove that money can't make me happy." - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] remove usage of memmem from scripts/kallsyms.c
Mike Frysinger wrote: On Tuesday 19 June 2007, Satyam Sharma wrote: So we could remove the "#define _GNU_SOURCE" at the top of scripts/kallsyms.c too, presumably? If not (i.e. if there are more GNUisms left in that file anyway), then I'm not sure if we really gain by the change. yes, i believe this is true I only tried in on x86 with my toolchain and it works, but I don't know if it is worth the risk of breaking someone's setup for virtually no gain... -- Paulo Marques - www.grupopie.com "God is real, unless declared integer." - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] remove usage of memmem from scripts/kallsyms.c
Christoph Hellwig wrote: On Tue, Jun 19, 2007 at 05:15:56PM +0100, Paulo Marques wrote: The only in-kernel user of "memmem" is scripts/kallsyms.c and it only uses it to find tokens that are 2 bytes in size. It is trivial to replace it with a simple function that finds 2-byte tokens. This should help users from systems that don't have the memmem GNU extension available. Please add a comment describing why it's there so that it's not ripped out again by the first janitor looking over the code. I don't see why it would seem a good idea to replace a simple find_token function that searches for 2 byte tokens with a call to memmem. So, I think this is not something a janitor would do. The call to memmem was actually a left-over from a previous algorithm that used variable sized tokens. With fixed size, 2 byte tokens, having a specialized function is probably more efficient anyway. -- Paulo Marques - www.grupopie.com "Nostalgia isn't what it used to be." - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] remove usage of memmem from scripts/kallsyms.c
The only in-kernel user of "memmem" is scripts/kallsyms.c and it only
uses it to find tokens that are 2 bytes in size. It is trivial to
replace it with a simple function that finds 2-byte tokens.
This should help users from systems that don't have the memmem GNU
extension available.
Signed-off-by: Paulo Marques <[EMAIL PROTECTED]>
--
Paulo Marques - www.grupopie.com
"667: The neighbor of the beast."
--- ./scripts/kallsyms.c.orig 2007-06-08 12:55:49.0 +0100
+++ ./scripts/kallsyms.c2007-06-08 13:19:52.0 +0100
@@ -378,6 +378,17 @@ static void build_initial_tok_table(void
table_cnt = pos;
}
+static void *find_token(unsigned char *str, int len, unsigned char *token)
+{
+ int i;
+
+ for (i = 0; i < len - 1; i++) {
+ if (str[i] == token[0] && str[i+1] == token[1])
+ return &str[i];
+ }
+ return NULL;
+}
+
/* replace a given token in all the valid symbols. Use the sampled symbols
* to update the counts */
static void compress_symbols(unsigned char *str, int idx)
@@ -391,7 +402,7 @@ static void compress_symbols(unsigned ch
p1 = table[i].sym;
/* find the token on the symbol */
- p2 = memmem(p1, len, str, 2);
+ p2 = find_token(p1, len, str);
if (!p2) continue;
/* decrease the counts for this symbol's tokens */
@@ -410,7 +421,7 @@ static void compress_symbols(unsigned ch
if (size < 2) break;
/* find the token on the symbol */
- p2 = memmem(p1, size, str, 2);
+ p2 = find_token(p1, size, str);
} while (p2);
Re: Dual-Licensing Linux Kernel with GPL V2 and GPL V3
Alan Cox wrote: But COPYING *is* the entire text and starts with: " GNU GENERAL PUBLIC LICENSE Version 2, June 1991" so there is no confusion about the version. The version of the COPYING file (and the licence document), not of the licence on the code. Wrong. Why do you say "Wrong"? Have you contributed some code to the kernel thinking that the kernel was "v2 or later", only to find out later that it wasn't? A fair bit of the kernel is probably v2 or later but not all of it and that shouldn't really matter as regards the kernel anyway, the GPLv2 only bits (if v2 only is a valid status) anchor it. So we are violently agreeing, then? This sub-thread started by me showing that: $ find -name "*.c" | xargs grep "any later version" | wc -l 3138 $ find -name "*.c" | wc -l 9482 This is a somewhat crude measure but it shows that only about 30% of the kernel is "v2 or later" and those pieces could be used on some other "v2 or later" project (including v3). But the kernel as a whole is v2 and my point was that the claim that there are just a few "v2 only" files was bogus. -- Paulo Marques - www.grupopie.com "As far as we know, our computer has never had an undetected error." Weisert - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Dual-Licensing Linux Kernel with GPL V2 and GPL V3
Bernd Paysan wrote: On Friday 15 June 2007 13:49, Paulo Marques wrote: I've contributed some code for the kernel (unlike yourself, AFAICT), and believe me, I did so under GPL v2. The COPYING file is pretty much self explanatory, so I didn't need to add any explicit license statement to my code. It's not, it's a personal comment from a misunderstanding of the GPL text. It's as valid as the "closed source kernel modules are legal" comment that was there some years ago. These are not changes to the license text. These are just clarifications to help people understand the license. They don't change what the license already said. People seem to forget that the kernel license in COPYING *never had* the "v2 or later" clause. Never. Period. It's there in section 9. The section 9 is meant to explain how you select one version of the license in a program without having to copy the entire license text to it, i.e., in simple programs you can just put the small text, suggested by FSF at the bottom of the gpl, and have the version number there, and that should be enough to reference the entire text. But COPYING *is* the entire text and starts with: " GNU GENERAL PUBLIC LICENSE Version 2, June 1991" so there is no confusion about the version. [...] And people who write kernel code are perfectly aware that the kernel license is GPL v2 only, and always has been (except for the initial linus license). Wrong. Why do you say "Wrong"? Have you contributed some code to the kernel thinking that the kernel was "v2 or later", only to find out later that it wasn't? In case you haven't followed previous discussions, here's a pointer: http://lkml.org/lkml/2006/9/22/176 The major kernel developers (and probably most of the total number of developers) are perfectly aware of the kernel license and chose GPL v2. I'm getting pretty tired of listening to people that just _know_ what I should do with _my_ code. And people who treat kernel developers as morons who can't read a license. We definitely need more Al Viro style comments on this thread ;) -- Paulo Marques - www.grupopie.com "The Mexicans have the Chupacabra. We have Al Viro. If you hear him roar, just _pray_ he's about to dissect somebody elses code than yours.. There is no point in running." Linus Torvalds - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Dual-Licensing Linux Kernel with GPL V2 and GPL V3
Bernd Paysan wrote: On Thursday 14 June 2007 19:20, Paulo Marques wrote: Watching the output of the first grep without "wc -l" shows that, although it is not 100% accurate, it is still ok just to get a rough estimate. So yes, ~6300 files are definitely more than a couple ;) I knew I shouldn't post into the yearly GPL flame-fest... :( Most of them don't say anything, so they are "any GPL" by the author. I've contributed some code for the kernel (unlike yourself, AFAICT), and believe me, I did so under GPL v2. The COPYING file is pretty much self explanatory, so I didn't need to add any explicit license statement to my code. When do you people accept that Linus can't change the GPL, he can only add comments of what he thinks is the case! His interpretation of the GPLv2 might be that not saying anything about the version means "v2 only", but if he does so, he's simply wrong. He was wrong in the module case, as well, and dropped this comment a while ago. He might drop this comment in future, as well. In fact, anybody can drop this comment, as it's just a comment. Linus can't and is not _changing_ the GPL. He can however use whatever license he sees fit for _his_ code just like all the other kernel developers do. People seem to forget that the kernel license in COPYING *never had* the "v2 or later" clause. Never. Period. The only change in license was from the previous hand-made one from Linus into GPL v2 only. And that is perfectly fine since the previous license was even more permissive than GPL v2. The kernel *as a whole* is clearly under GPLv2 only from Linus' comment, which is in fact true, since the common subset of GPL versions from all authors is indeed GPLv2 (by virtue of some files from Al Viro, and maybe some other explicit GPL v2 files). The author must specify the version himself, there simply is no other way. If you don't specify any, it's "any version", because I can license all patches straight from the authors. No, it is not "any version". It is the license specified in COPYING and nothing else. The way the GPLv2 allows you to explicitely specify "any version" is by not saying anything about the version at all. Linus isn't in the positition to change that unless he does a substantial change to the file, and also adds a comment that this file is now GPLv2 only. Man, I sure ain't a lawyer, but people in these discussions seem to not understand the basics at all. And the basics are: "people who write the code decide the license to give it". And that's just it. And people who write kernel code are perfectly aware that the kernel license is GPL v2 only, and always has been (except for the initial linus license). So don't go around saying that because people don't put explicit license statements they don't care about the license. I care very much about the license, and would have never contributed to the kernel if it had a BSD license of some sort. Putting a license statement in _every_ file in the kernel tree would just be idiotic when there is such a clear COPYING file in the root of the kernel tree. -- Paulo Marques - www.grupopie.com "Oh dear, I think you'll find reality's on the blink again." Marvin The Paranoid Android - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Dual-Licensing Linux Kernel with GPL V2 and GPL V3
Al Viro wrote: On Thu, Jun 14, 2007 at 01:01:20PM -0400, Dmitry Torokhov wrote: Multiple authors == need permission from each author with enough contributions to that file to make the contributions in question copyrightable. And in my case (and case of gregkh, and...) that would be considerably more than a couple of files. Really. I would expect that if you contribute to a file that explicitely says "GPL v2 or later" and you do not change that wording then you agree GPL v2 or later for that particular contribution. So for example drivers/net/plip.c could be changed to GPL v3 even though you contributed to it. After you exclude such cases it's still more than a couple of files... FWIW, $ find -name "*.c" | xargs grep "any later version" | wc -l 3138 $ find -name "*.c" | wc -l 9482 Watching the output of the first grep without "wc -l" shows that, although it is not 100% accurate, it is still ok just to get a rough estimate. So yes, ~6300 files are definitely more than a couple ;) -- Paulo Marques - www.grupopie.com "God is love. Love is blind. Ray Charles is blind. Ray Charles is God." - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: PC speaker
Maciej W. Rozycki wrote: On Tue, 12 Jun 2007, Jan Engelhardt wrote: 4) It assumes the current will be sufficient to burn out the speaker. (I know it will get very hot on older machines, whether it will burn out -- might even depend on the exact speaker model.) Since you can set the x86's crystals frequency from 1193182 to 18 Hz (PIT_TICK_RATE / 1 to PIT_TICK_RATE / 65535) [*], you can never really bust it. But even then, what would a speaker do it was constanly given I am fairly sure you have a choice between a steady low and a steady high level on the speaker output available if you switch the 8254 to the right single-shot mode. In case you have not been into such details -- the 8254 offers six modes of operation, selected for each channel separately, of which only two are periodic. Can we please stop this non-sense thread? Anyone designing the speaker circuit would certainly place a small capacitor in series with the speaker to kill the DC component of the signal. Since the speaker itself wouldn't be able to play very low frequencies anyway, the capacitor wouldn't have to be that big. So I'm pretty sure that on any half-way decent piece of hardware, you won't be able to kill the speaker with software... -- Paulo Marques - www.grupopie.com "Don't hit a man when he's down -- kick him; it's easier." - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch/rfc] implement memmem() locally in kallsyms.c
Mike Frysinger wrote:
This patch basically copies the gnulib version of memmem() into
scripts/kallsyms.c. While a useful function, it isn't in POSIX so some
systems (like Darwin) choose to omit it. How do others feel ?
Well, the only use of memmem in scripts/kallsyms.c is to find tokens of
size 2, so we can use a simplified version instead (and probably get
better code in the process).
How about the attached patch instead?
If you approve it, I'll post it appropriately for inclusion in -mm.
--
Paulo Marques - www.grupopie.com
"God is real, unless declared integer."
--- ./scripts/kallsyms.c.orig 2007-06-08 12:55:49.0 +0100
+++ ./scripts/kallsyms.c2007-06-08 13:19:52.0 +0100
@@ -378,6 +378,17 @@ static void build_initial_tok_table(void
table_cnt = pos;
}
+static void *find_token(unsigned char *str, int len, unsigned char *token)
+{
+ int i;
+
+ for (i = 0; i < len - 1; i++) {
+ if (str[i] == token[0] && str[i+1] == token[1])
+ return &str[i];
+ }
+ return NULL;
+}
+
/* replace a given token in all the valid symbols. Use the sampled symbols
* to update the counts */
static void compress_symbols(unsigned char *str, int idx)
@@ -391,7 +402,7 @@ static void compress_symbols(unsigned ch
p1 = table[i].sym;
/* find the token on the symbol */
- p2 = memmem(p1, len, str, 2);
+ p2 = find_token(p1, len, str);
if (!p2) continue;
/* decrease the counts for this symbol's tokens */
@@ -410,7 +421,7 @@ static void compress_symbols(unsigned ch
if (size < 2) break;
/* find the token on the symbol */
- p2 = memmem(p1, size, str, 2);
+ p2 = find_token(p1, size, str);
} while (p2);
Re: [Patch 05/18] fs/logfs/logfs.h
Jörn Engel wrote: --- /dev/null 2007-03-13 19:15:28.862769062 +0100 +++ linux-2.6.21logfs/fs/logfs/logfs.h 2007-06-03 19:34:07.0 +0200 @@ -0,0 +1,398 @@ +/* + * fs/logfs/logfs.h + * + * As should be obvious for Linux kernel code, license is GPLv2 + * + * Copyright (c) 2005-2007 Joern Engel + * + * Private header for logfs. + */ +#ifndef fs_logfs_logfs_h +#define fs_logfs_logfs_h + +#define __CHECK_ENDIAN__ + + +#include +#include +#include Including kallsyms.h is not really needed in this header file, is it? I don't have time / knowledge to review the rest of the code, but I always keep an eye out for kallsyms usage... -- Paulo Marques - www.grupopie.com "Very funny Scotty. Now beam up my clothes." - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND 1/1] crypto API: RSA algorithm patch (kernel version 2.6.20.1)
Matt Mackall wrote: [...] +/* Allocate space for the mpi and its data */ +s = (size / 4) + ((size % 4)? 1: 0); Uhhh.. (size + 1) / 4? You mean "(size + 3) / 4", no? Overall, I agree with your comments: this file looks like it needs a lot more CodingStyle ;) Redefining standard kernel interfaces (error numbers, memory allocation, printk, etc.) is not the right way to get code merged. -- Paulo Marques - www.grupopie.com "For every problem there is one solution which is simple, neat, and wrong." H. L. Mencken - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3] Race between cat /proc/kallsyms and rmmod
Alexey Dobriyan wrote: Iterating code of /proc/kallsyms calls module_get_kallsym() which grabs and drops module_mutex internally and returns "struct module *", module is removed, aforementioned "struct module *" is used in non-trivial way. Steps to reproduce: modprobe/rmmod loop cat /proc/kallsyms >/dev/null loop Copy all needed info under module_mutex. NOTE: this patch keeps module_mutex static. Yes, this patch fixes the "cat /proc/kallsyms" race without changing any "external" interfaces, so I think it should go into mainline in any case. Acked-by: Paulo Marques <[EMAIL PROTECTED]> -- Paulo Marques - www.grupopie.com "All I ask is a chance to prove that money can't make me happy." - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND 2/2] Fix some kallsyms_lookup() vs rmmod races
Alexey Dobriyan wrote: On Sat, Mar 17, 2007 at 08:37:18PM +1100, Rusty Russell wrote: On Fri, 2007-03-16 at 12:51 +0100, Ingo Molnar wrote: [...] looking at the problem from another angle: wouldnt this be something that would benefit from freeze_processes()/unfreeze_processes(), and hence no locking would be required? Actually, the list manipulation is done with stop_machine for this reason. mmm, my changelog is slightly narrow than it should be. Non-emergency code is traversing modules list. It finds "struct module *". module is removed. "struct module *" is now meaningless, but still dereferenced. How would all this refrigerator stuff would help? It wouldn't, Non-emergency code is traversing modules list. It finds "struct module *". Everything is freezed. Module is removed. Everything is unfreezed. "struct module *" is now meaningless, but still dereferenced. That is why I asked if the refrigerator would preempt processes or not. AFAICS there is no path where the "struct module *" that is returned is used after a voluntary preemption point, so it should be safe. I might be missing something, though. However, this isn't very robust. Since the functions are still returning pointers to module data, some changes in the future might keep the pointer, and use it after a valid freezing point -> oops. Alexey, is preempt enabled in your kernel? Yes. FWIW, CONFIG_PREEMPT=y CONFIG_PREEMPT_BKL=y CONFIG_DEBUG_PREEMPT=y I very much agree with proto-patch which _copies_ all relevant information into caller-supplied structure, keeping module_mutex private. Time to split it sanely. That depends on the roadmap: if we think the freezer approach is the best in the long run, maybe your less intrusive (in the sense that it changes less stuff) patch should go in now (as a quick fix to mainline) so that after we've sorted out the bugs from the freezer in -mm, it will be easier to revert. However, if we think the most reliable solution would be to not return internal module information through pointers and keep all that logic internal to module.c, then the "proto-patch" with some improvements might be the way to go... -- Paulo Marques - www.grupopie.com "God is love. Love is blind. Ray Charles is blind. Ray Charles is God." - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND 2/2] Fix some kallsyms_lookup() vs rmmod races
Andrew Morton wrote:
On Fri, 16 Mar 2007 17:16:39 + Paulo Marques <[EMAIL PROTECTED]> wrote:
Does freeze_processes() / unfreeze_processes() solve this by only
freezing processes that have voluntarily scheduled (opposed to just
being preempted)?
It goes much much further than that. Those processes need to actually
perform an explicit call to try_to_freeze().
Ok, I've just done a few tests with the attached patch. It basically
creates a freeze_machine_run function that is equivalent in interface to
stop_machine_run, but uses freeze_processes / thaw_processes to stop the
machine.
This is more of a proof of concept than an actual patch. At the very
least "freeze_machine_run" should be moved to kernel/power/process.c and
declared at include/linux/freezer.h so that it could be treated as a
more general purpose function and not something that is module specific.
Anyway, I then tested it by running a modprobe/rmmod loop while running
a "cat /proc/kallsyms" loop.
On the first run I forgot to remove the mutex_lock(module_mutex) from
the /proc/kallsyms read path and the freezer was unable to freeze the
"cat" process that was waiting for the same mutex that the freezer
process was holding :P
After removing the module_mutex locking from "module_get_kallsym"
everything was going fine (at least I got no oopses) until I got this:
kernel: Stopping user space processes timed out after 20 seconds (1
tasks refusing to freeze):
kernel: kbluetoothd
kernel: Restarting tasks ... <4> Strange, kseriod not stopped
kernel: Strange, pdflush not stopped
kernel: Strange, pdflush not stopped
kernel: Strange, kswapd0 not stopped
kernel: Strange, cifsoplockd not stopped
kernel: Strange, cifsdnotifyd not stopped
kernel: Strange, jfsIO not stopped
kernel: Strange, jfsCommit not stopped
kernel: Strange, jfsCommit not stopped
kernel: Strange, jfsSync not stopped
kernel: Strange, xfslogd/0 not stopped
kernel: Strange, xfslogd/1 not stopped
kernel: Strange, xfsdatad/0 not stopped
kernel: Strange, xfsdatad/1 not stopped
kernel: Strange, kjournald not stopped
kernel: Strange, khubd not stopped
kernel: Strange, khelper not stopped
kernel: Strange, kbluetoothd not stopped
kernel: done.
I repeated the test and did a Alt+SysRq+T to try to find out what
kbluetoothd was doing and got this:
kernel: kbluetoothd D 79A11860 0 19156 1 19142
(NOTLB)
kernel: 9a269e4c 0082 0001 79a11860 79a09860 c7018030
0003
kernel: 9a269e71 78475100 c7ebe000 c6730e40 0001 0001
0001
kernel: 9a2d7570 79a11860 c7018140 1832 42430d03
00ab
kernel: Call Trace:
kernel: [<7845dba3>] wait_for_completion+0x7d/0xb7
kernel: [<781190ba>] default_wake_function+0x0/0xc
kernel: [<781190ba>] default_wake_function+0x0/0xc
kernel: [<7812c759>] call_usermodehelper_keys+0xd1/0xf1
kernel: [<7812c41e>] request_module+0x96/0xd9
kernel: [<783e30fe>] sock_alloc_inode+0x20/0x4e
kernel: [<78172559>] alloc_inode+0x15/0x115
kernel: [<78172d87>] new_inode+0x24/0x81
kernel: [<783e4003>] __sock_create+0x111/0x199
kernel: [<783e40a3>] sock_create+0x18/0x1d
kernel: [<783e40e1>] sys_socket+0x1c/0x43
kernel: [<783e51da>] sys_socketcall+0x247/0x24c
kernel: [<78121b2d>] sys_gettimeofday+0x2c/0x65
kernel: [<78103f10>] sysenter_past_esp+0x5d/0x81
And this was as far as I got...
This actually seems like a better approach than to hold module_mutex
everywhere to account for an operation that should be "rare" (module
loading/unloading). If something like this goes in, there are probably a
few more places inside module.c where we can drop the locking completely.
However, it still has a few gotchas. Apart from the problem above (which
may still be me doing something wrong) it makes module loading /
unloading depend on CONFIG_PM which is somewhat unexpected for the user.
Would it make sense to separate the process freezing / thawing API from
actual power management and create a new config option (CONFIG_FREEZER?)
that was automatically selected by the systems that used it (CONFIG_PM,
CONFIG_MODULES, etc.)? or is that overkill?
--
Paulo Marques - www.grupopie.com
"Nostalgia isn't what it used to be."
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -35,7 +35,7 @@
#include
#include
#include
-#include
+#include
#include
#include
#include
@@ -618,13 +618,23 @@ static int __try_stop_module(void *_sref
return 0;
}
+static int freeze_machine_run(int (*fn)(void *), void *data, unsigned int cpu)
+{
+ int ret;
+ freeze_processes();
+ ret = fn(data);
+ thaw_processes();
+ return ret;
+}
+
static int try_stop_module(struct module *mod, int flags, int *forced)
{
struct stopref sref = { mod, flags, forced };
- return
Re: [PATCH RESEND 2/2] Fix some kallsyms_lookup() vs rmmod races
Ingo Molnar wrote:
* Paulo Marques <[EMAIL PROTECTED]> wrote:
looking at the problem from another angle: wouldnt this be something
that would benefit from freeze_processes()/unfreeze_processes(), and
hence no locking would be required?
I also considered this, but it seemed a little too "blunt" to stop
everything (including completely unrelated processes and kernel
threads) just to remove a module.
'just to remove a module' is very, very rare, on the timescale of most
kernel ops. Almost no distro does it. Furthermore, because we want to do
CPU-hotplug that way, we really want to make
freeze_processes()/unfreeze_processes() 'instantaneous' to the human -
and it is that already. (if it isnt in some case we can make it so)
Ok. I started to look at this approach and realized that module.c
already does this:
static int __unlink_module(void *_mod)
{
struct module *mod = _mod;
list_del(&mod->list);
return 0;
}
/* Free a module, remove from lists, etc (must hold module mutex). */
static void free_module(struct module *mod)
{
/* Delete from various lists */
stop_machine_run(__unlink_module, mod, NR_CPUS);
However stop_machine_run doesn't seem like the right thing to do,
because users of the "modules" list don't seem to do anything to prevent
preemption. Am I missing something?
Does freeze_processes() / unfreeze_processes() solve this by only
freezing processes that have voluntarily scheduled (opposed to just
being preempted)?
--
Paulo Marques - www.grupopie.com
"The Computer made me do it."
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND 2/2] Fix some kallsyms_lookup() vs rmmod races
Ingo Molnar wrote:
* Alexey Dobriyan <[EMAIL PROTECTED]> wrote:
[cc'ing folks whose proc files are affected]
kallsyms_lookup() can call module_address_lookup() which iterates over
modules list without module_mutex taken. Comment at the top of
module_address_lookup() says it's for oops resolution so races are
irrelevant, but in some cases it's reachable from regular code:
looking at the problem from another angle: wouldnt this be something
that would benefit from freeze_processes()/unfreeze_processes(), and
hence no locking would be required?
I also considered this, but it seemed a little too "blunt" to stop
everything (including completely unrelated processes and kernel threads)
just to remove a module.
How about something like this instead? (just for review)
It's a little more intrusive, since the interface for symbol lookup is
changed (and it affects the callers), but it makes the caller aware that
it should set "safe_to_lock" if possible.
This way we avoid exposing module_mutex outside of module.c and avoid
returning any data pointer to some data structure that might disappear
before the caller tries to use it.
Some of these changes are actually cleanups, because the callers where
creating a dummy modname variables that they didn't use afterwards.
The thing I like the less about this patch is the increase of stack
usage on some code paths by about 60 bytes.
Anyway, I don't have very strong feelings about this, so if you think it
would be better to use freeze_processes()/unfreeze_processes(), I can
cook up a patch for that too...
--
Paulo Marques - www.grupopie.com
"Very funny Scotty. Now beam up my clothes."
diffstat:
arch/parisc/kernel/unwind.c |3 --
arch/powerpc/xmon/xmon.c| 11 -
arch/ppc/xmon/xmon.c|8 +++---
arch/sh64/kernel/unwind.c |4 +--
arch/x86_64/kernel/traps.c | 10
fs/proc/base.c |3 --
include/linux/kallsyms.h|6 +++-
include/linux/module.h | 44 +++-
kernel/kallsyms.c | 53 +---
kernel/kprobes.c|6 ++--
kernel/lockdep.c|3 --
kernel/module.c | 44 +++-
kernel/time/timer_list.c|3 --
kernel/time/timer_stats.c |3 --
mm/slab.c |6 ++--
15 files changed, 114 insertions(+), 93 deletions(-)
diff --git a/arch/parisc/kernel/unwind.c b/arch/parisc/kernel/unwind.c
--- a/arch/parisc/kernel/unwind.c
+++ b/arch/parisc/kernel/unwind.c
@@ -216,11 +216,10 @@ static void unwind_frame_regs(struct unw
/* Handle some frequent special cases */
{
char symname[KSYM_NAME_LEN+1];
- char *modname;
unsigned long symsize, offset;
kallsyms_lookup(info->ip, &symsize, &offset,
- &modname, symname);
+ NULL, symname, 0);
dbg("info->ip = 0x%lx, name = %s\n", info->ip, symname);
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -1218,7 +1218,6 @@ static void get_function_bounds(unsigned
{
unsigned long size, offset;
const char *name;
- char *modname;
*startp = *endp = 0;
if (pc == 0)
@@ -1226,7 +1225,7 @@ static void get_function_bounds(unsigned
if (setjmp(bus_error_jmp) == 0) {
catch_memory_errors = 1;
sync();
- name = kallsyms_lookup(pc, &size, &offset, &modname, tmpstr);
+ name = kallsyms_lookup(pc, &size, &offset, NULL, tmpstr, 0);
if (name != NULL) {
*startp = pc - offset;
*endp = pc - offset + size;
@@ -2496,7 +2495,7 @@ symbol_lookup(void)
static void xmon_print_symbol(unsigned long address, const char *mid,
const char *after)
{
- char *modname;
+ char modname[MODULE_NAME_LEN + 1];
const char *name = NULL;
unsigned long offset, size;
@@ -2504,8 +2503,8 @@ static void xmon_print_symbol(unsigned l
if (setjmp(bus_error_jmp) == 0) {
catch_memory_errors = 1;
sync();
- name = kallsyms_lookup(address, &size, &offset, &modname,
- tmpstr);
+ name = kallsyms_lookup(address, &size, &offset, modname,
+ tmpstr, 0);
sync();
/* wait a little while to see if we get a machine check */
__delay(200);
@@ -2515,7 +2514,7 @@ static void xmon_pri
Re: [PATCH] Fix some kallsyms_lookup() vs rmmod races
Alexey Dobriyan wrote: [cc'ing folks whose proc files are affected] kallsyms_lookup() can call module_address_lookup() which iterates over modules list without module_mutex taken. Comment at the top of module_address_lookup() says it's for oops resolution so races are irrelevant, but in some cases it's reachable from regular code: So maybe we should just add a new parameter to "kallsyms_lookup" to inform it if it is safe to take a mutex or not. Spreading module_mutex everywhere doesn't seem like the right interface for several reasons: - new users of "kallsyms_lookup" might not be aware that they should take module_mutex if it is safe - many times we will be taking module_mutex even when we are fetching a kernel symbol that shouldn't require the mutex at all - it just creates new dependencies (hint: this patch shouldn't even compile with current git since module_mutex is not declared in module.h, not to mention compile when CONFIG_MODULES not set) IMHO we should not expose module_mutex outside of module.c. That is just wrong from an encapsulation point of view. -- Paulo Marques - www.grupopie.com "667: The neighbor of the beast." - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Race between cat /proc/kallsyms and rmmod
Alexey Dobriyan wrote: Iterating code of /proc/kallsyms calls module_get_kallsym() which grabs and drops module_mutex internally and returns "struct module *", module is removed, aforementioned "struct module *" is used in non-trivial way. So, grab module_mutex for entire operation like /proc/modules does. I would still prefer the other solution to avoid exposing "module_mutex" outside of module.c like this :( I'll try to send in a patch today for review. -- Paulo Marques - www.grupopie.com "As far as we know, our computer has never had an undetected error." Weisert - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: /proc/kallsyms race vs module unload
Alexey Dobriyan wrote: On Tue, Mar 13, 2007 at 06:49:50PM +, Paulo Marques wrote: Alexey Dobriyan wrote: [...] What happens is that module_get_kallsym() drops module_mutex, returns "struct module *", module unloaded, "struct module *" used. The only use for the "struct module *" is to display the name of the module. Ehh? This can be solved by adding a "char mod_name[MODULE_NAME_LEN];" field to "kallsym_iter" and copy the name of the module over, while still holding module_mutex. It would be slightly slower, but safer. iter->owner = module_get_kallsym(iter->pos - kallsyms_num_syms, &iter->value, &iter->type, iter->name, sizeof(iter->name)); if (iter->owner == NULL) return 0; /* Label it "global" if it is exported, "local" if not exported. */ iter->type = is_exported(iter->name, iter->owner) ^^^ Yes, there is this "is_exported" call, but his can be moved completely into "module_get_kallsym" and have the "type" returned be already upper / lower case. That, together with filling the module name "module_get_kallsym()" would make the returned "struct module *" unneeded. Since kallsyms is the only caller of that function, we can change its interface to not return a "struct module *" at all, and return just an integer that means "symbol found" or "no more symbols". I'm still volunteering to do that patch, but you seem more active than me at the moment... -- Paulo Marques Software Development Department - Grupo PIE, S.A. Phone: +351 252 290600, Fax: +351 252 290601 Web: www.grupopie.com - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: /proc/kallsyms race vs module unload
Alexey Dobriyan wrote: [...] What happens is that module_get_kallsym() drops module_mutex, returns "struct module *", module unloaded, "struct module *" used. The only use for the "struct module *" is to display the name of the module. This can be solved by adding a "char mod_name[MODULE_NAME_LEN];" field to "kallsym_iter" and copy the name of the module over, while still holding module_mutex. It would be slightly slower, but safer. We can even change the function's interface, so that it doesn't return a "struct module *" at all, since AFAICS kallsyms is the only user of that function. It will still produce strange artifacts, though. If the iterator is already past the removed module symbols, it will skip as many symbols as the module symbol count, failing to show some symbols from unrelated modules. It won't oops, though. I'll try to cook up a patch, if no one objects to this approach, -- Paulo Marques - www.grupopie.com "There cannot be a crisis today; my schedule is already full." - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2.6.21-rc1] Extend print_symbol capability TRY #3
Robert Peterson wrote: [...] It probably would be easy, but it's beyond the scope of this fix, and therefore probably best left to a separate patch. Feel free to send that out in another patch if you want. If you do, I'd be happy to test it, review it, and ACK it. Fair enough :) For what is worth, Acked-by: Paulo Marques <[EMAIL PROTECTED]> -- Paulo Marques - www.grupopie.com "Very funny Scotty. Now beam up my clothes." - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2.6.21-rc1] Extend print_symbol capability TRY #3
Robert Peterson wrote:
[...]
@@ -47,6 +52,11 @@ static inline const char *kallsyms_lookup(unsigned
long addr,
return NULL;
}
+static inline void sprint_symbol(char *buffer, unsigned long addr)
+{
+return;
+}
I'm really sorry for not replying sooner (I've been really busy), but
this function still doesn't seem right.
If someone does something like:
void my_function(unsigned long addr)
{
char buffer[KSYM_SYMBOL_LEN];
sprint_symbol(buffer, addr);
...
// use buffer to print somewhere
...
}
which seems like a perfectly natural thing to do, it might just oops the
kernel if CONFIG_KALLSYMS is not set, because the buffer will be left
uninitialized.
That is why I suggested to change it to something like "*buffer = '\0'"
instead.
The really nice solution IMHO, would be to remove the print_symbol and
sprint_symbol functions from the the "#ifdef CONFIG_KALLSYMS" and just
let them be available even in a not CONFIG_KALLSYMS kernel.
Since kallsyms_lookup is already #ifdef'ed to something sane,
sprint_symbol will just print out the symbol address in that case, but
it is better than not printing anything at all.
--
Paulo Marques - www.grupopie.com
"Very funny Scotty. Now beam up my clothes."
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2.6.21-rc1] Extend print_symbol capability
Robert Peterson wrote:
[...]
#define KSYM_NAME_LEN 127
+#define KSYM_SYMBOL_LEN (sizeof("%s+%#lx/%#lx [%s]") + KSYM_NAME_LEN + \
+2*(BITS_PER_LONG*3/10) + MODULE_NAME_LEN + 1)
#ifdef CONFIG_KALLSYMS
/* Lookup the address for a symbol. Returns 0 if not found. */
@@ -22,6 +24,9 @@ const char *kallsyms_lookup(unsigned long addr,
unsigned long *offset,
char **modname, char *namebuf);
+/* Look up a kernel symbol and return it in a text buffer. */
+extern void lookup_symbol(unsigned long addr, char *buffer);
I don't like this name much :(
We already have kallsyms_lookup and kallsyms_lookup_name. The name of
this function should imply that it will print the formatted result into
the buffer, not just lookup a symbol.
Maybe "__sprint_symbol", and change the interface to
"__sprint_symbol(char *buffer, unsigned long addr)"?
+
/* Replace "%s" in format with address, if found */
extern void __print_symbol(const char *fmt, unsigned long address);
@@ -47,6 +52,11 @@ static inline const char *kallsyms_lookup(unsigned
long addr,
return NULL;
}
+static inline void lookup_symbol(unsigned long addr, char *buffer)
+{
+ return NULL;
+}
Returning NULL in a function returning "void" doesn't seem right :P
Maybe it should be something like this instead:
{
*buffer = '\0';
}
[...]
Anyway, the change looks useful, so thanks for the patch :)
--
Paulo Marques - www.grupopie.com
"Very funny Scotty. Now beam up my clothes."
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: SMP performance degradation with sysbench
Rik van Riel wrote:
J.A. Magallón wrote:
[...]
Its the same to answer 4+4 queries than 8 at half the speed, isn't it ?
That still doesn't fix the potential Linux problem that this
benchmark identified.
To clarify: I don't care as much about MySQL performance as
I care about identifying and fixing this potential bug in
Linux.
IIRC a long time ago there was a change in the scheduler to prevent a
low prio task running on a sibling of a hyperthreaded processor to slow
down a higher prio task on another sibling of the same processor.
Basically the scheduler would put the low prio task to sleep during an
adequate task slice to allow the other sibling to run at full speed for
a while.
I don't know the scheduler code well enough, but comments like this one
make me think that the change is still in place:
/*
* If an SMT sibling task has been put to sleep for priority
* reasons reschedule the idle task to see if it can now run.
*/
if (rq->nr_running) {
resched_task(rq->idle);
ret = 1;
}
If that is the case, turning off CONFIG_SCHED_SMT would solve the problem.
--
Paulo Marques - www.grupopie.com
"The face of a child can say it all, especially the
mouth part of the face."
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: linux-2.6.19.3 build faild with "Inconsistent kallsyms data"
Toralf Förster wrote: Hello, the build with the attached .config failed, make ends with: ... scripts/kconfig/conf -s arch/i386/Kconfig CHK include/linux/version.h CHK include/linux/utsrelease.h CHK include/linux/compile.h AS .tmp_kallsyms2.o LD vmlinux SYSMAP System.map SYSMAP .tmp_System.map Inconsistent kallsyms data Try setting CONFIG_KALLSYMS_EXTRA_PASS make: *** [vmlinux] Error 1 Here's the config: Hummm, I just tried that .config with a vanilla 2.6.19.3 and it built just fine. So either you have some patches that make a difference, or it's a binutils version problem. Can you send the output of scripts/ver_linux to see what binutils version you are using? Also you can try a "make debug_kallsyms" build that creates a .tmp_map1 and a .tmp_map2 files that might help diagnose the problem. -- Paulo Marques - www.grupopie.com "The face of a child can say it all, especially the mouth part of the face." - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: PROBLEM: 2.6.13, Inconsistent kallsyms data
Jim McCloskey wrote:
When I try to compile 2.6.13, using a complete tarball from
kernel.org, the compilation fails with:
---
SYSMAP System.map
SYSMAP .tmp_System.map
Inconsistent kallsyms data
Try setting CONFIG_KALLSYMS_EXTRA_PASS
make: *** [vmlinux] Error 1
---
When CONFIG_KALLSYMS_EXTRA_PASS is set, the compilation completes
successfully.
Please try the attached patch too see if it fixes the problem for you.
This patch is already in -mm and scheduled to go in 2.6.14 (or at least
I hope it is ;)
--
Paulo Marques - www.grupopie.com
It is a mistake to think you can solve any major problems
just with potatoes.
Douglas Adams
--- ./scripts/kallsyms.c.orig 2005-06-23 19:20:20.0 +0100
+++ ./scripts/kallsyms.c2005-06-23 19:20:34.0 +0100
@@ -24,75 +24,37 @@
*
*/
+#define _GNU_SOURCE
+
#include
#include
#include
#include
-/* maximum token length used. It doesn't pay to increase it a lot, because
- * very long substrings probably don't repeat themselves too often. */
-#define MAX_TOK_SIZE 11
#define KSYM_NAME_LEN 127
-/* we use only a subset of the complete symbol table to gather the token count,
- * to speed up compression, at the expense of a little compression ratio */
-#define WORKING_SET1024
-
-/* first find the best token only on the list of tokens that would profit more
- * than GOOD_BAD_THRESHOLD. Only if this list is empty go to the "bad" list.
- * Increasing this value will put less tokens on the "good" list, so the search
- * is faster. However, if the good list runs out of tokens, we must painfully
- * search the bad list. */
-#define GOOD_BAD_THRESHOLD 10
-
-/* token hash parameters */
-#define HASH_BITS 18
-#define HASH_TABLE_SIZE(1 << HASH_BITS)
-#define HASH_MASK (HASH_TABLE_SIZE - 1)
-#define HASH_BASE_OFFSET 2166136261U
-#define HASH_FOLD(a) ((a)&(HASH_MASK))
-
-/* flags to mark symbols */
-#define SYM_FLAG_VALID 1
-#define SYM_FLAG_SAMPLED 2
struct sym_entry {
unsigned long long addr;
- char type;
- unsigned char flags;
- unsigned char len;
+ unsigned int len;
unsigned char *sym;
};
static struct sym_entry *table;
-static int size, cnt;
+static unsigned int table_size, table_cnt;
static unsigned long long _stext, _etext, _sinittext, _einittext, _sextratext,
_eextratext;
static int all_symbols = 0;
static char symbol_prefix_char = '\0';
-struct token {
- unsigned char data[MAX_TOK_SIZE];
- unsigned char len;
- /* profit: the number of bytes that could be saved by inserting this
-* token into the table */
- int profit;
- struct token *next; /* next token on the hash list */
- struct token *right;/* next token on the good/bad list */
- struct token *left;/* previous token on the good/bad list */
- struct token *smaller; /* token that is less one letter than this one */
- };
-
-struct token bad_head, good_head;
-struct token *hash_table[HASH_TABLE_SIZE];
+int token_profit[0x1];
/* the table that holds the result of the compression */
-unsigned char best_table[256][MAX_TOK_SIZE+1];
+unsigned char best_table[256][2];
unsigned char best_table_len[256];
-static void
-usage(void)
+static void usage(void)
{
fprintf(stderr, "Usage: kallsyms [--all-symbols]
[--symbol-prefix=] < in.map > out.S\n");
exit(1);
@@ -102,21 +64,19 @@ usage(void)
* This ignores the intensely annoying "mapping symbols" found
* in ARM ELF files: $a, $t and $d.
*/
-static inline int
-is_arm_mapping_symbol(const char *str)
+static inline int is_arm_mapping_symbol(const char *str)
{
return str[0] == '$' && strchr("atd", str[1])
&& (str[2] == '\0' || str[2] == '.');
}
-static int
-read_symbol(FILE *in, struct sym_entry *s)
+static int read_symbol(FILE *in, struct sym_entry *s)
{
char str[500];
- char *sym;
+ char *sym, stype;
int rc;
- rc = fscanf(in, "%llx %c %499s\n", &s->addr, &s->type, str);
+ rc = fscanf(in, "%llx %c %499s\n", &s->addr, &stype, str);
if (rc != 3) {
if (rc != EOF) {
/* skip line */
@@ -143,7 +103,7 @@ read_symbol(FILE *in, struct sym_entry *
_sextratext = s->addr;
else if (strcmp(sym, "_eextratext") == 0)
_eextratext = s->addr;
- else if (toupper(s->type) == 'A')
+ else if (toupper(stype) == 'A')
{
/* Keep these useful absolute symbols */
if (strcmp(sym, &
Re: [PATCH] do not save thousands of useless symbols in KALLSYMS kernels
Denis Vlasenko wrote: Sample of my kernel's mostly useless symbols (starting_with:# of symbols): __func__: 624 __vendorstr_: 1760 __pci_fixup_PCI_: 116 __ksymtab_: 2597 __kstrtab_: 2597 __kcrctab_: 2597 __initcall_: 236 __devicestr_: 4686 __devices_: 1760 Total: 16973 Lines in System.map: 39735 Excluding them from in-kernel symbol table saves ~300kb: textdata bss dec hex filename 4337710 1054414 259296 5651420 563bdc vmlinux.carrier1 - w/o KALLSYMS 4342068 1296046 259296 5897410 59fcc2 vmlinux - with KALLSYMS+patch 4341948 1607634 259296 6208878 5ebd6e vmlinux.carrier - with KALLSYMS ^^^ Hummm these symbols should only go in if you config KALLSYMS_ALL. Are you sure your configuration doesn't have KALLSYMS_ALL enabled? If it does, it seems like the correct behavior to include all symbols if you specified that you wanted _all_ symbols :) By the way, there is a completely different version of scripts/kallsyms.c in -mm that you might want to look at. It will probably go in after 2.6.13 is out. -- Paulo Marques - www.grupopie.com It is a mistake to think you can solve any major problems just with potatoes. Douglas Adams - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 8/8] ALSA: convert kcalloc to kzalloc
Dmitry Torokhov wrote: On 8/5/05, Pekka Enberg <[EMAIL PROTECTED]> wrote: This patch converts kcalloc(1, ...) calls to use the new kzalloc() function. Hi, Have you seen the following in include/sound/core? ... #define kmalloc(size, flags) snd_hidden_kmalloc(size, flags) #define kcalloc(n, size, flags) snd_hidden_kcalloc(n, size, flags) #define kfree(obj) snd_hidden_kfree(obj) Arghh... I've been bitten by this before, too. Pekka, the sound subsystem uses its own allocation functions that are just defined to the standard kernel counterparts when CONFIG_SND_DEBUG_MEMORY is not defined, but that map to their own functions that add aditional debug information when that config is set. To play well with the current structure you have to define you own snd_hidden_kzalloc(size, flags) and use the same scheme. For my previous encounter with this problem search a thread named "replace snd_kmalloc_strdup by kstrdup". The patch there might useful to you. I really hate this "#define kmalloc" hack. It makes the code really unreadble, because you expect a kmalloc to be just a kmalloc... Couldn't we turn this into a generic kernel debugging option, so that it could be used for every kmalloc instead of just the ones from the sound system? If I get this right, what this code does is to track kfree's on pointers that were not alloc'ed with kmalloc (using a magic number) and keep track of all the allocations to detect leaks. We already have SLAB_DEBUG. We could add a list of allocations with a proc interface (or something) to give an histogram of kmalloc callers / number of allocations not yet freed. This way, if after stopping everything related to sound there were still callers like "snd_" (through kallsyms) you would know there is a leak there. What does CONFIG_SND_DEBUG_MEMORY provide that this more generic scheme does not? -- Paulo Marques Software Development Department - Grupo PIE, S.A. Phone: +351 252 290600, Fax: +351 252 290601 Web: www.grupopie.com It is a mistake to think you can solve any major problems just with potatoes. Douglas Adams - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: IBM HDAPS, I need a tip.
Jan Engelhardt wrote: So in order to calibrate it you need a readily available source of constant acceleration, preferably with a known value. Hint: -9.8 m/sec^2. Drop it out of the window? :) No, no. Constant gravity (like having the laptop sitting on the desk) "feels like" constant acceleration. Dropping it out of the window should measure 0 m/sec^2, because the accelerometer is not working on an inertial referential (I hope this is the correct term in english...). For the accelerometer, this is just like the feeling of free falling inside an elevator: no gravity :) -- Paulo Marques - www.grupopie.com It is a mistake to think you can solve any major problems just with potatoes. Douglas Adams - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
Jon Smirl wrote: On 7/28/05, Mitchell Blank Jr <[EMAIL PROTECTED]> wrote: [...] It looks sane-ish to me, but also more complicated than need be. Why can't you just do something like: while (count > 0 && isspace(x[count - 1])) count--; Do we need to deal with UTF8 here? I did the forward loop because you can't parse UTF8 backwards. If UTF8 is possible I need to change the pointer inc function. I don't think it matters here. Even with UTF8, any char that makes isspace return true, can't be part of a multi-byte char, as every byte in a multi-byte char in UTF8 has the MSB set, i.e., >= 0x80. -- Paulo Marques - www.grupopie.com It is a mistake to think you can solve any major problems just with potatoes. Douglas Adams - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] signed char fixes for scripts
Bernd Petrovitsch wrote:
On Thu, 2005-07-28 at 11:02 +0100, Paulo Marques wrote:
J.A. Magallon wrote:
[...]
All the problems are born here:
struct sym_entry {
unsigned long long addr;
unsigned int len;
unsigned char *sym;
};
What are you guys talking about?
"unsigned char *" is simply the wrong type for mere text strings. "char
*" ist the corrrect one. These are BTW two completely different types
(yes, "char" can be promoted into an "unsigned char" but essentially
these are two completely different types like "int" and "long long *").
You're comming really late in this thread :)
The problem is that "sym" isn't really a string. It starts out as a
string, but as the compression scheme begins to work it just becomes a
"bunch of bytes" using all the values in the range 0-255 for which
unsigned char is the perfect type.
Since only the loading of the symbols use string functions, and all the
compression process treats these as bytes, it seemed better to treat
them as unsigned chars and just typecast the first few uses.
The union suggested by J.A.Magallon might be a reasonable solution, but
we only need 4 casts in the 500 lines of code of scripts/kallsyms.c to
solve all problems, so this seems really overkill.
Is my compiler version the problem (3.3.2), or are you testing with the
Compiler version - zse gcc-4.*.
Yes, I know J.A.Magallon is trying to silence the warnings of gcc 4.0,
but as I understood it, gcc 3 would also complain of the same problems
if -Wsign-compare were specified. It was just that gcc4 would complain
even without -Wsign-compare.
So the question is: is gcc4 complaining about signedness problems that
gcc3 doesn't, even with -Wsign-compare?
Now that I look at the source, I can see that it must be complaining!
There are still 3 calls to strcmp that use sym directly, and gcc3
doesn't say a thing.
I thought that these were already taken care of. In any cased the
attached patch should fix those and make the code more readable too.
With this patch we end up only having 2 casts to (char *) in the whole
source.
Can someone with gcc 4 apply this to the latest -mm and check that it
fixes everything?
--
Paulo Marques - www.grupopie.com
It is a mistake to think you can solve any major problems
just with potatoes.
Douglas Adams
--- ./scripts/kallsyms.c.orig 2005-07-28 11:31:29.0 +0100
+++ ./scripts/kallsyms.c 2005-07-28 11:33:58.0 +0100
@@ -150,11 +150,13 @@ static int symbol_valid(struct sym_entry
"_SDA2_BASE_", /* ppc */
NULL };
int i;
- int offset = 1;
+ char *name;
+
+ name = (char *) s->sym + 1;
/* skip prefix char */
- if (symbol_prefix_char && *(s->sym + 1) == symbol_prefix_char)
- offset++;
+ if (symbol_prefix_char && *name == symbol_prefix_char)
+ name++;
/* if --all-symbols is not specified, then symbols outside the text
* and inittext sections are discarded */
@@ -169,18 +171,18 @@ static int symbol_valid(struct sym_entry
* move then they may get dropped in pass 2, which breaks the
* kallsyms rules.
*/
- if ((s->addr == _etext && strcmp(s->sym + offset, "_etext")) ||
- (s->addr == _einittext && strcmp(s->sym + offset, "_einittext")) ||
- (s->addr == _eextratext && strcmp(s->sym + offset, "_eextratext")))
+ if ((s->addr == _etext && strcmp(name, "_etext")) ||
+ (s->addr == _einittext && strcmp(name, "_einittext")) ||
+ (s->addr == _eextratext && strcmp(name, "_eextratext")))
return 0;
}
/* Exclude symbols which vary between passes. */
- if (strstr((char *)s->sym + offset, "_compiled."))
+ if (strstr(name, "_compiled."))
return 0;
for (i = 0; special_symbols[i]; i++)
- if( strcmp((char *)s->sym + offset, special_symbols[i]) == 0 )
+ if( strcmp(name, special_symbols[i]) == 0 )
return 0;
return 1;
Re: [PATCH] signed char fixes for scripts
J.A. Magallon wrote:
On 07.27, Sam Ravnborg wrote:
On Fri, Jul 15, 2005 at 10:14:43PM +, J.A. Magallon wrote:
On 07.16, J.A. Magallon wrote:
On 07.15, Andrew Morton wrote:
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.13-rc3/2.6.13-rc3-mm1/
This time I did not break anything... and they shut up gcc4 ;)
I have applied it to my tree. There still is a lot left when I compile
with -Wsign-compare.
All the problems are born here:
struct sym_entry {
unsigned long long addr;
unsigned int len;
unsigned char *sym;
};
What are you guys talking about?
I've just compiled the current version in -mm with -Wsign-compare and it
doesn't give me a single warning.
Is my compiler version the problem (3.3.2), or are you testing with the
old version of kallsyms?
--
Paulo Marques - www.grupopie.com
It is a mistake to think you can solve any major problems
just with potatoes.
Douglas Adams
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: xor as a lazy comparison
Lee Revell wrote: On Mon, 2005-07-25 at 13:55 -0400, Steven Rostedt wrote: Doesn't matter. The cycles saved for old compilers is not rational to have obfuscated code. Where do we draw the line with this? Is x *= 2 preferable to x <<= 2 as well? I guess this depends on what you logically want to do. If the problem requires you to shift some value N bits, then you should use a shift operation. If what you want is to multiply a value by a certain ammount, you should just use a multiplication. Using a shift to perform the multiplication should be left to the compiler IMHO. The proof that the shift is not so clear is that even you got the shift wrong in your own example ;) -- Paulo Marques - www.grupopie.com It is a mistake to think you can solve any major problems just with potatoes. Douglas Adams - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Problem with Asus P4C800-DX and P4 -Northwood-
Andreas Baer wrote: [...] Vmstat for Notebook P4 3.0 GHz 512 MB RAM: procs ---memory-- ---swap-- -io --system-- cpu r b swpd free buff cache si sobibo incs us sy id wa 1 0 0 179620 14812 228832003321 557 184 3 1 95 1 2 0 0 178828 14812 22883200 0 0 1295 819 6 2 92 0 1 0 0 175948 14812 22883200 0 0 1090 111 37 17 This vmstat output doesn't show any input / output happening. Are you sure this was taken *while* your test is running? If it is, then all files are already in pagecache. The fact that you have free memory at all times, and that the run on the notebook takes less than 20 seconds confirms this. The second takes a lot more time to execute. The 1Gb memory does make me suspicious, though. There is a known problem with BIOS that don't set up the mtrr's correctly for the whole memory and leave a small amount of memory on the top with the wrong settings. Accessing this memory becomes painfully slow. Can you send the output of /proc/mtrr and try to boot with something like "mem=768M" to see if that improves performance on the Desktop P4? -- Paulo Marques - www.grupopie.com It is a mistake to think you can solve any major problems just with potatoes. Douglas Adams - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Re: itimer oddness in 2.6.12
George Anzinger wrote: Tom Marshall wrote: On Fri, Jul 22, 2005 at 08:21:25PM +0100, Paulo Marques wrote: [...] Unfortunately, this is not so clear cut as it seems :( Oops! That patch is wrong. The +1 should be applied to the initial interval _only_. We KNOW when the repeating intervals start (i.e. at the jiffie edge) and don't need to adjust them. The patch, however, incorrectly, rolls them all into one. The attach patch should fix the problem. Warnning, it compiles and boots, but I have not tested it. Yes, this seems to be the Right Thing :) Acked-by: Paulo Marques <[EMAIL PROTECTED]> -- Paulo Marques - www.grupopie.com It is a mistake to think you can solve any major problems just with potatoes. Douglas Adams - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: itimer oddness in 2.6.12
Tom Marshall wrote: The patch to fix "setitimer timer expires too early" is causing issues for the Helix server. We have a timer processs that updates the server's timestamp on an itimer and it expects the signal to be delivered at roughly the interval retrieved from getitimer. This is very consistent on every platform, including Linux up to 2.6.11, but breaks on 2.6.12. On 2.6.12, setting the itimer to 10ms and retrieving the actual interval from getitimer reports 10.998ms, but the timer interrupts are consistently delivered at roughly 11.998ms. Unfortunately, this is not so clear cut as it seems :( I tested this on my system again, and if I set the timer to 9900us and put the system to some load I get intervals as low as 10022us with a vanilla 2.6.12.2 kernel. Removing this patch would cause the system to give me 9022us intervals which is just unnacceptable. The reason this misbehaves in your case is that 10ms is converted into 11 jiffies. I'm not really an expert in the time subsystem, but I guess this happens because 1000HZ aren't _exactly_ 1000HZ, they're probably a little more, so to guarantee 10 ms, we need 11 jiffies (or there is a bug in the timeval->jiffies conversions). If HZ is slightly greater than 1000, this means that each tick will come in slightly less than 1 ms. Lets assume we want 2 ms intervals: ---|||||-- ^ ^ ^^ 0 1 23 If you place your request at instant "1" and the time between ticks is slightly less than 1 ms, at instant "2" there is no guarantee that the time has ellapsed, only at instant "3" that is 4 ticks away. If you place that request at instant "0", you'll get almost 4ms. So, the fact that 10 ms are converted to 11 jiffies explains why getitimer returns 10.998ms. The fact that you are getting consistently 11.998ms just means that either your system is pretty idle, or the process that is requesting this timer has a very high priority so that it is able to request the timer again right after the timer tick (like in instant "0"). If this process is delayed for some reason and just requests the timer in the middle of the tick you would start seing values like 11.5ms. If 10ms in jiffies would be just 10, then you would see 11ms between alarms, and getitimer would report 10ms. The only way this could be better was if we knew "where" inside the tick we were when we set the timer (as discussed in the thread you mentioned), but in your case, even this wouldn't help because you're requesting a 10ms timer and to absolutely conform to the setitimer specification we can't just give you a 9.9ms interval. Anyway, making the software depend on the time a timer takes to expire when the man page states "Timers will never expire before the requested time, ... the delivery will be offset by a small time dependent on the system loading" doesn't seem like a very robust software design to me... -- Paulo Marques Software Development Department - Grupo PIE, S.A. Phone: +351 252 290600, Fax: +351 252 290601 Web: www.grupopie.com It is a mistake to think you can solve any major problems just with potatoes. Douglas Adams - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFD] FAT robustness
Hiroyuki Machida wrote: [...] Q3 : I'm not sure JBD can be used for FAT improvements. Do you have any comments ? I might not be the best person to answer this, but this just seems so obvious: If you plan to let a recently hot-unplugged device to be used in another OS that doesn't understand your journaling extensions, your disk will be corrupted. If this is supposed to work only on OS's that understand your journaling extensions, then there are much better filesystems out there with journaling already. You might be able to reduce the size of the time window where hot removing the media will cause problems, like writting all the data first and update the metadata in as few operations as possible. But that just reduces the probability of data corruption. It doesn't eliminate it at all. -- Paulo Marques - www.grupopie.com It is a mistake to think you can solve any major problems just with potatoes. Douglas Adams - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] signed char fixes for scripts
Paulo Marques wrote: Sam Ravnborg wrote: [...] Also this patch seems relative small compared to the others floating around to cure signed warnings in scripts/ Does this really fix all of them or only a subset of the warnings? Well, current -linus already has a patch from me to change the ^^ I meant -mm... :P -- Paulo Marques - www.grupopie.com It is a mistake to think you can solve any major problems just with potatoes. Douglas Adams - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] signed char fixes for scripts
Sam Ravnborg wrote: On Fri, Jul 15, 2005 at 10:14:43PM +, J.A. Magallon wrote: On 07.16, J.A. Magallon wrote: [...] This time I did not break anything... and they shut up gcc4 ;) Thanks. Can you please resend with proper changelog and signed-off-by. Diff should be done on top of latest -linus preferable. Also this patch seems relative small compared to the others floating around to cure signed warnings in scripts/ Does this really fix all of them or only a subset of the warnings? Well, current -linus already has a patch from me to change the compression scheme that also fixes most of the signedness problems. The ones below escaped me because my gcc3.3.2 didn't complain about them even with all the -W[xxx] switches I could find. This takes a big hunk out of previous patches I've seen, so that might explain the difference. -- Paulo Marques - www.grupopie.com It is a mistake to think you can solve any major problems just with potatoes. Douglas Adams - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Realtime Preemption, 2.6.12, Beginners Guide?
Ingo Molnar wrote:
(gdb)
(gdb) # c013ebf4, stack size: 388 bytes #
(gdb)
(gdb) 0xc013ebf4 is in __print_symbol (kernel/kallsyms.c:234).
The attached patch fixes this partially by reducing the stack usage by
128 bytes. Compile, boot and run tested and apparently it works fine.
I didn't want to use kmalloc's in there because this function is
probably called from very "hard" contexts (kernel OOPS, stack overflow
dumps, etc.).
The stack usage could be reduced even further (I can do a patch for this
if needed) by changing the function to receive a "prefix" and a "suffix"
string instead of a format string.
The function could then simply do:
printk(prefix);
printk(symbol);
printk(address);
if (module) printk(module name);
printk(suffix);
This way it wouldn't need to allocate a buffer big enough for the whole
string, just for one symbol name (128 bytes).
This is a much more intrusive change however (there are ~65 callers that
would need changing), so I leave the decision to more experienced hackers :)
--
Paulo Marques - www.grupopie.com
It is a mistake to think you can solve any major problems
just with potatoes.
Douglas Adams
--- ./kernel/kallsyms.c.orig 2005-07-11 12:32:32.0 +0100
+++ ./kernel/kallsyms.c 2005-07-11 12:34:42.0 +0100
@@ -232,23 +232,21 @@ const char *kallsyms_lookup(unsigned lon
/* Replace "%s" in format with address, or returns -errno. */
void __print_symbol(const char *fmt, unsigned long address)
{
- char *modname;
+ char *modname, *bufend;
const char *name;
unsigned long offset, size;
- char namebuf[KSYM_NAME_LEN+1];
char buffer[sizeof("%s+%#lx/%#lx [%s]") + KSYM_NAME_LEN +
2*(BITS_PER_LONG*3/10) + MODULE_NAME_LEN + 1];
- name = kallsyms_lookup(address, &size, &offset, &modname, namebuf);
+ name = kallsyms_lookup(address, &size, &offset, &modname, buffer);
if (!name)
sprintf(buffer, "0x%lx", address);
else {
+ bufend = strchr(buffer, '\0');
+ bufend += sprintf(bufend, "+%#lx/%#lx", offset, size);
if (modname)
- sprintf(buffer, "%s+%#lx/%#lx [%s]", name, offset,
-size, modname);
- else
- sprintf(buffer, "%s+%#lx/%#lx", name, offset, size);
+ sprintf(bufend, " [%s]", modname);
}
printk(fmt, buffer);
}
Re: function Named
raja wrote: hi, Is there any way to get the function address by only knowing the function Name See "kallsyms_lookup_name" in include/linux/kallsyms.h (implemented in kernel/kallsyms.c). Beware: - CONFIG_KALLSYMS might be undefined. In that case it always returns 0 - the function returns the address of *any* known symbol, not just functions (this can be easily improved to also return the symbol type) - the function is extremely inefficient, as it does a linear search over all the symbols. kallsyms is optimized to work the other way around: get the name from the address. -- Paulo Marques - www.grupopie.com It is a mistake to think you can solve any major problems just with potatoes. Douglas Adams - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Linux 2.6.13-rc2 - Inconsistent kallsyms data
Alexis Ballier wrote: Yes, that fixed it. Ok, it is probably the same problem, then. However, there was no problem with rc1 with the same .config. That's just the nature of it. It only triggers if you're unlucky. For more details check these threads: http://lkml.org/lkml/2005/5/10/70 http://seclists.org/lists/linux-kernel/2005/May/2010.html The fix in mm is actually very different from any proposed solution in those threads. For more details check here: http://lkml.org/lkml/2005/6/27/188 -- Paulo Marques - www.grupopie.com It is a mistake to think you can solve any major problems just with potatoes. Douglas Adams - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Slowdown with randomize_va_space in 2.6.12.2
Hi, all I have a bash script that calls a small application several times (around 50 calls) that just send and receives data through an already open tcp socket to a local server through the loopback device. It also launches another small app several times that just reads a small file from disk and does some processing on it in memory. We noticed a severe performance regression on this application under kernel 2.6.12.2 that we tracked down to the address space randomization patches: # echo 0 > randomize_va_space # time ./script real0m0.671s user0m0.293s sys 0m0.325s # echo 1 > randomize_va_space # time ./script real0m3.310s user0m2.712s sys 0m0.401s Notice that the real time is 5x slower with "randomize_va_space" turned on. This is on a Transmeta Crusoe TM5600 at 533MHz. What is weird is that most of the extra time is being accounted as user-space time, but the user-space application is exactly the same in both runs, only the "randomize_va_space" parameter changed. I browsed the randomization patch code and I don't think the random calculations themselves could account for all that time. Does anybody have a clue as to why this is happening or what I should do to debug this further? -- Paulo Marques - www.grupopie.com It is a mistake to think you can solve any major problems just with potatoes. Douglas Adams - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Linux 2.6.13-rc2 - Inconsistent kallsyms data
Alexis Ballier wrote: Hi ! I have a problem building the rc2 (or rc3, whatever ;) ) Here is the end of the log : [...] Inconsistent kallsyms data Try setting CONFIG_KALLSYMS_EXTRA_PASS make: *** [vmlinux] Erreur 1 Can you try to change this setting in scripts/kallsyms.c: #define WORKING_SET 1024 to somethig like: #define WORKING_SET65536 If this fixes it, then it is a known problem and the fix is already in -mm. The fix is more complex than this, however. -- Paulo Marques - www.grupopie.com It is a mistake to think you can solve any major problems just with potatoes. Douglas Adams - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: incoming
Geert Uytterhoeven wrote: On Tue, 12 Apr 2005, Andrew Morton wrote: As the commits list probably isn't working at present I'll cc linux-kernel on this lot. Fairly cruel, sorry, but I don't like the idea of people not knowing what's hitting the main tree. Is it me, or were really only 117 mails of the 198 sent to lkml? (?) I just double-checked, and I can say that I received all 198 emails from vger... -- Paulo Marques - www.grupopie.com All that is necessary for the triumph of evil is that good men do nothing. Edmund Burke (1729 - 1797) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] create a kstrdup library function
Paulo Marques wrote: Hi, This patch creates a new kstrdup library function and changes the "local" implementations in several places to use this function. Arkadiusz Miskiewicz reported that this breaks compilation under PPC. Apparently, PPC builds a bootloader that links against lib.a but doesn't expect any dependencies on slab. Since kstrdup calls kmalloc, this breaks compilation. I can fix this by moving kstrdup into slab.c. This way this is treated as an "allocation" function instead of a string function, so it makes some sense to do this. Andrew, do you prefer an incremental patch against the current tree, or a single clean patch against the current tree with all the current kstrdup patches taken out? -- Paulo Marques - www.grupopie.com All that is necessary for the triumph of evil is that good men do nothing. Edmund Burke (1729 - 1797) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] kallsyms C_SYMBOL_PREFIX support
Yoshinori Sato wrote:
kallsyms does not consider SYMBOL_PREFIX of C.
Consequently do not work in architecture using prefix character (h8300, v850)
really.
Because I can want to use this, I made a patch.
Please comment.
[...]
@@ -177,6 +184,11 @@
"_SDA2_BASE_",/* ppc */
NULL };
int i;
+ int offset = 1;
+
+ /* skip prefix char */
+ if (symbol_prefix_char && *(s->sym + 1) == symbol_prefix_char)
+ offset++;
maybe something like:
char *sname;
sname = s->sym + 1;
if (symbol_prefix_char && *(s->sym + 1) == symbol_prefix_char)
sname++;
would avoid all the "(s->sym + offset)" below, turning them to just "sname".
I know that it was "(s->sym + 1)" before, so its really not your fault,
but you could take this opportunity to clean that up, too.
/* if --all-symbols is not specified, then symbols outside the text
* and inittext sections are discarded */
@@ -190,17 +202,17 @@
* they may get dropped in pass 2, which breaks the kallsyms
* rules.
*/
- if ((s->addr == _etext && strcmp(s->sym + 1, "_etext")) ||
- (s->addr == _einittext && strcmp(s->sym + 1, "_einittext")))
+ if ((s->addr == _etext && strcmp(s->sym + offset, "_etext")) ||
+ (s->addr == _einittext && strcmp(s->sym + offset, "_einittext")))
return 0;
}
/* Exclude symbols which vary between passes. */
- if (strstr(s->sym + 1, "_compiled."))
+ if (strstr(s->sym + offset, "_compiled."))
return 0;
for (i = 0; special_symbols[i]; i++)
- if( strcmp(s->sym + 1, special_symbols[i]) == 0 )
+ if( strcmp(s->sym + offset, special_symbols[i]) == 0 )
return 0;
return 1;
@@ -225,9 +237,15 @@
[...]
/* uncompress a compressed symbol. When this function is called, the best table
@@ -665,6 +683,13 @@
insert_real_symbols_in_table();
+ /* When valid symbol is not registered, exit to error */
+ if (good_head.left == good_head.right &&
+ bad_head.left == bad_head.right) {
+ fprintf(stderr, "No valid symbol.\n");
+ exit(1);
+ }
+
optimize_result();
}
This should only trigger if there are no symbols at all, or if there are
some symbols that are considered invalid, and do not go into the final
result.
Maybe we should just do a return here instead of exit, so that even if
this happens, kallsyms will still produce an empty result, that will at
least allow the kernel to compile.
It should give the error output to warn the user that there is something
fishy, nevertheless. Maybe even a bigger message, since this should not
happen at all, and if this triggers it means that something is seriously
wrong.
@@ -672,9 +697,21 @@
int
main(int argc, char **argv)
{
- if (argc == 2 && strcmp(argv[1], "--all-symbols") == 0)
- all_symbols = 1;
- else if (argc != 1)
+ if (argc >= 2) {
This test is unnecessary.
+ int i;
+ for (i = 1; i < argc; i++) {
+ if(strcmp(argv[i], "--all-symbols") == 0)
+all_symbols = 1;
+ else if (strncmp(argv[i], "--symbol-prefix=", 16) == 0) {
+char *p = &argv[i][16];
+/* skip quote */
+if ((*p == '"' && *(p+2) == '"') || (*p == '\'' && *(p+2) == '\''))
+ p++;
+symbol_prefix_char = *p;
+ } else
+usage();
+ }
+ } else if (argc != 1)
usage();
and so is this.
read_map(stdin);
@@ -683,4 +720,3 @@
return 0;
}
At least the patch seems to not affect architectures that don't use the
"--symbol-prefix" option, so it should be harmless for most.
Anyway, appart from the few comments, it has my acknowledge.
--
Paulo Marques - www.grupopie.com
All that is necessary for the triumph of evil is that good men do nothing.
Edmund Burke (1729 - 1797)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: turn kmalloc+memset(,0,) into kcalloc
Adrian Bunk wrote: [...] On Tue, Apr 05, 2005 at 05:26:31PM +0100, Paulo Marques wrote: Hi Adrian, Hi Paolo, Paulo, please :) Paolo is Spanish (or Italian), whereas Paulo is a Portuguese name. [...] I think most will agree that the second piece of code is more "readable". In this case yes (but it could still use the normal kcalloc). Yes, but the cleanup is already just barely justifiable. 75% (or more) of the calls will use just one parameter. Adding a stupid "1" to all of those will make the cleanup even less worth while. [...] Joerg's list of recursions should be valid independent of the kernel version. Fixing any real stack problems [1] that might be in this list is a valuable task. And "make checkstack" in a kernel compiled with unit-at-a-time lists several possible problems at the top. Ok, I've read Jörn's mail also and I think I can help out. It seems however that there are more people working on this. Will it be better to coordinate so we don't duplicate efforts or is the "everyone looks at everything" approach better, so that its harder to miss something? -- Paulo Marques - www.grupopie.com All that is necessary for the triumph of evil is that good men do nothing. Edmund Burke (1729 - 1797) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.12-rc2-mm2
Jan Dittmer wrote: Andrew Morton wrote: create-a-kstrdup-library-function.patch create a kstrdup library function create-a-kstrdup-library-function-fixes.patch create-a-kstrdup-library-function-fixes Oops, forgot to include slab.h. I guess the other #include's were including it somewhere down the line on x86, so it went unnoticed :( The attached patch should fix this. [PATCH] create-a-kstrdup-library-function-fix-include-slab Signed-off-by: Paulo Marques <[EMAIL PROTECTED]> -- Paulo Marques - www.grupopie.com All that is necessary for the triumph of evil is that good men do nothing. Edmund Burke (1729 - 1797) --- ./lib/string.c.orig 2005-04-08 16:07:14.0 +0100 +++ ./lib/string.c 2005-04-08 16:08:29.0 +0100 @@ -23,6 +23,7 @@ #include #include #include +#include #ifndef __HAVE_ARCH_STRNICMP /**
Re: RFC: turn kmalloc+memset(,0,) into kcalloc
Adrian Bunk wrote: On Tue, Apr 05, 2005 at 05:26:31PM +0100, Paulo Marques wrote: [...] Hi Paulo, Hi Adrian, [...] pros: - smaller kernel image size - smaller (and more readable) source code Which is better readable depends on what you are used to. That's true to some degree, but look at code like this (in drivers/usb/input/hid-core.c): if (!(field = kmalloc(sizeof(struct hid_field) + usages * sizeof(struct hid_usage) + values * sizeof(unsigned), GFP_KERNEL))) return NULL; memset(field, 0, sizeof(struct hid_field) + usages * sizeof(struct hid_usage) + values * sizeof(unsigned)); and compare to this: field = kzalloc(sizeof(struct hid_field) + usages * sizeof(struct hid_usage) + values * sizeof(unsigned), GFP_KERNEL); if (!field) return NULL; In the first case you have to read carefully to make sure that the size argument in both the kmalloc and the memset are the same. Even more, if the size needs to be updated to include something more, a mistake can be made by inserting the extra size just in the kmalloc call. Also, you are assuming that the compiler is smart enough to notice that the two expressions are the same and cache the result, but this is probably true for gcc, anyway. I think most will agree that the second piece of code is more "readable". cons: - the NULL test is done twice - memset will not be optimized for constant sizes ... Would this be a good thing to clean up, or isn't it worth the effort at all? You do plan to patch 1200 places in the kernel for this micro-optimization? I was actually planning on sending a patch at a time for a reasonably sized subsection. Like "use kzalloc in usb/serial drivers", or "use kzalloc in sound/core", etc. This way, it shouldn't be more than 1150 patches ;) This sounds like a really big overhead for a pretty small gain. Yes it is. But at least it will remove 1000+ lines of redundant kernel code. I really don't see this as a priority at all. I'll probably submit a patch shortly to create the kzalloc function. This way developers become aware that it exists and that it should be used, and we don't get a lot of new code with the same constructs. The cleanup can then progress at a slowly pace, without breaking things and without producing too much merging problems. There are tasks of higher value that can be done. I couldn't agree more :) E.g. read my "Stack usage tasks" email. The benefits would only be present for people using GNU gcc 3.4 or SuSE gcc 3.3 on i386, but this is a reasonable subset of the kernel users - and it brings them a 2% kernel size improvement. I've read that thread, but it seems that it is at a dead end right now, since we don't have a good tool to find out which functions are abusing the stack with unit-at-a-time. Is there some way to even limit the search, like using a stack usage log from a compilation without unit-at-a-time, and going over the hotspots to check for problems? If we can get a list, even if it contains a lot of false positives, I would more than happy to help out... -- Paulo Marques - www.grupopie.com All that is necessary for the triumph of evil is that good men do nothing. Edmund Burke (1729 - 1797) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: turn kmalloc+memset(,0,) into kcalloc
Pekka Enberg wrote: Hi, On Apr 6, 2005 3:15 PM, Paulo Marques <[EMAIL PROTECTED]> wrote: However "calloc" is the standard C interface for doing this, so it makes some sense to use it here as well... :( I initally submitted kcalloc() with just one parameter but Arjan wanted it to be similar to standard calloc() so we could check for overflows. I don't see any reason not to introduce kzalloc() for the common case you mentioned (as suggested by Denis). kzalloc it is, then. By the way I did a quick measurement to see how much we could gain in kernel size by doing this. This is with a 2.6.11-rc2, defconfig kernel: with kmalloc+memset: vmlinuz: 5521614 bzImage: 2005274 with kzalloc: vmlinuz: 5513422 bzImage: 2003927 So we gain 8kB on the uncompressed image and 1347 bytes on the compressed one. This was just a dumb test and actual results might be better due to smarter human cleanups. Not a spectacular gain per se, but the increase in code readability is still worth it, IMHO. -- Paulo Marques - www.grupopie.com All that is necessary for the triumph of evil is that good men do nothing. Edmund Burke (1729 - 1797) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: turn kmalloc+memset(,0,) into kcalloc
Jörn Engel wrote:
On Tue, 5 April 2005 22:01:49 +0200, Jesper Juhl wrote:
On Tue, 5 Apr 2005, Roland Dreier wrote:
> or simply
> if (!(ptr = kcalloc(n, size, ...)))
> goto out;
> and save an additional line of screen realestate while you are at it...
No, please don't do that. The general kernel style is to avoid
assignments within conditionals.
FWIW, I also agree that assignments within conditionals are evil and
hurt readability a lot, for no actual benefit.
Since one of the advantages of this cleanup is improve readability, it
would be counterproductive to do this.
To explain why this cleanup improves readability take the following
sample from sound/usb/usx2y/usbusx2yaudio.c (a random sample):
us = kmalloc(sizeof(*us) + sizeof(struct urb*) * NOOF_SETRATE_URBS, GFP_KERNEL);
if (NULL == us) {
err = -ENOMEM;
goto cleanup;
}
memset(us, 0, sizeof(*us) + sizeof(struct urb*) * NOOF_SETRATE_URBS);
In this case you have to read the size portion of the kmalloc and the
memset carefully to make sure that they are exactly the same, whereas in
code like this:
us = kcalloc(1, sizeof(*us) + sizeof(struct urb*) *
NOOF_SETRATE_URBS, GFP_KERNEL);
if (NULL == us) {
err = -ENOMEM;
goto cleanup;
}
it is self-evident that the whole area is being cleared. It also leaves
a smaller room for mistakes.
I still don't like the kcalloc API with a "1" argument. Not only most of
these allocations fall into that case, but also kmalloc_zero (or
kmalloc_zeroed) is much more clear than kcalloc that changes just one
letter from kmalloc. Someone reading fast through the code may kcalloc
as if it were kmalloc.
More over, passing an extra parameter waste a few more bytes of code. I
know is not much, but if the cleanup will address hundreds of these then
it starts to be something to consider.
However "calloc" is the standard C interface for doing this, so it makes
some sense to use it here as well... :(
--
Paulo Marques - www.grupopie.com
All that is necessary for the triumph of evil is that good men do nothing.
Edmund Burke (1729 - 1797)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RFC: turn kmalloc+memset(,0,) into kcalloc
Hi, I noticed there are a number of places in the kernel that do: ptr = kmalloc(n * size, ...) if (!ptr) goto out; memset(ptr, 0, n * size); It seems that these could be replaced by: ptr = kcalloc(n, size, ...) if (!ptr) goto out; saving a few bytes. Most of the times the size isn't something "n * size", but simply "n". These could be replaced by kcalloc(n, 1, ...) or we could create a special "kmalloc_zero" function to do this without the need for the extra "1" parameter. A quick (and lame) grep through the tree shows about 1200 of these cases. This means that about one quarter of all the kmallocs in the kernel are actually zeroed right after allocation. I could send patches to slowly clean this up, like Adrian Bunk did for the static functions and Jesper Juhl for the kfree NULL checks. There are pros and cons to doing this: pros: - smaller kernel image size - smaller (and more readable) source code - explicit interface to request zeroed data. If in the future we have a good way of providing some zeroed-cachehot-super-duper-slabs, we can allocate space from there and avoid the memset altogether cons: - the NULL test is done twice - memset will not be optimized for constant sizes The disadvantages don't seem to matter much for non-critical paths. For really fast paths we should probably keep the kmalloc + memset. Attached is a sample of what one of those patches would look like. Would this be a good thing to clean up, or isn't it worth the effort at all? -- Paulo Marques - www.grupopie.com All that is necessary for the triumph of evil is that good men do nothing. Edmund Burke (1729 - 1797) --- ./lib/kobject_uevent.c.orig 2005-04-05 16:39:09.0 +0100 +++ ./lib/kobject_uevent.c 2005-04-05 17:01:26.0 +0100 @@ -234,10 +234,9 @@ void kobject_hotplug(struct kobject *kob if (!action_string) return; - envp = kmalloc(NUM_ENVP * sizeof (char *), GFP_KERNEL); + envp = kmalloc_zero(NUM_ENVP * sizeof (char *), GFP_KERNEL); if (!envp) return; - memset (envp, 0x00, NUM_ENVP * sizeof (char *)); buffer = kmalloc(BUFFER_SIZE, GFP_KERNEL); if (!buffer)
Re: [PATCH] create a kstrdup library function
Paulo Marques wrote: Adrian Bunk wrote: This patch contains a small bug: [...] Andrew, do you want me to send a new patch with this fixed, so you can back out the current one and apply the new one, or can you simply merge this one from Adrian? Never mind, I can see the fix is already in rc2-mm1. Thanks, -- Paulo Marques - www.grupopie.com All that is necessary for the triumph of evil is that good men do nothing. Edmund Burke (1729 - 1797) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] create a kstrdup library function
Adrian Bunk wrote: This patch contains a small bug: <-- snip --> ... WARNING: /lib/modules/2.6.12-rc1-mm4/kernel/net/sunrpc/sunrpc.ko needs unknown symbol kstrdup WARNING: /lib/modules/2.6.12-rc1-mm4/kernel/net/ipv6/ipv6.ko needs unknown symbol kstrdup WARNING: /lib/modules/2.6.12-rc1-mm4/kernel/drivers/parport/parport.ko needs unknown symbol kstrdup WARNING: /lib/modules/2.6.12-rc1-mm4/kernel/drivers/md/dm-mod.ko needs unknown symbol kstrdup <-- snip --> Damn, I did compile-test this thing, but didn't remember to compile even one of these as modules. Andrew, do you want me to send a new patch with this fixed, so you can back out the current one and apply the new one, or can you simply merge this one from Adrian? If I have to send a new patch, I might as well also fix the "int should be size_t" thing that Andres Salomon pointed out. -- Paulo Marques - www.grupopie.com All that is necessary for the triumph of evil is that good men do nothing. Edmund Burke (1729 - 1797) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] create a kstrdup library function
Hi,
This patch creates a new kstrdup library function and changes the
"local" implementations in several places to use this function.
This is just a cleanup to allow reusing the strdup code, and to prevent
bugs in future duplications of strdup.
Most of the changes come from the sound and net subsystems. The sound
part had already been acknowledged by Takashi Iwai and the net part by
David S. Miller.
I left UML alone for now because I would need more time to read the code
carefully before making changes there.
Signed-off-by: Paulo Marques <[EMAIL PROTECTED]>
drivers/md/dm-ioctl.c | 14 +++---
drivers/parport/probe.c| 18 +-
include/linux/netdevice.h |4
include/linux/string.h |2 ++
include/sound/core.h |3 ++-
lib/string.c | 20
net/core/neighbour.c |3 ++-
net/core/sysctl_net_core.c | 15 ---
net/ipv4/devinet.c |2 +-
net/ipv6/addrconf.c|3 ++-
net/sunrpc/svcauth_unix.c | 11 ++-
sound/core/info.c |3 ++-
sound/core/info_oss.c |3 ++-
sound/core/memory.c| 41
++---
sound/core/oss/mixer_oss.c |3 ++-
sound/core/oss/pcm_oss.c |3 ++-
sound/core/sound.c |2 +-
sound/core/timer.c |3 ++-
sound/isa/gus/gus_mem.c|7 ---
sound/synth/emux/emux.c|3 ++-
20 files changed, 70 insertions(+), 93 deletions(-)
--
Paulo Marques - www.grupopie.com
All that is necessary for the triumph of evil is that good men do nothing.
Edmund Burke (1729 - 1797)
diff -uprN -X dontdiff linux-2.6.12-rc1-mm4.vanilla/drivers/md/dm-ioctl.c
linux-2.6.12-rc1-mm4/drivers/md/dm-ioctl.c
--- linux-2.6.12-rc1-mm4.vanilla/drivers/md/dm-ioctl.c 2005-03-02
07:37:51.0 +
+++ linux-2.6.12-rc1-mm4/drivers/md/dm-ioctl.c 2005-04-04 20:17:36.0
+0100
@@ -122,14 +122,6 @@ static struct hash_cell *__get_uuid_cell
/*-
* Inserting, removing and renaming a device.
*---*/
-static inline char *kstrdup(const char *str)
-{
- char *r = kmalloc(strlen(str) + 1, GFP_KERNEL);
- if (r)
- strcpy(r, str);
- return r;
-}
-
static struct hash_cell *alloc_cell(const char *name, const char *uuid,
struct mapped_device *md)
{
@@ -139,7 +131,7 @@ static struct hash_cell *alloc_cell(cons
if (!hc)
return NULL;
- hc->name = kstrdup(name);
+ hc->name = kstrdup(name, GFP_KERNEL);
if (!hc->name) {
kfree(hc);
return NULL;
@@ -149,7 +141,7 @@ static struct hash_cell *alloc_cell(cons
hc->uuid = NULL;
else {
- hc->uuid = kstrdup(uuid);
+ hc->uuid = kstrdup(uuid, GFP_KERNEL);
if (!hc->uuid) {
kfree(hc->name);
kfree(hc);
@@ -273,7 +265,7 @@ static int dm_hash_rename(const char *ol
/*
* duplicate new.
*/
- new_name = kstrdup(new);
+ new_name = kstrdup(new, GFP_KERNEL);
if (!new_name)
return -ENOMEM;
diff -uprN -X dontdiff linux-2.6.12-rc1-mm4.vanilla/drivers/parport/probe.c
linux-2.6.12-rc1-mm4/drivers/parport/probe.c
--- linux-2.6.12-rc1-mm4.vanilla/drivers/parport/probe.c2005-04-01
18:01:29.0 +0100
+++ linux-2.6.12-rc1-mm4/drivers/parport/probe.c2005-04-04
20:17:36.0 +0100
@@ -48,14 +48,6 @@ static void pretty_print(struct parport
printk("\n");
}
-static char *strdup(char *str)
-{
- int n = strlen(str)+1;
- char *s = kmalloc(n, GFP_KERNEL);
- if (!s) return NULL;
- return strcpy(s, str);
-}
-
static void parse_data(struct parport *port, int device, char *str)
{
char *txt = kmalloc(strlen(str)+1, GFP_KERNEL);
@@ -88,16 +80,16 @@ static void parse_data(struct parport *p
if (!strcmp(p, "MFG") || !strcmp(p, "MANUFACTURER")) {
if (info->mfr)
kfree (info->mfr);
- info->mfr = strdup(sep);
+ info->mfr = kstrdup(sep, GFP_KERNEL);
} else if (!strcmp(p, "MDL") || !strcmp(p, "MODEL")) {
if (info->model)
kfree (info->model);
- info->model = strdup(sep);
+ info->model = kstrdup(sep, GFP_KERNEL);
} else if (!strcmp(p, "CLS") || !strcmp(p, "CLASS")) {
Re: Do not misuse Coverity please
Shankar Unni wrote:
Jean Delvare wrote:
v = p->field;
if (!p) return;
can be seen as equivalent to
if (!p) return;
v = p->field;
Heck, no.
You're missing the side-effect of a null pointer dereference crash (for
p->field) (even though v is unused before the return). The optimizer is
not allowed to make exceptions go away as a result of the hoisting.
I just had to try this out :)
Using gcc 3.3.2 this code sample:
struct test {
int code;
};
int test_func(struct test *a)
{
int ret;
if (!a) return -1;
ret = a->code;
return ret;
}
is compiled into:
0: 8b 54 24 04 mov0x4(%esp,1),%edx
4: 83 c8 ffor $0x,%eax
7: 85 d2 test %edx,%edx
9: 74 02 je d
b: 8b 02 mov(%edx),%eax
d: c3 ret
whereas this one:
int test_func(struct test *a)
{
int ret;
ret = a->code;
if (!a) return -1;
return ret;
}
is simply compiled into:
0: 8b 44 24 04 mov0x4(%esp,1),%eax
4: 8b 00 mov(%eax),%eax
6: c3 ret
It seems that gcc is smart enough to know that after we've dereferenced
a pointer, if it was NULL, it doesn't matter any more. So it just
assumes that if execution reaches that "if" statement then the pointer
can not be NULL at all.
So the 2 versions aren't equivalent, and gcc doesn't treat them as such
either.
Just a minor nitpick, though: wouldn't it be possible for an application
to catch the SIGSEGV and let the code proceed, making invalid the
assumption made by gcc?
--
Paulo Marques - www.grupopie.com
All that is necessary for the triumph of evil is that good men do nothing.
Edmund Burke (1729 - 1797)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][2/2] SquashFS
Andrew Morton wrote: [...] Also, this filesystem seems to do the same thing as cramfs. We'd need to understand in some detail what advantages squashfs has over cramfs to justify merging it. Again, that is something which is appropriate to the changelog for patch 1/1. Well, probably Phillip can answer this better than me, but the main differences that affect end users (and that is why we are using SquashFS right now) are: CRAMFS SquashFS Max File Size 16Mb 4Gb Max Filesystem Size256Mb 4Gb? UID/GID 8 bits 32 bits Block Size4K default 64k Probably the block size is the most responsible for this, but the compression ratio achieved by SquashFS is much higher than that achieved with cramfs. I just wanted to say one thing on behalf of SquashFS. We've been using SquashFS in production on a POS system we sell, and we have currently more than 1200 of these in use. There was never a problem reported that involved SquashFS. Although the workload patterns of these systems are probably very similar (so the quantity doesn't really matter much), it is a real world test of the filesystem, nevertheless. -- Paulo Marques - www.grupopie.com All that is necessary for the triumph of evil is that good men do nothing. Edmund Burke (1729 - 1797) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: inconsistent kallsyms data [2.6.11-mm2]
Sam Ravnborg wrote: On Thu, Mar 10, 2005 at 12:12:22PM +, Paulo Marques wrote: Paulo Marques wrote: [...] A simple and robust way is to do the sampling on a list of symbols sorted by symbol name. This way, even if the symbol positions that are given to scripts/kallsyms change, the symbols sampled will be the same. I'll do the patch to do this and send it ASAP. Ok, here it is. Dominik can you try the attached patch and see if it solves the problem? Hi Paulo. Hi Sam :) Alexander Stohr had similar problems with down and __sched_text_start. I figured out that what was causing the troubles was the fact that the linker generated symbol __sched_text_start changed value from pass 1 to pass 2. The reason for this was the alingment used within that section. Damn, you're right. Looking more carefully at Dominik's files I can see that on the first pass we have: T __sched_text_startPTR 0xc0420482 t __downPTR 0xc0420484 and on the second pass: t __downPTR 0xc0420484 T __sched_text_startPTR 0xc0420484 I only looked at the addresses on the second pass and noticed they were aliased symbols and that the symbol order changed from the first pass :P I never came around submitting this since I do not know what the correct number for function alignment is on different paltforms. If this will just align the beginning of a section, I don't think it will be a problem to always align at 8 bytes even on platforms that need only a 4 byte alignment. So I think that your patch should definitely go in, as it solves a real problem. As for my patch it could potentially solve problems that we don't currently have(*), so it is probably better to wait for them to appear before trying to solve an non-existent problem :) -- Paulo Marques - www.grupopie.com All that is necessary for the triumph of evil is that good men do nothing. Edmund Burke (1729 - 1797) (*) order of aliased symbols changing, or 'nm' returning non sorted addresses. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: inconsistent kallsyms data [2.6.11-mm2]
Paulo Marques wrote:
[...]
A simple and robust way is to do the sampling on a list of symbols
sorted by symbol name. This way, even if the symbol positions that are
given to scripts/kallsyms change, the symbols sampled will be the same.
I'll do the patch to do this and send it ASAP.
Ok, here it is.
Dominik can you try the attached patch and see if it solves the problem?
It it does, I'll do a correct [PATCH] post later with all the
signed-off-by and subject and stuff.
Please make sure you test with the same configuration that produces the
error, because this is a pretty hard to hit bug. The needed conditions are:
- 'nm' changes the order of 2 aliased symbols from the 1st to the 2nd pass
- one of those symbols get sampled for token optimization. With your
configuration the sampling was about 1 out of 12.
- the difference in the name of those symbols makes the algorithm
select different tokens. As 1024 symbols are used to produce the tokens,
changing just one of these symbols can easily go unnoticed.
- the difference in the tokens selected makes the size of the
compressed data change, so that it goes above (or below) an alignment
boundary. In your case it only changed 2 bytes in size, but it crossed a
4 byte alignment boundary.
With your .config file but a different set of tools (gcc, binutils
versions) I couldn't trigger the bug in my machine.
--
Paulo Marques - www.grupopie.com
All that is necessary for the triumph of evil is that good men do nothing.
Edmund Burke (1729 - 1797)
--- ./scripts/kallsyms.c.orig 2005-03-10 11:00:26.0 +
+++ ./scripts/kallsyms.c2005-03-10 11:11:50.0 +
@@ -499,11 +499,30 @@ static void forget_symbol(unsigned char
forget_token(symbol + i, len - i);
}
+/* sort the symbols by address->name so that even if aliased symbols
+ * change position, or the symbols are not supplied in address order
+ * the algorithm will work nevertheless */
+
+static int sort_by_address_name(const void *a, const void *b)
+{
+ struct sym_entry *sa, *sb;
+
+ sa = (struct sym_entry *) a;
+ sb = (struct sym_entry *) b;
+
+ if (sa->addr != sb->addr)
+ return sa->addr - sb->addr;
+
+ return strcmp(sa->sym + 1, sb->sym + 1);
+}
+
/* set all the symbol flags and do the initial token count */
static void build_initial_tok_table(void)
{
int i, use_it, valid;
+ qsort(table, cnt, sizeof(table[0]), sort_by_address_name);
+
valid = 0;
for (i = 0; i < cnt; i++) {
table[i].flags = 0;
Re: inconsistent kallsyms data [2.6.11-mm2]
Paulo Marques wrote: [...] Can you send me privately a tar.bz2 containing your .config, .tmp_kallsyms1.S and .tmp_kallsyms2.S so I can try to figure out what's going on? Ok, after some investigation into the files I was able to find out the problem. scripts/kallsyms.c uses a subset of the symbol table to optimize the tokens to use to compress the symbols. It does this because using the complete set of symbols would be much slower without a significant gain in compression. For some reason, in the files sent by Dominik, two aliased symbols change places from the first to the second step of the kallsyms build process (__sched_text_start, __down). Because of this, the subset used for optimization is different and so are the tokens selected, producing a 2 byte difference in the total size of the compressed symbol names :P So I must change the sampling algorithm in a way that is robust to symbol position changes. A simple and robust way is to do the sampling on a list of symbols sorted by symbol name. This way, even if the symbol positions that are given to scripts/kallsyms change, the symbols sampled will be the same. I'll do the patch to do this and send it ASAP. -- Paulo Marques - www.grupopie.com All that is necessary for the triumph of evil is that good men do nothing. Edmund Burke (1729 - 1797) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: inconsistent kallsyms data [2.6.11-mm2]
Dominik Brodowski wrote: On Tue, Mar 08, 2005 at 12:35:54PM -0800, Andrew Morton wrote: Dominik Brodowski <[EMAIL PROTECTED]> wrote: compiling -mm2 on my x86 box results in: SYSMAP .tmp_System.map Inconsistent kallsyms data Try setting CONFIG_KALLSYMS_EXTRA_PASS make: *** [vmlinux] Fehler 1 gcc-Version 3.4.3 20050110 (Gentoo Linux 3.4.3.20050110, ssp-3.4.3.20050110-0, pie-8.7.7) Did CONFIG_KALLSYMS_EXTRA_PASS fix it up? Yes. It doesn't happen to me here :( Can you send me privately a tar.bz2 containing your .config, .tmp_kallsyms1.S and .tmp_kallsyms2.S so I can try to figure out what's going on? TIA, -- Paulo Marques - www.grupopie.com All that is necessary for the triumph of evil is that good men do nothing. Edmund Burke (1729 - 1797) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] new driver for ITM Touch touchscreen
Hans-Christian Egtvedt wrote:
[...]
Any tips are welcome. Is this done before with a touchscreen?
Just a minor nitpick, not really related to the mouse problem. More of
coding style problem.
IMHO the UCP and UCOM macros just obfuscate the code. If you do not want
to write "((unsigned char *) urb->transfer_buffer)[0]" every time (I can
perfectly understand that), maybe using a local "u8 *" var would do the
trick.
Something like this:
static void itmtouch_irq(struct urb *urb, struct pt_regs *regs)
{
struct itmtouch_dev * itmtouch = urb->context;
int retval;
u8 *tbuf;
input_regs(&itmtouch->inputdev, regs);
tbuf = (u8 *)(urb->transfer_buffer);
/* if pressure has been released, then don't report X/Y */
if (!(tbuf[7] & 0x20)) {
input_report_abs(&itmtouch->inputdev, ABS_X,
(tbuf[0] & 0x1F) << 7 | (tbuf[3] & 0x7F));
input_report_abs(&itmtouch->inputdev, ABS_Y,
(tbuf[1] & 0x1F) << 7 | (tbuf[4] & 0x7F));
}
input_report_abs(&itmtouch->inputdev, ABS_PRESSURE,
(tbuf[2] & 0x1) << 7 | (tbuf[5] & 0x7F));
input_report_key(&itmtouch->inputdev, BTN_TOUCH, !(tbuf[7] & 0x20));
/* TODO: Do we need to use input_sync() ? */
/* input_sync(&itmtouch->inputdev); */
..
This is perfectly readable without one having to find out what those
macros mean, and it is even easier for the compiler to optimize (even
though gcc will probably optimize both versions just fine).
--
Paulo Marques - www.grupopie.com
All that is necessary for the triumph of evil is that good men do nothing.
Edmund Burke (1729 - 1797)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: sizeof(ptr) or sizeof(*ptr)?
Andrew Morton wrote:
"" <[EMAIL PROTECTED]> wrote:
Anyway, after improving the tool and checking for false positives, there is only
one more suspicious piece of code in drivers/acpi/video.c:561
status = acpi_video_device_lcd_query_levels(device, &obj);
if (obj && obj->type == ACPI_TYPE_PACKAGE && obj->package.count >= 2) {
int count = 0;
union acpi_object *o;
br = kmalloc(sizeof &br, GFP_KERNEL);
yup, bug.
if (!br) {
printk(KERN_ERR "can't allocate memory\n");
} else {
memset(br, 0, sizeof &br);
br->levels = kmalloc(obj->package.count * sizeof
&br->levels, GFP_KERNEL);
And another one, although it happens to work out OK.
I'll get these all fixed up, thanks.
I just checked the 2.6.11-mm1 release and this is only half-fixed there,
and it is the worst of both halves: the kmalloc only mallocs the size of
a pointer, but the memset is fixed, so it memset's the size of a
structure (oops). This is partially my fault for not sending the patch
in the first place, together with the bug report.
The attached patch against 2.6.11-mm1 should fix the kmalloc.
By the way, I haven't got any response from an alsa developer about the
bug in sound/core/control.c, but this is already fixed in 2.6.11-mm1,
along with several other changes to that file. So the status is: it was
a bug, but it is already fixed :)
--
Paulo Marques - www.grupopie.com
All that is necessary for the triumph of evil is that good men do nothing.
Edmund Burke (1729 - 1797)
--- ./drivers/acpi/video.c.orig 2005-03-08 13:07:42.0 +
+++ ./drivers/acpi/video.c 2005-03-08 13:09:05.0 +
@@ -564,11 +564,11 @@ acpi_video_device_find_cap (struct acpi_
int count = 0;
union acpi_object *o;
- br = kmalloc(sizeof &br, GFP_KERNEL);
+ br = kmalloc(sizeof(*br), GFP_KERNEL);
if (!br) {
printk(KERN_ERR "can't allocate memory\n");
} else {
- memset(br, 0, sizeof *br);
+ memset(br, 0, sizeof(*br));
br->levels = kmalloc(obj->package.count *
sizeof *(br->levels), GFP_KERNEL);
if (!br->levels)
Re: RFD: Kernel release numbering
Linus Torvalds wrote: [...] Ie I'd organize it like some of the "checkin committees" work for other projects that have nowhere _near_ as much work going on as Linux has. That seems to work well for small projects - and we can try to keep this "small" exactly by having the strict rules in place that would mean that 99% of all patches wouldn't even be a consideration. Maybe setting up a mailing list where all the patches have to be posted before inclusion in this tree, would help the maintainer(s) a lot. Since we expect little traffic (at least compared to LKML) a lot of developers (even "small" developers like myself) can review all the patches for correctness, and throw quite a few eyes on them. The more eyes, the less a chance for bugs to slip by. Just a thought, -- Paulo Marques - www.grupopie.com All that is necessary for the triumph of evil is that good men do nothing. Edmund Burke (1729 - 1797) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Module Vs app?
BZ Benny wrote: Hi, I have an app wich run in the user space. I want to create a virtual network device for catching data under the IP layer and sending data to IP layer. I want to creaate such a second eth0. I know how we can do this with /linux/netdevice.h but how can we do that from the user space? I think you want a TUN device. Read Documentation/networking/tuntap.txt -- Paulo Marques - www.grupopie.com All that is necessary for the triumph of evil is that good men do nothing. Edmund Burke (1729 - 1797) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Writng daemon and wake up on demand.
Payasam Manohar wrote: I have two doubts, 1) Can we design a linux daemon which will call some shell scripts. 2) How to call this daemon from the keyboard driver and how to kill it on demand. I don't want to be impolite, but you're not following the mailing list etiquette *at all*. In the last five days you sent 15 messages to the list, and you still haven't been able to tell what you're trying to accomplish. So I can only recommend a few urls: http://www.tux.org/lkml/ http://www.kernelnewbies.org/ http://linuxconsole.sourceforge.net/input/input.html -- Paulo Marques - www.grupopie.com All that is necessary for the triumph of evil is that good men do nothing. Edmund Burke (1729 - 1797) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Potentially dead bttv cards from 2.6.10
James Bruce wrote: [...] The card= option didn't help in my case since my card is not in the list; For thess cards we went off the reccomendation of other people doing machine vision in Linux; Next time I guess we'll go name brand again... I think you should try it anyway, using all the options, because it is very likely that your card might be compatible with one of the listed ones. This is specially true if you don't care about the tuner. Just modprobe the bttv module with card=X option, test it, rmmod it, modprobe it again with card=X+1, etc., until you find a number that fits. -- Paulo Marques - www.grupopie.com All that is necessary for the triumph of evil is that good men do nothing. Edmund Burke (1729 - 1797) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: ARM undefined symbols. Again.
Sam Ravnborg wrote: On Fri, Feb 25, 2005 at 08:54:56PM +, Paulo Marques wrote: The patch (against 2.6.11-rc5) is attached, should you decide to use it. How does the patch help rmk with respect to the tools issue? From the thread I gathered that the problem Russell King was having was caused by the fact that kallsyms used "weak" symbols. He proposed a solution where he created an empty assembly file with just the symbols defined to make the symbols exist already on the first pass. This way they wouldn't have to be defined as weak anymore. His patch created the assembly file using "echo's" from the Makefile. I just suggested that it would be better to do it from scripts/kallsyms.c directly, so that it would be easier to maintain in case new symbols need to be added in the future. That's what this patch does. By the way, I'm not really sure about my changes to the Makefile, although they comply with Linus Confidence Level 3(*). I think you're the one with the best understanding on the kbuild process to comment on them. -- Paulo Marques - www.grupopie.com All that is necessary for the triumph of evil is that good men do nothing. Edmund Burke (1729 - 1797) (*) It works - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: ARM undefined symbols. Again.
Linus Torvalds wrote:
[...]
That makes no sense. Or, more likely, it means that the toolchain people
are incompetent bastards who don't care about bugs and have no pride at
all in what they do.
Errmm... I really feel pretty small coming in on a Russell King / Linus
Torvalds discussion, but I was the one who promised the patch and I just
wanted to keep my promises.
The patch (against 2.6.11-rc5) is attached, should you decide to use it.
IMHO it makes the kallsyms code look nicer, by getting rid of the
__attribute__((weak)) statements int kernel/kallsyms.c code.
Me getting out of here now....
--
Paulo Marques - www.grupopie.com
All that is necessary for the triumph of evil is that good men do nothing.
Edmund Burke (1729 - 1797)
diff -uprN -X dontdiff linux-2.6.11-rc5-vanilla/kernel/kallsyms.c
linux-2.6.11-rc5/kernel/kallsyms.c
--- linux-2.6.11-rc5-vanilla/kernel/kallsyms.c 2005-02-25 20:36:44.0
+
+++ linux-2.6.11-rc5/kernel/kallsyms.c 2005-02-25 20:15:19.0 +
@@ -29,14 +29,14 @@
#endif
/* These will be re-linked against their real values during the second link
stage */
-extern unsigned long kallsyms_addresses[] __attribute__((weak));
-extern unsigned long kallsyms_num_syms __attribute__((weak,section("data")));
-extern u8 kallsyms_names[] __attribute__((weak));
+extern unsigned long kallsyms_addresses[];
+extern unsigned long kallsyms_num_syms;
+extern u8 kallsyms_names[];
-extern u8 kallsyms_token_table[] __attribute__((weak));
-extern u16 kallsyms_token_index[] __attribute__((weak));
+extern u8 kallsyms_token_table[];
+extern u16 kallsyms_token_index[];
-extern unsigned long kallsyms_markers[] __attribute__((weak));
+extern unsigned long kallsyms_markers[];
static inline int is_kernel_inittext(unsigned long addr)
{
diff -uprN -X dontdiff linux-2.6.11-rc5-vanilla/Makefile
linux-2.6.11-rc5/Makefile
--- linux-2.6.11-rc5-vanilla/Makefile 2005-02-25 20:36:15.0 +
+++ linux-2.6.11-rc5/Makefile 2005-02-25 20:25:44.0 +
@@ -702,14 +702,20 @@ quiet_cmd_kallsyms = KSYM$@
cmd_kallsyms = $(NM) -n $< | $(KALLSYMS) \
$(if $(CONFIG_KALLSYMS_ALL),--all-symbols) > $@
-.tmp_kallsyms1.o .tmp_kallsyms2.o .tmp_kallsyms3.o: %.o: %.S scripts FORCE
+quiet_cmd_kallsyms0 = KSYM$@
+ cmd_kallsyms0 = $(KALLSYMS) -0 > $@
+
+.tmp_kallsyms0.o .tmp_kallsyms1.o .tmp_kallsyms2.o .tmp_kallsyms3.o: %.o: %.S
scripts FORCE
$(call if_changed_dep,as_o_S)
+.tmp_kallsyms0.S: $(KALLSYMS) FORCE
+ $(call cmd,kallsyms0)
+
.tmp_kallsyms%.S: .tmp_vmlinux% $(KALLSYMS)
$(call cmd,kallsyms)
# .tmp_vmlinux1 must be complete except kallsyms, so update vmlinux version
-.tmp_vmlinux1: $(vmlinux-lds) $(vmlinux-all) FORCE
+.tmp_vmlinux1: $(vmlinux-lds) $(vmlinux-all) .tmp_kallsyms0.o FORCE
$(call if_changed_rule,ksym_ld)
.tmp_vmlinux2: $(vmlinux-lds) $(vmlinux-all) .tmp_kallsyms1.o FORCE
diff -uprN -X dontdiff linux-2.6.11-rc5-vanilla/scripts/kallsyms.c
linux-2.6.11-rc5/scripts/kallsyms.c
--- linux-2.6.11-rc5-vanilla/scripts/kallsyms.c 2005-02-25 20:36:45.0
+
+++ linux-2.6.11-rc5/scripts/kallsyms.c 2005-02-25 20:33:25.0 +
@@ -93,7 +93,7 @@ unsigned char best_table_len[256];
static void
usage(void)
{
- fprintf(stderr, "Usage: kallsyms [--all-symbols] < in.map > out.S\n");
+ fprintf(stderr, "Usage: kallsyms [--all-symbols] [-0] < in.map >
out.S\n");
exit(1);
}
@@ -230,6 +230,20 @@ static void output_label(char *label)
printf("%s:\n",label);
}
+static void output_header(void)
+{
+ printf("#include \n");
+ printf("#if BITS_PER_LONG == 64\n");
+ printf("#define PTR .quad\n");
+ printf("#define ALGN .align 8\n");
+ printf("#else\n");
+ printf("#define PTR .long\n");
+ printf("#define ALGN .align 4\n");
+ printf("#endif\n");
+
+ printf(".data\n");
+}
+
/* uncompress a compressed symbol. When this function is called, the best table
* might still be compressed itself, so the function needs to be recursive */
static int expand_symbol(unsigned char *data, int len, char *result)
@@ -257,6 +271,25 @@ static int expand_symbol(unsigned char *
return total;
}
+/* this function writes an empty assembly output with just the definitions
+ * of the variables */
+
+static void write_src_zero_pass(void)
+{
+ output_header();
+
+ output_label("kallsyms_addresses");
+ output_label("kallsyms_num_syms");
+ output_label("kallsyms_names");
+ output_label("kallsyms_markers");
+ output_label("kallsyms_token_table");
+ output_label("kallsyms_token_index");
+
+ printf("\t.byte\t0\n");
+}
+
+/* this one writes the real deal */
+
static voi
Re: Xterm Hangs - Possible scheduler defect?
Ingo Oeser wrote: Chris Friesen wrote: Ingo Oeser wrote: [...] You would need to change the priority of task 1 until it releases the mutex. Ideally the owner gets the maximum priority of his and all the waiters on it, until it releases his mutex, where he regains its old priority after release of mutex. But this priority elevation happens only, if he is runnable. If not, he gets his old priority back, until he is runnable. This is called a "priority inversion" problem, and there was some work done by Ingo Molnar to make the scheduler aware of such cases and handle them appropriatelly. You can follow this thread for more info: http://marc.theaimsgroup.com/?l=linux-kernel&m=110106915415886&w=2 I really don't know what's the current state, but this is nothing new... -- Paulo Marques - www.grupopie.com All that is necessary for the triumph of evil is that good men do nothing. Edmund Burke (1729 - 1797) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Inconsistent kallsyms data (since 2.6.11-rc3 or so)
Geert Uytterhoeven wrote: One of my m68k configs has been giving | Inconsistent kallsyms data | Try setting CONFIG_KALLSYMS_EXTRA_PASS since 2.6.11-rc3 or so. Setting CONFIG_KALLSYMS_EXTRA_PASS, or applying Keith Owen's patch to fix an issue for SH (http://seclists.org/lists/linux-kernel/2005/Jan/0017.html) doesn't help. The diffs between the human-readable tables (as generated by Keith's kallsyms_uncompress.pl) show lots of changes (see below). There is something weird going on here. For starters all the symbols that move are of type either '?'(unknown type) or 'b' (local bss). From the first to the second run a few more symbols pop up, and that moves symbols around. From a quick visual inspection I spotted these: +b log_startPTR 0x1b72c8 +b con_startPTR 0x1b72cc +b log_end PTR 0x1b72d0 There might be a few more, but these would be enough to give problems. Although marked as 'b' type, their addresses are between _sinittext and _einittext. These are actualy "local bss", static vars defined in printk.c. So the question is: why don't they appear on the first link phase on m68k? -- Paulo Marques - www.grupopie.com All that is necessary for the triumph of evil is that good men do nothing. Edmund Burke (1729 - 1797) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Xterm Hangs - Possible scheduler defect?
Chad N. Tindel wrote: Low-latency userspace apps. The audio guys, for instance, are trying to get latencies down to the 100us range. If random kernel threads can preempt userspace at any time, they wreak havoc with latency as seen by userspace. Come now. There is no such thing as a random kernel thread. Any General Purpose kernel needs the ability to do work that keeps the entire system from grinding to a halt. FYI most kernel threads do background work, that doesn't have hard real-time constraints. Why should my audio recording session get interrupted (read: "sent to the trashcan") just because the swap daemon decided that it was a good time to write some pages out? Couldn't it have waited just a few more milliseconds? You don't seem to realize that you have just arrived to this mailing list and missed years of discussions on kernel architecture. If you keep a learning attitude, there is a chance for this discussion to go on. However, if you keep the "Come now, don't bullshit me, this is a broken architecture and you're just trying to cover up" attitude, you're just going to get discarded as a troll. I personally like the linux way: "root has the ability to shoot himself in the foot if he wants to". This is my computer, damn it, I am the one who tells it what to do. This is much, much better than the "users are stupid, we must protect them from themselves" kind of way that other OS'es use. Just my 0.02 euros, -- Paulo Marques - www.grupopie.com All that is necessary for the triumph of evil is that good men do nothing. Edmund Burke (1729 - 1797) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: OT: Why is usb data many times the cpu hog that firewire is?
Matt Mackall wrote: [...] JPEG data is DCT of 8x8 pixel chunks. If you can get at that, you can compare the DC terms of each chunk with minimal decoding. Various thumbnailers do this for speed already. I really doubt that this would work. It seems to me that you can have very different DC terms with very similar results. In other words, even a little noise in the picture might produce very different DC terms. Instead of comparing the DC terms, you could compare just the luminance. You would have to decompress just half the data for that and you wouldn't need to make the YUV->RGB conversion. That would probably save a few cycles. -- Paulo Marques - www.grupopie.com All that is necessary for the triumph of evil is that good men do nothing. Edmund Burke (1729 - 1797) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: OT: Why is usb data many times the cpu hog that firewire is?
Gene Heskett wrote: On Monday 21 February 2005 12:58, Oliver Neukum wrote: [...] A video stream over usb1.1 must be compressed due to bandwidth available. Decompression needs cpu. Thats what I was afraid of, which makes using it for a motion detected burgular alarm source considerably less than practical since the machine must be able to do other things too. Darn. And its usb1.1 even when plugged into a 2.0 capable port. Depending on the camera model you can try some bandwidth reduction measures to try to make it send uncompressed video: - reduce frame rate. Something as low as 2 fps should be enough for motion detection. - reduce requested resolution. This of course depends on whether you have enough resolution or not. - selecting gray scale images. I don't know if your motion detection software is greatly affected by this, or not. USB1.1 bandwidth is enought for 640x480, 8 bits gray scale (or color, 8 bits bayer pattern), at 3 fps. Of course, you can always buy a USB2.0 camera :) -- Paulo Marques - www.grupopie.com All that is necessary for the triumph of evil is that good men do nothing. Edmund Burke (1729 - 1797) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [TTY] 2 points seems strange to me.
linux-os wrote: On Fri, 18 Feb 2005, Paul Fulghum wrote: Paulo Marques wrote: Paul Fulghum wrote: No, it limits the size to 80 bytes, which is the size of buf. sizeof returns the size of the char array buf[80] (standard C) Looking at the code, I think Franck is right. buf is a "const unsigned char *" for which sizeof(buf) is the size of a pointer. What kernel version are you looking at? I'm looking at 2.4.20 n_tty.c opost_block() and buf is a char array. -- Paul Fulghum Microgate Systems, Ltd. Ahaa! That's how the bug got introduced. It used to be an array and then it got changed to a pointer! linux-2.4.26 also shows a local array. Yes, just looked at the revision history in linux.bkbits.net and Linus just fixed this 67 hours ago... So we're too late :) -- Paulo Marques - www.grupopie.com All that is necessary for the triumph of evil is that good men do nothing. Edmund Burke (1729 - 1797) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
