linux-kernel - Re: [PATCH 01/10] fs: turn inode ctime fields into a single ktime

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4ec1fbdc6568e16da40f41789081805e764fd83e.camel@kernel.org>
Date: Tue, 02 Jul 2024 11:58:02 -0400
From: Jeff Layton <jlayton@...nel.org>
To: Christoph Hellwig <hch@...radead.org>
Cc: Jan Kara <jack@...e.cz>, "Darrick J. Wong" <djwong@...nel.org>, 
 Alexander Viro <viro@...iv.linux.org.uk>, Christian Brauner
 <brauner@...nel.org>, Steven Rostedt <rostedt@...dmis.org>, Masami
 Hiramatsu <mhiramat@...nel.org>, Mathieu Desnoyers
 <mathieu.desnoyers@...icios.com>, Chandan Babu R <chandan.babu@...cle.com>,
 Theodore Ts'o <tytso@....edu>, Andreas Dilger <adilger.kernel@...ger.ca>,
 Chris Mason <clm@...com>, Josef Bacik <josef@...icpanda.com>, David Sterba
 <dsterba@...e.com>,  Hugh Dickins <hughd@...gle.com>, Andrew Morton
 <akpm@...ux-foundation.org>, kernel-team@...com, 
 linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org, 
 linux-trace-kernel@...r.kernel.org, linux-xfs@...r.kernel.org, 
 linux-ext4@...r.kernel.org, linux-btrfs@...r.kernel.org,
 linux-mm@...ck.org,  linux-nfs@...r.kernel.org
Subject: Re: [PATCH 01/10] fs: turn inode ctime fields into a single ktime_t

On Tue, 2024-07-02 at 08:12 -0700, Christoph Hellwig wrote:
> On Tue, Jul 02, 2024 at 08:21:42AM -0400, Jeff Layton wrote:
> > Many of the existing callers of inode_ctime_to_ts are in void
> > return
> > functions. They're just copying data from an internal
> > representation to
> > struct inode and assume it always succeeds. For those we'll
> > probably
> > have to catch bad ctime values earlier.
> > 
> > So, I think I'll probably have to roll bespoke error handling in
> > all of
> > the relevant filesystems if we go this route. There are also
> > differences between filesystems -- does it make sense to refuse to
> > load
> > an inode with a bogus ctime on NFS or AFS? Probably not.
> > 
> > Hell, it may be simpler to just ditch this patch and reimplement
> > mgtimes using the nanosecond fields like the earlier versions did.
> 
> Thatdoes for sure sound simpler.  What is the big advantage of the
> ktime_t?  Smaller size?
> 

Yeah, mostly. We shrink struct inode by 8 bytes with that patch, and we
(probably) get a better cache footprint, since i_version ends up in the
same cacheline as the ctime. That's really a separate issue though, so
I'm not too worked up about dropping that patch.

As a bonus, leaving it split across separate fields means that we can
use unused bits in the nsec field for the flag, so we don't need to
sacrifice any timestamp granularity either.

I've got a draft rework that does this that I'm testing now. Assuming
it works OK, I'll resend in a few days.

Thanks for the feedback!
-- 
Jeff Layton <jlayton@...nel.org>