[PATCH] add romfs_get_size()

[PATCH] add romfs_get_size()

[Yasushi SHOJI]

Many embedded linux products have been using romfs and it's still
growing.  most, if not all, of them implement thier own way to check
its romfs size.

this patch provides this commonly used function.

Signed-off-by: Yasushi SHOJI <yashi@atmark-techno.com>
---
diff --git a/fs/romfs/inode.c b/fs/romfs/inode.c
--- a/fs/romfs/inode.c
+++ b/fs/romfs/inode.c
@@ -84,6 +84,17 @@ struct romfs_inode_info {
 	struct inode vfs_inode;
 };
 
+int romfs_get_size(struct romfs_super_block *rsb)
+{
+	int sz = be32_to_cpu(rsb->size);
+	if (rsb->word0 != ROMSB_WORD0 || rsb->word1 != ROMSB_WORD1
+	    || sz < ROMFH_SIZE) {
+		sz = -1;
+	}
+	return sz;
+}
+EXPORT_SYMBOL(romfs_get_size);
+
 /* instead of private superblock data */
 static inline unsigned long romfs_maxsize(struct super_block *sb)
 {
@@ -132,9 +143,8 @@ static int romfs_fill_super(struct super
 	}
 
 	rsb = (struct romfs_super_block *)bh->b_data;
-	sz = be32_to_cpu(rsb->size);
-	if (rsb->word0 != ROMSB_WORD0 || rsb->word1 != ROMSB_WORD1
-	   || sz < ROMFH_SIZE) {
+	sz = romfs_get_size(rsb);
+	if (sz == -1) {
 		if (!silent)
 			printk ("VFS: Can't find a romfs filesystem on dev "
 				"%s.\n", s->s_id);
diff --git a/include/linux/romfs_fs.h b/include/linux/romfs_fs.h
--- a/include/linux/romfs_fs.h
+++ b/include/linux/romfs_fs.h
@@ -55,7 +55,7 @@ struct romfs_inode {
 
 #ifdef __KERNEL__
 
-/* Not much now */
+int romfs_get_size(struct romfs_super_block *rsb);
 
 #endif /* __KERNEL__ */
 #endif

Re: [PATCH] add romfs_get_size()

[Christoph Hellwig]

On Wed, Sep 07, 2005 at 11:22:19PM +0900, Yasushi SHOJI wrote:
> Many embedded linux products have been using romfs and it's still
> growing.  most, if not all, of them implement thier own way to check
> its romfs size.
> 
> this patch provides this commonly used function.

Used where.  Please come back as soon as you have a caller in-tree
which makes sense..

Re: [PATCH] add romfs_get_size()

[Yasushi SHOJI]

At Wed, 7 Sep 2005 15:26:04 +0100,
Christoph Hellwig wrote:
> 
> On Wed, Sep 07, 2005 at 11:22:19PM +0900, Yasushi SHOJI wrote:
> > Many embedded linux products have been using romfs and it's still
> > growing.  most, if not all, of them implement thier own way to check
> > its romfs size.
> > 
> > this patch provides this commonly used function.
> 
> Used where.  Please come back as soon as you have a caller in-tree
> which makes sense..

i don't know this one make sense but the biggest user is uclinux mtd
map. in uclinux_mtd_init():

http://lxr.linux.no/source/drivers/mtd/maps/uclinux.c#L66
--
        yashi

Re: [PATCH] add romfs_get_size()

[Christoph Hellwig]

On Wed, Sep 07, 2005 at 11:31:36PM +0900, Yasushi SHOJI wrote:
> > On Wed, Sep 07, 2005 at 11:22:19PM +0900, Yasushi SHOJI wrote:
> > > Many embedded linux products have been using romfs and it's still
> > > growing.  most, if not all, of them implement thier own way to check
> > > its romfs size.
> > > 
> > > this patch provides this commonly used function.
> > 
> > Used where.  Please come back as soon as you have a caller in-tree
> > which makes sense..
> 
> i don't know this one make sense but the biggest user is uclinux mtd
> map. in uclinux_mtd_init():

I don't quite see the corelation.  Anyway, please submit a patch series
that converts whatever wrong variant to the new one, describing each
patch in detail, and adding proper ROMFS depencies to the places using
it.

Re: [PATCH] add romfs_get_size()

[Yasushi SHOJI]

At Wed, 7 Sep 2005 16:04:39 +0100,
Christoph Hellwig wrote:
> 
> On Wed, Sep 07, 2005 at 11:31:36PM +0900, Yasushi SHOJI wrote:
> > > On Wed, Sep 07, 2005 at 11:22:19PM +0900, Yasushi SHOJI wrote:
> > > > Many embedded linux products have been using romfs and it's still
> > > > growing.  most, if not all, of them implement thier own way to check
> > > > its romfs size.
> > > > 
> > > > this patch provides this commonly used function.
> > > 
> > > Used where.  Please come back as soon as you have a caller in-tree
> > > which makes sense..
> > 
> > i don't know this one make sense but the biggest user is uclinux mtd
> > map. in uclinux_mtd_init():
> 
> I don't quite see the corelation.  Anyway, please submit a patch series
> that converts whatever wrong variant to the new one, describing each
> patch in detail, and adding proper ROMFS depencies to the places using
> it.

I don't have most of platform to test. sure it's easy to just convert
them using romfs_get_size() but I don't wanna submit any patch that I
can't test.

So, if the patch is not that bad, it'd be much easier to just tell all
platform maintainer that the infra. is in place and they can start
converting their code to use new function.

Anyway I just converted uclinux.c to use romfs_get_size(). Greg, would
you kindly comment on the attached patch?

Signed-off-by: Yasushi SHOJI <yashi@atmark-techno.com>
---
diff --git a/drivers/mtd/maps/uclinux.c b/drivers/mtd/maps/uclinux.c
--- a/drivers/mtd/maps/uclinux.c
+++ b/drivers/mtd/maps/uclinux.c
@@ -16,6 +16,7 @@
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/fs.h>
+#include <linux/romfs_fs.h>
 #include <linux/major.h>
 #include <linux/root_dev.h>
 #include <linux/mtd/mtd.h>
@@ -63,7 +64,7 @@ int __init uclinux_mtd_init(void)
 
 	mapp = &uclinux_ram_map;
 	mapp->phys = (unsigned long) &_ebss;
-	mapp->size = PAGE_ALIGN(*((unsigned long *)((&_ebss) + 8)));
+	mapp->size = PAGE_ALIGN(romfs_get_size((struct romfs_super_block *)&_ebss));
 	mapp->bankwidth = 4;
 
 	printk("uclinux[mtd]: RAM probe address=0x%x size=0x%x\n",

Re: [PATCH] add romfs_get_size()

[Greg Ungerer]

Hi Yashi,

Yasushi SHOJI wrote:
> At Wed, 7 Sep 2005 16:04:39 +0100,
> Christoph Hellwig wrote:
> 
>>On Wed, Sep 07, 2005 at 11:31:36PM +0900, Yasushi SHOJI wrote:
>>
>>>>On Wed, Sep 07, 2005 at 11:22:19PM +0900, Yasushi SHOJI wrote:
>>>>
>>>>>Many embedded linux products have been using romfs and it's still
>>>>>growing.  most, if not all, of them implement thier own way to check
>>>>>its romfs size.
>>>>>
>>>>>this patch provides this commonly used function.
>>>>
>>>>Used where.  Please come back as soon as you have a caller in-tree
>>>>which makes sense..
>>>
>>>i don't know this one make sense but the biggest user is uclinux mtd
>>>map. in uclinux_mtd_init():
>>
>>I don't quite see the corelation.  Anyway, please submit a patch series
>>that converts whatever wrong variant to the new one, describing each
>>patch in detail, and adding proper ROMFS depencies to the places using
>>it.
> 
> 
> I don't have most of platform to test. sure it's easy to just convert
> them using romfs_get_size() but I don't wanna submit any patch that I
> can't test.
> 
> So, if the patch is not that bad, it'd be much easier to just tell all
> platform maintainer that the infra. is in place and they can start
> converting their code to use new function.
> 
> Anyway I just converted uclinux.c to use romfs_get_size(). Greg, would
> you kindly comment on the attached patch?

I don't see any problem with it.

The uclinux.c code as it stands is probably a little sloppy.
It doesn't do any checks for a valid ROMfs first - it should at
least check the magic number.

Regards
Greg



> Signed-off-by: Yasushi SHOJI <yashi@atmark-techno.com>
> ---
> diff --git a/drivers/mtd/maps/uclinux.c b/drivers/mtd/maps/uclinux.c
> --- a/drivers/mtd/maps/uclinux.c
> +++ b/drivers/mtd/maps/uclinux.c
> @@ -16,6 +16,7 @@
>  #include <linux/init.h>
>  #include <linux/kernel.h>
>  #include <linux/fs.h>
> +#include <linux/romfs_fs.h>
>  #include <linux/major.h>
>  #include <linux/root_dev.h>
>  #include <linux/mtd/mtd.h>
> @@ -63,7 +64,7 @@ int __init uclinux_mtd_init(void)
>  
>  	mapp = &uclinux_ram_map;
>  	mapp->phys = (unsigned long) &_ebss;
> -	mapp->size = PAGE_ALIGN(*((unsigned long *)((&_ebss) + 8)));
> +	mapp->size = PAGE_ALIGN(romfs_get_size((struct romfs_super_block *)&_ebss));
>  	mapp->bankwidth = 4;
>  
>  	printk("uclinux[mtd]: RAM probe address=0x%x size=0x%x\n",
> 
> 

-- 
------------------------------------------------------------------------
Greg Ungerer  --  Chief Software Dude       EMAIL:     gerg@snapgear.com
SnapGear -- a CyberGuard Company            PHONE:       +61 7 3435 2888
825 Stanley St,                             FAX:         +61 7 3891 3630
Woolloongabba, QLD, 4102, Australia         WEB: http://www.SnapGear.com

Re: [PATCH] add romfs_get_size()

[Yasushi SHOJI]

Hi Greg,

At Thu, 08 Sep 2005 17:07:42 +1000,
Greg Ungerer wrote:
> 
> Yasushi SHOJI wrote:
> > At Wed, 7 Sep 2005 16:04:39 +0100,
> > Christoph Hellwig wrote:
> > 
> >>On Wed, Sep 07, 2005 at 11:31:36PM +0900, Yasushi SHOJI wrote:
> >>
> >>>>On Wed, Sep 07, 2005 at 11:22:19PM +0900, Yasushi SHOJI wrote:
> >>>>
> >>>>>Many embedded linux products have been using romfs and it's still
> >>>>>growing.  most, if not all, of them implement thier own way to check
> >>>>>its romfs size.
> >>>>>
> >>>>>this patch provides this commonly used function.
> >>>>
> >>>>Used where.  Please come back as soon as you have a caller in-tree
> >>>>which makes sense..
> >>>
> >>>i don't know this one make sense but the biggest user is uclinux mtd
> >>>map. in uclinux_mtd_init():
> >>
> >>I don't quite see the corelation.  Anyway, please submit a patch series
> >>that converts whatever wrong variant to the new one, describing each
> >>patch in detail, and adding proper ROMFS depencies to the places using
> >>it.
> > 
> > 
> > I don't have most of platform to test. sure it's easy to just convert
> > them using romfs_get_size() but I don't wanna submit any patch that I
> > can't test.
> > 
> > So, if the patch is not that bad, it'd be much easier to just tell all
> > platform maintainer that the infra. is in place and they can start
> > converting their code to use new function.
> > 
> > Anyway I just converted uclinux.c to use romfs_get_size(). Greg, would
> > you kindly comment on the attached patch?
> 
> I don't see any problem with it.

great.

> The uclinux.c code as it stands is probably a little sloppy.
> It doesn't do any checks for a valid ROMfs first - it should at
> least check the magic number.

Yeah that's right. At least with romfs_get_size(), we can check the
return val to see it ok or not. it might be better if we have

    mapp->size = PAGE_ALIGN(romfs_get_size((struct romfs_super_block *)&_ebss));
    if (mapp->size != -1)

kind of code. what we can do once we detected mismatched magic number
is deferment story, though. ;)

regards,
--
         yashi