netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Avi Kivity <avi@redhat.com>
To: "Nicholas A. Bellinger" <nab@linux-iscsi.org>
Cc: Ingo Molnar <mingo@elte.hu>,
	Anthony Liguori <anthony@codemonkey.ws>,
	kvm@vger.kernel.org, alacrityvm-devel@lists.sourceforge.net,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"Ira W. Snyder" <iws@ovro.caltech.edu>,
	Joel Becker <joel.becker@oracle.com>
Subject: configfs/sysfs
Date: Wed, 19 Aug 2009 23:12:43 +0300	[thread overview]
Message-ID: <4A8C5CBB.10605@redhat.com> (raw)
In-Reply-To: <1250706227.27590.156.camel@haakon2.linux-iscsi.org>

On 08/19/2009 09:23 PM, Nicholas A. Bellinger wrote:
> Anyways, I was wondering if you might be interesting in sharing your
> concerns wrt to configfs (conigfs maintainer CC'ed), at some point..?
>    

My concerns aren't specifically with configfs, but with all the text 
based pseudo filesystems that the kernel exposes.

My high level concern is that we're optimizing for the active sysadmin, 
not for libraries and management programs.  configfs and sysfs are easy 
to use from the shell, discoverable, and easily scripted.  But they 
discourage documentation, the text format is ambiguous, and they require 
a lot of boilerplate to use in code.

You could argue that you can wrap *fs in a library that hides the 
details of accessing it, but that's the wrong approach IMO.  We should 
make the information easy to use and manipulate for programs; one of 
these programs can be a fuse filesystem for the active sysadmin if 
someone thinks it's important.

Now for the low level concerns:

- efficiency

Each attribute access requires an open/read/close triplet and 
binary->ascii->binary conversions.  In contrast an ordinary 
syscall/ioctl interface can fetch all attributes of an object, or even 
all attributes of all objects, in one call.

- atomicity

One attribute per file means that, lacking userspace-visible 
transactions, there is no way to change several attributes at once.  
When you read attributes, there is no way to read several attributes 
atomically so you can be sure their values correlate.  Another example 
of a problem is when an object disappears while reading its attributes.  
Sure, openat() can mitigate this, but it's better to avoid introducing 
problem than having a fix.

- ambiguity

What format is the attribute?  does it accept lowercase or uppercase hex 
digits?  is there a newline at the end?  how many digits can it take 
before the attribute overflows?  All of this has to be documented and 
checked by the OS, otherwise we risk regressions later.  In contrast, 
__u64 says everything in a binary interface.

- lifetime and access control

If a process brings an object into being (using mkdir) and then dies, 
the object remains behind.  The syscall/ioctl approach ties the object 
into an fd, which will be destroyed when the process dies, and which can 
be passed around using SCM_RIGHTS, allowing a server process to create 
and configure an object before passing it to an unprivileged program

- notifications

It's hard to notify users about changes in attributes.  Sure, you can 
use inotify, but that limits you to watching subtrees.  Once you do get 
the notification, you run into the atomicity problem.  When do you know 
all attributes are valid?  This can be solved using sequence counters, 
but that's just gratuitous complexity.  Netlink type interfaces are much 
more robust and flexible.

- readdir

You can either list everything, or nothing.  Sure, you can have trees to 
ease searching, even multiple views of the same data, but it's painful.

You may argue, correctly, that syscalls and ioctls are not as flexible.  
But this is because no one has invested the effort in making them so.  A 
struct passed as an argument to a syscall is not extensible.  But if you 
pass the size of the structure, and also a bitmap of which attributes 
are present, you gain extensibility and retain the atomicity property of 
a syscall interface.  I don't think a lot of effort is needed to make an 
extensible syscall interface just as usable and a lot more efficient 
than configfs/sysfs.  It should also be simple to bolt a fuse interface 
on top to expose it to us commandline types.

> As you may recall, I have been using configfs extensively for the 3.x
> generic target core infrastructure and iSCSI fabric modules living in
> lio-core-2.6.git/drivers/target/target_core_configfs.c and
> lio-core-2.6.git/drivers/lio-core/iscsi_target_config.c, and have found
> it to be extraordinarly useful for the purposes of a implementing a
> complex kernel level target mode stack that is expected to manage
> massive amounts of metadata, allow for real-time configuration, share
> data structures (eg: SCSI Target Ports) between other kernel fabric
> modules and manage the entire set of fabrics using only intrepetered
> userspace code.
>
> Using the 10000 1:1 mapped TCM Virtual HBA+FILEIO LUNs<->  iSCSI Target
> Endpoints inside of a KVM Guest (from the results in May posted with
> IOMMU aware 10 Gb on modern Nahelem hardware, see
> http://linux-iscsi.org/index.php/KVM-LIO-Target), we have been able to
> dump the entire running target fabric configfs hierarchy to a single
> struct file on a KVM Guest root device using python code on the order of
> ~30 seconds for those 10000 active iSCSI endpoints.  In configfs terms,
> this means:
>
> *) 7 configfs groups (directories), ~50 configfs attributes (files) per
> Virtual HBA+FILEIO LUN
> *) 15 configfs groups (directories), ~60 configfs attributes (files per
> iSCSI fabric Endpoint
>
> Which comes out to a total of ~220000 groups and ~1100000 attributes
> active configfs objects living in the configfs_dir_cache that are being
> dumped inside of the single KVM guest instances, including symlinks
> between the fabric modules to establish the SCSI ports containing
> complete set of SPC-4 and RFC-3720 features, et al.
>    

You achieved 3 million syscalls/sec from Python code?  That's very 
impressive.

Note with syscalls you could have done it with 10K syscalls (Python 
supports packing and unpacking structs quite well, and also directly 
calling C code IIRC).

> Also on the kernel<->  user API interaction compatibility side, I have
> found the 3.x configfs enabled code adventagous over the LIO 2.9 code
> (that used an ioctl for everything) because it allows us to do backwards
> compat for future versions without using any userspace C code, which in
> IMHO makes maintaining userspace packages for complex kernel stacks with
> massive amounts of metadata + real-time configuration considerations.
> No longer having ioctl compatibility issues between LIO versions as the
> structures passed via ioctl change, and being able to do backwards
> compat with small amounts of interpreted code against configfs layout
> changes makes maintaining the kernel<->  user API really have made this
> that much easier for me.
>    

configfs is more maintainable that a bunch of hand-maintained ioctls.  
But if we put some effort into an extendable syscall infrastructure 
(perhaps to the point of using an IDL) I'm sure we can improve on that 
without the problems pseudo filesystems introduce.

> Anyways, I though these might be useful to the discussion as it releates
> to potental uses of configfs on the KVM Host or other projects that
> really make sense, and/or to improve the upstream implementation so that
> other users (like myself) can benefit from improvements to configfs.
>    

I can't really fault a project for using configfs; it's an accepted and 
recommented (by the community) interface.  I'd much prefer it though if 
there was an effort to create a usable fd/struct based alternative.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.


  parent reply	other threads:[~2009-08-19 20:12 UTC|newest]

Thread overview: 120+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-08-14 15:42 [PATCH v3 0/6] AlacrityVM guest drivers Gregory Haskins
2009-08-14 15:42 ` [PATCH v3 1/6] shm-signal: shared-memory signals Gregory Haskins
2009-08-14 15:43 ` [PATCH v3 2/6] ioq: Add basic definitions for a shared-memory, lockless queue Gregory Haskins
2009-08-14 15:43 ` [PATCH v3 3/6] vbus: add a "vbus-proxy" bus model for vbus_driver objects Gregory Haskins
2009-08-15 10:32   ` Ingo Molnar
2009-08-15 19:15     ` Anthony Liguori
2009-08-16  7:16       ` Ingo Molnar
2009-08-17 13:54         ` Anthony Liguori
2009-08-17 14:23           ` Ingo Molnar
2009-08-17 14:14       ` Gregory Haskins
2009-08-17 14:58         ` Avi Kivity
2009-08-17 15:05           ` Ingo Molnar
2009-08-17 17:41         ` Michael S. Tsirkin
2009-08-17 20:17           ` Gregory Haskins
2009-08-18  8:46             ` Michael S. Tsirkin
2009-08-18 15:19               ` Gregory Haskins
2009-08-18 16:25                 ` Michael S. Tsirkin
2009-08-18 15:53               ` [Alacrityvm-devel] " Ira W. Snyder
2009-08-18 16:51                 ` Avi Kivity
2009-08-18 17:27                   ` Ira W. Snyder
2009-08-18 17:47                     ` Avi Kivity
2009-08-18 18:27                       ` Ira W. Snyder
2009-08-18 18:52                         ` Avi Kivity
2009-08-18 20:59                           ` Ira W. Snyder
2009-08-18 21:26                             ` Avi Kivity
2009-08-18 22:06                               ` Avi Kivity
2009-08-19  0:44                                 ` Ira W. Snyder
2009-08-19  5:26                                   ` Avi Kivity
2009-08-19  0:38                               ` Ira W. Snyder
2009-08-19  5:40                                 ` Avi Kivity
2009-08-19 15:28                                   ` Ira W. Snyder
2009-08-19 15:37                                     ` Avi Kivity
2009-08-19 16:29                                       ` Ira W. Snyder
2009-08-19 16:38                                         ` Avi Kivity
2009-08-19 21:05                                           ` Hollis Blanchard
2009-08-20  9:57                                             ` Stefan Hajnoczi
2009-08-20 10:08                                               ` Avi Kivity
2009-08-18 20:35                         ` Michael S. Tsirkin
2009-08-18 21:04                           ` Arnd Bergmann
2009-08-18 20:39                     ` Michael S. Tsirkin
2009-08-18 20:57                 ` Michael S. Tsirkin
2009-08-18 23:24                   ` Ira W. Snyder
2009-08-18  1:08         ` Anthony Liguori
2009-08-18  7:38           ` Avi Kivity
2009-08-18  8:54           ` Michael S. Tsirkin
2009-08-18 13:16           ` Gregory Haskins
2009-08-18 13:45             ` Avi Kivity
2009-08-18 15:51               ` Gregory Haskins
2009-08-18 16:14                 ` Ingo Molnar
2009-08-19  4:27                   ` Gregory Haskins
2009-08-19  5:22                     ` Avi Kivity
2009-08-19 13:27                       ` Gregory Haskins
2009-08-19 14:35                         ` Avi Kivity
2009-08-18 16:47                 ` Avi Kivity
2009-08-18 16:51                 ` Michael S. Tsirkin
2009-08-19  5:36                   ` Gregory Haskins
2009-08-19  5:48                     ` Avi Kivity
2009-08-19  6:40                       ` Gregory Haskins
2009-08-19  7:13                         ` Avi Kivity
2009-08-19 11:40                           ` Gregory Haskins
2009-08-19 11:49                             ` Avi Kivity
2009-08-19 11:52                               ` Gregory Haskins
2009-08-19 14:33                     ` Michael S. Tsirkin
2009-08-20 12:12                     ` Michael S. Tsirkin
2009-08-16  8:30     ` Avi Kivity
2009-08-17 14:16       ` Gregory Haskins
2009-08-17 14:59         ` Avi Kivity
2009-08-17 15:09           ` Gregory Haskins
2009-08-17 15:14             ` Ingo Molnar
2009-08-17 19:35               ` Gregory Haskins
2009-08-17 15:18             ` Avi Kivity
2009-08-17 13:02     ` Gregory Haskins
2009-08-17 14:25       ` Ingo Molnar
2009-08-17 15:05         ` Gregory Haskins
2009-08-17 15:08           ` Ingo Molnar
2009-08-17 19:33             ` Gregory Haskins
2009-08-18  8:33               ` Avi Kivity
2009-08-18 14:46                 ` Gregory Haskins
2009-08-18 16:27                   ` Avi Kivity
2009-08-19  6:28                     ` Gregory Haskins
2009-08-19  7:11                       ` Avi Kivity
2009-08-19 18:23                         ` Nicholas A. Bellinger
2009-08-19 18:39                           ` Gregory Haskins
2009-08-19 19:19                             ` Nicholas A. Bellinger
2009-08-19 19:34                               ` Nicholas A. Bellinger
2009-08-19 20:12                           ` Avi Kivity [this message]
2009-08-19 20:48                             ` configfs/sysfs Ingo Molnar
2009-08-19 20:53                               ` configfs/sysfs Avi Kivity
2009-08-19 21:19                             ` configfs/sysfs Nicholas A. Bellinger
2009-08-19 22:15                             ` configfs/sysfs Gregory Haskins
2009-08-19 22:16                             ` configfs/sysfs Joel Becker
2009-08-19 23:48                               ` [Alacrityvm-devel] configfs/sysfs Alex Tsariounov
2009-08-19 23:54                               ` configfs/sysfs Nicholas A. Bellinger
2009-08-20  6:09                               ` configfs/sysfs Avi Kivity
     [not found]                               ` <4A8CE891.2010502@redhat.com>
2009-08-20 22:48                                 ` configfs/sysfs Joel Becker
2009-08-21  4:14                                   ` configfs/sysfs Avi Kivity
2009-08-19 18:26                         ` [PATCH v3 3/6] vbus: add a "vbus-proxy" bus model for vbus_driver objects Gregory Haskins
2009-08-19 20:37                           ` Avi Kivity
2009-08-19 20:53                             ` Ingo Molnar
2009-08-20 17:25                             ` Muli Ben-Yehuda
2009-08-20 20:58                             ` Caitlin Bestler
2009-08-18 18:20                   ` Arnd Bergmann
2009-08-18 19:08                     ` Avi Kivity
2009-08-19  5:36                     ` Gregory Haskins
2009-08-18  9:53               ` Michael S. Tsirkin
2009-08-18 10:00                 ` Avi Kivity
2009-08-18 10:09                   ` Michael S. Tsirkin
2009-08-18 10:13                     ` Avi Kivity
2009-08-18 10:28                       ` Michael S. Tsirkin
2009-08-18 10:45                         ` Avi Kivity
2009-08-18 11:07                           ` Michael S. Tsirkin
2009-08-18 11:15                             ` Avi Kivity
2009-08-18 11:49                               ` Michael S. Tsirkin
2009-08-18 11:54                                 ` Avi Kivity
2009-08-18 15:39                 ` Gregory Haskins
2009-08-18 16:39                   ` Michael S. Tsirkin
2009-08-17 15:13           ` Avi Kivity
2009-08-14 15:43 ` [PATCH v3 4/6] vbus-proxy: add a pci-to-vbus bridge Gregory Haskins
2009-08-14 15:43 ` [PATCH v3 5/6] ioq: add driver-side vbus helpers Gregory Haskins
2009-08-14 15:43 ` [PATCH v3 6/6] net: Add vbus_enet driver Gregory Haskins

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=4A8C5CBB.10605@redhat.com \
    --to=avi@redhat.com \
    --cc=alacrityvm-devel@lists.sourceforge.net \
    --cc=anthony@codemonkey.ws \
    --cc=iws@ovro.caltech.edu \
    --cc=joel.becker@oracle.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=mst@redhat.com \
    --cc=nab@linux-iscsi.org \
    --cc=netdev@vger.kernel.org \
    /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).