public inbox for ecos-maintainers@sourceware.org
 help / color / mirror / Atom feed
* FWD: JFFS2 : Node info mutex fix suggestion.
@ 2004-02-16 17:07 Andrew Lunn
  2004-02-17 14:11 ` Gary Thomas
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Lunn @ 2004-02-16 17:07 UTC (permalink / raw)
  To: eCos Maintainers, dwmw2

Hi Folks

Does anybody know how locking should work in the jffs2 package? 

To me this patch looks the wrong solution. 

   Thanks
        Andrew

----- Forwarded message from Alf Nilsson <alf.nilsson@abem.se> -----

Date: Thu, 12 Feb 2004 17:19:31 +0100
From: Alf Nilsson <alf.nilsson@abem.se>
To: ecos-patches@sources.redhat.com
Subject: JFFS2 : Node info mutex fix suggestion.
X-Spam-Status: No, hits=-4.9 required=5.0 tests=BAYES_00 autolearn=ham 
	version=2.63

When running a project with priority inheritance we noticed that all the 
threads that passed though JFFS did not return to their original 
priority. This due to the mutex count in the jffs2_inode_info was in 
some cases 2 as lowest. We found that this depended on a call to 
fs-ecos.c:jffs2_init_inode_info in fs-ecos.c:jffs2_new_inode. The 2nd 
call in fs-ecos.c:jffs2_new_inode that increases the mutex occurs in 
read.c:jffs2_do_new_inode.
This might not be the ultimate solution, but it seemed like the right 
place to place the patch, but I'm open for suggestions.

Regards,
Alf Nilsson

Index: current/ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/fs/jffs2/current/ChangeLog,v
retrieving revision 1.24
diff -u -w -p -r1.24 ChangeLog
--- current/ChangeLog	27 Jan 2004 10:49:06 -0000	1.24
+++ current/ChangeLog	12 Feb 2004 16:01:10 -0000
@@ -1,3 +1,10 @@
+2004-02-12  Alf Nilsson  <alf.nilsson@abem.se>
+
+	* src/fs-ecos.c: Removed call to jffs2_init_inode_info in
+	jffs2_new_inode. The call resulted in the mutex in the inode_info
+	being locked twice. The 2nd lock is in the call to 
+	jffs2_do_new_inode.
+
 2004-01-09  Thomas Koeller  <thomas.koeller@baslerweb.com>
 
 	* src/fs-ecos.c: Fixed inode reference counting in jffs2_ops_link().
Index: current/src/fs-ecos.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/fs/jffs2/current/src/fs-ecos.c,v
retrieving revision 1.17
diff -u -w -p -r1.17 fs-ecos.c
--- current/src/fs-ecos.c	27 Jan 2004 10:49:06 -0000	1.17
+++ current/src/fs-ecos.c	12 Feb 2004 16:01:11 -0000
@@ -1849,7 +1849,7 @@ struct _inode *jffs2_new_inode (struct _
 		return ERR_PTR(-ENOMEM);
 
 	f = JFFS2_INODE_INFO(inode);
-	jffs2_init_inode_info(f);
+	memset(f, 0, sizeof(*f));
 
 	memset(ri, 0, sizeof(*ri));
 	/* Set OS-specific defaults for new inodes */


----- End forwarded message -----

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

* Re: FWD: JFFS2 : Node info mutex fix suggestion.
  2004-02-16 17:07 FWD: JFFS2 : Node info mutex fix suggestion Andrew Lunn
@ 2004-02-17 14:11 ` Gary Thomas
  2004-02-17 14:58   ` David Woodhouse
  0 siblings, 1 reply; 6+ messages in thread
From: Gary Thomas @ 2004-02-17 14:11 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: eCos Maintainers, dwmw2

On Mon, 2004-02-16 at 10:07, Andrew Lunn wrote:
> Hi Folks
> 
> Does anybody know how locking should work in the jffs2 package? 
> 
> To me this patch looks the wrong solution. 

I agree.  I did already pass this to David, with no reply :-(

> 
>    Thanks
>         Andrew
> 
> ----- Forwarded message from Alf Nilsson <alf.nilsson@abem.se> -----
> 
> Date: Thu, 12 Feb 2004 17:19:31 +0100
> From: Alf Nilsson <alf.nilsson@abem.se>
> To: ecos-patches@sources.redhat.com
> Subject: JFFS2 : Node info mutex fix suggestion.
> X-Spam-Status: No, hits=-4.9 required=5.0 tests=BAYES_00 autolearn=ham 
> 	version=2.63
> 
> When running a project with priority inheritance we noticed that all the 
> threads that passed though JFFS did not return to their original 
> priority. This due to the mutex count in the jffs2_inode_info was in 
> some cases 2 as lowest. We found that this depended on a call to 
> fs-ecos.c:jffs2_init_inode_info in fs-ecos.c:jffs2_new_inode. The 2nd 
> call in fs-ecos.c:jffs2_new_inode that increases the mutex occurs in 
> read.c:jffs2_do_new_inode.
> This might not be the ultimate solution, but it seemed like the right 
> place to place the patch, but I'm open for suggestions.
> 
> Regards,
> Alf Nilsson
> 
> Index: current/ChangeLog
> ===================================================================
> RCS file: /cvs/ecos/ecos/packages/fs/jffs2/current/ChangeLog,v
> retrieving revision 1.24
> diff -u -w -p -r1.24 ChangeLog
> --- current/ChangeLog	27 Jan 2004 10:49:06 -0000	1.24
> +++ current/ChangeLog	12 Feb 2004 16:01:10 -0000
> @@ -1,3 +1,10 @@
> +2004-02-12  Alf Nilsson  <alf.nilsson@abem.se>
> +
> +	* src/fs-ecos.c: Removed call to jffs2_init_inode_info in
> +	jffs2_new_inode. The call resulted in the mutex in the inode_info
> +	being locked twice. The 2nd lock is in the call to 
> +	jffs2_do_new_inode.
> +
>  2004-01-09  Thomas Koeller  <thomas.koeller@baslerweb.com>
>  
>  	* src/fs-ecos.c: Fixed inode reference counting in jffs2_ops_link().
> Index: current/src/fs-ecos.c
> ===================================================================
> RCS file: /cvs/ecos/ecos/packages/fs/jffs2/current/src/fs-ecos.c,v
> retrieving revision 1.17
> diff -u -w -p -r1.17 fs-ecos.c
> --- current/src/fs-ecos.c	27 Jan 2004 10:49:06 -0000	1.17
> +++ current/src/fs-ecos.c	12 Feb 2004 16:01:11 -0000
> @@ -1849,7 +1849,7 @@ struct _inode *jffs2_new_inode (struct _
>  		return ERR_PTR(-ENOMEM);
>  
>  	f = JFFS2_INODE_INFO(inode);
> -	jffs2_init_inode_info(f);
> +	memset(f, 0, sizeof(*f));
>  
>  	memset(ri, 0, sizeof(*ri));
>  	/* Set OS-specific defaults for new inodes */
> 
> 
> ----- End forwarded message -----
-- 
Gary Thomas <gary@mlbassoc.com>
MLB Associates

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

* Re: FWD: JFFS2 : Node info mutex fix suggestion.
  2004-02-17 14:11 ` Gary Thomas
@ 2004-02-17 14:58   ` David Woodhouse
  2004-02-17 15:09     ` Gary Thomas
  0 siblings, 1 reply; 6+ messages in thread
From: David Woodhouse @ 2004-02-17 14:58 UTC (permalink / raw)
  To: Gary Thomas; +Cc: Andrew Lunn, eCos Maintainers

On Tue, 2004-02-17 at 07:11 -0700, Gary Thomas wrote:
> On Mon, 2004-02-16 at 10:07, Andrew Lunn wrote:
> > Hi Folks
> > 
> > Does anybody know how locking should work in the jffs2 package? 
> > 
> > To me this patch looks the wrong solution. 
> 
> I agree.  I did already pass this to David, with no reply :-(

Hmmm. I think the answer is to remove the one in jffs2_do_new_inode()
instead.

That function should rely on the OS-specific code to pass it a struct
jffs2_inode_info which is initialised correctly, and that includes
having a locked semaphore in it.

I've committed the appropriate change to my CVS tree.

-- 
dwmw2

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

* Re: FWD: JFFS2 : Node info mutex fix suggestion.
  2004-02-17 14:58   ` David Woodhouse
@ 2004-02-17 15:09     ` Gary Thomas
  2004-02-17 15:42       ` Gary Thomas
  0 siblings, 1 reply; 6+ messages in thread
From: Gary Thomas @ 2004-02-17 15:09 UTC (permalink / raw)
  To: David Woodhouse; +Cc: Andrew Lunn, eCos Maintainers

On Tue, 2004-02-17 at 07:58, David Woodhouse wrote:
> On Tue, 2004-02-17 at 07:11 -0700, Gary Thomas wrote:
> > On Mon, 2004-02-16 at 10:07, Andrew Lunn wrote:
> > > Hi Folks
> > > 
> > > Does anybody know how locking should work in the jffs2 package? 
> > > 
> > > To me this patch looks the wrong solution. 
> > 
> > I agree.  I did already pass this to David, with no reply :-(
> 
> Hmmm. I think the answer is to remove the one in jffs2_do_new_inode()
> instead.
> 
> That function should rely on the OS-specific code to pass it a struct
> jffs2_inode_info which is initialised correctly, and that includes
> having a locked semaphore in it.
> 
> I've committed the appropriate change to my CVS tree.

Thanks, I'll merge this to the eCos repository.  There are also some 
changes in our tree (CDL stuff mostly) that need to be pushed your
way.  I'll take care of that as well.

-- 
Gary Thomas <gary@mlbassoc.com>
MLB Associates

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

* Re: FWD: JFFS2 : Node info mutex fix suggestion.
  2004-02-17 15:09     ` Gary Thomas
@ 2004-02-17 15:42       ` Gary Thomas
  2004-02-17 15:59         ` David Woodhouse
  0 siblings, 1 reply; 6+ messages in thread
From: Gary Thomas @ 2004-02-17 15:42 UTC (permalink / raw)
  To: David Woodhouse; +Cc: Andrew Lunn, eCos Maintainers

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

On Tue, 2004-02-17 at 08:09, Gary Thomas wrote:
> On Tue, 2004-02-17 at 07:58, David Woodhouse wrote:
> > On Tue, 2004-02-17 at 07:11 -0700, Gary Thomas wrote:
> > > On Mon, 2004-02-16 at 10:07, Andrew Lunn wrote:
> > > > Hi Folks
> > > > 
> > > > Does anybody know how locking should work in the jffs2 package? 
> > > > 
> > > > To me this patch looks the wrong solution. 
> > > 
> > > I agree.  I did already pass this to David, with no reply :-(
> > 
> > Hmmm. I think the answer is to remove the one in jffs2_do_new_inode()
> > instead.
> > 
> > That function should rely on the OS-specific code to pass it a struct
> > jffs2_inode_info which is initialised correctly, and that includes
> > having a locked semaphore in it.
> > 
> > I've committed the appropriate change to my CVS tree.
> 
> Thanks, I'll merge this to the eCos repository.  There are also some 
> changes in our tree (CDL stuff mostly) that need to be pushed your
> way.  I'll take care of that as well.

Done (diffs attached).

David, what magic do you use to run "syncmail" when you make CVS
changes?
-- 
Gary Thomas <gary@mlbassoc.com>
MLB Associates

[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 3220 bytes --]

? jffs2-200302041443.epk
? jffs2-200311190423.epk
? jffs2-200312111628.epk
? jffs2-200402170804.epk
Index: ChangeLog
===================================================================
RCS file: /home/cvs/mtd/fs/jffs2/ecos/ChangeLog,v
retrieving revision 1.3
diff -u -5 -p -r1.3 ChangeLog
--- ChangeLog	26 Nov 2003 22:23:08 -0000	1.3
+++ ChangeLog	17 Feb 2004 15:39:06 -0000
@@ -1,5 +1,22 @@
+2004-02-17  David Woodhouse  <dwmw2@redhat.com>
+
+	* src/fs-ecos.c:
+	Don't re-initialise the already-locked f->sem. It makes eCos unhappy. 
+
+2004-01-27  David Woodhouse  <dwmw2@redhat.com>
+
+	* src/write.c: 
+	Fix bug noted by Howard Gray; dirents belong to, and should dirty, 
+	the _parent_inode, not the child (which may be zero in the case 
+	of an unlink).
+
+004-01-05  Thomas Koeller  <thomas.koeller@baslerweb.com>
+
+	* cdl/jffs2.cdl: Re-added CYGPKG_FS_JFFS2_CFLAGS_REMOVE that had been
+	removed by previous change.
+	
 2003-11-26  David Woodhouse  <dwmw2@redhat.com>
 
 	JFFS2 cleanup and import of newer code. Remove last vestiges of
 	Linuxisms such as 'struct inode' from the core code, leaving eCos
 	with no excuse, and renaming the eCos 'struct inode' to make that
Index: mkepk.sh
===================================================================
RCS file: /home/cvs/mtd/fs/jffs2/ecos/mkepk.sh,v
retrieving revision 1.6
diff -u -5 -p -r1.6 mkepk.sh
--- mkepk.sh	3 Dec 2003 09:40:09 -0000	1.6
+++ mkepk.sh	17 Feb 2004 15:39:07 -0000
@@ -5,11 +5,11 @@ EPK_TMPDIR=/tmp/jffs2-epk-$$
 MTDDIR=`dirname $0`/../../..
 
 COREFILES="build.c compr_zlib.c LICENCE scan.c compr.c gc.c pushpull.h compr_rtime.c histo.h nodelist.c read.c write.c compr_rubin.c erase.c histo_mips.h nodelist.h readinode.c compr_rubin.h nodemgmt.c"
 ECOSFILES="os-ecos.h flashio.c gcthread.c dir-ecos.c  fs-ecos.c  malloc-ecos.c"
 INCFILES="jffs2.h jffs2_fs_i.h jffs2_fs_sb.h"
-DOCFILES="README.Locking CONTRIBUTORS TODO ecos/doc/TODO.eCos ecos/doc/readme.txt"
+DOCFILES="README.Locking TODO ecos/doc/TODO.eCos ecos/doc/readme.txt"
 mkdir $EPK_TMPDIR || exit 1
 
 mkdir -p $EPK_TMPDIR/fs/jffs2/current/src
 mkdir -p $EPK_TMPDIR/fs/jffs2/current/cdl
 mkdir -p $EPK_TMPDIR/fs/jffs2/current/include/linux
Index: cdl/jffs2.cdl
===================================================================
RCS file: /home/cvs/mtd/fs/jffs2/ecos/cdl/jffs2.cdl,v
retrieving revision 1.13
diff -u -5 -p -r1.13 jffs2.cdl
--- cdl/jffs2.cdl	29 Nov 2003 00:30:30 -0000	1.13
+++ cdl/jffs2.cdl	17 Feb 2004 15:39:07 -0000
@@ -198,10 +198,21 @@ cdl_package CYGPKG_FS_JFFS2 {
             building the JFFS2 package.
             These flags are used in addition
             to the set of global flags."
         }
 
+     cdl_option CYGPKG_FS_JFFS2_CFLAGS_REMOVE {
+         display "Suppressed compiler flags"
+         flavor  data
+         no_define
+         default_value { "" }
+         description   "
+             This option modifies the set of compiler flags for
+             building the JFFS2 package. These flags are removed from
+             the set of global flags if present."
+     }
+
     # ----------------------------------------------------------------
     # Tests
 
     cdl_option CYGPKG_FS_JFFS2_TESTS {
 	display "JFFS2 FS tests"

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

* Re: FWD: JFFS2 : Node info mutex fix suggestion.
  2004-02-17 15:42       ` Gary Thomas
@ 2004-02-17 15:59         ` David Woodhouse
  0 siblings, 0 replies; 6+ messages in thread
From: David Woodhouse @ 2004-02-17 15:59 UTC (permalink / raw)
  To: Gary Thomas; +Cc: Andrew Lunn, eCos Maintainers

On Tue, 2004-02-17 at 08:42 -0700, Gary Thomas wrote:
> David, what magic do you use to run "syncmail" when you make CVS
> changes?

Have it listed in loginfo for the module in question.

The removal of init_MUTEX_LOCKED() was in src/write.c btw, not in
src/fs-ecos.c

-- 
dwmw2

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

end of thread, other threads:[~2004-02-17 15:59 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-02-16 17:07 FWD: JFFS2 : Node info mutex fix suggestion Andrew Lunn
2004-02-17 14:11 ` Gary Thomas
2004-02-17 14:58   ` David Woodhouse
2004-02-17 15:09     ` Gary Thomas
2004-02-17 15:42       ` Gary Thomas
2004-02-17 15:59         ` David Woodhouse

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).