* 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).