Re: [PATCH] Fix race condition in ext2fs when remounting
Hello, James Clarke, le Thu 27 Aug 2015 17:22:11 +0100, a écrit : > On some systems, ext2fs.static would regularly hang at startup, as a > race condition meant it would process paging requests while remounting. > To fix this, libpager has been altered to allow inhibiting and resuming > its worker threads, and ext2fs uses this to inhibit paging while > remounting. Pushed, thanks! Samuel
[PATCH] Fix race condition in ext2fs when remounting
On some systems, ext2fs.static would regularly hang at startup, as a
race condition meant it would process paging requests while remounting.
To fix this, libpager has been altered to allow inhibiting and resuming
its worker threads, and ext2fs uses this to inhibit paging while
remounting.
* console/pager.c (pager_requests): New variable.
(user_pager_init): Updated call to pager_start_workers to use new
pager_requests variable.
* daemons/runsystem.sh: Removed artificial delay working around the race
condition.
* ext2fs/ext2fs.c (diskfs_reload_global_state): Call new
inhibit_ext2_pager and resume_ext2_pager functions, and leave sblock as
non-NULL so it will be munmapped.
* ext2fs/ext2fs.h (inhibit_ext2_pager,resume_ext2_pager): New functions.
* ext2fs/pager.c (file_pager_requests): New variable.
(create_disk_pager): Updated call to pager_start_workers to use new
file_pager_requests variable.
(inhibit_ext2_pager,resume_ext2_pager): New functions.
* fatfs/fatfs.h (inhibit_fat_pager,resume_fat_pager): New functions.
* fatfs/pager.c (file_pager_requests): New variable.
(create_fat_pager): Updated call to pager_start_workers to use new
file_pager_requests variable.
(inhibit_fat_pager,resume_fat_pager): New functions.
* libdiskfs/disk-pager.c (diskfs_disk_pager_requests): New variable.
(diskfs_start_disk_pager): Updated call to pager_start_workers to use
new diskfs_disk_pager_requests variable.
* libdiskfs/diskfs-pager.h (diskfs_disk_pager_requests): New variable.
* libpager/demuxer.c (struct pager_requests): Renamed struct requests to
struct pager_requests. Replaced queue with queue_in and queue_out
pointers. Added inhibit_wakeup field.
(pager_demuxer): Updated to use new queue_in/queue_out pointers. Only
wake up workers if not inhibited.
(worker_func): Updated to use new queue_in/queue_out pointers. Final
worker thread to sleep notifies the inhibit_wakeup condition variable.
(pager_start_workers): Added out parameter for the requests instance.
Allocate heap space shared by both queues. Initialise new inhibit_wakeup
condition.
(pager_inhibit_workers,pager_resume_workers): New functions.
* libpager/pager.h (struct pager_requests): Public forward definition.
(pager_start_workers): Added out parameter for the requests instance.
(pager_inhibit_workers,pager_resume_workers): New functions.
* libpager/queue.h (queue_empty): New function.
* storeio/pager.c (pager_requests): New variable.
(init_dev_paging): Updated call to pager_start_workers to use new
pager_requests variable.
---
console/pager.c | 3 +-
daemons/runsystem.sh | 3 --
ext2fs/ext2fs.c | 12 -
ext2fs/ext2fs.h | 6 +++
ext2fs/pager.c | 33 -
fatfs/fatfs.h| 2 +
fatfs/pager.c| 33 -
libdiskfs/disk-pager.c | 3 +-
libdiskfs/diskfs-pager.h | 1 +
libpager/demuxer.c | 119 ---
libpager/pager.h | 28 ++-
libpager/queue.h | 8
storeio/pager.c | 3 +-
13 files changed, 227 insertions(+), 27 deletions(-)
diff --git a/console/pager.c b/console/pager.c
index 5e13ba4..818e49d 100644
--- a/console/pager.c
+++ b/console/pager.c
@@ -42,6 +42,7 @@ struct user_pager_info
/* We need a separate bucket for the pager ports. */
static struct port_bucket *pager_bucket;
+static struct pager_requests *pager_requests;
/* Implement the pager_clear_user_data callback from the pager library. */
@@ -133,7 +134,7 @@ user_pager_init (void)
error (5, errno, Cannot create pager bucket);
/* Start libpagers worker threads. */
- err = pager_start_workers (pager_bucket);
+ err = pager_start_workers (pager_bucket, pager_requests);
if (err)
error (5, err, Cannot start pager worker threads);
}
diff --git a/daemons/runsystem.sh b/daemons/runsystem.sh
index ae25a7d..5d0ad01 100644
--- a/daemons/runsystem.sh
+++ b/daemons/runsystem.sh
@@ -118,9 +118,6 @@ esac
/hurd/mach-defpager
# This is necessary to make stat / return the correct device ids.
-# Work around a race condition (probably in the root translator).
-for i in `seq 1 10` ; do : ; done # XXX
-
fsysopts / --update --readonly
# Finally, start the actual init.
diff --git a/ext2fs/ext2fs.c b/ext2fs/ext2fs.c
index d0fdfe7..03c9eed 100644
--- a/ext2fs/ext2fs.c
+++ b/ext2fs/ext2fs.c
@@ -207,10 +207,20 @@ main (int argc, char **argv)
error_t
diskfs_reload_global_state ()
{
+ error_t err;
+
pokel_flush (global_pokel);
pager_flush (diskfs_disk_pager, 1);
- sblock = NULL;
+
+ /* libdiskfs is not responsible for inhibiting paging. */
+ err = inhibit_ext2_pager ();
+ if (err)
+return err;
+
get_hypermetadata ();
map_hypermetadata ();
+
+ resume_ext2_pager ();
+
return 0;
}
diff --git a/ext2fs/ext2fs.h b/ext2fs/ext2fs.h
index 96d8e9d..a744685 100644
--- a/ext2fs/ext2fs.h
+++ b/ext2fs/ext2fs.h
@@ -201,6 +201,12 @@ struct user_pager_info
/* Set up the disk pager. */
void
[PATCH] Fix race condition in ext2fs when remounting
On some systems, ext2fs.static would regularly hang at startup, as a
race condition meant it would process paging requests while reounting.
To fix this, libpager has been altered to allow inhibiting and resuming
its worker threads.
* console/pager.c (pager_requests): New variable.
(user_pager_init): Updated call to pager_start_workers to use new
pager_requests variable.
* daemons/runsystem.sh: Removed artificial delay working around the race
condition.
* ext2fs/ext2fs.c (diskfs_reload_global_state): Call new
inhibit_ext2_pager and resume_ext2_pager functions, and leave sblock as
non-NULL so it will be munmapped.
* ext2fs/ext2fs.h (inhibit_ext2_pager,resume_ext2_pager): New functions.
* ext2fs/pager.c (file_pager_requests): New variable.
(create_disk_pager): Updated call to pager_start_workers to use new
file_pager_requests variable.
(inhibit_ext2_pager,resume_ext2_pager): New functions.
* fatfs/fatfs.h (inhibit_fat_pager,resume_fat_pager): New functions.
* fatfs/pager.c (file_pager_requests): New variable.
(create_fat_pager): Updated call to pager_start_workers to use new
file_pager_requests variable.
(inhibit_fat_pager,resume_fat_pager): New functions.
* libdiskfs/disk-pager.c (diskfs_disk_pager_requests): New variable.
(diskfs_start_disk_pager): Updated call to pager_start_workers to use
new diskfs_disk_pager_requests variable.
* libdiskfs/diskfs-pager.h (diskfs_disk_pager_requests): New variable.
* libpager/demuxer.c (struct pager_requests): Renamed struct requests to
struct pager_requests. Replaced queue with queue_in and queue_out
pointers. Added inhibit_wakeup field.
(pager_demuxer): Updated to use new queue_in/queue_out pointers. Only
wake up workers if not inhibited.
(worker_func): Updated to use new queue_in/queue_out pointers. Final
worker thread to sleep notifies the inhibit_wakeup condition variable.
(pager_start_workers): Added out parameter for the requests instance.
Allocate heap space shared by both queues. Initialise new inhibit_wakeup
condition.
(pager_inhibit_workers,pager_resume_workers): New functions.
* libpager/pager.h (struct pager_requests): Public forward definition.
(pager_start_workers): Added out parameter for the requests instance.
(pager_inhibit_workers,pager_resume_workers): New functions.
* libpager/queue.h (queue_empty): New function.
* storeio/pager.c (pager_requests): New variable.
(init_dev_paging): Updated call to pager_start_workers to use new
pager_requests variable.
---
console/pager.c | 3 +-
daemons/runsystem.sh | 3 --
ext2fs/ext2fs.c | 12 -
ext2fs/ext2fs.h | 6 +++
ext2fs/pager.c | 29 +++-
fatfs/fatfs.h| 2 +
fatfs/pager.c| 29 +++-
libdiskfs/disk-pager.c | 3 +-
libdiskfs/diskfs-pager.h | 1 +
libpager/demuxer.c | 119 ---
libpager/pager.h | 28 ++-
libpager/queue.h | 8
storeio/pager.c | 3 +-
13 files changed, 219 insertions(+), 27 deletions(-)
diff --git a/console/pager.c b/console/pager.c
index 5e13ba4..818e49d 100644
--- a/console/pager.c
+++ b/console/pager.c
@@ -42,6 +42,7 @@ struct user_pager_info
/* We need a separate bucket for the pager ports. */
static struct port_bucket *pager_bucket;
+static struct pager_requests *pager_requests;
/* Implement the pager_clear_user_data callback from the pager library. */
@@ -133,7 +134,7 @@ user_pager_init (void)
error (5, errno, Cannot create pager bucket);
/* Start libpagers worker threads. */
- err = pager_start_workers (pager_bucket);
+ err = pager_start_workers (pager_bucket, pager_requests);
if (err)
error (5, err, Cannot start pager worker threads);
}
diff --git a/daemons/runsystem.sh b/daemons/runsystem.sh
index ae25a7d..5d0ad01 100644
--- a/daemons/runsystem.sh
+++ b/daemons/runsystem.sh
@@ -118,9 +118,6 @@ esac
/hurd/mach-defpager
# This is necessary to make stat / return the correct device ids.
-# Work around a race condition (probably in the root translator).
-for i in `seq 1 10` ; do : ; done # XXX
-
fsysopts / --update --readonly
# Finally, start the actual init.
diff --git a/ext2fs/ext2fs.c b/ext2fs/ext2fs.c
index d0fdfe7..03c9eed 100644
--- a/ext2fs/ext2fs.c
+++ b/ext2fs/ext2fs.c
@@ -207,10 +207,20 @@ main (int argc, char **argv)
error_t
diskfs_reload_global_state ()
{
+ error_t err;
+
pokel_flush (global_pokel);
pager_flush (diskfs_disk_pager, 1);
- sblock = NULL;
+
+ /* libdiskfs is not responsible for inhibiting paging. */
+ err = inhibit_ext2_pager ();
+ if (err)
+return err;
+
get_hypermetadata ();
map_hypermetadata ();
+
+ resume_ext2_pager ();
+
return 0;
}
diff --git a/ext2fs/ext2fs.h b/ext2fs/ext2fs.h
index 96d8e9d..a744685 100644
--- a/ext2fs/ext2fs.h
+++ b/ext2fs/ext2fs.h
@@ -201,6 +201,12 @@ struct user_pager_info
/* Set up the disk pager. */
void create_disk_pager (void);
+/* Inhibit the disk pager. */
Re: [PATCH] Fix race condition in ext2fs when remounting
Hi
This is me being picky about a corner case :-)
2015-07-22 19:42 GMT-03:00 James Clarke jrt...@jrtc27.com:
+error_t
+inhibit_ext2_pager (void)
+{
+ error_t err;
+
+ /* The file pager can rely on the disk pager, so inhibit the file
+ pager first. */
+
+ err = pager_inhibit_workers (file_pager_requests);
+ if (err)
+return err;
+
+ err = pager_inhibit_workers (diskfs_disk_pager_requests);
+ return err;
+}
It looks like the file pager workers will remain inhibited if the
'pager_inhibit_workers' function
fails to inhibit the disk pager. fatfs is affected by this problem too.
Should a call to 'pager_resume_workers' be inserted before returning
in case of error?
Regards
Re: [PATCH] Fix race condition in ext2fs when remounting
Perhaps; I was following what diskfs_remount does when inhibiting RPCs, which
stay inhibited on error.
James
On 23 Jul 2015, at 00:51, Diego Nieto Cid dnie...@gmail.com wrote:
Hi
This is me being picky about a corner case :-)
2015-07-22 19:42 GMT-03:00 James Clarke jrt...@jrtc27.com:
+error_t
+inhibit_ext2_pager (void)
+{
+ error_t err;
+
+ /* The file pager can rely on the disk pager, so inhibit the file
+ pager first. */
+
+ err = pager_inhibit_workers (file_pager_requests);
+ if (err)
+return err;
+
+ err = pager_inhibit_workers (diskfs_disk_pager_requests);
+ return err;
+}
It looks like the file pager workers will remain inhibited if the
'pager_inhibit_workers' function
fails to inhibit the disk pager. fatfs is affected by this problem too.
Should a call to 'pager_resume_workers' be inserted before returning
in case of error?
Regards
