From: Nick Garnett <nickg@ecoscentric.com>
To: ecos-patches@ecos.sourceware.org
Subject: Fix FILEIO sychronization
Date: Fri, 12 Dec 2008 10:53:00 -0000 [thread overview]
Message-ID: <m31vwdbr84.fsf@xl5.calivar.com> (raw)
Checked in as noted in discuss list.
Index: ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/io/fileio/current/ChangeLog,v
retrieving revision 1.68
diff -u -5 -r1.68 ChangeLog
--- ChangeLog 1 May 2008 09:31:09 -0000 1.68
+++ ChangeLog 11 Dec 2008 14:25:41 -0000
@@ -1,5 +1,12 @@
+2008-12-11 Nick Garnett <nickg@ecoscentric.com>
+
+ * src/fd.cxx (fp_ucount_dec, fd_close): Fix locking strategy so
+ that the fdlock is free when filesystem close entry is
+ called. This allows other threads to peform descriptor operations
+ during the close.
+
2008-04-02 Xinghua Yang <yxinghua@sunnorth.com.cn>
Andrew Lunn <andrew.lunn@ascom.ch>
* cdl/fileio.cdl: Use CYGPKG_FILEIO_DIRENT_DTYPE to enable/disable
d_type field of struct dirent.
Index: src/fd.cxx
===================================================================
RCS file: /cvs/ecos/ecos/packages/io/fileio/current/src/fd.cxx,v
retrieving revision 1.7
diff -u -5 -r1.7 fd.cxx
--- src/fd.cxx 7 Apr 2004 11:18:54 -0000 1.7
+++ src/fd.cxx 11 Dec 2008 14:25:41 -0000
@@ -147,24 +147,45 @@
// These must all be called with the fdlock already locked.
//--------------------------------------------------------------------------
// Decrement the use count on a file object and if it goes to zero,
// close the file and deallocate the file object.
+//
+// A word on locking here: It is necessary for the filesystem
+// fo_close() function to be called with the file lock claimed, but
+// the fdlock released, to permit other threads to perform fd-related
+// operations. The original code here took the file lock and released
+// the fdlock before the call and then locked the fdlock and released
+// the file lock after. The idea was that there was no point at which
+// a lock of some sort was not held. However, if two threads are
+// running through this code simultaneously, this could lead to
+// deadlock, particularly if the filesystem's syncmode specifies fstab
+// or mtab level locking. So the code now unlocks the file lock before
+// reclaiming the fdlock. This leaves a small window where no locks
+// are held, where in theory some other thread could jump in and mess
+// things up. However, this is benign; if the other thread is
+// accessing some other file object there will be no conflict and by
+// definition no other thread can access this file object since we are
+// executing here because no file descriptors point to this file
+// object any longer. Additionally, the file object is only marked
+// free, by zeroing the f_flag field, once the fdlock has been
+// reclaimed.
static int fp_ucount_dec( cyg_file *fp )
{
int error = 0;
if( (--fp->f_ucount) <= 0 )
{
cyg_file_lock( fp, fp->f_syncmode );
+ FILEIO_MUTEX_UNLOCK(fdlock);
error = fp->f_ops->fo_close(fp);
cyg_file_unlock( fp, fp->f_syncmode );
+ FILEIO_MUTEX_LOCK(fdlock);
- if( error == 0 )
- fp->f_flag = 0;
+ fp->f_flag = 0;
}
return error;
}
@@ -178,21 +199,20 @@
cyg_file *fp;
CYG_ASSERT(((0 <= fd) && (fd<CYGNUM_FILEIO_NFD)), "fd out of range");
fp = desc[fd];
+ desc[fd] = FD_ALLOCATED;
if( fp != FD_ALLOCATED && fp != NULL)
{
// The descriptor is occupied, decrement its usecount and
// close the file if it goes zero.
error = fp_ucount_dec( fp );
}
- desc[fd] = FD_ALLOCATED;
-
return error;
}
//==========================================================================
--
Nick Garnett eCos Kernel Architect
eCosCentric Limited http://www.eCosCentric.com The eCos experts
Barnwell House, Barnwell Drive, Cambridge, UK. Tel: +44 1223 245571
Registered in England and Wales: Reg No: 4422071
reply other threads:[~2008-12-12 10:53 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=m31vwdbr84.fsf@xl5.calivar.com \
--to=nickg@ecoscentric.com \
--cc=ecos-patches@ecos.sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).