[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2025052100-depravity-founder-6332@gregkh>
Date: Wed, 21 May 2025 13:24:36 +0200
From: Greg KH <gregkh@...uxfoundation.org>
To: Max Kellermann <max.kellermann@...os.com>
Cc: tj@...nel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] fs/kernfs: implement STATX_BTIME
On Tue, May 06, 2025 at 08:30:26PM +0200, Max Kellermann wrote:
> On Tue, May 6, 2025 at 7:44 PM Greg KH <gregkh@...uxfoundation.org> wrote:
> > You did just make this structure bigger, which has a real effect on many
> > systems (think 32bit s390 systems with 30k disks.) Are you sure this is
> > really needed?
>
> No, it's not "needed", and I understand that you question any increase
> in struct sizes.
>
> If you really worry about memory usage, kernfs has a lot of potential
> for optimizations. One simple example: embed struct kernfs_node into
> struct kobject/cgroup_file instead of having a pointer. That would
> save 8 bytes plus the overhead for a memory allocation (i.e. that
> alone would save more than my patch adds). Would you accept such an
> optimization, if I were to submit a patch?
Sure, if that saves memory that would be a good change.
> > What userspace tools want this in such that they can not determine this
> > any other way? What do they want this information for? What is going
> > to depend and require this to warrent it being added like this?
>
> We have a process that reaps empty cgroups and reads resource usage
> statistics (https://github.com/CM4all/spawn and
> https://cm4all-spawn.readthedocs.io/en/latest/#resource-accounting),
> and this process would like to know when that cgroup was created. That
> means we can measure the lifetime duration of the cgroup, and for
> example we can estimate the average CPU usage over the whole lifetime.
> Using the cgroup's btime is the natural canonical way to determine
> that time stamp, but cgroupfs doesn't implement it.
>
> Sure, our container manager could store the birth time in an xattr ...
> but that feels like a lousy kludge. If we have a concept of btime, I
> should use that.
>
> (It's okay if you don't like the patch and it doesn't get merged - as
> always, I can happily keep it in our private kernel fork. I'm only
> offering my work to everybody, because I'm a strong believer in open
> source.)
I feel like the creation time here is odd, but if others really
need/want it we could take it. I would like to see others review it
though.
> > And knowing when a device shows up in the system isn't that, sorry, the
> > kernel log shows that for you already, right?
>
> That was a theoretical example that's of no interest for me currently
> (just a side effect of my patch), but it might be interesting for
> others.
> But are you suggesting that programs should read and parse the kernel
> log for that? I don't think any program should ever do that.
Many programs read the kernel log for various reasons, that's why we
have some common macros/functions for displaying data properly there,
like the dev_err() call.
thanks,
greg k-h
Powered by blists - more mailing lists