Re: [PATCH v2 2/2] tmpfs: Support 64-bit inums per-sb

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Amir Goldstein writes:
+config TMPFS_INODE64
+       bool "Use 64-bit ino_t by default in tmpfs"
+       depends on TMPFS && 64BIT
+       default n
+       help
+         tmpfs has historically used only inode numbers as wide as an unsigned
+         int. In some cases this can cause wraparound, potentially resulting in
+         multiple files with the same inode number on a single device. This option
+         makes tmpfs use the full width of ino_t by default, similarly to the
+         inode64 mount option.
+
+         tmpfs mounts that are used privately by the kernel and are not visible to
+         users are unaffected.

Admins won't know what the line above means and they shouldn't care.
It adds no information, so better remove it.

Sure thing.

+
+       /*
+        * Showing inode{64,32} might be useful even if it's the system default,
+        * since then people don't have to resort to checking both here and
+        * /proc/config.gz to confirm 64-bit inums were successfully applied
+        * (which may not even exist if IKCONFIG_PROC isn't enabled).
+        *
+        * We hide it when inode64 isn't the default and we are using 32-bit
+        * inodes, since that probably just means the feature isn't even under
+        * consideration.
+        *
+        * As such:
+        *
+        *                     +-----------------+-----------------+
+        *                     | TMPFS_INODE64=y | TMPFS_INODE64=n |
+        *  +------------------+-----------------+-----------------+
+        *  | full_inums=true  | show            | show            |
+        *  | full_inums=false | show            | hide            |
+        *  +------------------+-----------------+-----------------+
+        *
+        */
+       if (IS_ENABLED(CONFIG_TMPFS_INODE64) || !sbinfo->full_inums)

Condition does not match comment - should be || sbinfo->full_inums)

Good catch! Thanks.

@@ -3915,6 +3969,7 @@ int shmem_init_fs_context(struct fs_context *fc)
        ctx->mode = 0777 | S_ISVTX;
        ctx->uid = current_fsuid();
        ctx->gid = current_fsgid();
+       ctx->full_inums = IS_ENABLED(CONFIG_TMPFS_INODE64);


This is the wrong place for this - it is also being set for the kern_mount.
Follow the lead of shmem_default_max_inodes.

Hmm, full_inums is intended to be simply ignored for SB_KERNMOUNT though, so it seems harmless, but I agree maybe it makes the intent of the code clearer to move it to a more specific place.

Thanks! I'll fix these up for v3.



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux