linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Anand Jain <Anand.Jain@oracle.com>
To: kreijack@inwind.it, dsterba@suse.cz, Chris Mason <clm@fb.com>
Cc: Miao Xie <miaox@cn.fujitsu.com>, linux-btrfs@vger.kernel.org
Subject: Re: [PATCH RFC 0/5] Scan all devices to build fs device list
Date: Wed, 10 Sep 2014 14:06:56 +0800	[thread overview]
Message-ID: <540FEA80.6040007@oracle.com> (raw)
In-Reply-To: <540F63BC.6080601@inwind.it>



Goffredo,


Thanks for comments, more in line below.

On 10/09/2014 04:31, Goffredo Baroncelli wrote:
> On 09/09/2014 05:26 AM, Anand Jain wrote:
>
> Anand,
> first thanks to discuss this with me; below my comments
>
>>> You are talking about a ioctl: why not use (extending them when needed)
>>> the sysfs information ?
>>
>>   yeah, I understand I am the only person talking about ioctl,
>>   and all others recommend sysfs. I understand I am wrong, but
>>   just by mass. and I still have no idea why not ioctl ?
>
> Because it is an interface difficult to extend.

  Its quite common to make ioctl auto adaptable by using the
  structure size/version.  btrfs-devlist tool does it too.


>>   Problem with sysfs:
>>    - Note that sysfs supports only one parameter value with max length
>>    u64, to rebuilt entire kernel's fs_uuid list in the user space
>>    that would be quite a lot of sysfs files. should that be fine ?
>>    Further we would need another traverser tool (python ?) to read
>>    all these sysfs files. ? so that we could create fs_uuid list
>>    in the user-space.
> To me it seems that other fs interface scale well even if the number
> of items is big (think /proc)

  /proc supports larger content transfer using seq_file.  /sysfs doesn't.

>>    - we would need all info about the kernel fs_uuid, even when the
>>   device is not mounted.

> Could you elaborate ?
> We have two kind of objects: filesystems and devices. A filesystem is
> created when its first device is registered. When the filesystem
> is mounted, further information are available.
>
> Our hypothetical sysfs interface should export information about
> filesystem and/or devices until they exist. If a filesystem is
> mounted more information are exported. When it is umounted
> less are exported.
>
> This would not be different if we use a ioctl() interface.

  (Its not an on disk structure)
  The purpose of this interface is to read entire of volume.c
  static LIST_HEAD(fs_uuids) list from the btrfs memory to the
  user space. Sorry if it wasn't clear before.

  We desperately need this to solve some critical structural
  problem in btrs-progs.

>>    - thinking of nested seed with sysfs is more complicated, as we would
>>   have same btrfs_device at multiple fs_devices. still we must represent
>>   them in the sysfs.
> I am not used to use seed device; so I can't comment.
>
>>    - as of now fs_uuid can grow infinite with just "a" device.
>>     ref: [PATCH 1/2] btrfs: device list could grow infinite
> This is a btrfs problem. Doesn't make sense to store these information
> if there aren't any device. When the last device disappear (is replaced),
> the filesystem object should disappear too.
> If a sysfs approach point out this problem... to me it seems good ! :-)


>>     can't imagine anybody traversing through /sysfs to understand
>>     what btrfs-kernel think of devices. user would prefer a cli
>>     to know that instead.
> sysfs is an interface, that doesn't means that is THE user interface.
> Of course a CLI or a GUI will need.


>>
>>    - sysfs layout has to be dynamic based on fs_uuids list changes,
>>    devices added, removed, mounted replaced etc.. isn't it making
>>    more unstable ?
>
> $ find /sys | wc -l
> 27362
> On my PC sysfs has more already than 20 thousands entries, and it is
> a quite simple machine. But this doesn't cause instability.
> May be more difficult, but I don't think that sysfs is not capable
> to sustain that.
>
>
> Already linux export a lot of sysfs files for each device. I.e.
>
> $ find /sys/block/sda/ | wc -l
> 	190
>
> these are hundreds entries for each disks. So I don't think that btrfs sysfs
> interface could cause more problem even if a filesystem has
> a lot of disks.


>>
>> appreciate your comments
> The major problem of an ioctl() interface is that is very difficult
> to extend. Typically when we need to extend it , the current is
> marked as old/deprecated and a new one is generated.
> However an ioctl() is an ABI so even the old interface have to be
> maintained.
>
> One point about I have to agree with you is that a sysfs interface
> is not good, is when:
> - we need to export huge quantity of data
> - we have an high rate of data
>
> But these to me don't seem to apply to a sysfs btrfs interface.


  I thought you will say sysfs doesn't have to depend on any
  custom made user space tool to read/write. :-)

  The challenge is to provide sysfs interface without cluttering
  it.  There are a lot talks on sysfs being cluttered.
  We definitely need sysfs interface for btrfs. But I am not
  too sure if we can get all the info without looking btrfs sysfs
  interface too cluttered, typically we might need all info as
  in btrfs-devlist.

Thanks, Anand


  reply	other threads:[~2014-09-10  6:07 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-03 13:36 [PATCH RFC 0/5] Scan all devices to build fs device list Miao Xie
2014-09-03 13:36 ` [PATCH 1/5] block: export disk_class and disk_type for btrfs Miao Xie
2014-09-03 13:36 ` [PATCH 2/5] Btrfs: don't return btrfs_fs_devices if the caller doesn't want it Miao Xie
2014-09-03 13:36 ` [PATCH 3/5] Btrfs: restructure btrfs_scan_one_device Miao Xie
2014-09-03 13:36 ` [PATCH 4/5] Btrfs: restructure btrfs_get_bdev_and_sb and pick up some code used later Miao Xie
2014-09-03 13:36 ` [PATCH 5/5] Btrfs: scan all the devices and build the fs device list by btrfs's self Miao Xie
2014-09-06 11:48   ` Goffredo Baroncelli
2014-09-09  4:06     ` Miao Xie
2014-09-06 20:05 ` [PATCH RFC 0/5] Scan all devices to build fs device list Chris Mason
2014-09-08  9:09   ` David Sterba
2014-09-08 11:04     ` Anand Jain
2014-09-08 17:15       ` Goffredo Baroncelli
2014-09-09  3:26         ` Anand Jain
2014-09-09 20:31           ` Goffredo Baroncelli
2014-09-10  6:06             ` Anand Jain [this message]
2014-09-08 16:59     ` Goffredo Baroncelli

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=540FEA80.6040007@oracle.com \
    --to=anand.jain@oracle.com \
    --cc=clm@fb.com \
    --cc=dsterba@suse.cz \
    --cc=kreijack@inwind.it \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=miaox@cn.fujitsu.com \
    /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).