linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Howells <dhowells@redhat.com>
To: Miklos Szeredi <miklos@szeredi.hu>
Cc: dhowells@redhat.com, viro@zeniv.linux.org.uk,
	linux-api@vger.kernel.org, torvalds@linux-foundation.org,
	raven@themaw.net, mszeredi@redhat.com, christian@brauner.io,
	jannh@google.com, darrick.wong@oracle.com, kzak@redhat.com,
	jlayton@redhat.com, linux-fsdevel@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 02/18] fsinfo: Add fsinfo() syscall to query filesystem information [ver #21]
Date: Tue, 04 Aug 2020 12:34:41 +0100	[thread overview]
Message-ID: <2041168.1596540881@warthog.procyon.org.uk> (raw)
In-Reply-To: <20200804101659.GA32719@miu.piliscsaba.redhat.com>

Miklos Szeredi <miklos@szeredi.hu> wrote:

> > 		__u32	Mth;
> 
> The Mth field seems to be unused in this patchset.  Since the struct is
> extensible, I guess there's no point in adding it now.

Yeah - I was using it to index through the server address lists for network
filesystems (ie. the Mth address of the Nth server), but I've dropped the nfs
patch and made afs return an array of addresses for the Nth server since the
address list can get reordered.

Ordinarily, I'd just take it out, but I don't want to cause the patchset to
get dropped for yet another merge cycle :-/

> > +#define FSINFO_ATTR_FSINFO_ATTRIBUTE_INFO 0x100	/* Information about attr N (for path) */
> > +#define FSINFO_ATTR_FSINFO_ATTRIBUTES	0x101	/* List of supported attrs (for path) */
> 
> I think it would make sense to move the actual attributes to a separate patch
> and leave this just being the infrastructure.

Maybe.  If there are no attributes, then it makes it a bit hard to test.

> > +struct fsinfo_u128 {
> ...
> 
> Shouldn't this belong in <linux/types.h>?

Maybe.  Ideally, I'd use a proper C type rather than a struct.

> Is there a reason these are 128 wide fields?  Are we approaching the limits of
> 64bits?

Dave Chinner was talking at LSF a couple of years ago, IIRC, about looking
beyond the 16 Exa limit in XFS.  I've occasionally talked to people who have
multi-Peta data sets in AFS or whatever they were using, streamed from science
experiments, so the limit isn't necessarily all *that* far off.

> > +struct fsinfo_limits {
> > +	struct fsinfo_u128 max_file_size;	/* Maximum file size */
> > +	struct fsinfo_u128 max_ino;		/* Maximum inode number */
> 
> Again, what's the reason.  AFACT we are not yet worried about overflowing 64
> bits.  Future proofing is good, but there has to be some rules and reasons
> behind the decisions.

This is cheap to do.  This information is expected to be static for the
lifetime a superblock and, for most filesystems, of the running kernel, so
simply copying it with memcpy() from rodata is going to suffice most of the
time.

But don't worry - 640K is sufficient for everyone ;-)

David


  parent reply	other threads:[~2020-08-04 11:35 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-03 13:36 [PATCH 00/18] VFS: Filesystem " David Howells
2020-08-03 13:36 ` [PATCH 01/18] fsinfo: Introduce a non-repeating system-unique superblock ID " David Howells
2020-08-04  9:34   ` Miklos Szeredi
2020-08-03 13:36 ` [PATCH 02/18] fsinfo: Add fsinfo() syscall to query filesystem information " David Howells
2020-08-04 10:16   ` Miklos Szeredi
2020-08-04 11:34   ` David Howells [this message]
2020-08-27 11:27   ` Michael Kerrisk (man-pages)
2020-08-03 13:36 ` [PATCH 03/18] fsinfo: Provide a bitmap of the features a filesystem supports " David Howells
2020-08-03 13:37 ` [PATCH 04/18] fsinfo: Allow retrieval of superblock devname, options and stats " David Howells
2020-08-03 13:37 ` [PATCH 05/18] fsinfo: Allow fsinfo() to look up a mount object by ID " David Howells
2020-08-04 10:33   ` Miklos Szeredi
2020-08-03 13:37 ` [PATCH 06/18] fsinfo: Add a uniquifier ID to struct mount " David Howells
2020-08-04 10:41   ` Miklos Szeredi
2020-08-04 12:32     ` Ian Kent
2020-08-05 14:13   ` David Howells
2020-08-05 14:46     ` Miklos Szeredi
2020-08-05 15:30     ` David Howells
2020-08-05 19:33       ` Matthew Wilcox
2020-08-06  5:43         ` Ian Kent
2020-08-03 13:37 ` [PATCH 07/18] fsinfo: Allow mount information to be queried " David Howells
2020-08-03 13:37 ` [PATCH 08/18] fsinfo: Allow mount topology and propagation info to be retrieved " David Howells
2020-08-04 13:38   ` Miklos Szeredi
2020-08-05 15:37   ` David Howells
2020-08-05 17:19     ` Miklos Szeredi
2020-08-03 13:37 ` [PATCH 09/18] watch_queue: Mount event counters " David Howells
2020-08-03 13:37 ` [PATCH 10/18] fsinfo: Provide notification overrun handling support " David Howells
2020-08-04 13:56   ` Miklos Szeredi
2020-08-05  2:05     ` Ian Kent
2020-08-05  2:46       ` Ian Kent
2020-08-05  7:45         ` Miklos Szeredi
2020-08-05 11:23           ` Ian Kent
2020-08-05 11:27             ` Miklos Szeredi
2020-08-06  1:47               ` Ian Kent
2020-08-05 16:06   ` David Howells
2020-08-05 17:26     ` Miklos Szeredi
2020-08-03 13:37 ` [PATCH 11/18] fsinfo: sample: Mount listing program " David Howells
2020-08-03 13:38 ` [PATCH 12/18] fsinfo: Add API documentation " David Howells
2020-08-03 13:38 ` [PATCH 13/18] fsinfo: Add support for AFS " David Howells
2020-08-03 13:38 ` [PATCH 14/18] fsinfo: Add support to ext4 " David Howells
2020-08-03 13:38 ` [PATCH 15/18] fsinfo: Add an attribute that lists all the visible mounts in a namespace " David Howells
2020-08-04 14:05   ` Miklos Szeredi
2020-08-05  0:59     ` Ian Kent
2020-08-05 16:44   ` David Howells
2020-08-03 13:38 ` [PATCH 16/18] errseq: add a new errseq_scrape function " David Howells
2020-08-03 13:38 ` [PATCH 17/18] vfs: allow fsinfo to fetch the current state of s_wb_err " David Howells
2020-08-03 13:39 ` [PATCH 18/18] samples: add error state information to test-fsinfo.c " David Howells
2020-08-04 15:39 ` [PATCH 00/18] VFS: Filesystem information " James Bottomley
2020-08-04 19:18   ` Miklos Szeredi
2020-08-05 17:13 ` David Howells

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=2041168.1596540881@warthog.procyon.org.uk \
    --to=dhowells@redhat.com \
    --cc=christian@brauner.io \
    --cc=darrick.wong@oracle.com \
    --cc=jannh@google.com \
    --cc=jlayton@redhat.com \
    --cc=kzak@redhat.com \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=mszeredi@redhat.com \
    --cc=raven@themaw.net \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    --subject='Re: [PATCH 02/18] fsinfo: Add fsinfo() syscall to query filesystem information [ver #21]' \
    /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

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).