linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Btrfs: add a disk info ioctl to get the disks attached to a filesystem
@ 2010-09-28 20:53 Josef Bacik
  2010-09-28 22:28 ` Goffredo Baroncelli
  2010-09-28 23:25 ` Christoph Hellwig
  0 siblings, 2 replies; 19+ messages in thread
From: Josef Bacik @ 2010-09-28 20:53 UTC (permalink / raw)
  To: linux-btrfs

This was a request from the systemd guys.  They need a quick and easy way to get
all devices attached to a Btrfs filesystem in order to check if any of the disks
are SSD for...something, I didn't ask :).   I've tested this with the
btrfs-progs patch that accompanies this patch.  Thanks,

Signed-off-by: Josef Bacik <josef@redhat.com>
---
 fs/btrfs/ioctl.c |   64 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/btrfs/ioctl.h |    7 ++++++
 2 files changed, 71 insertions(+), 0 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 9254b3d..f59b0bc 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -1957,6 +1957,68 @@ out:
 	return ret;
 }
 
+static noinline long btrfs_ioctl_disk_info(struct btrfs_root *root,
+					   void __user *arg)
+{
+	struct btrfs_ioctl_disk_info_args di_args;
+	u64 *user_dest;
+	u64 *dest = NULL;
+	struct btrfs_device *device;
+	struct list_head *devices;
+	int alloc_size = 0;
+	int ret = 0;
+
+	if (copy_from_user(&di_args,
+			   (struct btrfs_ioctl_disk_info_args __user *)arg,
+			   sizeof(di_args)))
+		return -EFAULT;
+
+	mutex_lock(&root->fs_info->fs_devices->device_list_mutex);
+	if (!di_args.num_devices) {
+		di_args.num_devices = root->fs_info->fs_devices->num_devices;
+		goto out;
+	}
+	alloc_size = sizeof(u64) * di_args.num_devices;
+
+	di_args.num_devices = 0;
+
+	/*
+	 * If we have more than 4k worth of space to hold a bunch of u64's,
+	 * somebody is misbehaving.
+	 */
+	if (alloc_size > PAGE_CACHE_SIZE) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	dest = kzalloc(alloc_size, GFP_NOFS);
+	if (!dest) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	devices = &root->fs_info->fs_devices->devices;
+
+	list_for_each_entry(device, devices, dev_list) {
+		dest[di_args.num_devices] =
+			huge_encode_dev(device->bdev->bd_dev);
+		di_args.num_devices++;
+	}
+
+	user_dest = (u64 *)
+		(arg + sizeof(struct btrfs_ioctl_disk_info_args));
+
+	if (copy_to_user(user_dest, dest, alloc_size))
+		ret = -EFAULT;
+out:
+	mutex_unlock(&root->fs_info->fs_devices->device_list_mutex);
+	if (ret == 0 && copy_to_user(arg, &di_args, sizeof(di_args)))
+		ret = -EFAULT;
+	kfree(dest);
+
+	return ret;
+}
+
 /*
  * there are many ways the trans_start and trans_end ioctls can lead
  * to deadlocks.  They should only be used by applications that
@@ -2031,6 +2093,8 @@ long btrfs_ioctl(struct file *file, unsigned int
 		return btrfs_ioctl_ino_lookup(file, argp);
 	case BTRFS_IOC_SPACE_INFO:
 		return btrfs_ioctl_space_info(root, argp);
+	case BTRFS_IOC_DISK_INFO:
+		return btrfs_ioctl_disk_info(root, argp);
 	case BTRFS_IOC_SYNC:
 		btrfs_sync_fs(file->f_dentry->d_sb, 1);
 		return 0;
diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h
index 424694a..294e8c3 100644
--- a/fs/btrfs/ioctl.h
+++ b/fs/btrfs/ioctl.h
@@ -138,6 +138,11 @@ struct btrfs_ioctl_space_args {
 	struct btrfs_ioctl_space_info spaces[0];
 };
 
+struct btrfs_ioctl_disk_info_args {
+	__u32 num_devices;
+	__u64 devices[0];
+};
+
 #define BTRFS_IOC_SNAP_CREATE _IOW(BTRFS_IOCTL_MAGIC, 1, \
 				   struct btrfs_ioctl_vol_args)
 #define BTRFS_IOC_DEFRAG _IOW(BTRFS_IOCTL_MAGIC, 2, \
@@ -178,4 +183,6 @@ struct btrfs_ioctl_space_args {
 #define BTRFS_IOC_DEFAULT_SUBVOL _IOW(BTRFS_IOCTL_MAGIC, 19, u64)
 #define BTRFS_IOC_SPACE_INFO _IOWR(BTRFS_IOCTL_MAGIC, 20, \
 				    struct btrfs_ioctl_space_args)
+#define BTRFS_IOC_DISK_INFO _IOWR(BTRFS_IOCTL_MAGIC, 21, \
+				  struct btrfs_ioctl_disk_info_args)
 #endif
-- 
1.6.6.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH] Btrfs: add a disk info ioctl to get the disks attached to a filesystem
  2010-09-28 20:53 [PATCH] Btrfs: add a disk info ioctl to get the disks attached to a filesystem Josef Bacik
@ 2010-09-28 22:28 ` Goffredo Baroncelli
  2010-09-29  0:24   ` Josef Bacik
  2010-09-28 23:25 ` Christoph Hellwig
  1 sibling, 1 reply; 19+ messages in thread
From: Goffredo Baroncelli @ 2010-09-28 22:28 UTC (permalink / raw)
  To: Josef Bacik; +Cc: linux-btrfs

Hi Josef,

what about using your ioctl to exporting also other info ? For example devid, 
size, bytes, uuid ...

I know that btrfs filesystem-show does the same, but it reads the partition 
(the disk) instead of query the filesystem. That means:
- It work even for unmounted filesystem (which is good)
- The results are inaccurate for a mounted filesystem, because the read data 
is not valid until a flush of the pages (which is bad.. very bad).

All the data are enclosed in the struct btrfs_device, so is very easy to 
extract.

Regards
G.Baroncelli


On Tuesday, 28 September, 2010, Josef Bacik wrote:
> This was a request from the systemd guys.  They need a quick and easy way to 
get
[...]



-- 
gpg key@ keyserver.linux.it: Goffredo Baroncelli (ghigo) <kreijack@inwind.it>
Key fingerprint = 4769 7E51 5293 D36C 814E  C054 BF04 F161 3DC5 0512

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] Btrfs: add a disk info ioctl to get the disks attached to a filesystem
  2010-09-28 20:53 [PATCH] Btrfs: add a disk info ioctl to get the disks attached to a filesystem Josef Bacik
  2010-09-28 22:28 ` Goffredo Baroncelli
@ 2010-09-28 23:25 ` Christoph Hellwig
  2010-09-29  0:08   ` Josef Bacik
  2010-09-29 12:19   ` Kay Sievers
  1 sibling, 2 replies; 19+ messages in thread
From: Christoph Hellwig @ 2010-09-28 23:25 UTC (permalink / raw)
  To: Josef Bacik; +Cc: linux-btrfs

On Tue, Sep 28, 2010 at 04:53:16PM -0400, Josef Bacik wrote:
> This was a request from the systemd guys.  They need a quick and easy way to get
> all devices attached to a Btrfs filesystem in order to check if any of the disks
> are SSD for...something, I didn't ask :).   I've tested this with the
> btrfs-progs patch that accompanies this patch.  Thanks,

So please tell the "systemd guys" to explain what the fuck they're doing
to linux-fsdevel and fiend a proper interface.  Chance is they will fuck
up as much as just about ever other lowlevel userspace tool are very
high.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] Btrfs: add a disk info ioctl to get the disks attached to a filesystem
  2010-09-28 23:25 ` Christoph Hellwig
@ 2010-09-29  0:08   ` Josef Bacik
  2010-09-29  0:19     ` Lennart Poettering
  2010-09-29 12:19   ` Kay Sievers
  1 sibling, 1 reply; 19+ messages in thread
From: Josef Bacik @ 2010-09-29  0:08 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Josef Bacik, linux-btrfs, mzerqung

On Tue, Sep 28, 2010 at 07:25:13PM -0400, Christoph Hellwig wrote:
> On Tue, Sep 28, 2010 at 04:53:16PM -0400, Josef Bacik wrote:
> > This was a request from the systemd guys.  They need a quick and easy way to get
> > all devices attached to a Btrfs filesystem in order to check if any of the disks
> > are SSD for...something, I didn't ask :).   I've tested this with the
> > btrfs-progs patch that accompanies this patch.  Thanks,
> 
> So please tell the "systemd guys" to explain what the fuck they're doing
> to linux-fsdevel and fiend a proper interface.  Chance is they will fuck
> up as much as just about ever other lowlevel userspace tool are very
> high.
> 

Lennart? :).  And Christoph, what would be a good interface?  LVM has a slaves/
subdir in sysfs which symlinks to all of their dev's, would you rather I
resurrect the sysfs stuff for Btrfs and do a similar thing?  I'm open to
suggestions, I just took the quick and painless way out.  Thanks,

Josef

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] Btrfs: add a disk info ioctl to get the disks attached to a filesystem
  2010-09-29  0:08   ` Josef Bacik
@ 2010-09-29  0:19     ` Lennart Poettering
  2010-09-29  7:25       ` Ric Wheeler
  0 siblings, 1 reply; 19+ messages in thread
From: Lennart Poettering @ 2010-09-29  0:19 UTC (permalink / raw)
  To: Josef Bacik; +Cc: Christoph Hellwig, linux-btrfs

On Tue, 28.09.10 20:08, Josef Bacik (josef@redhat.com) wrote:

> 
> On Tue, Sep 28, 2010 at 07:25:13PM -0400, Christoph Hellwig wrote:
> > On Tue, Sep 28, 2010 at 04:53:16PM -0400, Josef Bacik wrote:
> > > This was a request from the systemd guys.  They need a quick and easy way to get
> > > all devices attached to a Btrfs filesystem in order to check if any of the disks
> > > are SSD for...something, I didn't ask :).   I've tested this with the
> > > btrfs-progs patch that accompanies this patch.  Thanks,
> > 
> > So please tell the "systemd guys" to explain what the fuck they're doing
> > to linux-fsdevel and fiend a proper interface.  Chance is they will fuck
> > up as much as just about ever other lowlevel userspace tool are very
> > high.
> > 
> 
> Lennart? :).  And Christoph, what would be a good interface?  LVM has a slaves/
> subdir in sysfs which symlinks to all of their dev's, would you rather I
> resurrect the sysfs stuff for Btrfs and do a similar thing?  I'm open to
> suggestions, I just took the quick and painless way out.  Thanks,

When doing readahead you want to know whether you are on SSD or rotating
media, because you a) want to order the readahead requests on bootup
after access time on SSD and after location on disk on rotating
media. And b) because you might want to priorize readahead reads over
other reads on rotating media, but prefer other reads over readahead
reads on SSD.

This in fact is how all current readahead implementations work, be it
the fedora, the suse or ubuntu's readahead or Arjan's sreadahead. What's
new is that in the systemd case we try to test for ssd/rotating
properly, instead of just hardcoding a check for
/sys/class/block/sda/queue/rotational.

I hope this explains what the fuck we are doing.

Lennart

-- 
Lennart Poettering - Red Hat, Inc.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] Btrfs: add a disk info ioctl to get the disks attached to a filesystem
  2010-09-28 22:28 ` Goffredo Baroncelli
@ 2010-09-29  0:24   ` Josef Bacik
  0 siblings, 0 replies; 19+ messages in thread
From: Josef Bacik @ 2010-09-29  0:24 UTC (permalink / raw)
  To: kreijack; +Cc: Josef Bacik, linux-btrfs

On Wed, Sep 29, 2010 at 12:28:51AM +0200, Goffredo Baroncelli wrote:
> Hi Josef,
> 
> what about using your ioctl to exporting also other info ? For example devid, 
> size, bytes, uuid ...
> 
> I know that btrfs filesystem-show does the same, but it reads the partition 
> (the disk) instead of query the filesystem. That means:
> - It work even for unmounted filesystem (which is good)
> - The results are inaccurate for a mounted filesystem, because the read data 
> is not valid until a flush of the pages (which is bad.. very bad).
>

Well a) the data on disk is always valid, thats the point :).  And b) the only
way the data is not uptodate is if we happened to add/remove/resize a disk right
before running btrfs-show, something that isn't going to happen often.  So I
think btrfs-show solves this problem well enough.  My ioctl is to give apps a
nice quick programmable way to get the list of disks in the fs so they can do
whatever they want instead of having to ship a libbtrfs or some such thing and
make them use that.  Thanks,

Josef

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] Btrfs: add a disk info ioctl to get the disks attached to a filesystem
  2010-09-29  0:19     ` Lennart Poettering
@ 2010-09-29  7:25       ` Ric Wheeler
  2010-09-29  8:04         ` Kay Sievers
  2010-09-29 11:59         ` Lennart Poettering
  0 siblings, 2 replies; 19+ messages in thread
From: Ric Wheeler @ 2010-09-29  7:25 UTC (permalink / raw)
  To: Lennart Poettering; +Cc: Josef Bacik, Christoph Hellwig, linux-btrfs

  On 09/29/2010 09:19 AM, Lennart Poettering wrote:
> On Tue, 28.09.10 20:08, Josef Bacik (josef@redhat.com) wrote:
>
>> On Tue, Sep 28, 2010 at 07:25:13PM -0400, Christoph Hellwig wrote:
>>> On Tue, Sep 28, 2010 at 04:53:16PM -0400, Josef Bacik wrote:
>>>> This was a request from the systemd guys.  They need a quick and easy way to get
>>>> all devices attached to a Btrfs filesystem in order to check if any of the disks
>>>> are SSD for...something, I didn't ask :).   I've tested this with the
>>>> btrfs-progs patch that accompanies this patch.  Thanks,
>>> So please tell the "systemd guys" to explain what the fuck they're doing
>>> to linux-fsdevel and fiend a proper interface.  Chance is they will fuck
>>> up as much as just about ever other lowlevel userspace tool are very
>>> high.
>>>
>> Lennart? :).  And Christoph, what would be a good interface?  LVM has a slaves/
>> subdir in sysfs which symlinks to all of their dev's, would you rather I
>> resurrect the sysfs stuff for Btrfs and do a similar thing?  I'm open to
>> suggestions, I just took the quick and painless way out.  Thanks,
> When doing readahead you want to know whether you are on SSD or rotating
> media, because you a) want to order the readahead requests on bootup
> after access time on SSD and after location on disk on rotating
> media. And b) because you might want to priorize readahead reads over
> other reads on rotating media, but prefer other reads over readahead
> reads on SSD.
>
> This in fact is how all current readahead implementations work, be it
> the fedora, the suse or ubuntu's readahead or Arjan's sreadahead. What's
> new is that in the systemd case we try to test for ssd/rotating
> properly, instead of just hardcoding a check for
> /sys/class/block/sda/queue/rotational.
>

A couple of questions pop into mind - is systemd the right place to 
automatically tune readahead?  If this is a generic feature for the type of 
device, it sounds like something that we should be doing somewhere else in the 
stack (not relying on tuning from user space).

Second question is why is checking in /sys a big deal, would  you prefer an 
interface like we did for alignment in libblkid?

Ric


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] Btrfs: add a disk info ioctl to get the disks attached to a filesystem
  2010-09-29  7:25       ` Ric Wheeler
@ 2010-09-29  8:04         ` Kay Sievers
  2010-09-29 23:43           ` Christoph Hellwig
  2010-09-29 11:59         ` Lennart Poettering
  1 sibling, 1 reply; 19+ messages in thread
From: Kay Sievers @ 2010-09-29  8:04 UTC (permalink / raw)
  To: Ric Wheeler
  Cc: Lennart Poettering, Josef Bacik, Christoph Hellwig, linux-btrfs

On Wed, Sep 29, 2010 at 09:25, Ric Wheeler <rwheeler@redhat.com> wrote:

> Second question is why is checking in /sys a big deal, would =C2=A0yo=
u prefer an
> interface like we did for alignment in libblkid?

It's about knowing what's behind the 'nodev' major =3D=3D 0 of a btrfs
mount. There is no way to get that from /sys or anywhere else at the
moment.

Usually filesystems backed by a disk have the dev_t of the device, or
the fake block devices like md/dm/raid have their own major and the
slaves/ directory pointing to the devices.

This is not only about readahead, it's every other tool, that needs to
know what kind of disks are behind a btrfs 'nodev' major =3D=3D 0 mount=
=2E

Kay
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" =
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] Btrfs: add a disk info ioctl to get the disks attached to a filesystem
  2010-09-29  7:25       ` Ric Wheeler
  2010-09-29  8:04         ` Kay Sievers
@ 2010-09-29 11:59         ` Lennart Poettering
  2010-09-29 12:08           ` Ric Wheeler
  1 sibling, 1 reply; 19+ messages in thread
From: Lennart Poettering @ 2010-09-29 11:59 UTC (permalink / raw)
  To: Ric Wheeler; +Cc: Josef Bacik, Christoph Hellwig, linux-btrfs, Kay Sievers

On Wed, 29.09.10 16:25, Ric Wheeler (rwheeler@redhat.com) wrote:

> >This in fact is how all current readahead implementations work, be it
> >the fedora, the suse or ubuntu's readahead or Arjan's sreadahead. What's
> >new is that in the systemd case we try to test for ssd/rotating
> >properly, instead of just hardcoding a check for
> >/sys/class/block/sda/queue/rotational.
> >
> 
> A couple of questions pop into mind - is systemd the right place to
> automatically tune readahead?  If this is a generic feature for the
> type of device, it sounds like something that we should be doing
> somewhere else in the stack (not relying on tuning from user space).

Note that this is not the kind of readahead that is controllable via 
/sys/class/block/sda/queue/read_ahead_kb, this is about detecting "hot"
files at boot, and then preloading them on the next boot. i.e. the
problem Jens once proposed fcache for.

> Second question is why is checking in /sys a big deal, would  you
> prefer an interface like we did for alignment in libblkid?

Well, currently there's no way to discover the underlying block devices
if you have a btrfs mount point. This is what Josef's patch added for
us.

Lennart

-- 
Lennart Poettering - Red Hat, Inc.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] Btrfs: add a disk info ioctl to get the disks attached to a filesystem
  2010-09-29 11:59         ` Lennart Poettering
@ 2010-09-29 12:08           ` Ric Wheeler
  0 siblings, 0 replies; 19+ messages in thread
From: Ric Wheeler @ 2010-09-29 12:08 UTC (permalink / raw)
  To: Lennart Poettering
  Cc: Josef Bacik, Christoph Hellwig, linux-btrfs, Kay Sievers

  On 09/29/2010 08:59 PM, Lennart Poettering wrote:
> On Wed, 29.09.10 16:25, Ric Wheeler (rwheeler@redhat.com) wrote:
>
>>> This in fact is how all current readahead implementations work, be it
>>> the fedora, the suse or ubuntu's readahead or Arjan's sreadahead. What's
>>> new is that in the systemd case we try to test for ssd/rotating
>>> properly, instead of just hardcoding a check for
>>> /sys/class/block/sda/queue/rotational.
>>>
>> A couple of questions pop into mind - is systemd the right place to
>> automatically tune readahead?  If this is a generic feature for the
>> type of device, it sounds like something that we should be doing
>> somewhere else in the stack (not relying on tuning from user space).
> Note that this is not the kind of readahead that is controllable via
> /sys/class/block/sda/queue/read_ahead_kb, this is about detecting "hot"
> files at boot, and then preloading them on the next boot. i.e. the
> problem Jens once proposed fcache for.
>
>> Second question is why is checking in /sys a big deal, would  you
>> prefer an interface like we did for alignment in libblkid?
> Well, currently there's no way to discover the underlying block devices
> if you have a btrfs mount point. This is what Josef's patch added for
> us.
>
> Lennart
>

Makes sense to me,thanks!

Ric


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] Btrfs: add a disk info ioctl to get the disks attached to a filesystem
  2010-09-28 23:25 ` Christoph Hellwig
  2010-09-29  0:08   ` Josef Bacik
@ 2010-09-29 12:19   ` Kay Sievers
  1 sibling, 0 replies; 19+ messages in thread
From: Kay Sievers @ 2010-09-29 12:19 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Josef Bacik, linux-btrfs

On Wed, Sep 29, 2010 at 01:25, Christoph Hellwig <hch@infradead.org> wr=
ote:
> On Tue, Sep 28, 2010 at 04:53:16PM -0400, Josef Bacik wrote:
>> This was a request from the systemd guys. =C2=A0They need a quick an=
d easy way to get
>> all devices attached to a Btrfs filesystem in order to check if any =
of the disks
>> are SSD for...something, I didn't ask :). =C2=A0 I've tested this wi=
th the
>> btrfs-progs patch that accompanies this patch. =C2=A0Thanks,
>
> So please tell the "systemd guys" to explain what the fuck they're do=
ing
> to linux-fsdevel and fiend a proper interface. =C2=A0Chance is they w=
ill fuck
> up as much as just about ever other lowlevel userspace tool are very
> high.

=46uck like these comments make it incredibly hard to find the few
statements where you are right, in all the fucking noise you are
creating.

Thanks,
Kay
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" =
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] Btrfs: add a disk info ioctl to get the disks attached to a filesystem
  2010-09-29  8:04         ` Kay Sievers
@ 2010-09-29 23:43           ` Christoph Hellwig
  2010-09-30  0:32             ` Josef Bacik
                               ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Christoph Hellwig @ 2010-09-29 23:43 UTC (permalink / raw)
  To: Kay Sievers
  Cc: Ric Wheeler, Lennart Poettering, Josef Bacik, Christoph Hellwig,
	linux-btrfs

On Wed, Sep 29, 2010 at 10:04:31AM +0200, Kay Sievers wrote:
> On Wed, Sep 29, 2010 at 09:25, Ric Wheeler <rwheeler@redhat.com> wrote:
> 
> > Second question is why is checking in /sys a big deal, would ??you prefer an
> > interface like we did for alignment in libblkid?
> 
> It's about knowing what's behind the 'nodev' major == 0 of a btrfs
> mount. There is no way to get that from /sys or anywhere else at the
> moment.
> 
> Usually filesystems backed by a disk have the dev_t of the device, or
> the fake block devices like md/dm/raid have their own major and the
> slaves/ directory pointing to the devices.
> 
> This is not only about readahead, it's every other tool, that needs to
> know what kind of disks are behind a btrfs 'nodev' major == 0 mount.

Thanks for explaining the problem.  It's one that affects everything
with more than one underlying block device, so adding a
filesystem-specific ioctl hack is not a good idea.  As mentioned in this
mail we already have a solution for that - the block device slaves
links used for raid and volume managers.  The most logical fix is to
re-use that for btrfs as well and stop it from abusing the anonymous
block major that was never intended for block based filesystems (and
already has caused trouble in other areas).  One way to to this might
be to allocate a block major for btrfs that only gets used for
representing these links.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] Btrfs: add a disk info ioctl to get the disks attached to a filesystem
  2010-09-29 23:43           ` Christoph Hellwig
@ 2010-09-30  0:32             ` Josef Bacik
  2010-09-30  7:43             ` Kay Sievers
  2010-09-30 19:48             ` Josef Bacik
  2 siblings, 0 replies; 19+ messages in thread
From: Josef Bacik @ 2010-09-30  0:32 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Kay Sievers, Ric Wheeler, Lennart Poettering, Josef Bacik, linux-btrfs

On Wed, Sep 29, 2010 at 07:43:27PM -0400, Christoph Hellwig wrote:
> On Wed, Sep 29, 2010 at 10:04:31AM +0200, Kay Sievers wrote:
> > On Wed, Sep 29, 2010 at 09:25, Ric Wheeler <rwheeler@redhat.com> wrote:
> > 
> > > Second question is why is checking in /sys a big deal, would ??you prefer an
> > > interface like we did for alignment in libblkid?
> > 
> > It's about knowing what's behind the 'nodev' major == 0 of a btrfs
> > mount. There is no way to get that from /sys or anywhere else at the
> > moment.
> > 
> > Usually filesystems backed by a disk have the dev_t of the device, or
> > the fake block devices like md/dm/raid have their own major and the
> > slaves/ directory pointing to the devices.
> > 
> > This is not only about readahead, it's every other tool, that needs to
> > know what kind of disks are behind a btrfs 'nodev' major == 0 mount.
> 
> Thanks for explaining the problem.  It's one that affects everything
> with more than one underlying block device, so adding a
> filesystem-specific ioctl hack is not a good idea.  As mentioned in this
> mail we already have a solution for that - the block device slaves
> links used for raid and volume managers.  The most logical fix is to
> re-use that for btrfs as well and stop it from abusing the anonymous
> block major that was never intended for block based filesystems (and
> already has caused trouble in other areas).  One way to to this might
> be to allocate a block major for btrfs that only gets used for
> representing these links.
>

Fair enough, I will look into this next week sometime.  Thanks,

Josef 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] Btrfs: add a disk info ioctl to get the disks attached to a filesystem
  2010-09-29 23:43           ` Christoph Hellwig
  2010-09-30  0:32             ` Josef Bacik
@ 2010-09-30  7:43             ` Kay Sievers
  2010-09-30 12:38               ` Josef Bacik
  2010-09-30 13:47               ` Andi Kleen
  2010-09-30 19:48             ` Josef Bacik
  2 siblings, 2 replies; 19+ messages in thread
From: Kay Sievers @ 2010-09-30  7:43 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Ric Wheeler, Lennart Poettering, Josef Bacik, linux-btrfs

On Thu, Sep 30, 2010 at 01:43, Christoph Hellwig <hch@infradead.org> wr=
ote:
> On Wed, Sep 29, 2010 at 10:04:31AM +0200, Kay Sievers wrote:
>> On Wed, Sep 29, 2010 at 09:25, Ric Wheeler <rwheeler@redhat.com> wro=
te:
>>
>> > Second question is why is checking in /sys a big deal, would ??you=
 prefer an
>> > interface like we did for alignment in libblkid?
>>
>> It's about knowing what's behind the 'nodev' major =3D=3D 0 of a btr=
fs
>> mount. There is no way to get that from /sys or anywhere else at the
>> moment.
>>
>> Usually filesystems backed by a disk have the dev_t of the device, o=
r
>> the fake block devices like md/dm/raid have their own major and the
>> slaves/ directory pointing to the devices.
>>
>> This is not only about readahead, it's every other tool, that needs =
to
>> know what kind of disks are behind a btrfs 'nodev' major =3D=3D 0 mo=
unt.
>
> Thanks for explaining the problem. =C2=A0It's one that affects everyt=
hing
> with more than one underlying block device, so adding a
> filesystem-specific ioctl hack is not a good idea. =C2=A0As mentioned=
 in this
> mail we already have a solution for that - the block device slaves
> links used for raid and volume managers. =C2=A0The most logical fix i=
s to
> re-use that for btrfs as well and stop it from abusing the anonymous
> block major that was never intended for block based filesystems (and
> already has caused trouble in other areas). =C2=A0One way to to this =
might
> be to allocate a block major for btrfs that only gets used for
> representing these links.

Yeah, we thought about that too, but a btrfs mount does not show up as
a block device, like md/dm, so there is no place for a slaves/
directory in /sys with the individual disks listed. How could be solve
that? Create some fake blockdev for every btrfs mount,  but that can't
be used to read/write raw blocks?

A generic solution, statfs()-like, which operates at the superblock
would be another option. Any idea if that could be made working?

Kay
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" =
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] Btrfs: add a disk info ioctl to get the disks attached to a filesystem
  2010-09-30  7:43             ` Kay Sievers
@ 2010-09-30 12:38               ` Josef Bacik
  2010-09-30 13:47               ` Andi Kleen
  1 sibling, 0 replies; 19+ messages in thread
From: Josef Bacik @ 2010-09-30 12:38 UTC (permalink / raw)
  To: Kay Sievers
  Cc: Christoph Hellwig, Ric Wheeler, Lennart Poettering, Josef Bacik,
	linux-btrfs

On Thu, Sep 30, 2010 at 09:43:00AM +0200, Kay Sievers wrote:
> On Thu, Sep 30, 2010 at 01:43, Christoph Hellwig <hch@infradead.org> =
wrote:
> > On Wed, Sep 29, 2010 at 10:04:31AM +0200, Kay Sievers wrote:
> >> On Wed, Sep 29, 2010 at 09:25, Ric Wheeler <rwheeler@redhat.com> w=
rote:
> >>
> >> > Second question is why is checking in /sys a big deal, would ??y=
ou prefer an
> >> > interface like we did for alignment in libblkid?
> >>
> >> It's about knowing what's behind the 'nodev' major =3D=3D 0 of a b=
trfs
> >> mount. There is no way to get that from /sys or anywhere else at t=
he
> >> moment.
> >>
> >> Usually filesystems backed by a disk have the dev_t of the device,=
 or
> >> the fake block devices like md/dm/raid have their own major and th=
e
> >> slaves/ directory pointing to the devices.
> >>
> >> This is not only about readahead, it's every other tool, that need=
s to
> >> know what kind of disks are behind a btrfs 'nodev' major =3D=3D 0 =
mount.
> >
> > Thanks for explaining the problem. =A0It's one that affects everyth=
ing
> > with more than one underlying block device, so adding a
> > filesystem-specific ioctl hack is not a good idea. =A0As mentioned =
in this
> > mail we already have a solution for that - the block device slaves
> > links used for raid and volume managers. =A0The most logical fix is=
 to
> > re-use that for btrfs as well and stop it from abusing the anonymou=
s
> > block major that was never intended for block based filesystems (an=
d
> > already has caused trouble in other areas). =A0One way to to this m=
ight
> > be to allocate a block major for btrfs that only gets used for
> > representing these links.
>=20
> Yeah, we thought about that too, but a btrfs mount does not show up a=
s
> a block device, like md/dm, so there is no place for a slaves/
> directory in /sys with the individual disks listed. How could be solv=
e
> that? Create some fake blockdev for every btrfs mount,  but that can'=
t
> be used to read/write raw blocks?
>=20

That's what I was going to do.  We essentially do that anyway with the =
anonymous
superblock, so instead I'll just make /dev/btrfs-# whatever and do the
bd_claim_by_disk stuff to make all of our devices slaves of that parent=
 virtual
device.  Does this seem like a resonable solution?  Thanks,

Josef
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" =
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] Btrfs: add a disk info ioctl to get the disks attached to a filesystem
  2010-09-30  7:43             ` Kay Sievers
  2010-09-30 12:38               ` Josef Bacik
@ 2010-09-30 13:47               ` Andi Kleen
  1 sibling, 0 replies; 19+ messages in thread
From: Andi Kleen @ 2010-09-30 13:47 UTC (permalink / raw)
  To: Kay Sievers
  Cc: Christoph Hellwig, Ric Wheeler, Lennart Poettering, Josef Bacik,
	linux-btrfs

Kay Sievers <kay.sievers@vrfy.org> writes:
>
> Yeah, we thought about that too, but a btrfs mount does not show up as
> a block device, like md/dm, so there is no place for a slaves/
> directory in /sys with the individual disks listed. How could be solve
> that? Create some fake blockdev for every btrfs mount,  but that can't
> be used to read/write raw blocks?

You could simply create a new class for btrfs? (or maybe a generic
"compound" class)

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] Btrfs: add a disk info ioctl to get the disks attached to a filesystem
  2010-09-29 23:43           ` Christoph Hellwig
  2010-09-30  0:32             ` Josef Bacik
  2010-09-30  7:43             ` Kay Sievers
@ 2010-09-30 19:48             ` Josef Bacik
  2010-09-30 19:59               ` Kay Sievers
  2 siblings, 1 reply; 19+ messages in thread
From: Josef Bacik @ 2010-09-30 19:48 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Kay Sievers, Ric Wheeler, Lennart Poettering, Josef Bacik,
	linux-btrfs, chris.mason

On Wed, Sep 29, 2010 at 07:43:27PM -0400, Christoph Hellwig wrote:
> On Wed, Sep 29, 2010 at 10:04:31AM +0200, Kay Sievers wrote:
> > On Wed, Sep 29, 2010 at 09:25, Ric Wheeler <rwheeler@redhat.com> wrote:
> > 
> > > Second question is why is checking in /sys a big deal, would ??you prefer an
> > > interface like we did for alignment in libblkid?
> > 
> > It's about knowing what's behind the 'nodev' major == 0 of a btrfs
> > mount. There is no way to get that from /sys or anywhere else at the
> > moment.
> > 
> > Usually filesystems backed by a disk have the dev_t of the device, or
> > the fake block devices like md/dm/raid have their own major and the
> > slaves/ directory pointing to the devices.
> > 
> > This is not only about readahead, it's every other tool, that needs to
> > know what kind of disks are behind a btrfs 'nodev' major == 0 mount.
> 
> Thanks for explaining the problem.  It's one that affects everything
> with more than one underlying block device, so adding a
> filesystem-specific ioctl hack is not a good idea.  As mentioned in this
> mail we already have a solution for that - the block device slaves
> links used for raid and volume managers.  The most logical fix is to
> re-use that for btrfs as well and stop it from abusing the anonymous
> block major that was never intended for block based filesystems (and
> already has caused trouble in other areas).  One way to to this might
> be to allocate a block major for btrfs that only gets used for
> representing these links.
> 

Ok I've spent a few hours on this and I'm hitting a wall.  In order to get the
sort of /sys/block/btrfs-# sort of thing I have to do

1) register_blkdev to get a major
2) setup a gendisk
3) do a bdget_disk
4) Loop through all of our devices and do a bd_claim_by_disk on each of them

This sucks because for step #2 I have to have a request_queue for the disk.
It's a bogus disk, and theres no way to not have a request_queue, so I'd have to
wire that up and put a bunch of WARN_ON()'s to make sure nobody is trying to
write to our special disk (since I assume that if I go through all this crap I'm
going to end up with a /dev/btrfs-# that people are going to try to write to).

So my question is, is this what we want?  Do I just need to quit bitching and
make it work?  Or am I doing something wrong?  This is a completely new area for
me so I'm just looking around at what md/dm does and trying to mirror it for my
own uses, if thats not what I should be doing please tell me, otherwise this
seems like alot of work for a very shitty solution to our problem.  Thanks,

Josef

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] Btrfs: add a disk info ioctl to get the disks attached to a filesystem
  2010-09-30 19:48             ` Josef Bacik
@ 2010-09-30 19:59               ` Kay Sievers
  2010-09-30 20:37                 ` Lennart Poettering
  0 siblings, 1 reply; 19+ messages in thread
From: Kay Sievers @ 2010-09-30 19:59 UTC (permalink / raw)
  To: Josef Bacik
  Cc: Christoph Hellwig, Ric Wheeler, Lennart Poettering, linux-btrfs,
	chris.mason

On Thu, Sep 30, 2010 at 21:48, Josef Bacik <josef@redhat.com> wrote:
> On Wed, Sep 29, 2010 at 07:43:27PM -0400, Christoph Hellwig wrote:
>> On Wed, Sep 29, 2010 at 10:04:31AM +0200, Kay Sievers wrote:
>> > On Wed, Sep 29, 2010 at 09:25, Ric Wheeler <rwheeler@redhat.com> w=
rote:
>> >
>> > > Second question is why is checking in /sys a big deal, would ??y=
ou prefer an
>> > > interface like we did for alignment in libblkid?
>> >
>> > It's about knowing what's behind the 'nodev' major =3D=3D 0 of a b=
trfs
>> > mount. There is no way to get that from /sys or anywhere else at t=
he
>> > moment.
>> >
>> > Usually filesystems backed by a disk have the dev_t of the device,=
 or
>> > the fake block devices like md/dm/raid have their own major and th=
e
>> > slaves/ directory pointing to the devices.
>> >
>> > This is not only about readahead, it's every other tool, that need=
s to
>> > know what kind of disks are behind a btrfs 'nodev' major =3D=3D 0 =
mount.
>>
>> Thanks for explaining the problem. =C2=A0It's one that affects every=
thing
>> with more than one underlying block device, so adding a
>> filesystem-specific ioctl hack is not a good idea. =C2=A0As mentione=
d in this
>> mail we already have a solution for that - the block device slaves
>> links used for raid and volume managers. =C2=A0The most logical fix =
is to
>> re-use that for btrfs as well and stop it from abusing the anonymous
>> block major that was never intended for block based filesystems (and
>> already has caused trouble in other areas). =C2=A0One way to to this=
 might
>> be to allocate a block major for btrfs that only gets used for
>> representing these links.
>>
>
> Ok I've spent a few hours on this and I'm hitting a wall. =C2=A0In or=
der to get the
> sort of /sys/block/btrfs-# sort of thing I have to do
>
> 1) register_blkdev to get a major
> 2) setup a gendisk
> 3) do a bdget_disk
> 4) Loop through all of our devices and do a bd_claim_by_disk on each =
of them
>
> This sucks because for step #2 I have to have a request_queue for the=
 disk.
> It's a bogus disk, and theres no way to not have a request_queue, so =
I'd have to
> wire that up and put a bunch of WARN_ON()'s to make sure nobody is tr=
ying to
> write to our special disk (since I assume that if I go through all th=
is crap I'm
> going to end up with a /dev/btrfs-# that people are going to try to w=
rite to).
>
> So my question is, is this what we want? =C2=A0Do I just need to quit=
 bitching and
> make it work? =C2=A0Or am I doing something wrong? =C2=A0This is a co=
mpletely new area for
> me so I'm just looking around at what md/dm does and trying to mirror=
 it for my
> own uses, if thats not what I should be doing please tell me, otherwi=
se this
> seems like alot of work for a very shitty solution to our problem. =C2=
=A0Thanks,

Yeah, that matches what I was experiencing when thinking about the
options. Making a btrfs mount a fake blockdev of zero size seems like
a pretty weird hack, just get some 'dead' directories in sysfs. A
btrfs mount is just not a raw blockdev, and should probably not
pretend to be one.

I guess a statfs()-like call from the filesystem side and not the
block side, which can put out such information in some generic way,
would better fit here.

Kay
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" =
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] Btrfs: add a disk info ioctl to get the disks attached to  a filesystem
  2010-09-30 19:59               ` Kay Sievers
@ 2010-09-30 20:37                 ` Lennart Poettering
  0 siblings, 0 replies; 19+ messages in thread
From: Lennart Poettering @ 2010-09-30 20:37 UTC (permalink / raw)
  To: Kay Sievers
  Cc: Josef Bacik, Christoph Hellwig, Ric Wheeler, linux-btrfs, chris.mason

On Thu, 30.09.10 21:59, Kay Sievers (kay.sievers@vrfy.org) wrote:

> > So my question is, is this what we want? =A0Do I just need to quit =
bitching and
> > make it work? =A0Or am I doing something wrong? =A0This is a comple=
tely new area for
> > me so I'm just looking around at what md/dm does and trying to mirr=
or it for my
> > own uses, if thats not what I should be doing please tell me, other=
wise this
> > seems like alot of work for a very shitty solution to our problem. =
=A0Thanks,
>=20
> Yeah, that matches what I was experiencing when thinking about the
> options. Making a btrfs mount a fake blockdev of zero size seems like
> a pretty weird hack, just get some 'dead' directories in sysfs. A
> btrfs mount is just not a raw blockdev, and should probably not
> pretend to be one.
>=20
> I guess a statfs()-like call from the filesystem side and not the
> block side, which can put out such information in some generic way,
> would better fit here.

Note that for my particular usecase it would even suffice to have two
flags in struct statfs or struct statvfs that encode whether there's a =
at
least one SSD in the fs, resp. at least one rotating disk in the fs.

if (statvfs.f_flag & ST_SSD)=20
        printf("FS contains at least one SSD disk");
if (statvfs.f_flag & ST_ROTATING)=20
        printf("FS contains at least one rotating disk");

Lennart

--=20
Lennart Poettering - Red Hat, Inc.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" =
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2010-09-30 20:37 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-28 20:53 [PATCH] Btrfs: add a disk info ioctl to get the disks attached to a filesystem Josef Bacik
2010-09-28 22:28 ` Goffredo Baroncelli
2010-09-29  0:24   ` Josef Bacik
2010-09-28 23:25 ` Christoph Hellwig
2010-09-29  0:08   ` Josef Bacik
2010-09-29  0:19     ` Lennart Poettering
2010-09-29  7:25       ` Ric Wheeler
2010-09-29  8:04         ` Kay Sievers
2010-09-29 23:43           ` Christoph Hellwig
2010-09-30  0:32             ` Josef Bacik
2010-09-30  7:43             ` Kay Sievers
2010-09-30 12:38               ` Josef Bacik
2010-09-30 13:47               ` Andi Kleen
2010-09-30 19:48             ` Josef Bacik
2010-09-30 19:59               ` Kay Sievers
2010-09-30 20:37                 ` Lennart Poettering
2010-09-29 11:59         ` Lennart Poettering
2010-09-29 12:08           ` Ric Wheeler
2010-09-29 12:19   ` Kay Sievers

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