* Re: [PATCH] powerpc: fix large nvram access [not found] <200601091916.k09JGMJY004116@hera.kernel.org> @ 2006-01-10 22:13 ` Olaf Hering 2006-01-10 23:22 ` Andreas Schwab 2006-01-10 23:34 ` Arnd Bergmann 0 siblings, 2 replies; 5+ messages in thread From: Olaf Hering @ 2006-01-10 22:13 UTC (permalink / raw) To: linuxppc-dev, arndb On Mon, Jan 09, Linux Kernel Mailing List wrote: > tree da8f883f72d08f9534e9eca3bba662413b9bd865 > parent d52771fce4e774fa786097d34412a057d487c697 > author Arnd Bergmann <arnd@arndb.de> Fri, 09 Dec 2005 19:21:44 +0100 > committer Paul Mackerras <paulus@samba.org> Mon, 09 Jan 2006 14:53:31 +1100 > > [PATCH] powerpc: fix large nvram access > > /dev/nvram uses the user-provided read/write size > for kmalloc, which fails, if a large number is passed. > This will always use a single page at most, which > can be expected to succeed. > > Signed-off-by: Arnd Bergmann <arndb@de.ibm.com> > Signed-off-by: Paul Mackerras <paulus@samba.org> > > arch/powerpc/kernel/nvram_64.c | 114 +++++++++++++++++++---------------------- this change breaks my nvsetenv binary on JS20. (none):/# mount proc ; nvsetenv Error reading /dev/nvram: Success (none):/# strace -f nvsetenv execve("/sbin/nvsetenv", ["nvsetenv"], [/* 61 vars */]) = 0 brk(0) = 0x10014000 mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xf7fe0000 access("/etc/ld.so.preload", R_OK) = -1 ENOENT (No such file or directory) open("/etc/ld.so.cache", O_RDONLY) = 3 fstat64(3, {st_mode=S_IFREG|0644, st_size=171714, ...}) = 0 mmap(NULL, 171714, PROT_READ, MAP_PRIVATE, 3, 0) = 0xf7fb6000 close(3) = 0 open("/lib/libc.so.6", O_RDONLY) = 3 read(3, "\177ELF\1\2\1\0\0\0\0\0\0\0\0\0\0\3\0\24\0\0\0\1\0\1\325"..., 512) = 512 fstat64(3, {st_mode=S_IFREG|0755, st_size=1569515, ...}) = 0 mmap(0xfe99000, 1401836, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0xfe99000 madvise(0xfe99000, 1401836, MADV_SEQUENTIAL|0x1) = 0 mprotect(0xffd8000, 65536, PROT_NONE) = 0 mmap(0xffe8000, 20480, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x13f000) = 0xffe8000 mmap(0xffed000, 9196, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0xffed000 close(3) = 0 mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xf7fb5000 mprotect(0xffe8000, 4096, PROT_READ) = 0 munmap(0xf7fb6000, 171714) = 0 brk(0) = 0x10014000 brk(0x10035000) = 0x10035000 open("/proc/device-tree/compatible", O_RDONLY) = 3 fstat64(3, {st_mode=S_IFREG|0444, st_size=13, ...}) = 0 mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xf7fdf000 read(3, "IBM,1234-123\0", 1024) = 13 read(3, "", 1024) = 0 close(3) = 0 munmap(0xf7fdf000, 4096) = 0 open("/dev/nvram", O_RDONLY) = 3 read(3, "P\347\0\34of-config\0\0\0", 16) = 16 lseek(3, 432, SEEK_CUR) = 448 read(3, "p\375\2\0common\0\0\0\0\0\0", 16) = 16 read(3, "ibm,fw-phandle-enable?=false\0lit"..., 8176) = 4096 dup(2) = 4 fcntl64(4, F_GETFL) = 0x2 (flags O_RDWR) fstat64(4, {st_mode=S_IFCHR|0600, st_rdev=makedev(5, 1), ...}) = 0 ioctl(4, TCGETS, {B38400 opost isig icanon echo ...}) = 0 mmap(NULL, 131072, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xf7fc0000 _llseek(4, 0, 0xffd63fd8, SEEK_CUR) = -1 ESPIPE (Illegal seek) write(4, "Error reading /dev/nvram: Succes"..., 34Error reading /dev/nvram: Success ) = 34 close(4) = 0 munmap(0xf7fc0000, 131072) = 0 exit_group(1) = ? (none):/# -- short story of a lazy sysadmin: alias appserv=wotan ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] powerpc: fix large nvram access 2006-01-10 22:13 ` [PATCH] powerpc: fix large nvram access Olaf Hering @ 2006-01-10 23:22 ` Andreas Schwab 2006-01-10 23:34 ` Arnd Bergmann 1 sibling, 0 replies; 5+ messages in thread From: Andreas Schwab @ 2006-01-10 23:22 UTC (permalink / raw) To: Olaf Hering; +Cc: linuxppc-dev, arndb Olaf Hering <olh@suse.de> writes: > this change breaks my nvsetenv binary on JS20. > > (none):/# mount proc ; nvsetenv > Error reading /dev/nvram: Success It was broken to begin with. Andreas. -- Andreas Schwab, SuSE Labs, schwab@suse.de SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany PGP key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] powerpc: fix large nvram access 2006-01-10 22:13 ` [PATCH] powerpc: fix large nvram access Olaf Hering 2006-01-10 23:22 ` Andreas Schwab @ 2006-01-10 23:34 ` Arnd Bergmann 2006-01-11 10:25 ` Olaf Hering 1 sibling, 1 reply; 5+ messages in thread From: Arnd Bergmann @ 2006-01-10 23:34 UTC (permalink / raw) To: linuxppc-dev; +Cc: arndb, Olaf Hering Am Dienstag, 10. Januar 2006 23:13 schrieb Olaf Hering: > On Mon, Jan 09, Linux Kernel Mailing List wrote: > > tree da8f883f72d08f9534e9eca3bba662413b9bd865 > > parent d52771fce4e774fa786097d34412a057d487c697 > > author Arnd Bergmann <arnd@arndb.de> Fri, 09 Dec 2005 19:21:44 +0100 > > committer Paul Mackerras <paulus@samba.org> Mon, 09 Jan 2006 14:53:31 > > +1100 > > > > [PATCH] powerpc: fix large nvram access > > > > /dev/nvram uses the user-provided read/write size > > for kmalloc, which fails, if a large number is passed. > > This will always use a single page at most, which > > can be expected to succeed. > > > > Signed-off-by: Arnd Bergmann <arndb@de.ibm.com> > > Signed-off-by: Paul Mackerras <paulus@samba.org> > > > > arch/powerpc/kernel/nvram_64.c | 114 > > +++++++++++++++++++---------------------- > > this change breaks my nvsetenv binary on JS20. > I think this has uncovered a bug that has been in nvsetenv for a long time. > open("/dev/nvram", O_RDONLY) = 3 > read(3, "P\347\0\34of-config\0\0\0", 16) = 16 > lseek(3, 432, SEEK_CUR) = 448 > read(3, "p\375\2\0common\0\0\0\0\0\0", 16) = 16 > read(3, "ibm,fw-phandle-enable?=false\0lit"..., 8176) = 4096 This is where my patch changed the behavior of the driver. It used to return all the data in one chunk, now it returns 4096 bytes at most. > dup(2) = 4 > fcntl64(4, F_GETFL) = 0x2 (flags O_RDWR) > fstat64(4, {st_mode=S_IFCHR|0600, st_rdev=makedev(5, 1), ...}) = 0 > ioctl(4, TCGETS, {B38400 opost isig icanon echo ...}) = 0 > mmap(NULL, 131072, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xf7fc0000 > _llseek(4, 0, 0xffd63fd8, SEEK_CUR) = -1 ESPIPE > (Illegal seek) > write(4, "Error reading /dev/nvram: Succes"..., 34Error reading /dev/nvram: Success ) = 34 > close(4) = 0 > munmap(0xf7fc0000, 131072) = 0 > exit_group(1) = ? > (none):/# And then it got here: | if (lseek(nvfd, NVSTART, 0) < 0 | || read(nvfd, &nvbuf, NVSIZE) != NVSIZE) { | perror("Error reading /dev/nvram"); | exit(EXIT_FAILURE); | } I'm pretty sure that the failing _llseek is part of the perror implementation and not the real problem, as we were first thinking. The problem is that nvsetenv bails out on a short read, while according to the man page, "It is not an error if this number is smaller than the number of bytes requested". Of course, character devices often differ in their behavior from what is in the man pages for file I/O. I'd suggest we change both ends: nvsetenv should really be able to deal with a short read, and we can double the size of the kmalloc buffer for the kernel implementation. Please test the patch below. --- Subject: powerpc: fix nvsetenv regression with fixed nvram read/write reading from /dev/nvram was recently fixed to not try to allocate the user-defined amount of kernel memory. This broke the nvsetenv tool which relied on never getting short reads from /dev/nvram. Signed-off-by: Arnd Bergmann <arnd@arndb.de> diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c index fd7db8d..8da6e57 100644 --- a/arch/powerpc/kernel/nvram_64.c +++ b/arch/powerpc/kernel/nvram_64.c @@ -34,6 +34,15 @@ #undef DEBUG_NVRAM +/* + * Old nvsetenv versions expect us to be able to return 8kb + * of data in a single read, so use that as a limit for short + * reads. + * We can't simply use the requested size because large + * kmalloc allocations often fail. + */ +#define NVRAM_ALLOC_SIZE 8192 + static int nvram_scan_partitions(void); static int nvram_setup_partition(void); static int nvram_create_os_partition(void); @@ -94,7 +103,7 @@ static ssize_t dev_nvram_read(struct fil goto out; count = min_t(size_t, count, size - *ppos); - count = min(count, PAGE_SIZE); + count = min(count, NVRAM_ALLOC_SIZE); ret = -ENOMEM; tmp = kmalloc(count, GFP_KERNEL); @@ -131,7 +140,7 @@ static ssize_t dev_nvram_write(struct fi goto out; count = min_t(size_t, count, size - *ppos); - count = min(count, PAGE_SIZE); + count = min(count, NVRAM_ALLOC_SIZE); ret = -ENOMEM; tmp = kmalloc(count, GFP_KERNEL); ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] powerpc: fix large nvram access 2006-01-10 23:34 ` Arnd Bergmann @ 2006-01-11 10:25 ` Olaf Hering 2006-01-11 12:20 ` Olaf Hering 0 siblings, 1 reply; 5+ messages in thread From: Olaf Hering @ 2006-01-11 10:25 UTC (permalink / raw) To: Arnd Bergmann; +Cc: linuxppc-dev On Wed, Jan 11, Arnd Bergmann wrote: > Am Dienstag, 10. Januar 2006 23:13 schrieb Olaf Hering: > > On Mon, Jan 09, Linux Kernel Mailing List wrote: > > > tree da8f883f72d08f9534e9eca3bba662413b9bd865 > > > parent d52771fce4e774fa786097d34412a057d487c697 > > > author Arnd Bergmann <arnd@arndb.de> Fri, 09 Dec 2005 19:21:44 +0100 > > > committer Paul Mackerras <paulus@samba.org> Mon, 09 Jan 2006 14:53:31 > > > +1100 > > > > > > [PATCH] powerpc: fix large nvram access > > > > > > /dev/nvram uses the user-provided read/write size > > > for kmalloc, which fails, if a large number is passed. > > > This will always use a single page at most, which > > > can be expected to succeed. > > > > > > Signed-off-by: Arnd Bergmann <arndb@de.ibm.com> > > > Signed-off-by: Paul Mackerras <paulus@samba.org> > > > > > > arch/powerpc/kernel/nvram_64.c | 114 > > > +++++++++++++++++++---------------------- > > > > this change breaks my nvsetenv binary on JS20. > > > > I think this has uncovered a bug that has been in nvsetenv for a long time. Yeah. Dont bother, I will fix that tool. -- short story of a lazy sysadmin: alias appserv=wotan ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] powerpc: fix large nvram access 2006-01-11 10:25 ` Olaf Hering @ 2006-01-11 12:20 ` Olaf Hering 0 siblings, 0 replies; 5+ messages in thread From: Olaf Hering @ 2006-01-11 12:20 UTC (permalink / raw) To: Arnd Bergmann; +Cc: linuxppc-dev On Wed, Jan 11, Olaf Hering wrote: > > I think this has uncovered a bug that has been in nvsetenv for a long time. I made this change. Index: pmac-utils-2.1/nvsetenv.c =================================================================== --- pmac-utils-2.1.orig/nvsetenv.c +++ pmac-utils-2.1/nvsetenv.c @@ -110,12 +110,21 @@ int nvcsum( void ) static void nvload( int nvfd ) { int s; + ssize_t r, toread = 0; + char *buf = (char *)&nvbuf; - if (lseek(nvfd, NVSTART, 0) < 0 - || read(nvfd, &nvbuf, NVSIZE) != NVSIZE) { - perror("Error reading /dev/nvram"); + if ((off_t)-1 == lseek(nvfd, NVSTART, 0)) { + perror("Error seeking /dev/nvram"); exit(EXIT_FAILURE); } + do { + r = read(nvfd, buf + toread, NVSIZE - toread); + if (r < 0) { + perror("Error reading /dev/nvram"); + exit(EXIT_FAILURE); + } + toread += r; + } while (toread < NVSIZE && r); if (nvbuf.nv.magic != NVMAGIC) (void) fprintf(stderr, "Warning: Bad magic number %x\n", nvbuf.nv.magic); @@ -127,11 +136,21 @@ static void nvload( int nvfd ) static void nvstore(int nvfd) { - if (lseek(nvfd, NVSTART, 0) < 0 - || write(nvfd, &nvbuf, NVSIZE) != NVSIZE) { - perror("Error writing /dev/nvram"); + char *p = (char *)&nvbuf; + size_t count = 0; + off_t w; + if (lseek(nvfd, NVSTART, 0) < 0) { + perror("Error seeking /dev/nvram"); exit(EXIT_FAILURE); } + do { + w = write(nvfd, p + count, NVSIZE - count); + if (w < 0) { + perror("Error writing /dev/nvram"); + exit(EXIT_FAILURE); + } + count += w; + } while (count < NVSIZE); } void nvunpack( void ) Index: pmac-utils-2.1/nwnvsetenv.c =================================================================== --- pmac-utils-2.1.orig/nwnvsetenv.c +++ pmac-utils-2.1/nwnvsetenv.c @@ -88,15 +88,20 @@ int nvscan(int nvfd, chrp_header* chrph, static char* nvload( int nvfd , int nvsize) { char* nvbuf = malloc(nvsize); + ssize_t r, toread = 0; if (!nvbuf) { perror("Error allocating buffer"); exit(EXIT_FAILURE); } - if (read(nvfd, nvbuf, nvsize) != nvsize) { - perror("Error reading /dev/nvram"); - exit(EXIT_FAILURE); - } + do { + r = read(nvfd, nvbuf + toread, nvsize - toread); + if (r < 0) { + perror("Error reading /dev/nvram"); + exit(EXIT_FAILURE); + } + toread += r; + } while (toread < nvsize && r); return nvbuf; } @@ -182,6 +187,7 @@ print_var(char* nvbuf, int nvsize, char* printf("%s\n",buf); } +#if 0 /* This fucntion is not used here, it is left her for the curious */ @@ -197,26 +203,28 @@ unsigned short chrp_checksum(chrp_header sum = (sum & 0xFF) + (sum>>8); return sum; } +#endif static void nvstore(int nvfd, chrp_header* chrph, char* nvbuf, int nvstart, int nvsize) { // mmh, the checksum is calculated for the header only // since we did not modify the header we can just ignore it. - ssize_t written; - ssize_t seek = nvstart + sizeof(chrp_header); - written = lseek(nvfd, seek, SEEK_SET); - if (written != seek) - { + size_t count = 0; + off_t w, seek = nvstart + sizeof(chrp_header); + w = lseek(nvfd, seek, SEEK_SET); + if (w != seek) { fprintf(stderr,"Error seeking /dev/nvram\n"); exit(EXIT_FAILURE); } - written = write(nvfd, nvbuf, nvsize); - if (written != nvsize) - { - fprintf(stderr,"Error writing /dev/nvram %x %x\n", nvsize, seek); - exit(EXIT_FAILURE); - } + do { + w = write(nvfd, nvbuf + count, nvsize - count); + if (w < 0) { + perror("Error writing /dev/nvram"); + exit(EXIT_FAILURE); + } + count += w; + } while (count < (size_t)nvsize); } /* print / set the New World NVRAM */ -- short story of a lazy sysadmin: alias appserv=wotan ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2006-01-11 12:20 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <200601091916.k09JGMJY004116@hera.kernel.org>
2006-01-10 22:13 ` [PATCH] powerpc: fix large nvram access Olaf Hering
2006-01-10 23:22 ` Andreas Schwab
2006-01-10 23:34 ` Arnd Bergmann
2006-01-11 10:25 ` Olaf Hering
2006-01-11 12:20 ` Olaf Hering
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).