From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: vfs: change inode->i_ino from unsigned long to u64 Date: Wed, 04 Mar 2026 08:15:01 +1000 Message-ID: In-Reply-To: <20260302-iino-u64-v2-0-e5388800dae0@kernel.org> References: <20260302-iino-u64-v2-0-e5388800dae0@kernel.org> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Overall Series Review Subject: vfs: change inode->i_ino from unsigned long to u64 Author: Jeff Layton Patches: 166 Reviewed: 2026-03-04T08:15:01.964295 --- This is a well-structured 110-patch series by Jeff Layton that widens `struct inode`'s `i_ino` field from `unsigned long` to `u64`. The motivation is sound: on 32-bit architectures, `unsigned long` is only 32 bits, forcing filesystems like NFS, CIFS, XFS, Ceph, and FUSE to hash or fold their native 64-bit inode numbers, losing information and creating collision risks. **Strategy**: The series uses a clean three-phase approach for bisectability: 1. Patches 1-59: Introduce `kino_t` typedef (initially `unsigned long`) and `PRIino` format macro (initially `"l"`), convert all format strings and trace events to use them. 2. Patch 60: Flip `kino_t` from `unsigned long` to `u64` and `PRIino` from `"l"` to `"ll"` -- the actual type widening. 3. Patches 61-110: Replace `PRIino` with concrete `%llu`/`%llx` and `kino_t` with `u64`, then remove the typedef/macro. This is a good strategy that ensures every intermediate commit compiles warning-free on both 32-bit and 64-bit. The series touches 220 files but the vast majority of changes are mechanical format string updates. **Key concerns**: 1. **`hash()` function truncation on 32-bit**: In patch 002, the `hash()` function in `fs/inode.c` accepts `u64 hashval` but the function body operates entirely on `unsigned long` intermediates and returns `unsigned long`. On 32-bit, the upper 32 bits of a 64-bit inode number are silently truncated in the hash computation. This is not a regression (the hash table is sized for the machine's address space), but it means two inodes differing only in the upper 32 bits will hash to the same bucket on 32-bit. Since the lookup functions compare the full `u64 ino`, this is safe for correctness, just suboptimal for hash distribution. Worth a comment. 2. **ext4 arithmetic casts**: Patch 007 introduces `(unsigned int)` casts for `i_ino % ngroups` and `i_ino % s_mb_nr_global_goals`. Since ext4 inode numbers are always 32-bit (the on-disk format is `__le32`), this truncation is correct in practice, but the cast to `unsigned int` rather than `u32` is slightly inconsistent with kernel conventions. Also, the `div_u64()` usage in `fs/ext4/migrate.c` is correct and necessary since `EXT4_INODES_PER_GROUP` is `unsigned long` (32-bit on 32-bit arches), and 64-bit division by a 32-bit value needs `div_u64()` on 32-bit. 3. **UAPI/procfs format change**: Patch 004 changes `/proc/net/tcp`, `/proc/net/udp`, etc. from `%lu` to `%llu` for the inode number column. While the actual values won't change on 64-bit (same width), on 32-bit the field width could theoretically expand. Userspace parsers that use positional field extraction should be fine since the format is space-delimited, but this is a visible change worth noting in the commit message. The commit message does mention the UAPI diag struct truncation, which is good. 4. **`struct sock` packing on 32-bit**: Patch 004 widens `sk_ino` from `unsigned long` to `u64` in `struct sock`. The cover letter mentions struct inode growing by 4 bytes on 32-bit, but doesn't mention the `struct sock` growth. This is a hot networking structure and the packing impact on 32-bit should be considered. 5. **LLM attribution**: The cover letter notes "Much of this set was generated by LLM" which is honest and appropriate, and the mechanical nature of the bulk conversions makes this a reasonable use case. Overall, the series is in good shape. The approach is sound, the changes are largely mechanical and correct, and the few non-trivial changes (VFS API widening, ext4 arithmetic, audit, networking) are handled appropriately. --- Generated by Claude Code Patch Reviewer