From 2a8ae3f056c153d1581d012ee81593fb0636ae0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=B3=B0=E5=8F=8B?= Date: Wed, 3 Jan 2024 08:19:07 +0000 Subject: [PATCH 1/4] fix: residual fuse mountpoint after graceful shutdown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Case1: Fuse server exits in thread not main. There is possibility that process finishes before shutdown of server. 2. Case2: Fuse server exits in thread of state machine. There is possibiltiy that state machine not responses to signal catch thread. Then dead lock happens. Process exits before shutdown of server. This pr aims to seperator shutdown actions from signal catch handler. It only notifies controller. Controller exits with shutdown of fuse server. No race. No deadlock. Signed-off-by: 泰友 --- service/src/daemon.rs | 7 +++++-- src/bin/nydusd/main.rs | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/service/src/daemon.rs b/service/src/daemon.rs index 3170f41cab9..c858e05ceb8 100644 --- a/service/src/daemon.rs +++ b/service/src/daemon.rs @@ -422,13 +422,16 @@ impl DaemonController { self.fs_service.lock().unwrap().clone() } - /// Shutdown all services managed by the controller. - pub fn shutdown(&self) { + /// Notify controller shutdown + pub fn notify_shutdown(&self) { // Marking exiting state. self.active.store(false, Ordering::Release); // Signal the `run_loop()` working thread to exit. let _ = self.waker.wake(); + } + /// Shutdown all services managed by the controller. + pub fn shutdown(&self) { let daemon = self.daemon.lock().unwrap().take(); if let Some(d) = daemon { if let Err(e) = d.trigger_stop() { diff --git a/src/bin/nydusd/main.rs b/src/bin/nydusd/main.rs index e06693e7a55..fc5e4b7a6b8 100644 --- a/src/bin/nydusd/main.rs +++ b/src/bin/nydusd/main.rs @@ -720,7 +720,7 @@ mod nbd { } extern "C" fn sig_exit(_sig: std::os::raw::c_int) { - DAEMON_CONTROLLER.shutdown(); + DAEMON_CONTROLLER.notify_shutdown(); } fn main() -> Result<()> { From f4fde1c86142c445f3c541fea73ca17590cbdaab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=B3=B0=E5=8F=8B?= Date: Thu, 1 Feb 2024 08:44:34 +0000 Subject: [PATCH 2/4] fix: compatibility to image without ext table for blob cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There are scenes that cache file is smaller than expect size. Such as: 1. Nydusd 1.6 generates cache file by prefetch, which is smaller than size in boot. 2. Nydusd 2.2 generates cache file by prefetch, when image not provide ext blob tables. 3. Nydusd not have enough time to fill cache for blob. Equality check for size is too much strict for both 1.6 compatibility and 2.2 concurrency. This pr ensures blob size smaller or equal than expect size. It also truncates blob cache when smaller than expect size. Signed-off-by: 泰友 --- storage/src/cache/filecache/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/storage/src/cache/filecache/mod.rs b/storage/src/cache/filecache/mod.rs index 1e38f3b3072..fd561c8a60c 100644 --- a/storage/src/cache/filecache/mod.rs +++ b/storage/src/cache/filecache/mod.rs @@ -257,7 +257,7 @@ impl FileCacheEntry { } else { blob_info.uncompressed_size() }; - if file_size == 0 { + if file_size == 0 || file_size < cached_file_size { file.set_len(cached_file_size)?; } else if cached_file_size != 0 && file_size != cached_file_size { let msg = format!( From 8df5d7f4e584424433239197138aa54598def845 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=B3=B0=E5=8F=8B?= Date: Tue, 23 Jan 2024 07:49:46 +0000 Subject: [PATCH 3/4] feat: reuse blob cache of old version MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nydus2.2 has same format for blob cache and bitmap on disk. However, 2.2 nydusd not use blob cache of 1.6 nydusd. This pr counts ready chunk number for both 1.6 and 2.2, instead of thinking it as not filled at all. Signed-off-by: 泰友 --- smoke/tests/blobcache_test.go | 2 +- storage/src/cache/filecache/mod.rs | 2 +- storage/src/cache/state/persist_map.rs | 51 ++++++++++++-------------- 3 files changed, 26 insertions(+), 29 deletions(-) diff --git a/smoke/tests/blobcache_test.go b/smoke/tests/blobcache_test.go index 5dd10a25f88..ca5a44eabd3 100644 --- a/smoke/tests/blobcache_test.go +++ b/smoke/tests/blobcache_test.go @@ -212,7 +212,7 @@ func (a *BlobCacheTestSuite) TestGenerateBlobcache(t *testing.T) { ctx.Binary.Builder, ctx.Env.BootstrapPath, blobcacheDir, filepath.Join(ctx.Env.BlobDir, ociBlobDigest.Hex()))) - a.compareTwoFiles(t, filepath.Join(blobcacheDir, fmt.Sprintf("%s.blob.data", ociBlobDigest.Hex())), filepath.Join(ctx.Env.CacheDir, fmt.Sprintf("%s.blob.data", ociBlobDigest.Hex()))) + a.compareTwoFiles(t, filepath.Join(blobcacheDir, fmt.Sprintf("%s.blob.data", ociBlobDigest.Hex())), filepath.Join(ctx.Env.CacheDir, ociBlobDigest.Hex())) a.compareTwoFiles(t, filepath.Join(blobcacheDir, fmt.Sprintf("%s.blob.meta", ociBlobDigest.Hex())), filepath.Join(ctx.Env.CacheDir, fmt.Sprintf("%s.blob.meta", ociBlobDigest.Hex()))) } diff --git a/storage/src/cache/filecache/mod.rs b/storage/src/cache/filecache/mod.rs index fd561c8a60c..88485c32036 100644 --- a/storage/src/cache/filecache/mod.rs +++ b/storage/src/cache/filecache/mod.rs @@ -25,7 +25,7 @@ use crate::cache::{BlobCache, BlobCacheMgr}; use crate::device::{BlobFeatures, BlobInfo}; pub const BLOB_RAW_FILE_SUFFIX: &str = ".blob.raw"; -pub const BLOB_DATA_FILE_SUFFIX: &str = ".blob.data"; +pub const BLOB_DATA_FILE_SUFFIX: &str = ""; /// An implementation of [BlobCacheMgr](../trait.BlobCacheMgr.html) to improve performance by /// caching uncompressed blob with local storage. diff --git a/storage/src/cache/state/persist_map.rs b/storage/src/cache/state/persist_map.rs index bf434174607..bfe39734d0f 100644 --- a/storage/src/cache/state/persist_map.rs +++ b/storage/src/cache/state/persist_map.rs @@ -111,36 +111,33 @@ impl PersistMap { } let header = filemap.get_mut::
(0)?; - let mut not_ready_count = chunk_count; - if header.version >= 1 { - if header.magic2 != MAGIC2 { - return Err(einval!(format!( - "invalid blob chunk_map file header: {:?}", - filename - ))); + if header.version >= 1 && header.magic2 != MAGIC2 { + return Err(einval!(format!( + "invalid blob chunk_map file header: {:?}", + filename + ))); + } + let not_ready_count = if new_content { + chunk_count + } else if header.version >= 1 && header.all_ready == MAGIC_ALL_READY { + 0 + } else { + let mut ready_count = 0; + for idx in HEADER_SIZE..expected_size as usize { + let current = filemap.get_ref::(idx)?; + let val = current.load(Ordering::Acquire); + ready_count += val.count_ones() as u32; } - if header.all_ready == MAGIC_ALL_READY { - not_ready_count = 0; - } else if new_content { - not_ready_count = chunk_count; - } else { - let mut ready_count = 0; - for idx in HEADER_SIZE..expected_size as usize { - let current = filemap.get_ref::(idx)?; - let val = current.load(Ordering::Acquire); - ready_count += val.count_ones() as u32; - } - if ready_count >= chunk_count { - let header = filemap.get_mut::
(0)?; - header.all_ready = MAGIC_ALL_READY; - let _ = file.sync_all(); - not_ready_count = 0; - } else { - not_ready_count = chunk_count - ready_count; - } + if ready_count >= chunk_count { + let header = filemap.get_mut::
(0)?; + header.all_ready = MAGIC_ALL_READY; + let _ = file.sync_all(); + 0 + } else { + chunk_count - ready_count } - } + }; readahead(file.as_raw_fd(), 0, expected_size); if !persist { From aa0236d51dde0b56e0e43a274e1de003eca19388 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=B3=B0=E5=8F=8B?= Date: Mon, 4 Mar 2024 10:02:36 +0000 Subject: [PATCH 4/4] fix: bad read by wrong data region MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit User io may involve discontinuous segments in different chunks. Bad read is produced by merging them into continuous one. That is what Region does. This pr separate discontinuous segments into different regions, avoiding merging forcibly. Signed-off-by: 泰友 --- storage/src/cache/cachedfile.rs | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/storage/src/cache/cachedfile.rs b/storage/src/cache/cachedfile.rs index 4122384a250..d30bcb1762b 100644 --- a/storage/src/cache/cachedfile.rs +++ b/storage/src/cache/cachedfile.rs @@ -1662,6 +1662,20 @@ impl FileIoMergeState { tag: BlobIoTag, chunk: Option>, ) -> Result<()> { + // Make sure user io of same region continuous + if !self.regions.is_empty() && self.joinable(region_type) { + let region = &self.regions[self.regions.len() - 1]; + if !region.seg.is_empty() && tag.is_user_io() { + if let BlobIoTag::User(ref seg) = tag { + if seg.offset as u64 + start + != region.blob_address + region.seg.offset as u64 + region.seg.len as u64 + { + self.commit(); + } + } + } + } + if self.regions.is_empty() || !self.joinable(region_type) { self.regions.push(Region::new(region_type)); self.last_region_joinable = true; @@ -1793,7 +1807,7 @@ mod tests { let tag = BlobIoTag::User(BlobIoSegment { offset: 0x1800, - len: 0x1800, + len: 0x800, }); state .push(RegionType::CacheFast, 0x1000, 0x2000, tag, None) @@ -1810,8 +1824,8 @@ mod tests { assert_eq!(state.regions.len(), 1); let tag = BlobIoTag::User(BlobIoSegment { - offset: 0x0000, - len: 0x2000, + offset: 0x0001, + len: 0x1fff, }); state .push(RegionType::CacheSlow, 0x5000, 0x2000, tag, None)