Is JFFS a full featured filesystem?

Is JFFS a full featured filesystem?

[Markus Thiesmeyer]

[-- Attachment #1: Type: text/plain, Size: 823 bytes --]

Hello,

I've integrated the JFFS into a 2.2.16 kernel and it works almost fine.
But I cannot use an editor like pico to create a file on a JFFS-filesystem. It leads to an serious error. Other operations like copying or moving files are no problem.
So I've written a small program which uses file-mapping to change given files. And the results were very strange: When I run this application it should open a file called map.txt and change the first bytes into the sequence "abcdef...xyz". But when I read the file after I've run that application, the file seems to be unchanged. Changes appear first when I unmount that fs and mount in again. Then the file looks like a concatenation of the unchanged and the changed version.

Is this a bug? And if it is a bug, is this a known bug?


regards

Markus Thiesmeyer

[-- Attachment #2: Type: text/html, Size: 1491 bytes --]

Re: Is JFFS a full featured filesystem?

[Philipp Rumpf]

On Tue, Aug 01, 2000 at 06:15:31PM +0200, Markus Thiesmeyer wrote:
> So I've written a small program which uses file-mapping to change given
> files.

At least on 2.3 you shouldn't be able to get a shared writable mmap of a
JFFS file.  I don't think this should be any different in 2.2.

Can you send your code ?



To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org

Re: Is JFFS a full featured filesystem?

[Alexander Larsson]

On Tue, 1 Aug 2000, Markus Thiesmeyer wrote:

> Hello,
> 
> I've integrated the JFFS into a 2.2.16 kernel and it works almost fine.
> But I cannot use an editor like pico to create a file on a JFFS-filesystem. It leads to an serious error. Other operations like copying or moving files are no problem.
> So I've written a small program which uses file-mapping to change given files. And the results were very strange: When I run this application it should open a file called map.txt and change the first bytes into the sequence "abcdef...xyz". But when I read the file after I've run that application, the file seems to be unchanged. Changes appear first when I unmount that fs and mount in again. Then the file looks like a concatenation of the unchanged and the changed version.
> 
> Is this a bug? And if it is a bug, is this a known bug?

Well, i haven't had time to work on jffs lately so i haven't talked much
about this... But, as currently implemented, writing to mmaped jffs files
is serioiusly broken. Of the address space operations only readpage is
implemented. To get a correcly working mmap implementation writepage and
prepare_write_page (or whatever it's called, don't have source handy right
now) should be implemented. This could cause consistancy problems though,
because we're not using the (possibly modified) mmaped data when writing
directly to a file, we also in some way guarantee that all writes are
done to the log in the correct order. It will also be an inefficient way
to write data to jffs, since we have to rewrite whole pages instead of
only the written part.

All this, and the fact that the missing needed mutex around the writing to
the log makes the jffs filesystem quite seriously borked. It can be used,
but it's *clearly* marked experimental in the config, and should be
treated so.

/ Alex (On vacation)




To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org

Re: Is JFFS a full featured filesystem?

[David Woodhouse]

alex@cendio.se said:
>  This could cause consistancy problems though, because we're not using
> the (possibly modified) mmaped data when writing directly to a file,
> we also in some way guarantee that all writes are done to the log in
> the correct order. It will also be an inefficient way to write data to
> jffs, since we have to rewrite whole pages instead of only the written
> part.

I believe that we can address both of those problems - we can write data 
directly from the page cache and we can also work out which ranges within
a page have actually been dirtied. 


alex@cendio.se said:
> All this, and the fact that the missing needed mutex around the
> writing to the log makes the jffs filesystem quite seriously borked.

This compiles. Wonder if it works...

Index: fs/jffs/intrep.c
===================================================================
RCS file: /home/cvs/mtd/fs/jffs/intrep.c,v
retrieving revision 1.25
diff -u -w -r1.25 intrep.c
--- fs/jffs/intrep.c	2000/07/27 19:45:35	1.25
+++ fs/jffs/intrep.c	2000/08/02 12:08:55
@@ -377,6 +377,17 @@
 	return sum;
 }
 
+static __inline__ void jffs_fm_write_lock(struct jffs_fmcontrol *fmc)
+{
+	down(&fmc->wlock);
+}
+
+static __inline__ void jffs_fm_write_unlock(struct jffs_fmcontrol *fmc)
+{
+	up(&fmc->wlock);
+}
+
+
 /* Create and initialize a new struct jffs_file.  */
 static struct jffs_file *
 jffs_create_file(struct jffs_control *c,
@@ -1373,8 +1384,11 @@
 		  (name ? name : ""), raw_inode->ino,
 		  raw_inode->version, total_size));
 
+	jffs_fm_write_lock(fmc);
+
 	/* First try to allocate some flash memory.  */
 	if ((err = jffs_fmalloc(fmc, total_size, node, &fm)) < 0) {
+		jffs_fm_write_unlock(fmc);
 		D(printk("jffs_write_node(): jffs_fmalloc(0x%p, %u) "
 			 "failed!\n", fmc, total_size));
 		return err;
@@ -1383,14 +1397,16 @@
 		/* The jffs_fm struct that we got is not good enough.
 		   Make that space dirty.  */
 		if ((err = jffs_write_dummy_node(c, fm)) < 0) {
-			D(printk("jffs_write_node(): "
-				 "jffs_write_dummy_node(): Failed!\n"));
 			kfree(fm);
 			DJM(no_jffs_fm--);
+			jffs_fm_write_unlock(fmc);
+			D(printk("jffs_write_node(): "
+				 "jffs_write_dummy_node(): Failed!\n"));
 			return err;
 		}
 		/* Get a new one.  */
 		if ((err = jffs_fmalloc(fmc, total_size, node, &fm)) < 0) {
+			jffs_fm_write_unlock(fmc);
 			D(printk("jffs_write_node(): Second "
 				 "jffs_fmalloc(0x%p, %u) failed!\n",
 				 fmc, total_size));
@@ -1427,6 +1443,7 @@
 				    sizeof(struct jffs_raw_inode))) < 0) {
 		jffs_fmfree_partly(fmc, fm,
 				   total_name_size + total_data_size);
+		jffs_fm_write_unlock(fmc);
 		printk(KERN_ERR "JFFS: jffs_write_node: Failed to write "
 		       "raw_inode.\n");
 		return err;
@@ -1439,6 +1456,7 @@
 					    (u_char *)name,
 					    raw_inode->nsize)) < 0) {
 			jffs_fmfree_partly(fmc, fm, total_data_size);
+			jffs_fm_write_unlock(fmc);
 			printk(KERN_ERR "JFFS: jffs_write_node: Failed to "
                               "write the name.\n");
 			return err;
@@ -1451,12 +1469,13 @@
 		if ((err = flash_safe_write(fmc->mtd, pos, data,
 					    raw_inode->dsize)) < 0) {
 			jffs_fmfree_partly(fmc, fm, 0);
+			jffs_fm_write_unlock(fmc);
 			printk(KERN_ERR "JFFS: jffs_write_node: Failed to "
 			       "write the data.\n");
 			return err;
 		}
 	}
-
+	jffs_fm_write_unlock(fmc);
 	D3(printk("jffs_write_node(): Leaving...\n"));
 	return raw_inode->dsize;
 } /* jffs_write_node()  */
@@ -2211,23 +2230,28 @@
 	new_node->fm_offset = sizeof(struct jffs_raw_inode)
 			      + total_name_size;
 
+	jffs_fm_write_lock(fmc);
+
 	if ((err = jffs_fmalloc(fmc, total_size, new_node, &fm)) < 0) {
+		DJM(no_jffs_node--);
+		jffs_fm_write_unlock(fmc);
 		D(printk("jffs_rewrite_data(): Failed to allocate fm.\n"));
 		kfree(new_node);
-		DJM(no_jffs_node--);
 		return err;
 	}
 	else if (!fm->nodes) {
 		/* The jffs_fm struct that we got is not good enough.  */
 		if ((err = jffs_write_dummy_node(c, fm)) < 0) {
+			DJM(no_jffs_fm--);
+			jffs_fm_write_unlock(fmc);
 			D(printk("jffs_rewrite_data(): "
 				 "jffs_write_dummy_node() Failed!\n"));
 			kfree(fm);
-			DJM(no_jffs_fm--);
 			return err;
 		}
 		/* Get a new one.  */
 		if ((err = jffs_fmalloc(fmc, total_size, node, &fm)) < 0) {
+			jffs_fm_write_unlock(fmc);
 			D(printk("jffs_rewrite_data(): Second "
 				 "jffs_fmalloc(0x%p, %u) failed!\n",
 				 fmc, total_size));
@@ -2276,10 +2300,11 @@
 				    sizeof(struct jffs_raw_inode)
 				    - sizeof(__u32)
 				    - sizeof(__u16) - sizeof(__u16))) < 0) {
-		printk(KERN_ERR "JFFS: jffs_rewrite_data: Write error during "
-		       "rewrite. (raw inode)\n");
 		jffs_fmfree_partly(fmc, fm,
 				   total_name_size + total_data_size);
+		jffs_fm_write_unlock(fmc);
+		printk(KERN_ERR "JFFS: jffs_rewrite_data: Write error during "
+		       "rewrite. (raw inode)\n");
 		return err;
 	}
 	pos += sizeof(struct jffs_raw_inode);
@@ -2291,9 +2316,10 @@
 		if ((err = flash_safe_write(fmc->mtd, pos,
 					    (u_char *)f->name,
 					    f->nsize)) < 0) {
+			jffs_fmfree_partly(fmc, fm, total_data_size);
+			jffs_fm_write_unlock(fmc);
 			printk(KERN_ERR "JFFS: jffs_rewrite_data: Write "
 			       "error during rewrite. (name)\n");
-			jffs_fmfree_partly(fmc, fm, total_data_size);
 			return err;
 		}
 		pos += total_name_size;
@@ -2315,20 +2341,22 @@
 			__u32 s = jffs_min(size, PAGE_SIZE);
 			if ((r = jffs_read_data(f, (char *)page,
 						offset, s)) < s) {
+				free_page((unsigned long)page);
+				jffs_fmfree_partly(fmc, fm, 0);
+				jffs_fm_write_unlock(fmc);
 				printk(KERN_ERR "JFFS: jffs_rewrite_data: "
 					 "jffs_read_data() "
 					 "failed! (r = %d)\n", r);
-				free_page((unsigned long)page);
-				jffs_fmfree_partly(fmc, fm, 0);
 				return -1;
 			}
 			if ((err = flash_safe_write(fmc->mtd,
 						    pos, page, r)) < 0) {
+				free_page((unsigned long)page);
+				jffs_fmfree_partly(fmc, fm, 0);
+				jffs_fm_write_unlock(fmc);
 				printk(KERN_ERR "JFFS: jffs_rewrite_data: "
 				       "Write error during rewrite. "
 				       "(data)\n");
-				free_page((unsigned long)page);
-				jffs_fmfree_partly(fmc, fm, 0);
 				return err;
 			}
 			pos += r;
@@ -2352,14 +2380,16 @@
 				&raw_inode)[JFFS_RAW_INODE_DCHKSUM_OFFSET],
 				sizeof(__u32) + sizeof(__u16)
 				+ sizeof(__u16))) < 0) {
+		jffs_fmfree_partly(fmc, fm, 0);
+		jffs_fm_write_unlock(fmc);
 		printk(KERN_ERR "JFFS: jffs_rewrite_data: Write error during "
 		       "rewrite. (checksum)\n");
-		jffs_fmfree_partly(fmc, fm, 0);
 		return err;
 	}
 
 	/* Now make the file system aware of the newly written node.  */
 	jffs_insert_node(c, f, &raw_inode, f->name, new_node);
+	jffs_fm_write_unlock(fmc);
 
 	D3(printk("jffs_rewrite_data(): Leaving...\n"));
 	return 0;
Index: fs/jffs/jffs_fm.c
===================================================================
RCS file: /home/cvs/mtd/fs/jffs/jffs_fm.c,v
retrieving revision 1.8
diff -u -w -r1.8 jffs_fm.c
--- fs/jffs/jffs_fm.c	2000/07/13 13:15:33	1.8
+++ fs/jffs/jffs_fm.c	2000/08/02 12:08:55
@@ -77,6 +77,7 @@
 	fmc->tail = 0;
 	fmc->head_extra = 0;
 	fmc->tail_extra = 0;
+	init_MUTEX(&fmc->wlock);
 	return fmc;
 }
 
Index: fs/jffs/jffs_fm.h
===================================================================
RCS file: /home/cvs/mtd/fs/jffs/jffs_fm.h,v
retrieving revision 1.3
diff -u -w -r1.3 jffs_fm.h
--- fs/jffs/jffs_fm.h	2000/07/04 16:15:42	1.3
+++ fs/jffs/jffs_fm.h	2000/08/02 12:08:55
@@ -30,7 +30,9 @@
 /* Mark the on-flash space as obsolete when appropriate.  */
 #define JFFS_MARK_OBSOLETE 0
 
-#define CONFIG_JFFS_FS_VERBOSE 0
+#if !defined(CONFIG_JFFS_FS) && !defined(CONFIG_JFFS_FS_MODULE)
+#define CONFIG_JFFS_FS_VERBOSE 1
+#endif
 
 /* How many padding bytes should be inserted between two chunks of data
    on the flash?  */
@@ -77,6 +79,7 @@
 	struct jffs_fm *tail;
 	struct jffs_fm *head_extra;
 	struct jffs_fm *tail_extra;
+	struct semaphore wlock;
 };
 
 /* Notice the two members head_extra and tail_extra in the jffs_control



--
dwmw2




To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org

Re: Is JFFS a full featured filesystem?

[David Woodhouse]

dwmw2@infradead.org said:
>  alex@cendio.se said:
> > All this, and the fact that the missing needed mutex around the
> > writing to the log makes the jffs filesystem quite seriously borked.

> This compiles. Wonder if it works...

Hmmm. I didn't really expect it to work without deadlocking - I expected
there to be some path from jffs_write_node() through jffs_fmalloc() to
jffs_garbage_collect() and hence jffs_rewrite_data() - which would attempt
to obtain the lock for a second time.

In fact, it seems that it does work, because there's no attempt made to
trigger immediate GC if jffs_write_node() finds that there's not enough
space. Am I on crack or is this really the case?


--
dwmw2




To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org

Re: Is JFFS a full featured filesystem?

[Alexander Larsson]

On Wed, 2 Aug 2000, David Woodhouse wrote:

> 
> alex@cendio.se said:
> >  This could cause consistancy problems though, because we're not using
> > the (possibly modified) mmaped data when writing directly to a file,
> > we also in some way guarantee that all writes are done to the log in
> > the correct order. It will also be an inefficient way to write data to
> > jffs, since we have to rewrite whole pages instead of only the written
> > part.
> 
> I believe that we can address both of those problems - we can write data 
> directly from the page cache and we can also work out which ranges within
> a page have actually been dirtied. 

What about the log-ordering constraints? That kinda voids any write
caching at all. It could possibly be solved using a kernel thread that
does all the flash writing (in a serialized fashion), but the caching
problems get a lot harder then.

/ Ale




To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org

Re: Is JFFS a full featured filesystem?

[David Woodhouse]

alex@cendio.se said:
> What about the log-ordering constraints? That kinda voids any write
> caching at all.

Not really. It's not that difficult, you just need a warped mind. (Mine 
isn't currently warped _enough_, but I'm learning fast :)

> It could possibly be solved using a kernel thread that
> does all the flash writing (in a serialized fashion), 

yesyesyes

> but the caching problems get a lot harder then.

Not _so_ hard that it isn't worth doing, IMHO.

--
dwmw2




To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org

Re: Is JFFS a full featured filesystem?

[Alexander Larsson]

On Wed, 2 Aug 2000, David Woodhouse wrote:

> 
> alex@cendio.se said:
> >  This could cause consistancy problems though, because we're not using
> > the (possibly modified) mmaped data when writing directly to a file,
> > we also in some way guarantee that all writes are done to the log in
> > the correct order. It will also be an inefficient way to write data to
> > jffs, since we have to rewrite whole pages instead of only the written
> > part.
> 
> I believe that we can address both of those problems - we can write data 
> directly from the page cache and we can also work out which ranges within
> a page have actually been dirtied. 

Exactly how did you plan to work out the dirty ranges? Compare with
on-flash contents?

/ Alex



To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org

Re: Is JFFS a full featured filesystem?

[David Woodhouse]

alex@cendio.se said:
>  Exactly how did you plan to work out the dirty ranges? Compare with
> on-flash contents? 

Apparently it's easy when using generic_file_write, because prepare_write 
will give you the exact range. 

When doing writable mmap() we have to do something cleverer - basically yes,
comparing with the on-flash contents. We can either do that by going and
reading the flash nodes again on writepage(), or we can keep a copy of the 
clean page in RAM before it's dirtied. 

To start with, I'm inclined just to accept the hit of the 4Kb writes, and 
let the GC combine nodes later as necessary. Comparing with old contents 
can come later. This is only going to be a problem with writable mmap(), 
which isn't supported at the moment _anyway_. The normal write() case is 
fairly simple to optimise.


--
dwmw2




To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org

Re: Is JFFS a full featured filesystem?

[Alexander Larsson]

On Wed, 2 Aug 2000, David Woodhouse wrote:

> 
> alex@cendio.se said:
> >  Exactly how did you plan to work out the dirty ranges? Compare with
> > on-flash contents? 
> 
> Apparently it's easy when using generic_file_write, because prepare_write 
> will give you the exact range. 

Ok. I'll have to look at that some day.
 
> When doing writable mmap() we have to do something cleverer - basically yes,
> comparing with the on-flash contents. We can either do that by going and
> reading the flash nodes again on writepage(), or we can keep a copy of the 
> clean page in RAM before it's dirtied. 

I vote for comparing with on-flash content. Keeping the memory
requirements down is important for embedded systems low on memory.
 
> To start with, I'm inclined just to accept the hit of the 4Kb writes, and 
> let the GC combine nodes later as necessary. Comparing with old contents 
> can come later. This is only going to be a problem with writable mmap(), 
> which isn't supported at the moment _anyway_. The normal write() case is 
> fairly simple to optimise.

I agree. The current performance could easily be kept for files that just 
grow, and generic mmap writes in a file is pretty unusual.

/ Alex



To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org

Re: Is JFFS a full featured filesystem?

[Philipp Rumpf]

On Wed, Aug 02, 2000 at 01:13:34PM +0200, Alexander Larsson wrote:
> On Tue, 1 Aug 2000, Markus Thiesmeyer wrote:
> Well, i haven't had time to work on jffs lately so i haven't talked much
> about this... But, as currently implemented, writing to mmaped jffs files
> is serioiusly broken. Of the address space operations only readpage is

Note this is for 2.2 only - 2.4 properly handles filesystems that cannot be
modified through mmaps.  I think it might be a good idea to do:

diff -ur mtd/fs/jffs/inode-v22.c mtd-prumpf/fs/jffs/inode-v22.c
--- mtd/fs/jffs/inode-v22.c	Sun Jul 30 18:07:16 2000
+++ mtd-prumpf/fs/jffs/inode-v22.c	Wed Aug  2 09:39:05 2000
@@ -1562,13 +1562,21 @@
 	return 0;
 } /* jffs_ioctl()  */
 
+/* Don't allow shared writable mmaps - we don't handle them correctly */
+static int jffs_file_mmap(struct file * file, struct vm_area_struct * vma)
+{
+	if ((vma->vm_flags & VM_SHARED) && (vma->vm_flags & VM_MAYWRITE))
+		return -EINVAL;
+
+	return generic_file_mmap(file, vma);
+} /* jffs_file_mmap() */
 
 static struct file_operations jffs_file_operations =
 {
 	read:  generic_file_read,    /* read */
 	write: jffs_file_write,      /* write */
 	ioctl: jffs_ioctl,           /* ioctl */
-	mmap:  generic_file_mmap,    /* mmap */
+	mmap:  jffs_file_mmap,       /* mmap */
 };
 

[untested, as I don't have a local 2.2 tree]
 
> implemented. To get a correcly working mmap implementation writepage and
> prepare_write_page (or whatever it's called, don't have source handy right
> now) should be implemented. This could cause consistancy problems though,

But do we _want_ one ?  It basically means files will be modified 4 KB at a
time on most systems - I think the 2.4 behaviour is nicer.

> because we're not using the (possibly modified) mmaped data when writing
> directly to a file, we also in some way guarantee that all writes are
> done to the log in the correct order. It will also be an inefficient way

I am planning on breaking up my jffs compression patch into several patches
later today - writing through the page cache is the right thing for us to
do, even if we want to keep the fully synchronous behaviour.

> All this, and the fact that the missing needed mutex around the writing to
> the log makes the jffs filesystem quite seriously borked. It can be used,

OTOH, we don't really need a complicated locking structure - the vfs takes
care of managing per-inode mutexes and all we need to do is lock actual
physical flash accesses against each other;  things are slightly more
complicated for asynchronous writes (basically the code to do the physical
write would look somewhat like this:

	if (inode_has_modifications(inode)) {
		down(&inode->i_sem);
		compress, allocate, write;
		up(&inode->i_sem);
	}
), though the advantages are very interesting.

	Philipp Rumpf


To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org

Re: Is JFFS a full featured filesystem?

[Philipp Rumpf]

On Wed, Aug 02, 2000 at 03:20:04PM +0100, David Woodhouse wrote:
> alex@cendio.se said:
> >  Exactly how did you plan to work out the dirty ranges? Compare with
> > on-flash contents? 
> 
> Apparently it's easy when using generic_file_write, because prepare_write 
> will give you the exact range. 

Both prepare_write and commit_write, so we have full control over what
happens.  The arguments to {prepare,commit}_write are offsets within the
actual page (so <= PAGE_SIZE), but page->index holds the higher bits of
the logical offset.

> When doing writable mmap() we have to do something cleverer - basically yes,
> comparing with the on-flash contents. We can either do that by going and
> reading the flash nodes again on writepage(), or we can keep a copy of the 
> clean page in RAM before it's dirtied. 

Or we can just make writable mmap()s fail - applications should handle that
case, and it seems rather easy to fuck up and corrupt an fs by not handling
it correctly.

> To start with, I'm inclined just to accept the hit of the 4Kb writes, and 
> let the GC combine nodes later as necessary. Comparing with old contents 
> can come later. This is only going to be a problem with writable mmap(), 
> which isn't supported at the moment _anyway_. The normal write() case is 

I don't see where a writable mmap() fails in 2.2 - in 2.4 it's obvious.

	Philipp


To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org

Re: Is JFFS a full featured filesystem?

[Alexander Larsson]

On Wed, 2 Aug 2000, Philipp Rumpf wrote:

> On Wed, Aug 02, 2000 at 01:13:34PM +0200, Alexander Larsson wrote:
> > On Tue, 1 Aug 2000, Markus Thiesmeyer wrote:
> > Well, i haven't had time to work on jffs lately so i haven't talked much
> > about this... But, as currently implemented, writing to mmaped jffs files
> > is serioiusly broken. Of the address space operations only readpage is
> 
> Note this is for 2.2 only - 2.4 properly handles filesystems that cannot be
> modified through mmaps.  I think it might be a good idea to do:
> 
> diff -ur mtd/fs/jffs/inode-v22.c mtd-prumpf/fs/jffs/inode-v22.c
> --- mtd/fs/jffs/inode-v22.c	Sun Jul 30 18:07:16 2000
> +++ mtd-prumpf/fs/jffs/inode-v22.c	Wed Aug  2 09:39:05 2000
> @@ -1562,13 +1562,21 @@
>  	return 0;
>  } /* jffs_ioctl()  */
>  
> +/* Don't allow shared writable mmaps - we don't handle them correctly */
> +static int jffs_file_mmap(struct file * file, struct vm_area_struct * vma)
> +{
> +	if ((vma->vm_flags & VM_SHARED) && (vma->vm_flags & VM_MAYWRITE))
> +		return -EINVAL;
> +
> +	return generic_file_mmap(file, vma);
> +} /* jffs_file_mmap() */

Hmm. It can't handle private writable mmaps either. If such a thing
exists.

> [untested, as I don't have a local 2.2 tree]
>  
> > implemented. To get a correcly working mmap implementation writepage and
> > prepare_write_page (or whatever it's called, don't have source handy right
> > now) should be implemented. This could cause consistancy problems though,
> 
> But do we _want_ one ?  It basically means files will be modified 4 KB at a
> time on most systems - I think the 2.4 behaviour is nicer.

Isn't it possible to do normal writes as currently done, but mmaped writes
are written 4k at a time. It should be fairly easy to detect when an
append is done.
 
> > because we're not using the (possibly modified) mmaped data when writing
> > directly to a file, we also in some way guarantee that all writes are
> > done to the log in the correct order. It will also be an inefficient way
> 
> I am planning on breaking up my jffs compression patch into several patches
> later today - writing through the page cache is the right thing for us to
> do, even if we want to keep the fully synchronous behaviour.

 Yes, i agree.
 
> > All this, and the fact that the missing needed mutex around the writing to
> > the log makes the jffs filesystem quite seriously borked. It can be used,
> 
> OTOH, we don't really need a complicated locking structure - the vfs takes
> care of managing per-inode mutexes and all we need to do is lock actual
> physical flash accesses against each other;  things are slightly more
  David has done this in cvs. Looks ok to me.

> complicated for asynchronous writes (basically the code to do the physical
> write would look somewhat like this:
> 
> 	if (inode_has_modifications(inode)) {
> 		down(&inode->i_sem);
> 		compress, allocate, write;
> 		up(&inode->i_sem);
> 	}
> ), though the advantages are very interesting.
> 
> 	Philipp Rumpf



/ Alex





To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org

Re: Is JFFS a full featured filesystem?

[Philipp Rumpf]

On Wed, Aug 02, 2000 at 08:06:42PM +0200, Alexander Larsson wrote:
> On Wed, 2 Aug 2000, Philipp Rumpf wrote:
> > On Wed, Aug 02, 2000 at 01:13:34PM +0200, Alexander Larsson wrote:
> > > On Tue, 1 Aug 2000, Markus Thiesmeyer wrote:
> > > Well, i haven't had time to work on jffs lately so i haven't talked much
> > > about this... But, as currently implemented, writing to mmaped jffs files
> > > is serioiusly broken. Of the address space operations only readpage is
> > 
> > Note this is for 2.2 only - 2.4 properly handles filesystems that cannot be
> > modified through mmaps.  I think it might be a good idea to do:
> > 
> > diff -ur mtd/fs/jffs/inode-v22.c mtd-prumpf/fs/jffs/inode-v22.c
> > --- mtd/fs/jffs/inode-v22.c	Sun Jul 30 18:07:16 2000
> > +++ mtd-prumpf/fs/jffs/inode-v22.c	Wed Aug  2 09:39:05 2000
> > @@ -1562,13 +1562,21 @@
> >  	return 0;
> >  } /* jffs_ioctl()  */
> >  
> > +/* Don't allow shared writable mmaps - we don't handle them correctly */
> > +static int jffs_file_mmap(struct file * file, struct vm_area_struct * vma)
> > +{
> > +	if ((vma->vm_flags & VM_SHARED) && (vma->vm_flags & VM_MAYWRITE))
> > +		return -EINVAL;
> > +
> > +	return generic_file_mmap(file, vma);
> > +} /* jffs_file_mmap() */
> 
> Hmm. It can't handle private writable mmaps either. If such a thing
> exists.

It doesn't ?  A private writable mapping means all the changes the mmap()ing
process does get thrown away after the munmap, so it should work fine as long
as readpage works.

> > But do we _want_ one ?  It basically means files will be modified 4 KB at a
> > time on most systems - I think the 2.4 behaviour is nicer.
> 
> Isn't it possible to do normal writes as currently done, but mmaped writes
> are written 4k at a time. It should be fairly easy to detect when an
> append is done.

Possible, sure.  I'm just not sure it makes sense.

	Philipp


To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org

Re: Is JFFS a full featured filesystem?

[Alexander Larsson]

On Wed, 2 Aug 2000, Philipp Rumpf wrote:

> On Wed, Aug 02, 2000 at 08:06:42PM +0200, Alexander Larsson wrote:
> > Hmm. It can't handle private writable mmaps either. If such a thing
> > exists.
> 
> It doesn't ?  A private writable mapping means all the changes the mmap()ing
> process does get thrown away after the munmap, so it should work fine as long
> as readpage works.

I just saw that on l-k, sorry. Just ignore my ignorant rantings.

/ Alex



To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org