passthrough: reduce the memory footprint of file handle

File handles used by name_to_handle_at(2) are allocated with fixed max
bytes, which results in unnecessary memory pressure.

Per the manual of open_by_handle_at(2),

"The caller can discover the required size for the file_handle
structure by making a call in which handle->handle_bytes is zero; in
this case, the call fails with the error EOVERFLOW and
handle->handle_bytes is set to indicate the required size; the caller
can then use this information to allocate a structure of the correct
size (see EXAMPLES below)."

This follows the description and allocates the needed bytes.
With this,
sizeof::<CFileHandle> drops from 136 bytes to 16 bytes.
sizeof::<InodeData> drops from 328 bytes to 120 bytes.


Also there is no need to make CFileHandle public.

Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>

Author: liubogithub

src/passthrough/file_handle.rs

@@ -9,6 +9,7 @@ use std::fmt::{Debug, Formatter};
 use std::fs::File;
 use std::io;
 use std::os::unix::io::{AsRawFd, FromRawFd, RawFd};
+use std::ptr;
 use std::sync::{RwLock, RwLockReadGuard, RwLockWriteGuard};
 
 use crate::passthrough::PassthroughFs;
@@ -17,29 +18,33 @@ use crate::passthrough::PassthroughFs;
 ///
 /// According to Linux ABI, struct file_handle has a flexible array member 'f_handle', but it's
 /// hard-coded here for simplicity.
-pub const MAX_HANDLE_SZ: usize = 128;
+//pub const MAX_HANDLE_SZ: usize = 128;
 
 #[derive(Clone, Copy)]
 #[repr(C)]
-pub(crate) struct CFileHandle {
+struct CFileHandle {
     // Size of f_handle [in, out]
-    pub(crate) handle_bytes: libc::c_uint,
+    handle_bytes: libc::c_uint,
     // Handle type [out]
-    pub(crate) handle_type: libc::c_int,
+    handle_type: libc::c_int,
     // File identifier (sized by caller) [out]
-    pub(crate) f_handle: [libc::c_char; MAX_HANDLE_SZ],
+    f_handle: *mut libc::c_char,
 }
 
 impl CFileHandle {
     fn new() -> Self {
         CFileHandle {
-            handle_bytes: MAX_HANDLE_SZ as libc::c_uint,
+            handle_bytes: 0,
             handle_type: 0,
-            f_handle: [0; MAX_HANDLE_SZ],
+            f_handle: ptr::null_mut(),
         }
     }
 }
 
+// Safe because f_handle is readonly once FileHandle is initialized.
+unsafe impl Send for CFileHandle {}
+unsafe impl Sync for CFileHandle {}
+
 impl Ord for CFileHandle {
     fn cmp(&self, other: &Self) -> Ordering {
         if self.handle_bytes != other.handle_bytes {
@@ -49,12 +54,8 @@ impl Ord for CFileHandle {
             return self.handle_type.cmp(&other.handle_type);
         }
 
-        self.f_handle
-            .iter()
-            .zip(other.f_handle.iter())
-            .map(|(x, y)| x.cmp(y))
-            .find(|&ord| ord != std::cmp::Ordering::Equal)
-            .unwrap_or(Ordering::Equal)
+        // f_handle is left to be compared by FileHandle's buf.
+        Ordering::Equal
     }
 }
 
@@ -83,10 +84,12 @@ impl Debug for CFileHandle {
 }
 
 /// Struct to maintain information for a file handle.
-#[derive(Clone, Copy, PartialOrd, Ord, PartialEq, Eq, Debug)]
+#[derive(Clone, PartialOrd, Ord, PartialEq, Eq, Debug)]
 pub struct FileHandle {
     pub(crate) mnt_id: u64,
-    pub(crate) handle: CFileHandle,
+    handle: CFileHandle,
+    // internal buffer for handle.f_handle
+    buf: Vec<libc::c_char>,
 }
 
 extern "C" {
@@ -113,6 +116,38 @@ impl FileHandle {
         let mut mount_id: libc::c_int = 0;
         let mut c_fh = CFileHandle::new();
 
+        // Per name_to_handle_at(2), the caller can discover the required size
+        // for the file_handle structure by making a call in which
+        // handle->handle_bytes is zero.  In this case, the call fails with the
+        // error EOVERFLOW and handle->handle_bytes is set to indicate the
+        // required size; the caller can then use this information to allocate a
+        // structure of the correct size.
+        let ret = unsafe {
+            name_to_handle_at(
+                dir_fd,
+                path.as_ptr(),
+                &mut c_fh,
+                &mut mount_id,
+                libc::AT_EMPTY_PATH,
+            )
+        };
+        if ret == -1 {
+            let e = io::Error::last_os_error();
+            // unwrap is safe as e is obtained from last_os_error().
+            if e.raw_os_error().unwrap() != libc::EOVERFLOW {
+                return Err(e);
+            }
+        } else {
+            return Err(io::Error::from(io::ErrorKind::InvalidData));
+        }
+
+        let needed = c_fh.handle_bytes as usize;
+        let mut buf = vec![0; needed];
+
+        // get a unsafe pointer, FileHandle takes care of freeing the memory
+        // that 'f_handle' points to.
+        c_fh.f_handle = buf.as_mut_ptr();
+
         let ret = unsafe {
             name_to_handle_at(
                 dir_fd,
@@ -126,6 +161,7 @@ impl FileHandle {
             Ok(FileHandle {
                 mnt_id: mount_id as u64,
                 handle: c_fh,
+                buf,
             })
         } else {
             let e = io::Error::last_os_error();
@@ -296,43 +332,80 @@ mod tests {
 
     #[test]
     fn test_file_handle_derives() {
-        let mut h1 = CFileHandle {
+        let mut buf1 = vec![0; 128];
+        let h1 = CFileHandle {
             handle_bytes: 128,
             handle_type: 3,
-            f_handle: [0; MAX_HANDLE_SZ],
+            f_handle: buf1.as_mut_ptr(),
         };
+        let mut fh1 = FileHandle {
+            mnt_id: 0,
+            handle: h1,
+            buf: buf1,
+        };
+
+        let mut buf2 = vec![0; 129];
         let h2 = CFileHandle {
             handle_bytes: 129,
             handle_type: 3,
-            f_handle: [0; MAX_HANDLE_SZ],
+            f_handle: buf2.as_mut_ptr(),
         };
+        let fh2 = FileHandle {
+            mnt_id: 0,
+            handle: h2,
+            buf: buf2,
+        };
+
+        let mut buf3 = vec![0; 128];
         let h3 = CFileHandle {
             handle_bytes: 128,
             handle_type: 4,
-            f_handle: [0; MAX_HANDLE_SZ],
+            f_handle: buf3.as_mut_ptr(),
+        };
+        let fh3 = FileHandle {
+            mnt_id: 0,
+            handle: h3,
+            buf: buf3,
         };
+
+        let mut buf4 = vec![1; 128];
         let h4 = CFileHandle {
             handle_bytes: 128,
-            handle_type: 4,
-            f_handle: [1; MAX_HANDLE_SZ],
+            handle_type: 3,
+            f_handle: buf4.as_mut_ptr(),
         };
-        let mut h5 = CFileHandle {
+        let fh4 = FileHandle {
+            mnt_id: 0,
+            handle: h4,
+            buf: buf4,
+        };
+
+        let mut buf5 = vec![0; 128];
+        let h5 = CFileHandle {
             handle_bytes: 128,
             handle_type: 3,
-            f_handle: [0; MAX_HANDLE_SZ],
+            f_handle: buf5.as_mut_ptr(),
+        };
+        let mut fh5 = FileHandle {
+            mnt_id: 0,
+            handle: h5,
+            buf: buf5,
         };
 
-        assert!(h1 < h2);
-        assert!(h1 != h2);
-        assert!(h1 < h3);
-        assert!(h1 != h3);
-        assert!(h1 < h4);
-        assert!(h1 != h4);
-
-        assert!(h1 == h5);
-        h1.f_handle[0] = 1;
-        assert!(h1 > h5);
-        h5.f_handle[0] = 1;
-        assert!(h1 == h5);
+        assert!(fh1 < fh2);
+        assert!(fh1 != fh2);
+        assert!(fh1 < fh3);
+        assert!(fh1 != fh3);
+        assert!(fh1 < fh4);
+        assert!(fh1 != fh4);
+
+        assert!(fh1 == fh5);
+        fh1.buf[0] = 1;
+        fh1.handle.f_handle = fh1.buf.as_mut_ptr();
+        assert!(fh1 > fh5);
+
+        fh5.buf[0] = 1;
+        fh5.handle.f_handle = fh5.buf.as_mut_ptr();
+        assert!(fh1 == fh5);
     }
 }

src/passthrough/mod.rs

@@ -67,7 +67,7 @@ impl InodeStat {
     }
 }
 
-#[derive(Clone, Copy, PartialOrd, Ord, PartialEq, Eq, Debug)]
+#[derive(Clone, PartialOrd, Ord, PartialEq, Eq, Debug)]
 enum InodeAltKey {
     Ids {
         ino: libc::ino64_t,
@@ -622,7 +622,7 @@ impl<S: BitmapSlice + Send + Sync> PassthroughFs<S> {
                 fuse::ROOT_ID,
                 file_or_handle,
                 2,
-                ids_altkey,
+                ids_altkey.clone(),
                 st.get_stat().st_mode,
             ),
             ids_altkey,
@@ -834,7 +834,9 @@ impl<S: BitmapSlice + Send + Sync> PassthroughFs<S> {
         // Note that this will always be `None` if `cfg.inode_file_handles` is false, but we only
         // really need this alt key when we do not have an `O_PATH` fd open for every inode.  So if
         // `cfg.inode_file_handles` is false, we do not need this key anyway.
-        let handle_altkey = file_or_handle.handle().map(|h| InodeAltKey::Handle(*h));
+        let handle_altkey = file_or_handle
+            .handle()
+            .map(|h| InodeAltKey::Handle((*h).clone()));
 
         Ok((file_or_handle, inode_stat, ids_altkey, handle_altkey))
     }
@@ -937,7 +939,13 @@ impl<S: BitmapSlice + Send + Sync> PassthroughFs<S> {
                     InodeMap::insert_locked(
                         inodes.deref_mut(),
                         inode,
-                        InodeData::new(inode, file_or_handle, 1, ids_altkey, st.get_stat().st_mode),
+                        InodeData::new(
+                            inode,
+                            file_or_handle,
+                            1,
+                            ids_altkey.clone(),
+                            st.get_stat().st_mode,
+                        ),
                         ids_altkey,
                         handle_altkey,
                     );