scheduler fix
in [Kernel]
|
Prev: [GIT PULL IBFT] iBFT update for 2.6.34-rc3
Next: ibft, x86: Change reserve_ibft_region() to find_ibft_region()
From: Andreas Schwab on 8 Apr 2010 12:10 Linus Torvalds <torvalds(a)linux-foundation.org> writes: > On Thu, 8 Apr 2010, Ingo Molnar wrote: >> >> - if (len < nr_cpu_ids) >> + if ((len * BITS_PER_BYTE) < nr_cpu_ids) >> return -EINVAL; > > Not that it really matters, but this will now fail for no good reason if > you pass it a half-gigabyte area due to overflow. Which can easily be avoided. if (len < DIV_ROUND_UP(nr_cpu_ids, BITS_PER_BYTE)) Andreas. -- Andreas Schwab, schwab(a)redhat.com GPG Key fingerprint = D4E8 DBE3 3813 BB5D FA84 5EC7 45C6 250E 6F00 984E "And now for something completely different." -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Ingo Molnar on 8 Apr 2010 14:30 * Andreas Schwab <schwab(a)redhat.com> wrote: > Linus Torvalds <torvalds(a)linux-foundation.org> writes: > > > On Thu, 8 Apr 2010, Ingo Molnar wrote: > >> > >> - if (len < nr_cpu_ids) > >> + if ((len * BITS_PER_BYTE) < nr_cpu_ids) > >> return -EINVAL; > > > > Not that it really matters, but this will now fail for no good reason if > > you pass it a half-gigabyte area due to overflow. > > Which can easily be avoided. > > if (len < DIV_ROUND_UP(nr_cpu_ids, BITS_PER_BYTE)) nr_cpu_ids is a signed integer which turns the DIV_ROUND_UP into a somewhat suboptimal instruction sequence. (havent checked it though) So i'd suggest changing nr_cpu_ids to unsigned int [unless there's some strong reason to have it signed] plus doing something like: if (len < (nr_cpu_ids >> BITS_PER_BYTE_BITS)) ought to both result in better code and should be more readable. We'd have to add: #define BITS_PER_BYTE_BITS 3 to linux/bitops.h. Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Linus Torvalds on 8 Apr 2010 14:50 On Thu, 8 Apr 2010, Ingo Molnar wrote: > > So i'd suggest changing nr_cpu_ids to unsigned int [unless there's some strong > reason to have it signed] plus doing something like: > > if (len < (nr_cpu_ids >> BITS_PER_BYTE_BITS)) No workee. It really should round up. If you worry about code generation, I'd suggest looking at whether nr_cpu_ids could just be made unsigned. Anyway, this all was _not_ the point of my original email. I really don't care about this particular instance. I care more about the whole "in general people should think _way_ more about validating user-supplied arguments than clearly happened this time". Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Ingo Molnar on 8 Apr 2010 15:00
* Linus Torvalds <torvalds(a)linux-foundation.org> wrote: > > > On Thu, 8 Apr 2010, Ingo Molnar wrote: > > > > So i'd suggest changing nr_cpu_ids to unsigned int [unless there's some strong > > reason to have it signed] plus doing something like: > > > > if (len < (nr_cpu_ids >> BITS_PER_BYTE_BITS)) > > No workee. > > It really should round up. Indeed. > If you worry about code generation, I'd suggest looking at whether > nr_cpu_ids could just be made unsigned. > > Anyway, this all was _not_ the point of my original email. I really don't > care about this particular instance. I care more about the whole "in general > people should think _way_ more about validating user-supplied arguments than > clearly happened this time". Yeah, no argument about that, point taken and accepted. Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |