Re: [PATCH 06/14] Pramfs: Include files

2009-06-24 Thread Marco Stornelli
2009/6/23 Arnd Bergmann a...@arndb.de:
 On Tuesday 23 June 2009, David Woodhouse wrote:
 And dd on /dev/mem would work, surely?

 Actually, reading from /dev/mem is only valid on real RAM. If the nvram
 is part of an IO memory mapping, you have to do mmap()+memcpy() rather
 than read(). So dd won't do it, but it's still easy to read from user
 space.

For security reasons pram reserve the region of memory with
reserve_mem_region_exclusive().


 I'd definitely recommend making it fixed-endian. Not doing so for JFFS2
 was a mistake I frequently regretted.

 Right.

        Arnd 

--
To unsubscribe from this list: send the line unsubscribe linux-embedded in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 06/14] Pramfs: Include files

2009-06-24 Thread Marco
Arnd Bergmann wrote:
 On Wednesday 24 June 2009, Marco Stornelli wrote:
 Actually, reading from /dev/mem is only valid on real RAM. If the nvram
 is part of an IO memory mapping, you have to do mmap()+memcpy() rather
 than read(). So dd won't do it, but it's still easy to read from user
 space.
 For security reasons pram reserve the region of memory with
 reserve_mem_region_exclusive().
 
 That will only prevent other device drivers from stepping on it,
 /dev/mem does not care about mem_region reservations.
 
   Arnd 
 

Userland may not map this resource, so /dev/mem and the sysfs MMIO
access will not be allowed. This restriction depends on STRICT_DEVMEM
option. It's true that currently is only implemented in the x86 world.

Marco

--
To unsubscribe from this list: send the line unsubscribe linux-embedded in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 06/14] Pramfs: Include files

2009-06-23 Thread Marco
Sam Ravnborg wrote:
 It should be possible to read a file-system on your x86 64bit
 box that you wrote with your small powerpc target.
 For a (NV)RAM-based filesystem??  WTH???
 
 dd the full image - dig into it.
 Usefull is you have post-mortem info there.
 
   Sam
 

dd? You haven't got any device file to have a dump. I think we're going
a bit out of scope. I had some doubt to support rootfs in pram and after
some feedback and the comments of this review I think I'll remove it
from the next release (to understand some aspects of this fs with the
kernel community was my main goal for this review). I agree to use the
native endian. As I said the important thing is that if an user want to
use it in a 64bit environment then the fs must work well and then it
must be designed to support even this situation, I think it's obvious.

Marco

--
To unsubscribe from this list: send the line unsubscribe linux-embedded in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 06/14] Pramfs: Include files

2009-06-23 Thread Arnd Bergmann
On Tuesday 23 June 2009, David Woodhouse wrote:
 And dd on /dev/mem would work, surely?

Actually, reading from /dev/mem is only valid on real RAM. If the nvram
is part of an IO memory mapping, you have to do mmap()+memcpy() rather
than read(). So dd won't do it, but it's still easy to read from user
space.

 I'd definitely recommend making it fixed-endian. Not doing so for JFFS2
 was a mistake I frequently regretted.

Right.

Arnd 
--
To unsubscribe from this list: send the line unsubscribe linux-embedded in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 06/14] Pramfs: Include files

2009-06-22 Thread Marco Stornelli
2009/6/21 Arnd Bergmann a...@arndb.de:
 On Sunday 21 June 2009, Marco wrote:
 I was thinking about your comment and I think I'll use __kernel_off_t
 for the exported headers. I know that it will differ between 32 and 64
 bit architectures, but for this kind of fs there isn't any compatibility
 problem at layout level. You cannot remove a chip of RAM from a board
 32bit little endian and attach it to a board with a cpu 64bit big
 endian, the memory isn't a disk. Indeed, I see that tmpfs uses simply
 unsigned long in the exported header file without any problems to
 little or big endian.

 It's still a problem. You might be creating a file system image
 for an embedded board with a different endianess.

It's not possible to create an image with pramfs, it's like tmpfs.

 Or even on the same machine, you could be looking at the file system contents
 with a 32 bit process running on a 64 bit kernel.

        Arnd 


Yes, indeed the most important thing is to be sure that a 64bit kernel
works well. I'll try to test it in this environment. If there are
64bit guys to help me to test it, it'd be great.

Marco
--
To unsubscribe from this list: send the line unsubscribe linux-embedded in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 06/14] Pramfs: Include files

2009-06-22 Thread Marco
Arnd Bergmann wrote:
 On Monday 22 June 2009, Marco Stornelli wrote:
 It's still a problem. You might be creating a file system image
 for an embedded board with a different endianess.
 It's not possible to create an image with pramfs, it's like tmpfs.
 
 But the data is persistant, you even support using it as a root file
 system, so the data has to have a way to get there. Even if you
 don't do it right now, I don't see any fundamental limitation that
 prevents you from creating an image on one machine and dumping it
 into the nvram of another machine as part of manufacturing or testing.
 

Sorry, I meant it's not *currently* possible. At the moment the only way
to use it as rootfs it's to copy all the data in an already mounted
(empty) ram partition and reboot. However it's not my first item on my
todo list because I think that it's possible to use it as rootfs but it
isn't the standard use for this fs.


 Or even on the same machine, you could be looking at the file system 
 contents
 with a 32 bit process running on a 64 bit kernel.
 Yes, indeed the most important thing is to be sure that a 64bit kernel
 works well. I'll try to test it in this environment. If there are
 64bit guys to help me to test it, it'd be great.
 
 This particular problem (__kernel_off_t on 64-bit machines) can be avoided
 by just switching to __kernel_loff_t, which is 64 bit long on all machines,
 while __kernel_off_t is always the register length (32 or 64 bits).
 
   Arnd 
 

Yep, it can be a good idea, thanks for the tip.

Marco
--
To unsubscribe from this list: send the line unsubscribe linux-embedded in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 06/14] Pramfs: Include files

2009-06-22 Thread Arnd Bergmann
On Monday 22 June 2009, Marco wrote:
 
 Sorry, I meant it's not currently possible. At the moment the only way
 to use it as rootfs it's to copy all the data in an already mounted
 (empty) ram partition and reboot. However it's not my first item on my
 todo list because I think that it's possible to use it as rootfs but it
 isn't the standard use for this fs.

Well, it doesn't have to work right away. What I'm asking to
define the data structures in a way that keeps the layout stable
across kernel updates. Since a future version of the file system
might support cross-endian image creation, it would be good to
define the data structures in a fixed endian mode already, so
you don't have to change it in the future.

Arnd 
--
To unsubscribe from this list: send the line unsubscribe linux-embedded in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 06/14] Pramfs: Include files

2009-06-22 Thread Chris Simmonds

Arnd Bergmann wrote:

On Monday 22 June 2009, Marco wrote:

Sorry, I meant it's not currently possible. At the moment the only way
to use it as rootfs it's to copy all the data in an already mounted
(empty) ram partition and reboot. However it's not my first item on my
todo list because I think that it's possible to use it as rootfs but it
isn't the standard use for this fs.


Well, it doesn't have to work right away. What I'm asking to
define the data structures in a way that keeps the layout stable
across kernel updates. Since a future version of the file system
might support cross-endian image creation, it would be good to
define the data structures in a fixed endian mode already, so
you don't have to change it in the future.

Arnd 
--
To unsubscribe from this list: send the line unsubscribe linux-embedded in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html



I disagree: that adds an unnecessary overhead for those architectures 
where the cpu byte order does not match the data structure ordering. I 
think the data structures should be native endian and when mkpramfs is 
written it can take a flag (e.g. -r) in the same way mkcramfs does.


Chris Simmonds
--
To unsubscribe from this list: send the line unsubscribe linux-embedded in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 06/14] Pramfs: Include files

2009-06-22 Thread Sam Ravnborg
On Mon, Jun 22, 2009 at 08:31:10PM +0100, Chris Simmonds wrote:
 Arnd Bergmann wrote:
 On Monday 22 June 2009, Marco wrote:
 Sorry, I meant it's not currently possible. At the moment the only way
 to use it as rootfs it's to copy all the data in an already mounted
 (empty) ram partition and reboot. However it's not my first item on my
 todo list because I think that it's possible to use it as rootfs but it
 isn't the standard use for this fs.
 
 Well, it doesn't have to work right away. What I'm asking to
 define the data structures in a way that keeps the layout stable
 across kernel updates. Since a future version of the file system
 might support cross-endian image creation, it would be good to
 define the data structures in a fixed endian mode already, so
 you don't have to change it in the future.
 
  Arnd 
 --
 To unsubscribe from this list: send the line unsubscribe linux-embedded 
 in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 
 
 I disagree: that adds an unnecessary overhead for those architectures 
 where the cpu byte order does not match the data structure ordering.
It is not that we are talking big and complex stuff here.
pramfs is likely to be used for small things and then having to
fix endian on a few headers in the on-dsk format does not matter.
Not compared to the potential disadvantages.

It should be possible to read a file-system on your x86 64bit
box that you wrote with your small powerpc target.

Sam
--
To unsubscribe from this list: send the line unsubscribe linux-embedded in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 06/14] Pramfs: Include files

2009-06-22 Thread Jörn Engel
On Mon, 22 June 2009 20:31:10 +0100, Chris Simmonds wrote:
 
 I disagree: that adds an unnecessary overhead for those architectures 
 where the cpu byte order does not match the data structure ordering. I 
 think the data structures should be native endian and when mkpramfs is 
 written it can take a flag (e.g. -r) in the same way mkcramfs does.

Just to quantify this point, I've written a small crap program:
#include stdio.h
#include stdint.h
#include byteswap.h
#include sys/time.h

long long delta(struct timeval *t1, struct timeval *t2)
{
long long delta;

delta  = 100ull * t2-tv_sec + t2-tv_usec;
delta -= 100ull * t1-tv_sec + t1-tv_usec;
return delta;
}

#define LOOPS 1
int main(void)
{
long native = 0;
uint32_t narrow = 0;
uint64_t wide = 0, native_wide = 0;
struct timeval t1, t2, t3, t4, t5;
int i;

gettimeofday(t1, NULL);
for (i = 0; i  LOOPS; i++)
native++;
gettimeofday(t2, NULL);
for (i = 0; i  LOOPS; i++)
narrow = bswap_32(bswap_64(narrow) + 1);
gettimeofday(t3, NULL);
for (i = 0; i  LOOPS; i++)
native_wide++;
gettimeofday(t4, NULL);
for (i = 0; i  LOOPS; i++)
wide = bswap_64(bswap_64(wide) + 1);
gettimeofday(t5, NULL);
printf(long:  %9lld us\n, delta(t1, t2));
printf(we32:  %9lld us\n, delta(t2, t3));
printf(u64:   %9lld us\n, delta(t3, t4));
printf(we64:  %9lld us\n, delta(t4, t5));
printf(loops: %9d\n, LOOPS);
return 0;
}

Four loops doing the same increment with different data types: long,
u64, we32 (wrong-endian) and we64.  Compile with _no_ optimizations.

Results on my i386 notebook:
long: 453953 us
we32: 880273 us
u64:  504214 us
we64:2259953 us
loops: 1

Or thereabouts, not completely stable.  Increasing the data width is 10%
slower, 32bit endianness conversions is 2x slower, 64bit conversion is
5x slower.

However, even the we64 loop still munches through 353MB/s (100M
conversions in 2.2s, 8bytes per converion.  Double the number if you
count both conversion to/from wrong endianness).  Elsewhere in this
thread someone claimed the filesystem peaks out at 13MB/s.  One might
further note that only filesystem metadata has to go through endianness
conversion, so on this particular machine it is completely lost in the
noise.

Feel free to run the program on any machine you care about.  If you get
numbers to back up your position, I'm willing to be convinced.  Until
then, I consider the alleged overhead of endianness conversion a prime
example of premature optimization.

Jörn

-- 
Joern's library part 7:
http://www.usenix.org/publications/library/proceedings/neworl/full_papers/mckusick.a
--
To unsubscribe from this list: send the line unsubscribe linux-embedded in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 06/14] Pramfs: Include files

2009-06-22 Thread Arnd Bergmann
On Monday 22 June 2009, Jörn Engel wrote:
 Four loops doing the same increment with different data types: long,
 u64, we32 (wrong-endian) and we64.  Compile with no optimizations.
 
 Results on my i386 notebook:
 long: 453953 us
 we32: 880273 us
 u64:  504214 us
 we64:2259953 us
 loops: 1

(couldn't resist)

The we64 number is artificially high because the glibc bswap_64
implementation forces the conversion to be done on the stack.
Using __builtin_bswap64 make this look more logical, and
makes your point even stronger (on core 2, using -m32):

long: 236792 us
we32: 500827 us
u64:  265990 us
we64: 757380 us
loops: 1

Arnd 
--
To unsubscribe from this list: send the line unsubscribe linux-embedded in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 06/14] Pramfs: Include files

2009-06-22 Thread Sam Ravnborg
 
  It should be possible to read a file-system on your x86 64bit
  box that you wrote with your small powerpc target.
 For a (NV)RAM-based filesystem??  WTH???

dd the full image - dig into it.
Usefull is you have post-mortem info there.

Sam
--
To unsubscribe from this list: send the line unsubscribe linux-embedded in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 06/14] Pramfs: Include files

2009-06-22 Thread Jörn Engel
On Mon, 22 June 2009 23:20:39 +0100, David Woodhouse wrote:
 On Mon, 2009-06-22 at 23:41 +0200, Jörn Engel wrote:
  Four loops doing the same increment with different data types: long,
  u64, we32 (wrong-endian) and we64.  Compile with _no_ optimizations.
 
 That's a bit of a poor test then. Especially on architectures with a
 load-and-swap instruction where it really shouldn't be any slower at
 all.
 
 (Although since GCC doesn't have an __attribute__((littleendian)) I'm
 not entirely sure how to entice it into _using_ said instruction for the
 purpose of the test... I think the kernel does manage somehow though, if
 you get the sources _just_ right.)

Feel free to improve the test.  It is admittedly crap and designed to
support Chris' argument.  But seeing that it still fails to do so and
Arnd has already shown one improvement that weakened Chris' argument, I
guess we can all agree that further improvments won't change the
conclusion, can we? ;)

Jörn

-- 
It's just what we asked for, but not what we want!
-- anonymous
--
To unsubscribe from this list: send the line unsubscribe linux-embedded in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 06/14] Pramfs: Include files

2009-06-21 Thread Marco
Arnd Bergmann wrote:
 On Saturday 13 June 2009, Sam Ravnborg wrote:
 + union {
 + struct {
 + /*
 +  * ptr to row block of 2D block pointer array,
 +  * file block #'s 0 to (blocksize/4)^2 - 1.
 +  */
 + off_t row_block;
 It is my understanding that we shall use: __kernel_off_t
 in exported headers.
 
 That is a correct understanding in general, however this case is
 different, because it describes an on-disk data structure,
 not a kernel to user space interface. Here, __kernel_off_t is just
 as wrong as off_t, because it will differ between 32 and 64 bit
 architectures, making the fs layout incompatible. I'd suggest
 simply defining this as __u64.
 
 Moreover, file system layout should be described in terms of
 big-endian or little-endian types (e.g. __be64 or __le64),
 together with the right accessor functions.
 
   Arnd 
 

I was thinking about your comment and I think I'll use __kernel_off_t
for the exported headers. I know that it will differ between 32 and 64
bit architectures, but for this kind of fs there isn't any compatibility
problem at layout level. You cannot remove a chip of RAM from a board
32bit little endian and attach it to a board with a cpu 64bit big
endian, the memory isn't a disk. Indeed, I see that tmpfs uses simply
unsigned long in the exported header file without any problems to
little or big endian.

Regards,

Marco
--
To unsubscribe from this list: send the line unsubscribe linux-embedded in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 06/14] Pramfs: Include files

2009-06-13 Thread Sam Ravnborg
On Sat, Jun 13, 2009 at 03:21:48PM +0200, Marco wrote:
 From: Marco Stornelli marco.storne...@gmail.com
 
 Include files.
 
 Signed-off-by: Marco Stornelli marco.storne...@gmail.com
 ---
 
 diff -uprN linux-2.6.30-orig/fs/pramfs/pram_fs.h 
 linux-2.6.30/fs/pramfs/pram_fs.h
 --- linux-2.6.30-orig/fs/pramfs/pram_fs.h 1970-01-01 01:00:00.0 
 +0100
 +++ linux-2.6.30/fs/pramfs/pram_fs.h  2009-06-13 12:58:49.0 +0200
 @@ -0,0 +1,388 @@
 +/*
 + * FILE NAME include/linux/pram_fs.h
 + *
 + * BRIEF DESCRIPTION
 + *
 + * Definitions for the PRAMFS filesystem.
 + *
 + * Copyright 2009 Marco Stornelli marco.storne...@gmail.com
 + * Copyright 2003 Sony Corporation
 + * Copyright 2003 Matsushita Electric Industrial Co., Ltd.
 + * 2003-2004 (c) MontaVista Software, Inc. , Steve Longerbeam
 + * This file is licensed under the terms of the GNU General Public
 + * License version 2. This program is licensed as is without any
 + * warranty of any kind, whether express or implied.
 + */
 +#ifndef _LINUX_PRAM_FS_H
 +#define _LINUX_PRAM_FS_H
 +
 +#include linux/types.h
 +
 +#ifdef __KERNEL__
 +#include linux/sched.h
 +#include linux/buffer_head.h
 +#include pram_fs_sb.h
 +#endif

The only reason to have this header file in include/linux/
is that it is used by userspace.
So please split it up so we have one header suitable for exporting
and another header with all the promfs local stuff.
The latter should be in fs/pramsfs/


 +
 +/*
 + * The PRAM filesystem constants/structures
 + */
 +
 +/*
 + * Define PRAMFS_DEBUG to produce debug messages
 + */
 +#define PRAMFS_DEBUG
 +
 +/*
 + * Debug code
 + */
 +#ifdef __KERNEL__
 +#define PFX pramfs
 +#ifdef PRAMFS_DEBUG
 +#define pram_dbg(format, arg...) \
 +printk(KERN_DEBUG PFX :  format , ## arg)
 +#else
 +#define pram_dbg(format, arg...) do {} while (0)
 +#endif
 +#define pram_err(format, arg...) \
 +printk(KERN_ERR PFX :  format , ## arg)
 +#define pram_info(format, arg...) \
 +printk(KERN_INFO PFX :  format , ## arg)
 +#define pram_warn(format, arg...) \
 +printk(KERN_WARNING PFX :  format , ## arg)
 +#endif

For a typical drivers we have some pr_* to avoid the above.
Can they be used for a filesystem too?

 +
 +/*
 + * The PRAM file system magic number
 + */
 +#define PRAM_SUPER_MAGIC 0xEFFA

Move to include/linux/magic.h

 +
 +/*
 + * Structure of an inode in PRAMFS
 + */
 +struct pram_inode {
 + __u32   i_sum;  /* checksum of this inode */
 + __u32   i_uid;  /* Owner Uid */
 + __u32   i_gid;  /* Group Id */
 + __u16   i_mode; /* File mode */
 + __u16   i_links_count;  /* Links count */
 + __u32   i_blocks;   /* Blocks count */
 + __u32   i_size; /* Size of data in bytes */
 + __u32   i_atime;/* Access time */
 + __u32   i_ctime;/* Creation time */
 + __u32   i_mtime;/* Modification time */
 + __u32   i_dtime;/* Deletion Time */
 +
 + union {
 + struct {
 + /*
 +  * ptr to row block of 2D block pointer array,
 +  * file block #'s 0 to (blocksize/4)^2 - 1.
 +  */
 + off_t row_block;

It is my understanding that we shall use: __kernel_off_t
in exported headers.

The headers are not added to Kbuild - so it is not exported.
I assume thats an oversight.

Sam
--
To unsubscribe from this list: send the line unsubscribe linux-embedded in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html