All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: A few V7 fs improvements
  2010-07-19 17:16 A few V7 fs improvements Lubomir Rintel
@ 2010-07-19 14:53 ` Christoph Hellwig
  2010-07-19 16:10 ` Bill Davidsen
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 21+ messages in thread
From: Christoph Hellwig @ 2010-07-19 14:53 UTC (permalink / raw)
  To: Lubomir Rintel
  Cc: Christoph Hellwig, Linux Kernel Mailing List, Andrew Morton

On Mon, Jul 19, 2010 at 07:16:39PM +0200, Lubomir Rintel wrote:
> I'm resending these once again, since I suspect they may have gotten lost
> in a spam filter due to improper MTA configuration of mine.

Yes, I don't think I got them before.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/3] [fs/sysv] Add v7 alias
  2010-07-19 17:16 ` [PATCH 1/3] [fs/sysv] Add v7 alias Lubomir Rintel
@ 2010-07-19 14:53   ` Christoph Hellwig
  2010-07-19 17:16   ` [PATCH 2/3] [fs/sysv] V7: Adjust sanity checks for some volumes Lubomir Rintel
  1 sibling, 0 replies; 21+ messages in thread
From: Christoph Hellwig @ 2010-07-19 14:53 UTC (permalink / raw)
  To: Lubomir Rintel
  Cc: Christoph Hellwig, Linux Kernel Mailing List, Andrew Morton

Looks good, thanks.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/3] [fs/sysv] V7: Adjust sanity checks for some volumes
  2010-07-19 17:16   ` [PATCH 2/3] [fs/sysv] V7: Adjust sanity checks for some volumes Lubomir Rintel
@ 2010-07-19 14:54     ` Christoph Hellwig
  2010-07-20 10:31       ` Lubomir Rintel
  2010-07-19 17:16     ` [PATCH 3/3] [fs/sysv] V7: Add support for non-PDP11 v7 filesystems Lubomir Rintel
  1 sibling, 1 reply; 21+ messages in thread
From: Christoph Hellwig @ 2010-07-19 14:54 UTC (permalink / raw)
  To: Lubomir Rintel
  Cc: Christoph Hellwig, Linux Kernel Mailing List, Andrew Morton

>  	if (fs16_to_cpu(sbi, v7sb->s_nfree) > V7_NICFREE ||
>  	    fs16_to_cpu(sbi, v7sb->s_ninode) > V7_NICINOD ||
> -	    fs32_to_cpu(sbi, v7sb->s_time) == 0)
> -		goto failed;
> +	    fs32_to_cpu(sbi, v7sb->s_fsize) > V7_MAXSIZE)
> +		return 0;

Ok.

>  	v7i = (struct sysv_inode *)(bh2->b_data + 64);
>  	if ((fs16_to_cpu(sbi, v7i->i_mode) & ~0777) != S_IFDIR ||
>  	    (fs32_to_cpu(sbi, v7i->i_size) == 0) ||
> -	    (fs32_to_cpu(sbi, v7i->i_size) & 017) != 0)
> +	    (fs32_to_cpu(sbi, v7i->i_size) & 017)
> +	    (fs32_to_cpu(sbi, v7i->i_size) > V7_NFILES *
> +	     sizeof (struct sysv_dir_entry))) {

Maybe I'm missing something, but without and additional || on the
first line you added this doesn't look like it will compile.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 3/3] [fs/sysv] V7: Add support for non-PDP11 v7 filesystems
  2010-07-19 17:16     ` [PATCH 3/3] [fs/sysv] V7: Add support for non-PDP11 v7 filesystems Lubomir Rintel
@ 2010-07-19 14:58       ` Christoph Hellwig
  2010-07-20 10:41         ` Lubomir Rintel
  2010-07-24 13:51         ` [PATCH 3/3] [fs/sysv] " Al Viro
  0 siblings, 2 replies; 21+ messages in thread
From: Christoph Hellwig @ 2010-07-19 14:58 UTC (permalink / raw)
  To: Lubomir Rintel
  Cc: Christoph Hellwig, Linux Kernel Mailing List, Andrew Morton

On Mon, Jul 19, 2010 at 07:16:42PM +0200, Lubomir Rintel wrote:
> A mount-time option was added that makes it possible to override the
> endianness and an attempt is made to autodetect it (which seems easy,
> given the disk addresses are 3-byte.
> 
> No attempt is made to detect big-endian filesystems -- were there any?
> Tested with PDP-11 v7 filesystems and PC-IX maintenance floppy.

Do you actually need the mount option?  We get away just fine with
it for sysv filesystems.  And if not I'd be consistent and accept the
options for both sysv and v7 filesystems.

> +	/* plausibility check on root inode: it is a directory,
> +	   with a nonzero size that is a multiple of 16 */
> +	if ((bh2 = sb_bread(sb, 2)) == NULL) {
> +		return 0;
> +	}

A little style nitpick, this should be:

	bh2 = sb_bread(sb, 2);
	if (!bh)
		return 0;


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: A few V7 fs improvements
  2010-07-19 17:16 A few V7 fs improvements Lubomir Rintel
  2010-07-19 14:53 ` Christoph Hellwig
@ 2010-07-19 16:10 ` Bill Davidsen
  2010-07-19 17:16 ` [PATCH 1/3] [fs/sysv] Add v7 alias Lubomir Rintel
  2010-07-25  5:23 ` A few V7 fs improvements Artem Bityutskiy
  3 siblings, 0 replies; 21+ messages in thread
From: Bill Davidsen @ 2010-07-19 16:10 UTC (permalink / raw)
  To: linux-kernel; +Cc: Christoph Hellwig, Linux Kernel Mailing List, Andrew Morton

Lubomir Rintel wrote:
> I'm resending these once again, since I suspect they may have gotten lost
> in a spam filter due to improper MTA configuration of mine.
> 
> They basically improve usability of PC v7 filesystems; such as those of
> PC/IX.
> 
I remember PC/IX fondly, I had people using it on an old XT for data entry. Wish 
I still had an install kit, I bet the AT version would install nicely in a VM. 
Would be a good teaching tool.

-- 
Bill Davidsen <davidsen@tmr.com>
   "We have more to fear from the bungling of the incompetent than from
the machinations of the wicked."  - from Slashdot


^ permalink raw reply	[flat|nested] 21+ messages in thread

* A few V7 fs improvements
@ 2010-07-19 17:16 Lubomir Rintel
  2010-07-19 14:53 ` Christoph Hellwig
                   ` (3 more replies)
  0 siblings, 4 replies; 21+ messages in thread
From: Lubomir Rintel @ 2010-07-19 17:16 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Linux Kernel Mailing List, Andrew Morton

I'm resending these once again, since I suspect they may have gotten lost
in a spam filter due to improper MTA configuration of mine.

They basically improve usability of PC v7 filesystems; such as those of
PC/IX.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 1/3] [fs/sysv] Add v7 alias
  2010-07-19 17:16 A few V7 fs improvements Lubomir Rintel
  2010-07-19 14:53 ` Christoph Hellwig
  2010-07-19 16:10 ` Bill Davidsen
@ 2010-07-19 17:16 ` Lubomir Rintel
  2010-07-19 14:53   ` Christoph Hellwig
  2010-07-19 17:16   ` [PATCH 2/3] [fs/sysv] V7: Adjust sanity checks for some volumes Lubomir Rintel
  2010-07-25  5:23 ` A few V7 fs improvements Artem Bityutskiy
  3 siblings, 2 replies; 21+ messages in thread
From: Lubomir Rintel @ 2010-07-19 17:16 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Linux Kernel Mailing List, Andrew Morton, Lubomir Rintel,
	Christoph Hellwig

So that the module gets autoloaded when a v7 filesystem is mounted.

Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 fs/sysv/super.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/fs/sysv/super.c b/fs/sysv/super.c
index 5a903da..bd8d141 100644
--- a/fs/sysv/super.c
+++ b/fs/sysv/super.c
@@ -560,4 +560,5 @@ static void __exit exit_sysv_fs(void)
 
 module_init(init_sysv_fs)
 module_exit(exit_sysv_fs)
+MODULE_ALIAS("v7");
 MODULE_LICENSE("GPL");
-- 
1.6.5.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 2/3] [fs/sysv] V7: Adjust sanity checks for some volumes
  2010-07-19 17:16 ` [PATCH 1/3] [fs/sysv] Add v7 alias Lubomir Rintel
  2010-07-19 14:53   ` Christoph Hellwig
@ 2010-07-19 17:16   ` Lubomir Rintel
  2010-07-19 14:54     ` Christoph Hellwig
  2010-07-19 17:16     ` [PATCH 3/3] [fs/sysv] V7: Add support for non-PDP11 v7 filesystems Lubomir Rintel
  1 sibling, 2 replies; 21+ messages in thread
From: Lubomir Rintel @ 2010-07-19 17:16 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Linux Kernel Mailing List, Andrew Morton, Lubomir Rintel,
	Christoph Hellwig

Newly mkfs-ed filesystems from Seventh Edition have last modification
time set to zero, but are otherwise perfectly valid.

Also, tighten up other sanity checks to filter out most filesystems with
different bytesex than we're using.

Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 fs/sysv/super.c         |    8 +++++---
 include/linux/sysv_fs.h |   11 +++++++++++
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/fs/sysv/super.c b/fs/sysv/super.c
index bd8d141..17ac83d 100644
--- a/fs/sysv/super.c
+++ b/fs/sysv/super.c
@@ -470,8 +470,8 @@ static int v7_fill_super(struct super_block *sb, void *data, int silent)
 	v7sb = (struct v7_super_block *) bh->b_data;
 	if (fs16_to_cpu(sbi, v7sb->s_nfree) > V7_NICFREE ||
 	    fs16_to_cpu(sbi, v7sb->s_ninode) > V7_NICINOD ||
-	    fs32_to_cpu(sbi, v7sb->s_time) == 0)
-		goto failed;
+	    fs32_to_cpu(sbi, v7sb->s_fsize) > V7_MAXSIZE)
+		return 0;
 
 	/* plausibility check on root inode: it is a directory,
 	   with a nonzero size that is a multiple of 16 */
@@ -480,7 +480,9 @@ static int v7_fill_super(struct super_block *sb, void *data, int silent)
 	v7i = (struct sysv_inode *)(bh2->b_data + 64);
 	if ((fs16_to_cpu(sbi, v7i->i_mode) & ~0777) != S_IFDIR ||
 	    (fs32_to_cpu(sbi, v7i->i_size) == 0) ||
-	    (fs32_to_cpu(sbi, v7i->i_size) & 017) != 0)
+	    (fs32_to_cpu(sbi, v7i->i_size) & 017)
+	    (fs32_to_cpu(sbi, v7i->i_size) > V7_NFILES *
+	     sizeof (struct sysv_dir_entry))) {
 		goto failed;
 	brelse(bh2);
 	bh2 = NULL;
diff --git a/include/linux/sysv_fs.h b/include/linux/sysv_fs.h
index 9641130..0a7a232 100644
--- a/include/linux/sysv_fs.h
+++ b/include/linux/sysv_fs.h
@@ -148,6 +148,17 @@ struct v7_super_block {
 	char    s_fname[6];     /* file system name */
 	char    s_fpack[6];     /* file system pack name */
 };
+/* Constants to aid sanity checking */
+/* This is not a hard limit, nor enforced by v7 kernel. It's actually just
+ * the limit used by Seventh Edition's ls, though is high enough to assume
+ * that no reasonable file system would have that much entries in root
+ * directory. Thus, if we see anything higher, we just probably got the
+ * endiannes wrong. */
+#define V7_NFILES	1024
+/* Indirect blocks hold just three-byte addresses, therefore if see a file
+ * system whose length has the most significant byte non-zero something is
+ * most likely wrong (not a filesystem, bad bytesex). */
+#define V7_MAXSIZE	0x00ffffff
 
 /* Coherent super-block data on disk */
 #define COH_NICINOD	100	/* number of inode cache entries */
-- 
1.6.5.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 3/3] [fs/sysv] V7: Add support for non-PDP11 v7 filesystems
  2010-07-19 17:16   ` [PATCH 2/3] [fs/sysv] V7: Adjust sanity checks for some volumes Lubomir Rintel
  2010-07-19 14:54     ` Christoph Hellwig
@ 2010-07-19 17:16     ` Lubomir Rintel
  2010-07-19 14:58       ` Christoph Hellwig
  1 sibling, 1 reply; 21+ messages in thread
From: Lubomir Rintel @ 2010-07-19 17:16 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Linux Kernel Mailing List, Andrew Morton, Lubomir Rintel,
	Christoph Hellwig

A mount-time option was added that makes it possible to override the
endianness and an attempt is made to autodetect it (which seems easy,
given the disk addresses are 3-byte.

No attempt is made to detect big-endian filesystems -- were there any?
Tested with PDP-11 v7 filesystems and PC-IX maintenance floppy.

Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 fs/sysv/super.c         |  116 +++++++++++++++++++++++++++++++++++++----------
 include/linux/sysv_fs.h |    6 +-
 2 files changed, 95 insertions(+), 27 deletions(-)

diff --git a/fs/sysv/super.c b/fs/sysv/super.c
index 17ac83d..9cd5be0 100644
--- a/fs/sysv/super.c
+++ b/fs/sysv/super.c
@@ -24,6 +24,7 @@
 #include <linux/init.h>
 #include <linux/slab.h>
 #include <linux/buffer_head.h>
+#include <linux/parser.h>
 #include "sysv.h"
 
 /*
@@ -435,12 +436,55 @@ Ebadsize:
 	goto failed;
 }
 
-static int v7_fill_super(struct super_block *sb, void *data, int silent)
+static int v7_sanity_check(struct super_block *sb, struct buffer_head *bh)
 {
-	struct sysv_sb_info *sbi;
-	struct buffer_head *bh, *bh2 = NULL;
 	struct v7_super_block *v7sb;
 	struct sysv_inode *v7i;
+	struct buffer_head *bh2;
+	struct sysv_sb_info *sbi;
+
+	sbi = sb->s_fs_info;
+
+	/* plausibility check on superblock */
+	v7sb = (struct v7_super_block *) bh->b_data;
+	if (fs16_to_cpu(sbi, v7sb->s_nfree) > V7_NICFREE ||
+	    fs16_to_cpu(sbi, v7sb->s_ninode) > V7_NICINOD ||
+	    fs32_to_cpu(sbi, v7sb->s_fsize) > V7_MAXSIZE)
+		return 0;
+
+	/* plausibility check on root inode: it is a directory,
+	   with a nonzero size that is a multiple of 16 */
+	if ((bh2 = sb_bread(sb, 2)) == NULL) {
+		return 0;
+	}
+
+	v7i = (struct sysv_inode *)(bh2->b_data + 64);
+	if ((fs16_to_cpu(sbi, v7i->i_mode) & ~0777) != S_IFDIR ||
+	    (fs32_to_cpu(sbi, v7i->i_size) == 0) ||
+	    (fs32_to_cpu(sbi, v7i->i_size) & 017) ||
+	    (fs32_to_cpu(sbi, v7i->i_size) > V7_NFILES *
+             sizeof (struct sysv_dir_entry))) {
+		brelse(bh2);
+		return 0;
+	}
+
+	brelse(bh2);
+	return 1;
+}
+
+enum { Opt_err, Opt_bytesex_pdp, Opt_bytesex_le, Opt_bytesex_be };
+
+static const match_table_t v7_tokens = {
+	{Opt_bytesex_pdp, "bytesex=pdp"},
+	{Opt_bytesex_le, "bytesex=le"},
+	{Opt_bytesex_be, "bytesex=be"},
+	{Opt_err, NULL}
+};
+
+static int v7_fill_super(struct super_block *sb, void *data, int silent)
+{
+	struct sysv_sb_info *sbi;
+	struct buffer_head *bh;
 
 	if (440 != sizeof (struct v7_super_block))
 		panic("V7 FS: bad super-block size");
@@ -454,7 +498,6 @@ static int v7_fill_super(struct super_block *sb, void *data, int silent)
 	sbi->s_sb = sb;
 	sbi->s_block_base = 0;
 	sbi->s_type = FSTYPE_V7;
-	sbi->s_bytesex = BYTESEX_PDP;
 	sb->s_fs_info = sbi;
 	
 	sb_set_blocksize(sb, 512);
@@ -466,26 +509,51 @@ static int v7_fill_super(struct super_block *sb, void *data, int silent)
 		goto failed;
 	}
 
-	/* plausibility check on superblock */
-	v7sb = (struct v7_super_block *) bh->b_data;
-	if (fs16_to_cpu(sbi, v7sb->s_nfree) > V7_NICFREE ||
-	    fs16_to_cpu(sbi, v7sb->s_ninode) > V7_NICINOD ||
-	    fs32_to_cpu(sbi, v7sb->s_fsize) > V7_MAXSIZE)
-		return 0;
+	if (data) {
+		substring_t args[MAX_OPT_ARGS];
+		char *p;
+
+		while ((p = strsep ((char **)&data, ",")) != NULL) {
+			int token;
+			if (!*p)
+				continue;
+			token = match_token(p, v7_tokens, args);
+			switch (token) {
+			case Opt_bytesex_pdp:
+				sbi->s_bytesex = BYTESEX_PDP;
+				break;
+			case Opt_bytesex_le:
+				sbi->s_bytesex = BYTESEX_LE;
+				break;
+			case Opt_bytesex_be:
+				sbi->s_bytesex = BYTESEX_BE;
+				break;
+			default:
+				/* An unrecognized option */
+				goto failed;
+			}
+		}
+	}
 
-	/* plausibility check on root inode: it is a directory,
-	   with a nonzero size that is a multiple of 16 */
-	if ((bh2 = sb_bread(sb, 2)) == NULL)
-		goto failed;
-	v7i = (struct sysv_inode *)(bh2->b_data + 64);
-	if ((fs16_to_cpu(sbi, v7i->i_mode) & ~0777) != S_IFDIR ||
-	    (fs32_to_cpu(sbi, v7i->i_size) == 0) ||
-	    (fs32_to_cpu(sbi, v7i->i_size) & 017)
-	    (fs32_to_cpu(sbi, v7i->i_size) > V7_NFILES *
-	     sizeof (struct sysv_dir_entry))) {
-		goto failed;
-	brelse(bh2);
-	bh2 = NULL;
+	/* Endianness overridden by a mount option */
+	if (sbi->s_bytesex) {
+		if (!v7_sanity_check (sb, bh))
+			goto failed;
+		goto detected;
+	}
+
+	/* Try PDP-11 UNIX */
+	sbi->s_bytesex = BYTESEX_PDP;
+	if (v7_sanity_check (sb, bh))
+		goto detected;
+
+	/* Try PC/IX, v7/x86 */
+	sbi->s_bytesex = BYTESEX_LE;
+	if (v7_sanity_check (sb, bh))
+		goto detected;
+
+	goto failed;
+detected:
 
 	sbi->s_bh1 = bh;
 	sbi->s_bh2 = bh;
@@ -493,7 +561,7 @@ static int v7_fill_super(struct super_block *sb, void *data, int silent)
 		return 0;
 
 failed:
-	brelse(bh2);
+	printk("VFS: could not find a valid V7 on %s.\n", sb->s_id);
 	brelse(bh);
 	kfree(sbi);
 	return -EINVAL;
diff --git a/include/linux/sysv_fs.h b/include/linux/sysv_fs.h
index 0a7a232..e47d6d9 100644
--- a/include/linux/sysv_fs.h
+++ b/include/linux/sysv_fs.h
@@ -155,9 +155,9 @@ struct v7_super_block {
  * directory. Thus, if we see anything higher, we just probably got the
  * endiannes wrong. */
 #define V7_NFILES	1024
-/* Indirect blocks hold just three-byte addresses, therefore if see a file
- * system whose length has the most significant byte non-zero something is
- * most likely wrong (not a filesystem, bad bytesex). */
+/* The disk addresses are three-byte (despite direct block addresses being
+ * aligned word-wise in inode). If the most significant byte is non-zero,
+ * something is most likely wrong (not a filesystem, bad bytesex). */
 #define V7_MAXSIZE	0x00ffffff
 
 /* Coherent super-block data on disk */
-- 
1.6.5.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/3] [fs/sysv] V7: Adjust sanity checks for some volumes
  2010-07-19 14:54     ` Christoph Hellwig
@ 2010-07-20 10:31       ` Lubomir Rintel
  2010-07-22  1:11         ` [PATCH] " Lubomir Rintel
  0 siblings, 1 reply; 21+ messages in thread
From: Lubomir Rintel @ 2010-07-20 10:31 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Linux Kernel Mailing List, Andrew Morton

On Mon, 2010-07-19 at 16:54 +0200, Christoph Hellwig wrote:

> >  	v7i = (struct sysv_inode *)(bh2->b_data + 64);
> >  	if ((fs16_to_cpu(sbi, v7i->i_mode) & ~0777) != S_IFDIR ||
> >  	    (fs32_to_cpu(sbi, v7i->i_size) == 0) ||
> > -	    (fs32_to_cpu(sbi, v7i->i_size) & 017) != 0)
> > +	    (fs32_to_cpu(sbi, v7i->i_size) & 017)
> > +	    (fs32_to_cpu(sbi, v7i->i_size) > V7_NFILES *
> > +	     sizeof (struct sysv_dir_entry))) {
> 
> Maybe I'm missing something, but without and additional || on the
> first line you added this doesn't look like it will compile.

You're right. In fact this hunk is fixed in a commit that followed it, I
probably incorrectly merged the fixups in git (the latter commit also
had an useless comment change and such).

I'll follow up with a fixed commit tomorrow, since I'm unable to test it
now.

Thanks,
Lubo

-- 
Flash is the Web2.0 version of blink and animated gifs.
                                     -- Stephen Smoogen


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 3/3] [fs/sysv] V7: Add support for non-PDP11 v7 filesystems
  2010-07-19 14:58       ` Christoph Hellwig
@ 2010-07-20 10:41         ` Lubomir Rintel
  2010-07-22  1:17           ` Lubomir Rintel
  2010-07-24 13:51         ` [PATCH 3/3] [fs/sysv] " Al Viro
  1 sibling, 1 reply; 21+ messages in thread
From: Lubomir Rintel @ 2010-07-20 10:41 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Linux Kernel Mailing List, Andrew Morton

On Mon, 2010-07-19 at 16:58 +0200, Christoph Hellwig wrote:
> On Mon, Jul 19, 2010 at 07:16:42PM +0200, Lubomir Rintel wrote:
> > A mount-time option was added that makes it possible to override the
> > endianness and an attempt is made to autodetect it (which seems easy,
> > given the disk addresses are 3-byte.
> > 
> > No attempt is made to detect big-endian filesystems -- were there any?
> > Tested with PDP-11 v7 filesystems and PC-IX maintenance floppy.
> 
> Do you actually need the mount option?  We get away just fine with
> it for sysv filesystems.  And if not I'd be consistent and accept the
> options for both sysv and v7 filesystems.

Well, there's no reliable way to detect endiannes of a v7 filesystem
unless we do a deep check as fsck would do (there are cases where we can
be sure that a filesystem is not of a certain bytesex, which is what the
current sanity check does and what's abused for the lousy
autodetection).

In super.c it looks like xenix and sysv use a magic number which the
byte order can be determined from, thus the option would be useless
there.

Coherent seems to always use PDP-11 bytesex. I can not check at the
time, but I'm almost sure it never run on such machines (was PC and 68k
only?), so I suspect the coherent kernel might have always done the
translation to native byte order. I think I have some coherent (for PC)
floppies at home, so I can check tomorrow.

> > +	/* plausibility check on root inode: it is a directory,
> > +	   with a nonzero size that is a multiple of 16 */
> > +	if ((bh2 = sb_bread(sb, 2)) == NULL) {
> > +		return 0;
> > +	}
> 
> A little style nitpick, this should be:
> 
> 	bh2 = sb_bread(sb, 2);
> 	if (!bh)
> 		return 0;
> 

I actually did not write this myself, but merely moved from
v7_fill_super(). It would probably a good idea to keep it as it is (not
to obfuscate the changes) and fix it up in a separate commit if is
worth.

Take care,
Lubo

-- 
Flash is the Web2.0 version of blink and animated gifs.
                                     -- Stephen Smoogen


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH] V7: Adjust sanity checks for some volumes
  2010-07-20 10:31       ` Lubomir Rintel
@ 2010-07-22  1:11         ` Lubomir Rintel
  0 siblings, 0 replies; 21+ messages in thread
From: Lubomir Rintel @ 2010-07-22  1:11 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Linux Kernel Mailing List, Andrew Morton, Lubomir Rintel,
	Christoph Hellwig

Newly mkfs-ed filesystems from Seventh Edition have last modification
time set to zero, but are otherwise perfectly valid.

Also, tighten up other sanity checks to filter out most filesystems with
different bytesex than we're using.

Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 fs/sysv/super.c         |    6 ++++--
 include/linux/sysv_fs.h |   11 +++++++++++
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/fs/sysv/super.c b/fs/sysv/super.c
index bd8d141..ac7b008 100644
--- a/fs/sysv/super.c
+++ b/fs/sysv/super.c
@@ -470,7 +470,7 @@ static int v7_fill_super(struct super_block *sb, void *data, int silent)
 	v7sb = (struct v7_super_block *) bh->b_data;
 	if (fs16_to_cpu(sbi, v7sb->s_nfree) > V7_NICFREE ||
 	    fs16_to_cpu(sbi, v7sb->s_ninode) > V7_NICINOD ||
-	    fs32_to_cpu(sbi, v7sb->s_time) == 0)
+	    fs32_to_cpu(sbi, v7sb->s_fsize) > V7_MAXSIZE)
 		goto failed;
 
 	/* plausibility check on root inode: it is a directory,
@@ -480,7 +480,9 @@ static int v7_fill_super(struct super_block *sb, void *data, int silent)
 	v7i = (struct sysv_inode *)(bh2->b_data + 64);
 	if ((fs16_to_cpu(sbi, v7i->i_mode) & ~0777) != S_IFDIR ||
 	    (fs32_to_cpu(sbi, v7i->i_size) == 0) ||
-	    (fs32_to_cpu(sbi, v7i->i_size) & 017) != 0)
+	    (fs32_to_cpu(sbi, v7i->i_size) & 017) ||
+	    (fs32_to_cpu(sbi, v7i->i_size) > V7_NFILES *
+	     sizeof (struct sysv_dir_entry)))
 		goto failed;
 	brelse(bh2);
 	bh2 = NULL;
diff --git a/include/linux/sysv_fs.h b/include/linux/sysv_fs.h
index 9641130..e47d6d9 100644
--- a/include/linux/sysv_fs.h
+++ b/include/linux/sysv_fs.h
@@ -148,6 +148,17 @@ struct v7_super_block {
 	char    s_fname[6];     /* file system name */
 	char    s_fpack[6];     /* file system pack name */
 };
+/* Constants to aid sanity checking */
+/* This is not a hard limit, nor enforced by v7 kernel. It's actually just
+ * the limit used by Seventh Edition's ls, though is high enough to assume
+ * that no reasonable file system would have that much entries in root
+ * directory. Thus, if we see anything higher, we just probably got the
+ * endiannes wrong. */
+#define V7_NFILES	1024
+/* The disk addresses are three-byte (despite direct block addresses being
+ * aligned word-wise in inode). If the most significant byte is non-zero,
+ * something is most likely wrong (not a filesystem, bad bytesex). */
+#define V7_MAXSIZE	0x00ffffff
 
 /* Coherent super-block data on disk */
 #define COH_NICINOD	100	/* number of inode cache entries */
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH 3/3] [fs/sysv] V7: Add support for non-PDP11 v7 filesystems
  2010-07-20 10:41         ` Lubomir Rintel
@ 2010-07-22  1:17           ` Lubomir Rintel
  2010-07-22  1:18             ` [PATCH] " Lubomir Rintel
  0 siblings, 1 reply; 21+ messages in thread
From: Lubomir Rintel @ 2010-07-22  1:17 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Linux Kernel Mailing List, Andrew Morton

On Tue, 2010-07-20 at 12:41 +0200, Lubomir Rintel wrote:
> On Mon, 2010-07-19 at 16:58 +0200, Christoph Hellwig wrote:
... 
> > Do you actually need the mount option?  We get away just fine with
> > it for sysv filesystems.  And if not I'd be consistent and accept the
> > options for both sysv and v7 filesystems.
...
> Coherent seems to always use PDP-11 bytesex. I can not check at the
> time, but I'm almost sure it never run on such machines (was PC and 68k
> only?), so I suspect the coherent kernel might have always done the
> translation to native byte order. I think I have some coherent (for PC)
> floppies at home, so I can check tomorrow.

This was just partly correct. Coherent indeed run on PDP-11 (and not on
68k), but the PC version uses the very same bytesex as PDP-11 one,
translating the byte order on the fly (_canl() routine defined in
i386/as.inc file of Coherent 4.2.10 for i386 kernel is used).

Thus the mount option is really only useful with v7 filesystem and none
of those handled with sysv filesystem.

-- 
Flash is the Web2.0 version of blink and animated gifs.
                                     -- Stephen Smoogen


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH] V7: Add support for non-PDP11 v7 filesystems
  2010-07-22  1:17           ` Lubomir Rintel
@ 2010-07-22  1:18             ` Lubomir Rintel
  0 siblings, 0 replies; 21+ messages in thread
From: Lubomir Rintel @ 2010-07-22  1:18 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Linux Kernel Mailing List, Andrew Morton, Lubomir Rintel,
	Christoph Hellwig

A mount-time option was added that makes it possible to override the
endianness and an attempt is made to autodetect it (which seems easy,
given the disk addresses are 3-byte.

No attempt is made to detect big-endian filesystems -- were there any?
Tested with PDP-11 v7 filesystems and PC-IX maintenance floppy.

Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 fs/sysv/super.c |  116 +++++++++++++++++++++++++++++++++++++++++++-----------
 1 files changed, 92 insertions(+), 24 deletions(-)

diff --git a/fs/sysv/super.c b/fs/sysv/super.c
index ac7b008..ca4e8aa 100644
--- a/fs/sysv/super.c
+++ b/fs/sysv/super.c
@@ -24,6 +24,7 @@
 #include <linux/init.h>
 #include <linux/slab.h>
 #include <linux/buffer_head.h>
+#include <linux/parser.h>
 #include "sysv.h"
 
 /*
@@ -435,12 +436,55 @@ Ebadsize:
 	goto failed;
 }
 
-static int v7_fill_super(struct super_block *sb, void *data, int silent)
+static int v7_sanity_check(struct super_block *sb, struct buffer_head *bh)
 {
-	struct sysv_sb_info *sbi;
-	struct buffer_head *bh, *bh2 = NULL;
 	struct v7_super_block *v7sb;
 	struct sysv_inode *v7i;
+	struct buffer_head *bh2;
+	struct sysv_sb_info *sbi;
+
+	sbi = sb->s_fs_info;
+
+	/* plausibility check on superblock */
+	v7sb = (struct v7_super_block *) bh->b_data;
+	if (fs16_to_cpu(sbi, v7sb->s_nfree) > V7_NICFREE ||
+	    fs16_to_cpu(sbi, v7sb->s_ninode) > V7_NICINOD ||
+	    fs32_to_cpu(sbi, v7sb->s_fsize) > V7_MAXSIZE)
+		return 0;
+
+	/* plausibility check on root inode: it is a directory,
+	   with a nonzero size that is a multiple of 16 */
+	if ((bh2 = sb_bread(sb, 2)) == NULL) {
+		return 0;
+	}
+
+	v7i = (struct sysv_inode *)(bh2->b_data + 64);
+	if ((fs16_to_cpu(sbi, v7i->i_mode) & ~0777) != S_IFDIR ||
+	    (fs32_to_cpu(sbi, v7i->i_size) == 0) ||
+	    (fs32_to_cpu(sbi, v7i->i_size) & 017) ||
+	    (fs32_to_cpu(sbi, v7i->i_size) > V7_NFILES *
+             sizeof (struct sysv_dir_entry))) {
+		brelse(bh2);
+		return 0;
+	}
+
+	brelse(bh2);
+	return 1;
+}
+
+enum { Opt_err, Opt_bytesex_pdp, Opt_bytesex_le, Opt_bytesex_be };
+
+static const match_table_t v7_tokens = {
+	{Opt_bytesex_pdp, "bytesex=pdp"},
+	{Opt_bytesex_le, "bytesex=le"},
+	{Opt_bytesex_be, "bytesex=be"},
+	{Opt_err, NULL}
+};
+
+static int v7_fill_super(struct super_block *sb, void *data, int silent)
+{
+	struct sysv_sb_info *sbi;
+	struct buffer_head *bh;
 
 	if (440 != sizeof (struct v7_super_block))
 		panic("V7 FS: bad super-block size");
@@ -454,7 +498,6 @@ static int v7_fill_super(struct super_block *sb, void *data, int silent)
 	sbi->s_sb = sb;
 	sbi->s_block_base = 0;
 	sbi->s_type = FSTYPE_V7;
-	sbi->s_bytesex = BYTESEX_PDP;
 	sb->s_fs_info = sbi;
 	
 	sb_set_blocksize(sb, 512);
@@ -466,34 +509,59 @@ static int v7_fill_super(struct super_block *sb, void *data, int silent)
 		goto failed;
 	}
 
-	/* plausibility check on superblock */
-	v7sb = (struct v7_super_block *) bh->b_data;
-	if (fs16_to_cpu(sbi, v7sb->s_nfree) > V7_NICFREE ||
-	    fs16_to_cpu(sbi, v7sb->s_ninode) > V7_NICINOD ||
-	    fs32_to_cpu(sbi, v7sb->s_fsize) > V7_MAXSIZE)
-		goto failed;
+	if (data) {
+		substring_t args[MAX_OPT_ARGS];
+		char *p;
+
+		while ((p = strsep ((char **)&data, ",")) != NULL) {
+			int token;
+			if (!*p)
+				continue;
+			token = match_token(p, v7_tokens, args);
+			switch (token) {
+			case Opt_bytesex_pdp:
+				sbi->s_bytesex = BYTESEX_PDP;
+				break;
+			case Opt_bytesex_le:
+				sbi->s_bytesex = BYTESEX_LE;
+				break;
+			case Opt_bytesex_be:
+				sbi->s_bytesex = BYTESEX_BE;
+				break;
+			default:
+				/* An unrecognized option */
+				goto failed;
+			}
+		}
+	}
 
-	/* plausibility check on root inode: it is a directory,
-	   with a nonzero size that is a multiple of 16 */
-	if ((bh2 = sb_bread(sb, 2)) == NULL)
-		goto failed;
-	v7i = (struct sysv_inode *)(bh2->b_data + 64);
-	if ((fs16_to_cpu(sbi, v7i->i_mode) & ~0777) != S_IFDIR ||
-	    (fs32_to_cpu(sbi, v7i->i_size) == 0) ||
-	    (fs32_to_cpu(sbi, v7i->i_size) & 017) ||
-	    (fs32_to_cpu(sbi, v7i->i_size) > V7_NFILES *
-	     sizeof (struct sysv_dir_entry)))
-		goto failed;
-	brelse(bh2);
-	bh2 = NULL;
+	/* Endianness overridden by a mount option */
+	if (sbi->s_bytesex) {
+		if (!v7_sanity_check (sb, bh))
+			goto failed;
+		goto detected;
+	}
+
+	/* Try PDP-11 UNIX */
+	sbi->s_bytesex = BYTESEX_PDP;
+	if (v7_sanity_check (sb, bh))
+		goto detected;
+
+	/* Try PC/IX, v7/x86 */
+	sbi->s_bytesex = BYTESEX_LE;
+	if (v7_sanity_check (sb, bh))
+		goto detected;
 
+	goto failed;
+
+detected:
 	sbi->s_bh1 = bh;
 	sbi->s_bh2 = bh;
 	if (complete_read_super(sb, silent, 1))
 		return 0;
 
 failed:
-	brelse(bh2);
+	printk("VFS: could not find a valid V7 on %s.\n", sb->s_id);
 	brelse(bh);
 	kfree(sbi);
 	return -EINVAL;
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH 3/3] [fs/sysv] V7: Add support for non-PDP11 v7 filesystems
  2010-07-19 14:58       ` Christoph Hellwig
  2010-07-20 10:41         ` Lubomir Rintel
@ 2010-07-24 13:51         ` Al Viro
  2010-07-25 22:59           ` [PATCH] " Lubomir Rintel
  1 sibling, 1 reply; 21+ messages in thread
From: Al Viro @ 2010-07-24 13:51 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Lubomir Rintel, Linux Kernel Mailing List, Andrew Morton

On Mon, Jul 19, 2010 at 04:58:51PM +0200, Christoph Hellwig wrote:
> On Mon, Jul 19, 2010 at 07:16:42PM +0200, Lubomir Rintel wrote:
> > A mount-time option was added that makes it possible to override the
> > endianness and an attempt is made to autodetect it (which seems easy,
> > given the disk addresses are 3-byte.
> > 
> > No attempt is made to detect big-endian filesystems -- were there any?
> > Tested with PDP-11 v7 filesystems and PC-IX maintenance floppy.
> 
> Do you actually need the mount option?  We get away just fine with
> it for sysv filesystems.  And if not I'd be consistent and accept the
> options for both sysv and v7 filesystems.

Actually, it shouldn't be too hard to detect the damn thing even without magic.
Look - we always have the inode table starting at block 2, so on-disk root
inode is guaranteed to be found correctly.  Now, suppose we'd mistaken
l-e for pdp or vice versa; the half-words of i_size would get swapped.
What could pass both tests?  Suppose the right size is a * 65536 + b;
then we have: a and b are both multiples of 16 and at least one is non-zero.
So all we need is to reject root directories bigger than 1Mb.  And posted
patches do reject that (and lower than that, actually).

So I'd rather see a variant without that option.  Simply get both bh, then
try the same sanity checks with LE and PDP used for s_bytesex.  And use
one that works - we _know_ that it's impossible to have both pass at the
same time.

I'm fine with the rest of patch series as is; Lubomir, could you redo the
last one that way and resend?

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: A few V7 fs improvements
  2010-07-19 17:16 A few V7 fs improvements Lubomir Rintel
                   ` (2 preceding siblings ...)
  2010-07-19 17:16 ` [PATCH 1/3] [fs/sysv] Add v7 alias Lubomir Rintel
@ 2010-07-25  5:23 ` Artem Bityutskiy
  3 siblings, 0 replies; 21+ messages in thread
From: Artem Bityutskiy @ 2010-07-25  5:23 UTC (permalink / raw)
  To: Lubomir Rintel
  Cc: Christoph Hellwig, Linux Kernel Mailing List, Andrew Morton

On Mon, 2010-07-19 at 19:16 +0200, Lubomir Rintel wrote:
> I'm resending these once again, since I suspect they may have gotten lost
> in a spam filter due to improper MTA configuration of mine.
> 
> They basically improve usability of PC v7 filesystems; such as those of
> PC/IX.

Hi, do you by any chance know where I can find some docs about what we
have in fs/sysv ? I saw issues there, but could not fix them because I
could not find any docs. There are several flavors of FSes which fs/sysv
supports, do you have anything which makes sense out of it and describes
what exactly we support, what are the differences, what is the FS
layout, etc.

Thanks.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH] V7: Add support for non-PDP11 v7 filesystems
  2010-07-24 13:51         ` [PATCH 3/3] [fs/sysv] " Al Viro
@ 2010-07-25 22:59           ` Lubomir Rintel
  2010-07-26  0:26             ` Randy Dunlap
  2010-07-26 23:52             ` Andrew Morton
  0 siblings, 2 replies; 21+ messages in thread
From: Lubomir Rintel @ 2010-07-25 22:59 UTC (permalink / raw)
  To: Al Viro
  Cc: Linux Kernel Mailing List, Andrew Morton, Christoph Hellwig,
	Lubomir Rintel, Al Viro

This adds byte order autodetection (of PDP-11 and LE filesystems).
No attempt is made to detect big-endian filesystems -- were there any?
Tested with PDP-11 v7 filesystems and PC-IX maintenance floppy.

Cc: Christoph Hellwig <hch@lst.de>
Cc: Al Viro <viro@ZenIV.linux.org.uk>
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 fs/sysv/super.c |   74 +++++++++++++++++++++++++++++++++++++------------------
 1 files changed, 50 insertions(+), 24 deletions(-)

diff --git a/fs/sysv/super.c b/fs/sysv/super.c
index ac7b008..0433f23 100644
--- a/fs/sysv/super.c
+++ b/fs/sysv/super.c
@@ -24,6 +24,7 @@
 #include <linux/init.h>
 #include <linux/slab.h>
 #include <linux/buffer_head.h>
+#include <linux/parser.h>
 #include "sysv.h"
 
 /*
@@ -435,12 +436,46 @@ Ebadsize:
 	goto failed;
 }
 
-static int v7_fill_super(struct super_block *sb, void *data, int silent)
+static int v7_sanity_check(struct super_block *sb, struct buffer_head *bh)
 {
-	struct sysv_sb_info *sbi;
-	struct buffer_head *bh, *bh2 = NULL;
 	struct v7_super_block *v7sb;
 	struct sysv_inode *v7i;
+	struct buffer_head *bh2;
+	struct sysv_sb_info *sbi;
+
+	sbi = sb->s_fs_info;
+
+	/* plausibility check on superblock */
+	v7sb = (struct v7_super_block *) bh->b_data;
+	if (fs16_to_cpu(sbi, v7sb->s_nfree) > V7_NICFREE ||
+	    fs16_to_cpu(sbi, v7sb->s_ninode) > V7_NICINOD ||
+	    fs32_to_cpu(sbi, v7sb->s_fsize) > V7_MAXSIZE)
+		return 0;
+
+	/* plausibility check on root inode: it is a directory,
+	   with a nonzero size that is a multiple of 16 */
+	if ((bh2 = sb_bread(sb, 2)) == NULL) {
+		return 0;
+	}
+
+	v7i = (struct sysv_inode *)(bh2->b_data + 64);
+	if ((fs16_to_cpu(sbi, v7i->i_mode) & ~0777) != S_IFDIR ||
+	    (fs32_to_cpu(sbi, v7i->i_size) == 0) ||
+	    (fs32_to_cpu(sbi, v7i->i_size) & 017) ||
+	    (fs32_to_cpu(sbi, v7i->i_size) > V7_NFILES *
+             sizeof (struct sysv_dir_entry))) {
+		brelse(bh2);
+		return 0;
+	}
+
+	brelse(bh2);
+	return 1;
+}
+
+static int v7_fill_super(struct super_block *sb, void *data, int silent)
+{
+	struct sysv_sb_info *sbi;
+	struct buffer_head *bh;
 
 	if (440 != sizeof (struct v7_super_block))
 		panic("V7 FS: bad super-block size");
@@ -454,7 +489,6 @@ static int v7_fill_super(struct super_block *sb, void *data, int silent)
 	sbi->s_sb = sb;
 	sbi->s_block_base = 0;
 	sbi->s_type = FSTYPE_V7;
-	sbi->s_bytesex = BYTESEX_PDP;
 	sb->s_fs_info = sbi;
 	
 	sb_set_blocksize(sb, 512);
@@ -466,34 +500,26 @@ static int v7_fill_super(struct super_block *sb, void *data, int silent)
 		goto failed;
 	}
 
-	/* plausibility check on superblock */
-	v7sb = (struct v7_super_block *) bh->b_data;
-	if (fs16_to_cpu(sbi, v7sb->s_nfree) > V7_NICFREE ||
-	    fs16_to_cpu(sbi, v7sb->s_ninode) > V7_NICINOD ||
-	    fs32_to_cpu(sbi, v7sb->s_fsize) > V7_MAXSIZE)
-		goto failed;
+	/* Try PDP-11 UNIX */
+	sbi->s_bytesex = BYTESEX_PDP;
+	if (v7_sanity_check (sb, bh))
+		goto detected;
 
-	/* plausibility check on root inode: it is a directory,
-	   with a nonzero size that is a multiple of 16 */
-	if ((bh2 = sb_bread(sb, 2)) == NULL)
-		goto failed;
-	v7i = (struct sysv_inode *)(bh2->b_data + 64);
-	if ((fs16_to_cpu(sbi, v7i->i_mode) & ~0777) != S_IFDIR ||
-	    (fs32_to_cpu(sbi, v7i->i_size) == 0) ||
-	    (fs32_to_cpu(sbi, v7i->i_size) & 017) ||
-	    (fs32_to_cpu(sbi, v7i->i_size) > V7_NFILES *
-	     sizeof (struct sysv_dir_entry)))
-		goto failed;
-	brelse(bh2);
-	bh2 = NULL;
+	/* Try PC/IX, v7/x86 */
+	sbi->s_bytesex = BYTESEX_LE;
+	if (v7_sanity_check (sb, bh))
+		goto detected;
 
+	goto failed;
+
+detected:
 	sbi->s_bh1 = bh;
 	sbi->s_bh2 = bh;
 	if (complete_read_super(sb, silent, 1))
 		return 0;
 
 failed:
-	brelse(bh2);
+	printk("VFS: could not find a valid V7 on %s.\n", sb->s_id);
 	brelse(bh);
 	kfree(sbi);
 	return -EINVAL;
-- 
1.7.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH] V7: Add support for non-PDP11 v7 filesystems
  2010-07-25 22:59           ` [PATCH] " Lubomir Rintel
@ 2010-07-26  0:26             ` Randy Dunlap
  2010-07-26 23:52             ` Andrew Morton
  1 sibling, 0 replies; 21+ messages in thread
From: Randy Dunlap @ 2010-07-26  0:26 UTC (permalink / raw)
  To: Lubomir Rintel
  Cc: Al Viro, Linux Kernel Mailing List, Andrew Morton, Christoph Hellwig

On Mon, 26 Jul 2010 00:59:16 +0200 Lubomir Rintel wrote:

> This adds byte order autodetection (of PDP-11 and LE filesystems).
> No attempt is made to detect big-endian filesystems -- were there any?
> Tested with PDP-11 v7 filesystems and PC-IX maintenance floppy.
> 
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Al Viro <viro@ZenIV.linux.org.uk>
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> ---
>  fs/sysv/super.c |   74 +++++++++++++++++++++++++++++++++++++------------------
>  1 files changed, 50 insertions(+), 24 deletions(-)
> 
> diff --git a/fs/sysv/super.c b/fs/sysv/super.c
> index ac7b008..0433f23 100644
> --- a/fs/sysv/super.c
> +++ b/fs/sysv/super.c
> @@ -435,12 +436,46 @@ Ebadsize:
>  	goto failed;
>  }
>  
> -static int v7_fill_super(struct super_block *sb, void *data, int silent)
> +static int v7_sanity_check(struct super_block *sb, struct buffer_head *bh)
>  {
> -	struct sysv_sb_info *sbi;
> -	struct buffer_head *bh, *bh2 = NULL;
>  	struct v7_super_block *v7sb;
>  	struct sysv_inode *v7i;
> +	struct buffer_head *bh2;
> +	struct sysv_sb_info *sbi;
> +
> +	sbi = sb->s_fs_info;
> +
> +	/* plausibility check on superblock */
> +	v7sb = (struct v7_super_block *) bh->b_data;
> +	if (fs16_to_cpu(sbi, v7sb->s_nfree) > V7_NICFREE ||
> +	    fs16_to_cpu(sbi, v7sb->s_ninode) > V7_NICINOD ||
> +	    fs32_to_cpu(sbi, v7sb->s_fsize) > V7_MAXSIZE)
> +		return 0;
> +
> +	/* plausibility check on root inode: it is a directory,
> +	   with a nonzero size that is a multiple of 16 */
> +	if ((bh2 = sb_bread(sb, 2)) == NULL) {
> +		return 0;
> +	}

Don't need the braces.

> +
> +	v7i = (struct sysv_inode *)(bh2->b_data + 64);
> +	if ((fs16_to_cpu(sbi, v7i->i_mode) & ~0777) != S_IFDIR ||
> +	    (fs32_to_cpu(sbi, v7i->i_size) == 0) ||
> +	    (fs32_to_cpu(sbi, v7i->i_size) & 017) ||
> +	    (fs32_to_cpu(sbi, v7i->i_size) > V7_NFILES *
> +             sizeof (struct sysv_dir_entry))) {

	        sizeof(struct sysv_dir_entry))) {

> +		brelse(bh2);
> +		return 0;
> +	}
> +
> +	brelse(bh2);
> +	return 1;
> +}
> +
> +static int v7_fill_super(struct super_block *sb, void *data, int silent)
> +{
> +	struct sysv_sb_info *sbi;
> +	struct buffer_head *bh;
>  
>  	if (440 != sizeof (struct v7_super_block))
>  		panic("V7 FS: bad super-block size");
> @@ -454,7 +489,6 @@ static int v7_fill_super(struct super_block *sb, void *data, int silent)
>  	sbi->s_sb = sb;
>  	sbi->s_block_base = 0;
>  	sbi->s_type = FSTYPE_V7;
> -	sbi->s_bytesex = BYTESEX_PDP;
>  	sb->s_fs_info = sbi;
>  	
>  	sb_set_blocksize(sb, 512);
> @@ -466,34 +500,26 @@ static int v7_fill_super(struct super_block *sb, void *data, int silent)
>  		goto failed;
>  	}
>  
> -	/* plausibility check on superblock */
> -	v7sb = (struct v7_super_block *) bh->b_data;
> -	if (fs16_to_cpu(sbi, v7sb->s_nfree) > V7_NICFREE ||
> -	    fs16_to_cpu(sbi, v7sb->s_ninode) > V7_NICINOD ||
> -	    fs32_to_cpu(sbi, v7sb->s_fsize) > V7_MAXSIZE)
> -		goto failed;
> +	/* Try PDP-11 UNIX */
> +	sbi->s_bytesex = BYTESEX_PDP;
> +	if (v7_sanity_check (sb, bh))
> +		goto detected;
>  
> -	/* plausibility check on root inode: it is a directory,
> -	   with a nonzero size that is a multiple of 16 */
> -	if ((bh2 = sb_bread(sb, 2)) == NULL)
> -		goto failed;
> -	v7i = (struct sysv_inode *)(bh2->b_data + 64);
> -	if ((fs16_to_cpu(sbi, v7i->i_mode) & ~0777) != S_IFDIR ||
> -	    (fs32_to_cpu(sbi, v7i->i_size) == 0) ||
> -	    (fs32_to_cpu(sbi, v7i->i_size) & 017) ||
> -	    (fs32_to_cpu(sbi, v7i->i_size) > V7_NFILES *
> -	     sizeof (struct sysv_dir_entry)))
> -		goto failed;
> -	brelse(bh2);
> -	bh2 = NULL;
> +	/* Try PC/IX, v7/x86 */
> +	sbi->s_bytesex = BYTESEX_LE;
> +	if (v7_sanity_check (sb, bh))
> +		goto detected;
>  
> +	goto failed;
> +
> +detected:
>  	sbi->s_bh1 = bh;
>  	sbi->s_bh2 = bh;
>  	if (complete_read_super(sb, silent, 1))
>  		return 0;
>  
>  failed:
> -	brelse(bh2);
> +	printk("VFS: could not find a valid V7 on %s.\n", sb->s_id);

	                                    V7 fs (or V7 filesystem)

>  	brelse(bh);
>  	kfree(sbi);
>  	return -EINVAL;
> -- 


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] V7: Add support for non-PDP11 v7 filesystems
  2010-07-25 22:59           ` [PATCH] " Lubomir Rintel
  2010-07-26  0:26             ` Randy Dunlap
@ 2010-07-26 23:52             ` Andrew Morton
  2010-07-27  0:19               ` Lubomir Rintel
  1 sibling, 1 reply; 21+ messages in thread
From: Andrew Morton @ 2010-07-26 23:52 UTC (permalink / raw)
  To: Lubomir Rintel; +Cc: Al Viro, Linux Kernel Mailing List, Christoph Hellwig

On Mon, 26 Jul 2010 00:59:16 +0200
Lubomir Rintel <lkundrak@v3.sk> wrote:

> This adds byte order autodetection (of PDP-11 and LE filesystems).
> No attempt is made to detect big-endian filesystems -- were there any?
> Tested with PDP-11 v7 filesystems and PC-IX maintenance floppy.

Which kernel is this against?  My v7_fill_super() is significantly
different from yours.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] V7: Add support for non-PDP11 v7 filesystems
  2010-07-26 23:52             ` Andrew Morton
@ 2010-07-27  0:19               ` Lubomir Rintel
  2010-07-27  0:29                 ` Andrew Morton
  0 siblings, 1 reply; 21+ messages in thread
From: Lubomir Rintel @ 2010-07-27  0:19 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Al Viro, Linux Kernel Mailing List, Christoph Hellwig

On Mon, 2010-07-26 at 16:52 -0700, Andrew Morton wrote:
> On Mon, 26 Jul 2010 00:59:16 +0200
> Lubomir Rintel <lkundrak@v3.sk> wrote:
> 
> > This adds byte order autodetection (of PDP-11 and LE filesystems).
> > No attempt is made to detect big-endian filesystems -- were there any?
> > Tested with PDP-11 v7 filesystems and PC-IX maintenance floppy.
> 
> Which kernel is this against?  My v7_fill_super() is significantly
> different from yours.

This depends on [PATCH] V7: Adjust sanity checks for some volumes
<1279761108-302-1-git-send-email-lkundrak@v3.sk>.

It might be that I've messed up something in the communication so that
the dependency was not immediately clear. To be honest, I'm not very
familiar with the way reviews and adjustements to patches are tracked.
What have I done wrong, here? How am I supposed to follow up with a
change to a single patch of a multi-patch (be it just two in this case)
set?

-- 
Flash is the Web2.0 version of blink and animated gifs.
                                     -- Stephen Smoogen


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] V7: Add support for non-PDP11 v7 filesystems
  2010-07-27  0:19               ` Lubomir Rintel
@ 2010-07-27  0:29                 ` Andrew Morton
  0 siblings, 0 replies; 21+ messages in thread
From: Andrew Morton @ 2010-07-27  0:29 UTC (permalink / raw)
  To: Lubomir Rintel; +Cc: Al Viro, Linux Kernel Mailing List, Christoph Hellwig

On Tue, 27 Jul 2010 02:19:15 +0200 Lubomir Rintel <lkundrak@v3.sk> wrote:

> On Mon, 2010-07-26 at 16:52 -0700, Andrew Morton wrote:
> > On Mon, 26 Jul 2010 00:59:16 +0200
> > Lubomir Rintel <lkundrak@v3.sk> wrote:
> > 
> > > This adds byte order autodetection (of PDP-11 and LE filesystems).
> > > No attempt is made to detect big-endian filesystems -- were there any?
> > > Tested with PDP-11 v7 filesystems and PC-IX maintenance floppy.
> > 
> > Which kernel is this against?  My v7_fill_super() is significantly
> > different from yours.
> 
> This depends on [PATCH] V7: Adjust sanity checks for some volumes
> <1279761108-302-1-git-send-email-lkundrak@v3.sk>.
> 
> It might be that I've messed up something in the communication so that
> the dependency was not immediately clear. To be honest, I'm not very
> familiar with the way reviews and adjustements to patches are tracked.
> What have I done wrong, here? How am I supposed to follow up with a
> change to a single patch of a multi-patch (be it just two in this case)
> set?

ah, OK, I managed to confuse myself with
fs-sysv-v7-adjust-sanity-checks-for-some-volumes-checkpatch-fixes.patch.

This new patch triggers many warnings also:

ERROR: do not use assignment in if condition
#52: FILE: fs/sysv/super.c:456:
+	if ((bh2 = sb_bread(sb, 2)) == NULL) {

WARNING: braces {} are not necessary for single statement blocks
#52: FILE: fs/sysv/super.c:456:
+	if ((bh2 = sb_bread(sb, 2)) == NULL) {
+		return 0;
+	}

ERROR: code indent should use tabs where possible
#61: FILE: fs/sysv/super.c:465:
+             sizeof (struct sysv_dir_entry))) {$

WARNING: please, no space for starting a line, 
				excluding comments
#61: FILE: fs/sysv/super.c:465:
+             sizeof (struct sysv_dir_entry))) {$

WARNING: space prohibited between function name and open parenthesis '('
#61: FILE: fs/sysv/super.c:465:
+             sizeof (struct sysv_dir_entry))) {

WARNING: space prohibited between function name and open parenthesis '('
#97: FILE: fs/sysv/super.c:504:
+	if (v7_sanity_check (sb, bh))

WARNING: space prohibited between function name and open parenthesis '('
#115: FILE: fs/sysv/super.c:509:
+	if (v7_sanity_check (sb, bh))

WARNING: printk() should include KERN_ facility level
#128: FILE: fs/sysv/super.c:521:
+	printk("VFS: could not find a valid V7 on %s.\n", sb->s_id);

total: 2 errors, 6 warnings, 109 lines checked

./patches/v7-add-support-for-non-pdp11-v7-filesystems.patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.




--- a/fs/sysv/super.c~fs-sysv-superc-add-support-for-non-pdp11-v7-filesystems-checkpatch-fixes
+++ a/fs/sysv/super.c
@@ -453,16 +453,16 @@ static int v7_sanity_check(struct super_
 
 	/* plausibility check on root inode: it is a directory,
 	   with a nonzero size that is a multiple of 16 */
-	if ((bh2 = sb_bread(sb, 2)) == NULL) {
+	bh2 = sb_bread(sb, 2);
+	if (bh2 == NULL)
 		return 0;
-	}
 
 	v7i = (struct sysv_inode *)(bh2->b_data + 64);
 	if ((fs16_to_cpu(sbi, v7i->i_mode) & ~0777) != S_IFDIR ||
 	    (fs32_to_cpu(sbi, v7i->i_size) == 0) ||
 	    (fs32_to_cpu(sbi, v7i->i_size) & 017) ||
 	    (fs32_to_cpu(sbi, v7i->i_size) > V7_NFILES *
-             sizeof (struct sysv_dir_entry))) {
+	     sizeof(struct sysv_dir_entry))) {
 		brelse(bh2);
 		return 0;
 	}
@@ -501,12 +501,12 @@ static int v7_fill_super(struct super_bl
 
 	/* Try PDP-11 UNIX */
 	sbi->s_bytesex = BYTESEX_PDP;
-	if (v7_sanity_check (sb, bh))
+	if (v7_sanity_check(sb, bh))
 		goto detected;
 
 	/* Try PC/IX, v7/x86 */
 	sbi->s_bytesex = BYTESEX_LE;
-	if (v7_sanity_check (sb, bh))
+	if (v7_sanity_check(sb, bh))
 		goto detected;
 
 	goto failed;
@@ -518,7 +518,8 @@ detected:
 		return 0;
 
 failed:
-	printk("VFS: could not find a valid V7 on %s.\n", sb->s_id);
+	printk(KERN_ERR "VFS: could not find a valid V7 on %s.\n",
+		sb->s_id);
 	brelse(bh);
 	kfree(sbi);
 	return -EINVAL;
_


^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2010-07-27  0:29 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-07-19 17:16 A few V7 fs improvements Lubomir Rintel
2010-07-19 14:53 ` Christoph Hellwig
2010-07-19 16:10 ` Bill Davidsen
2010-07-19 17:16 ` [PATCH 1/3] [fs/sysv] Add v7 alias Lubomir Rintel
2010-07-19 14:53   ` Christoph Hellwig
2010-07-19 17:16   ` [PATCH 2/3] [fs/sysv] V7: Adjust sanity checks for some volumes Lubomir Rintel
2010-07-19 14:54     ` Christoph Hellwig
2010-07-20 10:31       ` Lubomir Rintel
2010-07-22  1:11         ` [PATCH] " Lubomir Rintel
2010-07-19 17:16     ` [PATCH 3/3] [fs/sysv] V7: Add support for non-PDP11 v7 filesystems Lubomir Rintel
2010-07-19 14:58       ` Christoph Hellwig
2010-07-20 10:41         ` Lubomir Rintel
2010-07-22  1:17           ` Lubomir Rintel
2010-07-22  1:18             ` [PATCH] " Lubomir Rintel
2010-07-24 13:51         ` [PATCH 3/3] [fs/sysv] " Al Viro
2010-07-25 22:59           ` [PATCH] " Lubomir Rintel
2010-07-26  0:26             ` Randy Dunlap
2010-07-26 23:52             ` Andrew Morton
2010-07-27  0:19               ` Lubomir Rintel
2010-07-27  0:29                 ` Andrew Morton
2010-07-25  5:23 ` A few V7 fs improvements Artem Bityutskiy

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.