Message ID | 156138532485.25627.7459410522109581052.stgit@warthog.procyon.org.uk (mailing list archive) |
---|---|
Headers | show |
Series | VFS: Introduce filesystem information query syscall [ver #14] | expand |
On Mon, Jun 24, 2019 at 03:08:45PM +0100, David Howells wrote: > > Hi Al, > > Here are a set of patches that adds a syscall, fsinfo(), that allows > attributes of a filesystem/superblock to be queried. Attribute values are > of four basic types: > > (1) Version dependent-length structure (size defined by type). > > (2) Variable-length string (up to PAGE_SIZE). > > (3) Array of fixed-length structures (up to INT_MAX size). > > (4) Opaque blob (up to INT_MAX size). > > Attributes can have multiple values in up to two dimensions and all the > values of a particular attribute must have the same type. > > Note that the attribute values *are* allowed to vary between dentries > within a single superblock, depending on the specific dentry that you're > looking at. > > I've tried to make the interface as light as possible, so integer/enum > attribute selector rather than string and the core does all the allocation > and extensibility support work rather than leaving that to the filesystems. > That means that for the first two attribute types, sb->s_op->fsinfo() may > assume that the provided buffer is always present and always big enough. > > Further, this removes the possibility of the filesystem gaining access to the > userspace buffer. > > > fsinfo() allows a variety of information to be retrieved about a filesystem > and the mount topology: > > (1) General superblock attributes: > > - The amount of space/free space in a filesystem (as statfs()). > - Filesystem identifiers (UUID, volume label, device numbers, ...) > - The limits on a filesystem's capabilities > - Information on supported statx fields and attributes and IOC flags. > - A variety single-bit flags indicating supported capabilities. > - Timestamp resolution and range. > - Sources (as per mount(2), but fsconfig() allows multiple sources). > - In-filesystem filename format information. > - Filesystem parameters ("mount -o xxx"-type things). > - LSM parameters (again "mount -o xxx"-type things). > > (2) Filesystem-specific superblock attributes: > > - Server names and addresses. > - Cell name. > > (3) Filesystem configuration metadata attributes: > > - Filesystem parameter type descriptions. > - Name -> parameter mappings. > - Simple enumeration name -> value mappings. > > (4) Mount topology: > > - General information about a mount object. > - Mount device name(s). > - Children of a mount object and their relative paths. > > (5) Information about what the fsinfo() syscall itself supports, including > the number of attibutes supported and the number of capability bits > supported. Phew, this patchset is a lot. It's good of course but can we please cut some of the more advanced features such as querying by mount id, submounts etc. pp. for now? I feel this would help with review and since your interface is extensible it's really not a big deal if we defer fancy features to later cycles after people had more time to review and the interface has seen some exposure. The mount api changes over the last months have honestly been so huge that any chance to make the changes smaller and easier to digest we should take. (I'm really not complaining. Good that the work is done and it's entirely ok that it's a lot of code.) It would also be great if after you have dropped some stuff from this patchset and gotten an Ack we could stuff it into linux-next for some time because it hasn't been so far... Christian
On Wed, 2019-06-26 at 12:05 +0200, Christian Brauner wrote: > On Mon, Jun 24, 2019 at 03:08:45PM +0100, David Howells wrote: > > Hi Al, > > > > Here are a set of patches that adds a syscall, fsinfo(), that allows > > attributes of a filesystem/superblock to be queried. Attribute values are > > of four basic types: > > > > (1) Version dependent-length structure (size defined by type). > > > > (2) Variable-length string (up to PAGE_SIZE). > > > > (3) Array of fixed-length structures (up to INT_MAX size). > > > > (4) Opaque blob (up to INT_MAX size). > > > > Attributes can have multiple values in up to two dimensions and all the > > values of a particular attribute must have the same type. > > > > Note that the attribute values *are* allowed to vary between dentries > > within a single superblock, depending on the specific dentry that you're > > looking at. > > > > I've tried to make the interface as light as possible, so integer/enum > > attribute selector rather than string and the core does all the allocation > > and extensibility support work rather than leaving that to the filesystems. > > That means that for the first two attribute types, sb->s_op->fsinfo() may > > assume that the provided buffer is always present and always big enough. > > > > Further, this removes the possibility of the filesystem gaining access to > > the > > userspace buffer. > > > > > > fsinfo() allows a variety of information to be retrieved about a filesystem > > and the mount topology: > > > > (1) General superblock attributes: > > > > - The amount of space/free space in a filesystem (as statfs()). > > - Filesystem identifiers (UUID, volume label, device numbers, ...) > > - The limits on a filesystem's capabilities > > - Information on supported statx fields and attributes and IOC flags. > > - A variety single-bit flags indicating supported capabilities. > > - Timestamp resolution and range. > > - Sources (as per mount(2), but fsconfig() allows multiple sources). > > - In-filesystem filename format information. > > - Filesystem parameters ("mount -o xxx"-type things). > > - LSM parameters (again "mount -o xxx"-type things). > > > > (2) Filesystem-specific superblock attributes: > > > > - Server names and addresses. > > - Cell name. > > > > (3) Filesystem configuration metadata attributes: > > > > - Filesystem parameter type descriptions. > > - Name -> parameter mappings. > > - Simple enumeration name -> value mappings. > > > > (4) Mount topology: > > > > - General information about a mount object. > > - Mount device name(s). > > - Children of a mount object and their relative paths. > > > > (5) Information about what the fsinfo() syscall itself supports, including > > the number of attibutes supported and the number of capability bits > > supported. > > Phew, this patchset is a lot. It's good of course but can we please cut > some of the more advanced features such as querying by mount id, > submounts etc. pp. for now? Did you mean the "vfs: Allow fsinfo() to look up a mount object by ID" patch? We would need to be very careful what was dropped. For example, I've found that the patch above is pretty much essential for fsinfo() to be useful from user space. > I feel this would help with review and since your interface is > extensible it's really not a big deal if we defer fancy features to > later cycles after people had more time to review and the interface has > seen some exposure. > > The mount api changes over the last months have honestly been so huge > that any chance to make the changes smaller and easier to digest we > should take. (I'm really not complaining. Good that the work is done and > it's entirely ok that it's a lot of code.) > > It would also be great if after you have dropped some stuff from this > patchset and gotten an Ack we could stuff it into linux-next for some > time because it hasn't been so far... > > Christian
On Wed, Jun 26, 2019 at 06:42:51PM +0800, Ian Kent wrote: > On Wed, 2019-06-26 at 12:05 +0200, Christian Brauner wrote: > > On Mon, Jun 24, 2019 at 03:08:45PM +0100, David Howells wrote: > > > Hi Al, > > > > > > Here are a set of patches that adds a syscall, fsinfo(), that allows > > > attributes of a filesystem/superblock to be queried. Attribute values are > > > of four basic types: > > > > > > (1) Version dependent-length structure (size defined by type). > > > > > > (2) Variable-length string (up to PAGE_SIZE). > > > > > > (3) Array of fixed-length structures (up to INT_MAX size). > > > > > > (4) Opaque blob (up to INT_MAX size). > > > > > > Attributes can have multiple values in up to two dimensions and all the > > > values of a particular attribute must have the same type. > > > > > > Note that the attribute values *are* allowed to vary between dentries > > > within a single superblock, depending on the specific dentry that you're > > > looking at. > > > > > > I've tried to make the interface as light as possible, so integer/enum > > > attribute selector rather than string and the core does all the allocation > > > and extensibility support work rather than leaving that to the filesystems. > > > That means that for the first two attribute types, sb->s_op->fsinfo() may > > > assume that the provided buffer is always present and always big enough. > > > > > > Further, this removes the possibility of the filesystem gaining access to > > > the > > > userspace buffer. > > > > > > > > > fsinfo() allows a variety of information to be retrieved about a filesystem > > > and the mount topology: > > > > > > (1) General superblock attributes: > > > > > > - The amount of space/free space in a filesystem (as statfs()). > > > - Filesystem identifiers (UUID, volume label, device numbers, ...) > > > - The limits on a filesystem's capabilities > > > - Information on supported statx fields and attributes and IOC flags. > > > - A variety single-bit flags indicating supported capabilities. > > > - Timestamp resolution and range. > > > - Sources (as per mount(2), but fsconfig() allows multiple sources). > > > - In-filesystem filename format information. > > > - Filesystem parameters ("mount -o xxx"-type things). > > > - LSM parameters (again "mount -o xxx"-type things). > > > > > > (2) Filesystem-specific superblock attributes: > > > > > > - Server names and addresses. > > > - Cell name. > > > > > > (3) Filesystem configuration metadata attributes: > > > > > > - Filesystem parameter type descriptions. > > > - Name -> parameter mappings. > > > - Simple enumeration name -> value mappings. > > > > > > (4) Mount topology: > > > > > > - General information about a mount object. > > > - Mount device name(s). > > > - Children of a mount object and their relative paths. > > > > > > (5) Information about what the fsinfo() syscall itself supports, including > > > the number of attibutes supported and the number of capability bits > > > supported. > > > > Phew, this patchset is a lot. It's good of course but can we please cut > > some of the more advanced features such as querying by mount id, > > submounts etc. pp. for now? > > Did you mean the "vfs: Allow fsinfo() to look up a mount object by ID" > patch? > > We would need to be very careful what was dropped. Not dropped as in never implement but rather defer it by one merge window to give us a) more time to review and settle the interface while b) not stalling the overall patch. > > For example, I've found that the patch above is pretty much essential > for fsinfo() to be useful from user space. Yeah, but that interface is not clearly defined yet as can be seen from the commit message and that's what's bothering me most.
On Wed, Jun 26, 2019 at 12:05:25PM +0200, Christian Brauner wrote: > On Mon, Jun 24, 2019 at 03:08:45PM +0100, David Howells wrote: > > > > Hi Al, > > > > Here are a set of patches that adds a syscall, fsinfo(), that allows > > attributes of a filesystem/superblock to be queried. Attribute values are > > of four basic types: > > > > (1) Version dependent-length structure (size defined by type). > > > > (2) Variable-length string (up to PAGE_SIZE). > > > > (3) Array of fixed-length structures (up to INT_MAX size). > > > > (4) Opaque blob (up to INT_MAX size). > > > > Attributes can have multiple values in up to two dimensions and all the > > values of a particular attribute must have the same type. > > > > Note that the attribute values *are* allowed to vary between dentries > > within a single superblock, depending on the specific dentry that you're > > looking at. > > > > I've tried to make the interface as light as possible, so integer/enum > > attribute selector rather than string and the core does all the allocation > > and extensibility support work rather than leaving that to the filesystems. > > That means that for the first two attribute types, sb->s_op->fsinfo() may > > assume that the provided buffer is always present and always big enough. > > > > Further, this removes the possibility of the filesystem gaining access to the > > userspace buffer. > > > > > > fsinfo() allows a variety of information to be retrieved about a filesystem > > and the mount topology: > > > > (1) General superblock attributes: > > > > - The amount of space/free space in a filesystem (as statfs()). > > - Filesystem identifiers (UUID, volume label, device numbers, ...) > > - The limits on a filesystem's capabilities > > - Information on supported statx fields and attributes and IOC flags. > > - A variety single-bit flags indicating supported capabilities. > > - Timestamp resolution and range. > > - Sources (as per mount(2), but fsconfig() allows multiple sources). > > - In-filesystem filename format information. > > - Filesystem parameters ("mount -o xxx"-type things). > > - LSM parameters (again "mount -o xxx"-type things). > > > > (2) Filesystem-specific superblock attributes: > > > > - Server names and addresses. > > - Cell name. > > > > (3) Filesystem configuration metadata attributes: > > > > - Filesystem parameter type descriptions. > > - Name -> parameter mappings. > > - Simple enumeration name -> value mappings. > > > > (4) Mount topology: > > > > - General information about a mount object. > > - Mount device name(s). > > - Children of a mount object and their relative paths. > > > > (5) Information about what the fsinfo() syscall itself supports, including > > the number of attibutes supported and the number of capability bits > > supported. > > Phew, this patchset is a lot. It's good of course but can we please cut > some of the more advanced features such as querying by mount id, > submounts etc. pp. for now? > I feel this would help with review and since your interface is > extensible it's really not a big deal if we defer fancy features to > later cycles after people had more time to review and the interface has > seen some exposure. > > The mount api changes over the last months have honestly been so huge > that any chance to make the changes smaller and easier to digest we > should take. (I'm really not complaining. Good that the work is done and > it's entirely ok that it's a lot of code.) > > It would also be great if after you have dropped some stuff from this > patchset and gotten an Ack we could stuff it into linux-next for some > time because it hasn't been so far... And I also very much recommend to remove any potential cross-dependency between the fsinfo() and the notification patchset. Ideally, I'd like to see fsinfo() to be completely independent to not block it on something way more controversial. Furthermore, I can't possibly keep the context of another huge patchset not yet merged in the back of my mind while reviewing this patchset. :) Christian
Christian Brauner <christian@brauner.io> wrote: > And I also very much recommend to remove any potential cross-dependency > between the fsinfo() and the notification patchset. The problem with that is that to make the notification patchset useful for mount notifications, you need some information that you would obtain through fsinfo(). David
On Wed, Jun 26, 2019 at 03:31:37PM +0100, David Howells wrote: > Christian Brauner <christian@brauner.io> wrote: > > > And I also very much recommend to remove any potential cross-dependency > > between the fsinfo() and the notification patchset. > > The problem with that is that to make the notification patchset useful for > mount notifications, you need some information that you would obtain through > fsinfo(). But would it really be that bad if you'd just land fsinfo() and then focus on the notification stuff. This very much rather looks like a timing issue than a conceptual one, i.e. you could very much just push fsinfo() and leave the notification stuff alone until that is done. Once fsinfo() has landed you can then go on to put additional bits you need from or for fsinfo() for the notification patchset in there. Seems you have at least sketched both APIs sufficiently that you know what you need to look out for to not cause any regressions later on when you need to expand them. Christian
On Wed, 2019-06-26 at 12:47 +0200, Christian Brauner wrote: > On Wed, Jun 26, 2019 at 06:42:51PM +0800, Ian Kent wrote: > > On Wed, 2019-06-26 at 12:05 +0200, Christian Brauner wrote: > > > On Mon, Jun 24, 2019 at 03:08:45PM +0100, David Howells wrote: > > > > Hi Al, > > > > > > > > Here are a set of patches that adds a syscall, fsinfo(), that allows > > > > attributes of a filesystem/superblock to be queried. Attribute values > > > > are > > > > of four basic types: > > > > > > > > (1) Version dependent-length structure (size defined by type). > > > > > > > > (2) Variable-length string (up to PAGE_SIZE). > > > > > > > > (3) Array of fixed-length structures (up to INT_MAX size). > > > > > > > > (4) Opaque blob (up to INT_MAX size). > > > > > > > > Attributes can have multiple values in up to two dimensions and all the > > > > values of a particular attribute must have the same type. > > > > > > > > Note that the attribute values *are* allowed to vary between dentries > > > > within a single superblock, depending on the specific dentry that you're > > > > looking at. > > > > > > > > I've tried to make the interface as light as possible, so integer/enum > > > > attribute selector rather than string and the core does all the > > > > allocation > > > > and extensibility support work rather than leaving that to the > > > > filesystems. > > > > That means that for the first two attribute types, sb->s_op->fsinfo() > > > > may > > > > assume that the provided buffer is always present and always big enough. > > > > > > > > Further, this removes the possibility of the filesystem gaining access > > > > to > > > > the > > > > userspace buffer. > > > > > > > > > > > > fsinfo() allows a variety of information to be retrieved about a > > > > filesystem > > > > and the mount topology: > > > > > > > > (1) General superblock attributes: > > > > > > > > - The amount of space/free space in a filesystem (as statfs()). > > > > - Filesystem identifiers (UUID, volume label, device numbers, ...) > > > > - The limits on a filesystem's capabilities > > > > - Information on supported statx fields and attributes and IOC > > > > flags. > > > > - A variety single-bit flags indicating supported capabilities. > > > > - Timestamp resolution and range. > > > > - Sources (as per mount(2), but fsconfig() allows multiple > > > > sources). > > > > - In-filesystem filename format information. > > > > - Filesystem parameters ("mount -o xxx"-type things). > > > > - LSM parameters (again "mount -o xxx"-type things). > > > > > > > > (2) Filesystem-specific superblock attributes: > > > > > > > > - Server names and addresses. > > > > - Cell name. > > > > > > > > (3) Filesystem configuration metadata attributes: > > > > > > > > - Filesystem parameter type descriptions. > > > > - Name -> parameter mappings. > > > > - Simple enumeration name -> value mappings. > > > > > > > > (4) Mount topology: > > > > > > > > - General information about a mount object. > > > > - Mount device name(s). > > > > - Children of a mount object and their relative paths. > > > > > > > > (5) Information about what the fsinfo() syscall itself supports, > > > > including > > > > the number of attibutes supported and the number of capability bits > > > > supported. > > > > > > Phew, this patchset is a lot. It's good of course but can we please cut > > > some of the more advanced features such as querying by mount id, > > > submounts etc. pp. for now? > > > > Did you mean the "vfs: Allow fsinfo() to look up a mount object by ID" > > patch? > > > > We would need to be very careful what was dropped. > > Not dropped as in never implement but rather defer it by one merge > window to give us a) more time to review and settle the interface while > b) not stalling the overall patch. Sure, and I'm not saying something like what you recommend shouldn't be done. I'm working on user space mount table improvements that I want to get done ahead of the merge. Well, I would be but there's still mount-api conversions that need to be done so that fsinfo() patches don't end up with endless merge conflicts. The fsinfo() super block method will result in changes in the same area as the mount-api changes. The mount-api changes are proving to be a bit of a challenge. Anyway, the plan is to use the mount table handling improvements to try and locate bugs and missing or not quite right functionality. > > > For example, I've found that the patch above is pretty much essential > > for fsinfo() to be useful from user space. > > Yeah, but that interface is not clearly defined yet as can be seen from > the commit message and that's what's bothering me most. Yeah, but updating my cloned branch hasn't been difficult. There's a certain amount of functionality that I'd like to see retained for when I get back to the user space development. Using the notifications changes are something I'm not likely to get to for quite some time so breaking those out into a separate branch (like they were not so long ago) would be more sensible IMHO. There may be some other bits that David can identify too. Ian