linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Miklos Szeredi <miklos@szeredi.hu>
To: David Howells <dhowells@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Linux NFS list <linux-nfs@vger.kernel.org>,
	Andreas Dilger <adilger.kernel@dilger.ca>,
	Anna Schumaker <anna.schumaker@netapp.com>,
	"Theodore Ts'o" <tytso@mit.edu>,
	Linux API <linux-api@vger.kernel.org>,
	linux-ext4@vger.kernel.org,
	Trond Myklebust <trond.myklebust@hammerspace.com>,
	Ian Kent <raven@themaw.net>, Miklos Szeredi <mszeredi@redhat.com>,
	Christian Brauner <christian@brauner.io>,
	Jann Horn <jannh@google.com>,
	"Darrick J. Wong" <darrick.wong@oracle.com>,
	Karel Zak <kzak@redhat.com>, Jeff Layton <jlayton@redhat.com>,
	linux-fsdevel@vger.kernel.org,
	LSM <linux-security-module@vger.kernel.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 00/13] VFS: Filesystem information [ver #19]
Date: Thu, 19 Mar 2020 13:36:58 +0100	[thread overview]
Message-ID: <CAJfpegv-_ai1LiW6=D+AnkozzmmXbB8=g8QDCS15bh==Wn3yoA@mail.gmail.com> (raw)
In-Reply-To: <3085880.1584614257@warthog.procyon.org.uk>

On Thu, Mar 19, 2020 at 11:37 AM David Howells <dhowells@redhat.com> wrote:
>
> Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> > >  (2) It's more efficient as we can return specific binary data rather than
> > >      making huge text dumps.  Granted, sysfs and procfs could present the
> > >      same data, though as lots of little files which have to be
> > >      individually opened, read, closed and parsed.
> >
> > Asked this a number of times, but you haven't answered yet:  what
> > application would require such a high efficiency?
>
> Low efficiency means more time doing this when that time could be spent doing
> other things - or even putting the CPU in a powersaving state.  Using an
> open/read/close render-to-text-and-parse interface *will* be slower and less
> efficient as there are more things you have to do to use it.
>
> Then consider doing a walk over all the mounts in the case where there are
> 10000 of them - we have issues with /proc/mounts for such.  fsinfo() will end
> up doing a lot less work.

Current /proc/mounts problems arise from the fact that mount info can
only be queried for the whole namespace, and hence changes related to
a single mount will require rescanning the complete mount list.  If
mount info can be queried for individual mounts, then the need to scan
the complete list will be rare.  That's *the* point of this change.

> > >  (3) We wouldn't have the overhead of open and close (even adding a
> > >      self-contained readfile() syscall has to do that internally
> >
> > Busted: add f_op->readfile() and be done with all that.   For example
> > DEFINE_SHOW_ATTRIBUTE() could be trivially moved to that interface.
>
> Look at your example.  "f_op->".  That's "file->f_op->" I presume.
>
> You would have to make it "i_op->" to avoid the open and the close - and for
> things like procfs and sysfs, that's probably entirely reasonable - but bear
> in mind that you still have to apply all the LSM file security controls, just
> in case the backing filesystem is, say, ext4 rather than procfs.
>
> > We could optimize existing proc, sys, etc. interfaces, but it's not
> > been an issue, apparently.
>
> You can't get rid of or change many of the existing interfaces.  A lot of them
> are effectively indirect system calls and are, as such, part of the fixed
> UAPI.  You'd have to add a parallel optimised set.

Sure.

We already have the single_open() internal API that is basically a
->readfile() wrapper.   Moving this up to the f_op level (no, it's not
an i_op, and yes, we do need struct file, but it can be simply
allocated on the stack) is a trivial optimization that would let a
readfile(2) syscall access that level.  No new complexity in that
case.    Same generally goes for seq_file: seq_readfile() is trivial
to implement without messing with current implementation or any
existing APIs.

>
> > >  (6) Don't have to create/delete a bunch of sysfs/procfs nodes each time a
> > >      mount happens or is removed - and since systemd makes much use of
> > >      mount namespaces and mount propagation, this will create a lot of
> > >      nodes.
> >
> > Not true.
>
> This may not be true if you roll your own special filesystem.  It *is* true if
> you do it in procfs or sysfs.  The files don't exist if you don't create nodes
> or attribute tables for them.

That's one of the reasons why I opted to roll my own.  But the ideas
therein could be applied to kernfs, if found to be generally useful.
Nothing magic about that.

>
> > > The argument for doing this through procfs/sysfs/somemagicfs is that
> > > someone using a shell can just query the magic files using ordinary text
> > > tools, such as cat - and that has merit - but it doesn't solve the
> > > query-by-pathname problem.
> > >
> > > The suggested way around the query-by-pathname problem is to open the
> > > target file O_PATH and then look in a magic directory under procfs
> > > corresponding to the fd number to see a set of attribute files[*] laid out.
> > > Bash, however, can't open by O_PATH or O_NOFOLLOW as things stand...
> >
> > Bash doesn't have fsinfo(2) either, so that's not really a good argument.
>
> I never claimed that fsinfo() could be accessed directly from the shell.  For
> you proposal, you claimed "immediately usable from all programming languages,
> including scripts".

You are right.  Note however: only special files need the O_PATH
handling, regular files are directories can be opened by the shell
without side effects.

In any case, I think neither of us can be convinced of the other's
right, so I guess It's up to Al and Linus to make a decision.

Thanks,
Miklos

      reply	other threads:[~2020-03-19 12:37 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-18 15:08 [PATCH 00/13] VFS: Filesystem information [ver #19] David Howells
2020-03-18 15:08 ` [PATCH 01/13] fsinfo: Add fsinfo() syscall to query filesystem " David Howells
2020-03-18 15:08 ` [PATCH 02/13] fsinfo: Provide a bitmap of supported features " David Howells
2020-03-18 15:08 ` [PATCH 03/13] fsinfo: Allow retrieval of superblock devname, options and stats " David Howells
2020-03-18 15:08 ` [PATCH 04/13] fsinfo: Allow fsinfo() to look up a mount object by ID " David Howells
2020-03-18 15:08 ` [PATCH 05/13] fsinfo: Add a uniquifier ID to struct mount " David Howells
2020-03-18 15:09 ` [PATCH 06/13] fsinfo: Allow mount information to be queried " David Howells
2020-03-18 15:09 ` [PATCH 07/13] fsinfo: Allow mount topology and propagation info to be retrieved " David Howells
2020-03-18 15:09 ` [PATCH 08/13] fsinfo: Provide notification overrun handling support " David Howells
2020-03-18 15:09 ` [PATCH 09/13] fsinfo: sample: Mount listing program " David Howells
2020-03-18 15:09 ` [PATCH 10/13] fsinfo: Add API documentation " David Howells
2020-03-18 15:09 ` [PATCH 11/13] fsinfo: Add support for AFS " David Howells
2020-03-18 15:09 ` [PATCH 12/13] fsinfo: Example support for Ext4 " David Howells
2020-03-18 15:10 ` [PATCH 13/13] fsinfo: Example support for NFS " David Howells
2020-03-18 16:05 ` [PATCH 00/13] VFS: Filesystem information " Miklos Szeredi
2020-04-01  5:22   ` Ian Kent
2020-04-01  8:18     ` Miklos Szeredi
2020-04-01  8:27     ` David Howells
2020-04-01  8:37       ` Miklos Szeredi
2020-04-01 12:35         ` Miklos Szeredi
2020-04-01 15:51         ` David Howells
2020-04-02  1:38         ` Ian Kent
2020-04-02 14:14           ` Karel Zak
2020-03-19 10:37 ` David Howells
2020-03-19 12:36   ` Miklos Szeredi [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAJfpegv-_ai1LiW6=D+AnkozzmmXbB8=g8QDCS15bh==Wn3yoA@mail.gmail.com' \
    --to=miklos@szeredi.hu \
    --cc=adilger.kernel@dilger.ca \
    --cc=anna.schumaker@netapp.com \
    --cc=christian@brauner.io \
    --cc=darrick.wong@oracle.com \
    --cc=dhowells@redhat.com \
    --cc=jannh@google.com \
    --cc=jlayton@redhat.com \
    --cc=kzak@redhat.com \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=mszeredi@redhat.com \
    --cc=raven@themaw.net \
    --cc=torvalds@linux-foundation.org \
    --cc=trond.myklebust@hammerspace.com \
    --cc=tytso@mit.edu \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).