Skip to contentSkip to conversation

dmfs for 2.5.51

PATCH Greg KH 2 participants 7 messages Dec 13, 2002 1 message not in the archive

Subsystems (3)
lore
Share & tools
lore

Greg KH

author
#0 lore raw

Hi,

Here's a patch against 2.5.51 with a updated dmfs. It contains the two patches at:
http://people.sistina.com/~thornber/patches/2.5-unstable/2.5.50/2.5.50-dmfs-1/00012.patch
and
http://people.sistina.com/~thornber/patches/2.5-unstable/2.5.50/2.5.50-dmfs-1/00013.patch
with the following modifications:
- fixed locking type mismatch (trying to call spin_lock on a rw_semaphore).
- fixed compile time warnings with the dbg() macro (something better should be used here, I just commented it out...)
- changed the dev file to print out the kdev value, not be the actual block device.

With regards to the last change, I didn't follow the way the other files operate with their complex page creation structure, as this is only a simple one line file. If the lvm developers want me to change this, I will. If not, I would argue that a number of the other files created should be changed to use this simpler format. Or is there some reason for creating these lists of pages that I'm missing?

Comments welcome.

thanks,

greg k-h

Documentation/filesystems/dmfs.txt new file +74 -0
diff -Nru a/Documentation/filesystems/dmfs.txt b/Documentation/filesystems/dmfs.txt
@@ -0,0 +1,74 @@
+Overview
+--------
+
+dmfs is a little custom filesystem used as a user interface to the
+device-mapper driver.  The directory heirarchy follows this pattern:
+
+/
+  <namespace1>/
+    <device name1>/
+      table
+      suspend
+      status
+      devices
+      dev
+    <device name2>/
+    ...
+
+  <namespace2>
+   ...
+
+
+Examples
+--------
+
+Example commands will make things a bit clearer:
+
+- To mount the filesystem
+
+  mount -r dmfs none /dev/mapper
+
+- Create a namespace for LVM
+
+  cd /dev/mapper
+  mkdir LVM
+  cd LVM
+
+- Create a new device, that is a linear catenation of parts of 2
+  disks.
+
+  mkdir vg0-music
+  cd vg0-music
+  echo -e "0 56 linear /dev/hda3 0\n56 102344 linear /dev/hda4 0" > table
+
+- Remind yourself what table is running
+
+  cat table
+
+- The 'dev' file should now have appeared.  It shows the major/minor value
+  that you can use to mount your new device.
+
+- To suspend the device
+
+  echo -n 1 > suspend
+
+- To resume
+
+  echo -n 0 > suspend
+
+- To find out the status of the targets in the table
+
+  cat status
+
+- Which sub devices are in use ?
+
+  cat devices
+
+- You may hard link device directories between different namespaces to
+  create aliases (eg, LVM likes to associate a unique id with every
+  device as well as giving it a more human readable name).
+
+  cd /dev/mapper
+  mkdir UUID
+  cd UUID
+  ln ../LVM/vg0-music Qmd96y-771S-Esbb-Zp6u-8xo9-Cfmt-YvndHY
drivers/md/Kconfig +13 -0
diff -Nru a/drivers/md/Kconfig b/drivers/md/Kconfig
@@ -145,5 +145,18 @@
 
 	  If unsure, say N.
 
+config BLK_DEV_DM_FS
+	tristate "Filesystem interface (experimental)"
+	depends on BLK_DEV_DM
+	depends on EXPERIMENTAL
+	---help---
+	  Ram-based filesytem for controlling device-mapper.  Can co-exist
+	  with the ioctl interface.
+
+	  If you want to compile this as a module, say M here and read 
+	  <file:Documentation/modules.txt>.  The module will be called dm-fs.o.
+
+	  If unsure, say N.
+
 endmenu
 
drivers/md/Makefile +3 -3
diff -Nru a/drivers/md/Makefile b/drivers/md/Makefile
@@ -3,8 +3,7 @@
 #
 
 export-objs	:= md.o xor.o dm-table.o dm-target.o
-dm-mod-objs	:= dm.o dm-table.o dm-target.o dm-linear.o dm-stripe.o \
-		   dm-ioctl.o
+dm-mod-objs	:= dm.o dm-table.o dm-target.o dm-linear.o dm-stripe.o
 
 # Note: link order is important.  All raid personalities
 # and xor.o must come before md.o, as they each initialise 
@@ -17,7 +16,8 @@
 obj-$(CONFIG_MD_RAID5)		+= raid5.o xor.o
 obj-$(CONFIG_MD_MULTIPATH)	+= multipath.o
 obj-$(CONFIG_BLK_DEV_MD)	+= md.o
-obj-$(CONFIG_BLK_DEV_DM)	+= dm-mod.o
+obj-$(CONFIG_BLK_DEV_DM)	+= dm-mod.o dm-ioctl.o
+obj-$(CONFIG_BLK_DEV_DM_FS)	+= dm-fs.o
 
 include $(TOPDIR)/Rules.make
 
drivers/md/dm-fs.c new file +1237 -0

1,237 lines not on this page · raw message

drivers/md/dm-ioctl.c +5 -2
diff -Nru a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
@@ -1073,7 +1073,7 @@
 /*
  * Create misc character device and link to DM_DIR/control.
  */
-int __init dm_interface_init(void)
+static int dm_interface_init(void)
 {
 	int r;
 
@@ -1105,7 +1105,7 @@
 	return r;
 }
 
-void dm_interface_exit(void)
+static void dm_interface_exit(void)
 {
 	dm_hash_exit();
 
@@ -1114,3 +1114,6 @@
 	if (misc_deregister(&_dm_misc) < 0)
 		DMERR("misc_deregister failed for control device");
 }
+
+module_init(dm_interface_init);
+module_exit(dm_interface_exit);
drivers/md/dm.c +0 -1
diff -Nru a/drivers/md/dm.c b/drivers/md/dm.c
@@ -125,7 +125,6 @@
 	xx(dm_target)
 	xx(dm_linear)
 	xx(dm_stripe)
-	xx(dm_interface)
 #undef xx
 };
 
drivers/md/dm.h +0 -7
diff -Nru a/drivers/md/dm.h b/drivers/md/dm.h
@@ -130,13 +130,6 @@
 }
 
 /*
- * The device-mapper can be driven through one of two interfaces;
- * ioctl or filesystem, depending which patch you have applied.
- */
-int dm_interface_init(void);
-void dm_interface_exit(void);
-
-/*
  * Targets for linear and striped mappings
  */
 int dm_linear_init(void);

A message that is not in the archive (sent off-list or not yet received): [email protected]

author
#2 lore raw
Quoted 9 lines · Andrew Morton

Documentation/filesystems/dmfs.txt line 25

Quoted 6 lines · Greg KH

..

Documentation/filesystems/dmfs.txt new file +4 -0
+Examples
+--------
+
+Example commands will make things a bit clearer:

hm. The whole thing seems hokey to me. Not sure why.

I agree. It doesn't feel right. I mean, doing a mkdir(1) to create a device, which causes files to be created automagically in that directory? Something needs to change here, and I proposed a single file to write to that creates the device, but was shot down by the author.

Anyone else have any ideas?

Quoted 4 lines
Quoted 2 lines

...
+ echo -e "0 56 linear /dev/hda3 0\n56 102344 linear /dev/hda4 0" > table

Maybe this is why.

Heh, yeah, welcome to parsers in the kernel :)
But the dm code today does much the same thing with ioctls, passing a string down to the loaded modules below it. So there is a bit of president. Even if it is ugly :)

Quoted 9 lines
Quoted 2 lines

...
+static struct page *find_page(struct dmfs_file *f, loff_t len, int fill)

This is called under spinlock.

Quoted 2 lines

...
+ void *addr = (void *) __get_free_page(GFP_KERNEL);

whoops.

Nice catch. I'm not sure that the find_page(), __io() and friends functions are really needed at all.

Thanks for looking at this. I hope the dm authors can help explain more.

greg k-h

#3 lore raw
Quoted 7 lines · Greg KH
Quoted 1 line · Andrew Morton

hm. The whole thing seems hokey to me. Not sure why.

I agree. It doesn't feel right. I mean, doing a mkdir(1) to create a device, which causes files to be created automagically in that directory? Something needs to change here, and I proposed a single file to write to that creates the device, but was shot down by the author.

Greg, I didn't mean to make it sound like I was shooting you down, I just said that we'd leave it as it was for now. Having written the code I wanted a bit more feedback. When I started writing the fs interface a couple of people expressed concerns that I should try and map things properly onto fs semantics and not just create a single file. Given Andrews comment I guess I haven't done a good job. Could you flesh out your single file idea a bit more please, there's a lot of functionality to shoe horn in there.

Quoted 9 lines
Quoted 4 lines
Quoted 2 lines

...
+ echo -e "0 56 linear /dev/hda3 0\n56 102344 linear /dev/hda4 0" > table

Maybe this is why.

Heh, yeah, welcome to parsers in the kernel :)
But the dm code today does much the same thing with ioctls, passing a string down to the loaded modules below it. So there is a bit of president. Even if it is ugly :)

y, the dm targets have always accepted their arguments as ascii strings. So the file system interface just adds code to split the input into lines, and then sscanf the first three elements of the line
- this is probably less code than the marshalling of binary data that is done in the ioctl interface.

I see the fact that we're using ascii data as being the big advantage of the fs interface, which neccessarily means doing a small amount of parsing in kernel. You can't have things both ways.

Quoted 9 lines
Quoted 9 lines
Quoted 2 lines

...
+static struct page *find_page(struct dmfs_file *f, loff_t len, int fill)

This is called under spinlock.

Quoted 2 lines

...
+ void *addr = (void *) __get_free_page(GFP_KERNEL);

whoops.

My fault :(

Quoted 2 lines

Nice catch. I'm not sure that the find_page(), __io() and friends functions are really needed at all.

It would be nice to get rid of them, what shall we replace them with ?

- Joe

author
#4 lore raw
Quoted 11 lines · Joe Thornber
Quoted 7 lines · Greg KH
Quoted 1 line · Andrew Morton

hm. The whole thing seems hokey to me. Not sure why.

I agree. It doesn't feel right. I mean, doing a mkdir(1) to create a device, which causes files to be created automagically in that directory? Something needs to change here, and I proposed a single file to write to that creates the device, but was shot down by the author.

Greg, I didn't mean to make it sound like I was shooting you down, I just said that we'd leave it as it was for now.

Sorry, I didn't mean it that way. I understand and was trying to reach more people. Looks like I succeded :)

Quoted 7 lines

Having written the
code I wanted a bit more feedback. When I started writing the fs interface a couple of people expressed concerns that I should try and map things properly onto fs semantics and not just create a single file. Given Andrews comment I guess I haven't done a good job. Could you flesh out your single file idea a bit more please, there's a lot of functionality to shoe horn in there.

Ok, I'll go work on that and see how it turns out.

Quoted 16 lines
Quoted 9 lines
Quoted 4 lines
Quoted 2 lines

...
+ echo -e "0 56 linear /dev/hda3 0\n56 102344 linear /dev/hda4 0" > table

Maybe this is why.

Heh, yeah, welcome to parsers in the kernel :)
But the dm code today does much the same thing with ioctls, passing a string down to the loaded modules below it. So there is a bit of president. Even if it is ugly :)

y, the dm targets have always accepted their arguments as ascii strings. So the file system interface just adds code to split the input into lines, and then sscanf the first three elements of the line
- this is probably less code than the marshalling of binary data that is done in the ioctl interface.

Quoted 3 lines

I see the fact that we're using ascii data as being the big advantage of the fs interface, which neccessarily means doing a small amount of parsing in kernel. You can't have things both ways.

I agree, I like the interface. It's just a little strange the first time you see it.

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 2 lines

...
+static struct page *find_page(struct dmfs_file *f, loff_t len, int fill)

This is called under spinlock.

Quoted 2 lines

...
+ void *addr = (void *) __get_free_page(GFP_KERNEL);

Quoted 8 lines
Quoted 1 line
Quoted 1 line

whoops.

My fault :(

Quoted 2 lines

Nice catch. I'm not sure that the find_page(), __io() and friends functions are really needed at all.

It would be nice to get rid of them, what shall we replace them with ?

Something like only creating one page as I did in the dev file, or the seq_file interface. I'll play around with the sizes of the files to see how to fix this up.

thanks,

greg k-h

#5 lore raw
In reply to Greg KH

Greg,

Quoted 1 line · Greg KH

Here's a patch against 2.5.51 with a updated dmfs.

I've split out your two changes into seperate patches (patches 21 and
22) and made them available here:

http://people.sistina.com/~thornber/patches/2.5-unstable/2.5.51/2.5.51-dmfs-1/

Quoted 3 lines

with the following modifications:
- fixed compile time warnings with the dbg() macro (something better should be used here, I just commented it out...)

I'm not seeing any warnings, which compiler version are you using ?

Quoted 2 lines

- changed the dev file to print out the kdev value, not be the actual block device.

Should we really be exporting a kdev_t to userland, why not just print out

<major>:<minor>

?

Quoted 4 lines

With regards to the last change, I didn't follow the way the other files operate with their complex page creation structure, as this is only a simple one line file. If the lvm developers want me to change this, I will.

What you've done looks fine to me, though allocating a whole page to hold a single number seems overkill. Why don't you just snprintf into a char[] held on the stack ?

Quoted 3 lines

If not, I would argue that a number of the other files created should be changed to use this simpler format. Or is there some reason for creating these lists of pages that I'm missing?

The files can be larger than a single page, which complicates things somewhat.

- Joe

author
#6 lore raw
Quoted 9 lines · Joe Thornber

Greg,

Quoted 1 line · Greg KH

Here's a patch against 2.5.51 with a updated dmfs.

I've split out your two changes into seperate patches (patches 21 and
22) and made them available here:

http://people.sistina.com/~thornber/patches/2.5-unstable/2.5.51/2.5.51-dmfs-1/

Thanks.

Quoted 5 lines
Quoted 3 lines

with the following modifications:
- fixed compile time warnings with the dbg() macro (something better should be used here, I just commented it out...)

I'm not seeing any warnings, which compiler version are you using ?

The latest for Red Hat 7.2: gcc-2.96-112.7.2
Are you using 3.2?

Quoted 6 lines
Quoted 2 lines

- changed the dev file to print out the kdev value, not be the actual block device.

Should we really be exporting a kdev_t to userland, why not just print out

<major>:<minor>

No, look at the other dev files in sysfs, I stayed consistant with them.

Quoted 8 lines
Quoted 4 lines

With regards to the last change, I didn't follow the way the other files operate with their complex page creation structure, as this is only a simple one line file. If the lvm developers want me to change this, I will.

What you've done looks fine to me, though allocating a whole page to hold a single number seems overkill. Why don't you just snprintf into a char[] held on the stack ?

Because I forgot you could do copy_to_user() with data on the stack :) Good point, I'll change it...

Quoted 6 lines
Quoted 3 lines

If not, I would argue that a number of the other files created should be changed to use this simpler format. Or is there some reason for creating these lists of pages that I'm missing?

The files can be larger than a single page, which complicates things somewhat.

Hm, then using the seq_file interface might be easier. I'll look into this.

thanks,

greg k-h

#7 lore raw
Quoted 2 lines · Greg KH

The latest for Red Hat 7.2: gcc-2.96-112.7.2
Are you using 3.2?

gcc version 2.95.4 20011002 (Debian prerelease)

Quoted 5 lines
Quoted 2 lines

The files can be larger than a single page, which complicates things somewhat.

Hm, then using the seq_file interface might be easier. I'll look into this.

Remember that the table file needs to hold two files at times: the new table that is being read in and the old table in case the new table is invalid.

- Joe