linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC] device-mapper ioctl interface
@ 2003-06-05  9:39 Joe Thornber
  2003-06-05 16:47 ` Kevin Corry
  2003-06-06 17:17 ` Greg KH
  0 siblings, 2 replies; 13+ messages in thread
From: Joe Thornber @ 2003-06-05  9:39 UTC (permalink / raw)
  To: dm-devel, Linux Mailing List

[-- Attachment #1: Type: text/plain, Size: 921 bytes --]

Here's the header file for the the proposed new ioctl interface for
dm.  We've tried to change as little as possible to minimise code
changes in LVM2 and EVMS.

The changes address the following problems:

o) Alignment mistakes in the ioctl structures

o) Provide a way for the userland tools to back out of a series of
   table updates without having to reload the orginal tables (remember
   that an LV is often composed of a stack of 2 or more dm devices).

o) Pre-loading of a new table may now occur without suspending the
   device.  (Table loading can potentially allocate a lot of memory,
   eg. mirror buffers).

o) Minimise the amount of time that a device is suspended.  eg. if a
   simple linear mapping is being extended, there is no longer a need to
   call the suspend ioctl in between the table load and the resume.
   Instead the resume (which swaps in the new table), will do the right
   thing.

- Joe

[-- Attachment #2: dm-ioctl.h --]
[-- Type: text/x-chdr, Size: 6846 bytes --]

/*
 * Copyright (C) 2001, 2003 Sistina Software (UK) Limited.
 *
 * This file is released under the LGPL.
 */

#ifndef _LINUX_DM_IOCTL_H
#define _LINUX_DM_IOCTL_H

#include <linux/types.h>

#define DM_DIR "mapper"	/* Slashes not supported */
#define DM_MAX_TYPE_NAME 16
#define DM_NAME_LEN 128
#define DM_UUID_LEN 129

/*
 * A traditional ioctl interface for the device mapper.
 *
 * Each device can have two tables associated with it, an
 * 'active' table which is the one currently used by io passing
 * through the device, and an 'inactive' one which is a table
 * that is being prepared as a replacement for the 'active' one.
 *
 * DM_VERSION:
 * Just get the version information for the ioctl interface.
 *
 * DM_REMOVE_ALL:
 * Remove all dm devices, destroy all tables.  Only really used
 * for debug.
 *
 * DM_LIST_DEVICES:
 * Get a list of all the dm device names.
 *
 * DM_DEV_CREATE:
 * Create a new device, neither the 'active' or 'inactive' table
 * slots will be filled.  The device will be in suspended state
 * after creation, however any io to the device will get errored
 * since it will be out-of-bounds.
 *
 * DM_DEV_REMOVE:
 * Remove a device, destroy any tables.
 *
 * DM_DEV_RENAME:
 * Rename a device.
 *
 * DM_SUSPEND:
 * This performs both suspend and resume, depending which flag is
 * passed in.
 * Suspend: This command will not return until all pending io to
 * the device has completed.  Further io will be deferred until
 * the device is resumed.
 * Resume: It is no longer an error to issue this command on an
 * unsuspended device.  If a table is present in the 'inactive'
 * slot, it will be moved to the active slot, then the old table
 * from the active slot will be _destroyed_.  Finally the device
 * is resumed.
 *
 * DM_DEV_STATUS:
 * Retrieves the status for the table in the 'active' slot.
 *
 * DM_DEV_WAIT:
 * Wait for a significant event to occur to the device.  This
 * could either be caused by an event triggered by one of the
 * targets of the table in the 'active' slot, or a table change.
 *
 * DM_TABLE_LOAD:
 * Load a table into the 'inactive' slot for the device.  The
 * device does _not_ need to be suspended prior to this command.
 *
 * DM_TABLE_CLEAR:
 * Destroy any table in the 'inactive' slot (ie. abort).
 *
 * DM_TABLE_DEPS:
 * Return a set of device dependencies for the 'active' table.
 *
 * DM_TABLE_STATUS:
 * Return the targets status for the 'active' table.
 */

/*
 * All ioctl arguments consist of a single chunk of memory, with
 * this structure at the start.  If a uuid is specified any
 * lookup (eg. for a DM_INFO) will be done on that, *not* the
 * name.
 */
struct dm_ioctl {
	/*
	 * The version number is made up of three parts:
	 * major - no backward or forward compatibility,
	 * minor - only backwards compatible,
	 * patch - both backwards and forwards compatible.
	 *
	 * All clients of the ioctl interface should fill in the
	 * version number of the interface that they were
	 * compiled with.
	 *
	 * All recognised ioctl commands (ie. those that don't
	 * return -ENOTTY) fill out this field, even if the
	 * command failed.
	 */
	uint32_t version[3];	/* in/out */
	uint32_t data_size;	/* total size of data passed in
				 * including this struct */

	uint32_t data_start;	/* offset to start of data
				 * relative to start of this struct */

	uint32_t target_count;	/* in/out */
	int32_t open_count;	/* out */
	uint32_t flags;		/* in/out */
	uint32_t event_nr;      /* in/out */
	uint32_t padding;

	uint64_t dev;		/* in/out */

	char name[DM_NAME_LEN];	/* device name */
	char uuid[DM_UUID_LEN];	/* unique identifier for
				 * the block device */
};

/*
 * Used to specify tables.  These structures appear after the
 * dm_ioctl.
 */
struct dm_target_spec {
	uint64_t sector_start;
	uint64_t length;
	int32_t status;		/* used when reading from kernel only */

	/*
	 * Offset in bytes (from the start of this struct) to
	 * next target_spec.
	 */
	uint32_t next;

	char target_type[DM_MAX_TYPE_NAME];

	/*
	 * Parameter string starts immediately after this object.
	 * Be careful to add padding after string to ensure correct
	 * alignment of subsequent dm_target_spec.
	 */
};

/*
 * Used to retrieve the target dependencies.
 */
struct dm_target_deps {
	uint32_t count;		/* Array size */
	uint32_t padding;	/* unused */
	uint64_t dev[0];	/* out */
};

/*
 * Used to get a list of all dm devices.
 */
struct dm_name_list {
	uint64_t dev;
	uint32_t next;		/* offset to the next record from
				   the _start_ of this */
	char name[0];
};

/*
 * If you change this make sure you make the corresponding change
 * to dm-ioctl.c:lookup_ioctl()
 */
enum {
	/* Top level cmds */
	DM_VERSION_CMD = 0,
	DM_REMOVE_ALL_CMD,
	DM_LIST_DEVICES_CMD,

	/* device level cmds */
	DM_DEV_CREATE_CMD,
	DM_DEV_REMOVE_CMD,
	DM_DEV_RENAME_CMD,
	DM_DEV_SUSPEND_CMD,
	DM_DEV_STATUS_CMD,
	DM_DEV_WAIT_CMD,

	/* Table level cmds */
	DM_TABLE_LOAD_CMD,
	DM_TABLE_CLEAR_CMD,
	DM_TABLE_DEPS_CMD,
	DM_TABLE_STATUS_CMD,
};

#define DM_IOCTL 0xfd

#define DM_VERSION       _IOWR(DM_IOCTL, DM_VERSION_CMD, struct dm_ioctl)
#define DM_REMOVE_ALL    _IOWR(DM_IOCTL, DM_REMOVE_ALL_CMD, struct dm_ioctl)
#define DM_LIST_DEVICES  _IOWR(DM_IOCTL, DM_LIST_DEVICES_CMD, struct dm_ioctl)

#define DM_DEV_CREATE    _IOWR(DM_IOCTL, DM_DEV_CREATE_CMD, struct dm_ioctl)
#define DM_DEV_REMOVE    _IOWR(DM_IOCTL, DM_DEV_REMOVE_CMD, struct dm_ioctl)
#define DM_DEV_RENAME    _IOWR(DM_IOCTL, DM_DEV_RENAME_CMD, struct dm_ioctl)
#define DM_DEV_SUSPEND   _IOWR(DM_IOCTL, DM_DEV_SUSPEND_CMD, struct dm_ioctl)
#define DM_DEV_STATUS    _IOWR(DM_IOCTL, DM_DEV_STATUS_CMD, struct dm_ioctl)
#define DM_DEV_WAIT      _IOWR(DM_IOCTL, DM_DEV_WAIT_CMD, struct dm_ioctl)

#define DM_TABLE_LOAD    _IOWR(DM_IOCTL, DM_TABLE_LOAD_CMD, struct dm_ioctl)
#define DM_TABLE_CLEAR   _IOWR(DM_IOCTL, DM_TABLE_CLEAR_CMD, struct dm_ioctl)
#define DM_TABLE_DEPS    _IOWR(DM_IOCTL, DM_TABLE_DEPS_CMD, struct dm_ioctl)
#define DM_TABLE_STATUS  _IOWR(DM_IOCTL, DM_TABLE_STATUS_CMD, struct dm_ioctl)

#define DM_VERSION_MAJOR	4
#define DM_VERSION_MINOR	0
#define DM_VERSION_PATCHLEVEL	0
#define DM_VERSION_EXTRA	"-ioctl-cvs (2003-06-04)"

/* Status bits */
#define DM_READONLY_FLAG	(1 << 0) /* In/Out */
#define DM_SUSPEND_FLAG		(1 << 1) /* In/Out */
#define DM_PERSISTENT_DEV_FLAG	(1 << 3) /* In */

/*
 * Flag passed into ioctl STATUS command to get table information
 * rather than current status.
 */
#define DM_STATUS_TABLE_FLAG	(1 << 4) /* In */

/*
 * Flags that indicate whether a table is present in either of
 * the two table slots that a device has.
 */
#define DM_ACTIVE_PRESENT_FLAG   (1 << 5) /* Out */
#define DM_INACTIVE_PRESENT_FLAG (1 << 6) /* Out */

/*
 * Indicates that the buffer passed in wasn't big enough for the
 * results.
 */
#define DM_BUFFER_FULL_FLAG	(1 << 8) /* Out */

#endif				/* _LINUX_DM_IOCTL_H */

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

* Re: [RFC] device-mapper ioctl interface
  2003-06-05  9:39 [RFC] device-mapper ioctl interface Joe Thornber
@ 2003-06-05 16:47 ` Kevin Corry
  2003-06-05 17:00   ` [dm-devel] " Daniel Phillips
  2003-06-05 19:41   ` Joe Thornber
  2003-06-06 17:17 ` Greg KH
  1 sibling, 2 replies; 13+ messages in thread
From: Kevin Corry @ 2003-06-05 16:47 UTC (permalink / raw)
  To: dm-devel, Linux Mailing List

On Thursday 05 June 2003 04:39, Joe Thornber wrote:
> Here's the header file for the the proposed new ioctl interface for
> dm.  We've tried to change as little as possible to minimise code
> changes in LVM2 and EVMS.
>
> The changes address the following problems:
>
> o) Alignment mistakes in the ioctl structures
>
> o) Provide a way for the userland tools to back out of a series of
>    table updates without having to reload the orginal tables (remember
>    that an LV is often composed of a stack of 2 or more dm devices).

This should be very helpful for error recovery when setting up snapshots.

> o) Pre-loading of a new table may now occur without suspending the
>    device.  (Table loading can potentially allocate a lot of memory,
>    eg. mirror buffers).

Cool.

> o) Minimise the amount of time that a device is suspended.  eg. if a
>    simple linear mapping is being extended, there is no longer a need to
>    call the suspend ioctl in between the table load and the resume.
>    Instead the resume (which swaps in the new table), will do the right
>    thing.

Also very nice. This ought to elimate the possibility of deadlock due to 
suspending the device from user-space. I really like this solution!

I'll bring up a couple of the points Alasdair and I were discussing on irc 
yesterday:

1) Snapshots. Currently, the snapshot module, when it constructs a new table, 
reads the header and existing exception tables from disk to determine the 
initial state of the snapshot. With this new scheme, this setup really 
shouldn't happen until the device is resumed (if it is done when the 
"inactive" table is created, an existing "active" table could change the 
on-disk information before the tables are switched). This kind of implies a 
new entry-point into the target module will be required. 

2) Removing suspended devices. The current code (2.5.70) does not allow a 
suspended device to be removed/unlinked from the ioctl interface, since 
removing it would leave you with no way to resume it (and hence flush any 
pending I/Os). Alasdair mentioned a couple of new ideas. One would be to 
reload the device with an error-map and force it to resume, thus erroring any 
pending I/Os and allowing the device to be removed. This seems a bit 
heavy-handed. Another idea was to allow removing suspended-devices which 
aren't open. But that seems a bit racy. The current code doesn't seem to 
coordinate removal of a device from the interface with opening of the device. 
So you could potentially be opening the device at the same time you are 
removing it from the interface. Personally, I think the current behavior is 
fine. User-space can use the new list-devices command along with the 
dev-status command to determine if a device should be resumed before trying 
to remove it.

Any thoughts?

-- 
Kevin Corry
kevcorry@us.ibm.com
http://evms.sourceforge.net/


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

* Re: [dm-devel] Re: [RFC] device-mapper ioctl interface
  2003-06-05 16:47 ` Kevin Corry
@ 2003-06-05 17:00   ` Daniel Phillips
  2003-06-05 17:50     ` Kevin Corry
  2003-06-05 19:41   ` Joe Thornber
  1 sibling, 1 reply; 13+ messages in thread
From: Daniel Phillips @ 2003-06-05 17:00 UTC (permalink / raw)
  To: dm-devel, Kevin Corry, Linux Mailing List

On Thursday 05 June 2003 18:47, Kevin Corry wrote:
> 2) Removing suspended devices. The current code (2.5.70) does not allow a
> suspended device to be removed/unlinked from the ioctl interface, since
> removing it would leave you with no way to resume it (and hence flush any
> pending I/Os). Alasdair mentioned a couple of new ideas. One would be to
> reload the device with an error-map and force it to resume, thus erroring
> any pending I/Os and allowing the device to be removed. This seems a bit
> heavy-handed.

Which is the heavy-handed part?

Regards,

Daniel


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

* Re: [dm-devel] Re: [RFC] device-mapper ioctl interface
  2003-06-05 17:00   ` [dm-devel] " Daniel Phillips
@ 2003-06-05 17:50     ` Kevin Corry
  2003-06-05 18:53       ` Daniel Phillips
  0 siblings, 1 reply; 13+ messages in thread
From: Kevin Corry @ 2003-06-05 17:50 UTC (permalink / raw)
  To: dm-devel, Daniel Phillips, Linux Mailing List

On Thursday 05 June 2003 12:00, Daniel Phillips wrote:
> On Thursday 05 June 2003 18:47, Kevin Corry wrote:
> > 2) Removing suspended devices. The current code (2.5.70) does not allow a
> > suspended device to be removed/unlinked from the ioctl interface, since
> > removing it would leave you with no way to resume it (and hence flush any
> > pending I/Os). Alasdair mentioned a couple of new ideas. One would be to
> > reload the device with an error-map and force it to resume, thus erroring
> > any pending I/Os and allowing the device to be removed. This seems a bit
> > heavy-handed.
>
> Which is the heavy-handed part?

The part about automatically reloading the table with an error map and forcing 
it to resume. It just seemed to me that user-space ought to be able to gather 
enough information to determine that a device needed to be resumed before it 
could be removed. Thus the kernel driver wouldn't be forced to implement such 
a policy.

Talking with Alasadair again, he mentioned a case I hadn't considered. Devices 
would now be created without a mapping and initially suspended. If some other 
error occurred, and you decided to just delete the device before loading a 
mapping, it would fail.  And having to resume a device with no mapping just 
to be able to delete it definitely seems odd.

So, it's not like I'm dead-set against this idea. I was just curious what the 
reasoning was behind this change.

-- 
Kevin Corry
kevcorry@us.ibm.com
http://evms.sourceforge.net/


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

* Re: [dm-devel] Re: [RFC] device-mapper ioctl interface
  2003-06-05 17:50     ` Kevin Corry
@ 2003-06-05 18:53       ` Daniel Phillips
  0 siblings, 0 replies; 13+ messages in thread
From: Daniel Phillips @ 2003-06-05 18:53 UTC (permalink / raw)
  To: Kevin Corry, dm-devel, Linux Mailing List

On Thursday 05 June 2003 19:50, Kevin Corry wrote:
> On Thursday 05 June 2003 12:00, Daniel Phillips wrote:
> > On Thursday 05 June 2003 18:47, Kevin Corry wrote:
> > > 2) Removing suspended devices. The current code (2.5.70) does not allow
> > > a suspended device to be removed/unlinked from the ioctl interface,
> > > since removing it would leave you with no way to resume it (and hence
> > > flush any pending I/Os). Alasdair mentioned a couple of new ideas. One
> > > would be to reload the device with an error-map and force it to resume,
> > > thus erroring any pending I/Os and allowing the device to be removed.
> > > This seems a bit heavy-handed.
> >
> > Which is the heavy-handed part?
>
> The part about automatically reloading the table with an error map and
> forcing it to resume. It just seemed to me that user-space ought to be able
> to gather enough information to determine that a device needed to be
> resumed before it could be removed. Thus the kernel driver wouldn't be
> forced to implement such a policy.

I didn't see anything about doing that in-kernel.

> Talking with Alasadair again, he mentioned a case I hadn't considered.
> Devices would now be created without a mapping and initially suspended. If
> some other error occurred, and you decided to just delete the device before
> loading a mapping, it would fail.  And having to resume a device with no
> mapping just to be able to delete it definitely seems odd.
>
> So, it's not like I'm dead-set against this idea. I was just curious what
> the reasoning was behind this change.

It's similar to the way a lot of things work in Linux: you have to let 
operations run to completion so they can let go of resources.  One day we'll 
be able to shoot down transfers in mid-flight, but I doubt that's going to 
happen in this cycle.

So in general, the idea is: let any outstanding operations complete, but feed 
them errors.  What else can we do?

I don't see this as heavyweight at all.  Policy stays in user space, and a 
lightweight error path lives in the kernel.

Regards,

Daniel


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

* Re: [dm-devel] Re: [RFC] device-mapper ioctl interface
  2003-06-05 16:47 ` Kevin Corry
  2003-06-05 17:00   ` [dm-devel] " Daniel Phillips
@ 2003-06-05 19:41   ` Joe Thornber
  2003-06-06 16:11     ` Kevin Corry
  1 sibling, 1 reply; 13+ messages in thread
From: Joe Thornber @ 2003-06-05 19:41 UTC (permalink / raw)
  To: dm-devel; +Cc: Linux Mailing List

On Thu, Jun 05, 2003 at 11:47:10AM -0500, Kevin Corry wrote:
> 1) Snapshots. Currently, the snapshot module, when it constructs a new table, 
> reads the header and existing exception tables from disk to determine the 
> initial state of the snapshot. With this new scheme, this setup really 
> shouldn't happen until the device is resumed (if it is done when the 
> "inactive" table is created, an existing "active" table could change the 
> on-disk information before the tables are switched). This kind of implies a 
> new entry-point into the target module will be required. 

See the suspend and resume target methods in my recent dev tree.
We'll have to delay the metadata reading for both the snapshots and
mirror to the first 'resume'.

> 2) Removing suspended devices. The current code (2.5.70) does not allow a 
> suspended device to be removed/unlinked from the ioctl interface, since 
> removing it would leave you with no way to resume it (and hence flush any 
> pending I/Os).

I think removing a device that has deferred io against it should not
be possible, since it can only be in that state if the device is open.
We shouldn't start ripping devices out from under people.

The one place where we do want to do this is for the DM_REMOVE_ALL
ioctl cmd.  Which is really an emergency panic button.  I'll just
error any deferred io in this case.

- Joe

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

* Re: [dm-devel] Re: [RFC] device-mapper ioctl interface
  2003-06-05 19:41   ` Joe Thornber
@ 2003-06-06 16:11     ` Kevin Corry
  0 siblings, 0 replies; 13+ messages in thread
From: Kevin Corry @ 2003-06-06 16:11 UTC (permalink / raw)
  To: dm-devel; +Cc: Linux Mailing List

On Thursday 05 June 2003 14:41, Joe Thornber wrote:
> On Thu, Jun 05, 2003 at 11:47:10AM -0500, Kevin Corry wrote:
> > 1) Snapshots. Currently, the snapshot module, when it constructs a new
> > table, reads the header and existing exception tables from disk to
> > determine the initial state of the snapshot. With this new scheme, this
> > setup really shouldn't happen until the device is resumed (if it is done
> > when the "inactive" table is created, an existing "active" table could
> > change the on-disk information before the tables are switched). This kind
> > of implies a new entry-point into the target module will be required.
>
> See the suspend and resume target methods in my recent dev tree.
> We'll have to delay the metadata reading for both the snapshots and
> mirror to the first 'resume'.

Where is your dev tree located? I've checked your website on 
people.sistina.com and the various Sistina CVS trees, but I can't really find 
anything (regarding suspend and resume target methods) that's very recent. Do 
you have another ftp server somewhere?

> > 2) Removing suspended devices. The current code (2.5.70) does not allow a
> > suspended device to be removed/unlinked from the ioctl interface, since
> > removing it would leave you with no way to resume it (and hence flush any
> > pending I/Os).
>
> I think removing a device that has deferred io against it should not
> be possible, since it can only be in that state if the device is open.
> We shouldn't start ripping devices out from under people.

Right.

So are you saying it would be alright to remove a suspended device that has no 
pending I/O or isn't open? If so, the current code (in 2.5.70) doesn't seem 
to coordinate the removal of such a device with another process trying to 
open it or submit new I/O. Some new locking of the device would be necessary 
to prevent a device which is being removed from being opened at the same 
time.

Sorry if it sounds like I'm harping on this issue - I don't mean to. :)  Just 
interested in the details behind some of the proposed changes and some of the 
affects the changes might have. It will probably be much easier to just wait 
to see your new code, which will definitively answer these questions.

> The one place where we do want to do this is for the DM_REMOVE_ALL
> ioctl cmd.  Which is really an emergency panic button.  I'll just
> error any deferred io in this case.

Ok, this seems reasonable.

-- 
Kevin Corry
kevcorry@us.ibm.com
http://evms.sourceforge.net/


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

* Re: [RFC] device-mapper ioctl interface
  2003-06-05  9:39 [RFC] device-mapper ioctl interface Joe Thornber
  2003-06-05 16:47 ` Kevin Corry
@ 2003-06-06 17:17 ` Greg KH
  2003-06-06 19:53   ` [dm-devel] " Alasdair G Kergon
                     ` (2 more replies)
  1 sibling, 3 replies; 13+ messages in thread
From: Greg KH @ 2003-06-06 17:17 UTC (permalink / raw)
  To: Joe Thornber; +Cc: dm-devel, Linux Mailing List

On Thu, Jun 05, 2003 at 10:39:43AM +0100, Joe Thornber wrote:
> Here's the header file for the the proposed new ioctl interface for
> dm.  We've tried to change as little as possible to minimise code
> changes in LVM2 and EVMS.

Minor comment:
	- please do not use uint_32t types in kernel header files.  Use
	  the proper __u32 type which is guarenteed to be the proper
	  size across the user/kernel boundry.

thanks,

greg k-h

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

* Re: [dm-devel] Re: [RFC] device-mapper ioctl interface
  2003-06-06 17:17 ` Greg KH
@ 2003-06-06 19:53   ` Alasdair G Kergon
  2003-06-09 20:03   ` Daniel Phillips
  2003-06-09 22:08   ` Daniel Phillips
  2 siblings, 0 replies; 13+ messages in thread
From: Alasdair G Kergon @ 2003-06-06 19:53 UTC (permalink / raw)
  To: dm-devel; +Cc: Linux Mailing List

On Fri, Jun 06, 2003 at 10:17:00AM -0700, Greg KH wrote:
> Minor comment:
> 	- please do not use uint_32t types in kernel header files.  Use
> 	  the proper __u32 type which is guarenteed to be the proper
> 	  size across the user/kernel boundry.

So is uint32_t:

    linux/types.h: typedef         __u32           uint32_t;

I think this comment is about style conformity i.e. most linux kernel
developers avoid these (not-so-)new and more-portable C99 types.

Alasdair
-- 
agk@uk.sistina.com

Refs. http://www.xml.com/ldd/chapter/book/ch10.html
      http://www.unix-systems.org/whitepapers/64bit.html

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

* Re: [dm-devel] Re: [RFC] device-mapper ioctl interface
  2003-06-06 17:17 ` Greg KH
  2003-06-06 19:53   ` [dm-devel] " Alasdair G Kergon
@ 2003-06-09 20:03   ` Daniel Phillips
  2003-06-09 20:39     ` Greg KH
  2003-06-09 22:08   ` Daniel Phillips
  2 siblings, 1 reply; 13+ messages in thread
From: Daniel Phillips @ 2003-06-09 20:03 UTC (permalink / raw)
  To: dm-devel, Greg KH, Joe Thornber; +Cc: dm-devel, Linux Mailing List

On Friday 06 June 2003 19:17, Greg KH wrote:
> On Thu, Jun 05, 2003 at 10:39:43AM +0100, Joe Thornber wrote:
> > Here's the header file for the the proposed new ioctl interface for
> > dm.  We've tried to change as little as possible to minimise code
> > changes in LVM2 and EVMS.
>
> Minor comment:
> 	- please do not use uint_32t types in kernel header files.  Use
> 	  the proper __u32 type which is guarenteed to be the proper
> 	  size across the user/kernel boundry.

We even have a flavor without the __'s:

   http://lxr.linux.no/source/include/asm-i386/types.h?v=2.5.56#L47

A little easier on the eyes, imho.

Regards,

Daniel


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

* Re: [dm-devel] Re: [RFC] device-mapper ioctl interface
  2003-06-09 20:03   ` Daniel Phillips
@ 2003-06-09 20:39     ` Greg KH
  2003-06-09 21:39       ` Daniel Phillips
  0 siblings, 1 reply; 13+ messages in thread
From: Greg KH @ 2003-06-09 20:39 UTC (permalink / raw)
  To: Daniel Phillips; +Cc: dm-devel, Joe Thornber, Linux Mailing List

On Mon, Jun 09, 2003 at 10:03:50PM +0200, Daniel Phillips wrote:
> On Friday 06 June 2003 19:17, Greg KH wrote:
> > On Thu, Jun 05, 2003 at 10:39:43AM +0100, Joe Thornber wrote:
> > > Here's the header file for the the proposed new ioctl interface for
> > > dm.  We've tried to change as little as possible to minimise code
> > > changes in LVM2 and EVMS.
> >
> > Minor comment:
> > 	- please do not use uint_32t types in kernel header files.  Use
> > 	  the proper __u32 type which is guarenteed to be the proper
> > 	  size across the user/kernel boundry.
> 
> We even have a flavor without the __'s:
> 
>    http://lxr.linux.no/source/include/asm-i386/types.h?v=2.5.56#L47
> 
> A little easier on the eyes, imho.

But they are not the same.
 - "__" means this variable will be the same size across the
   kernel/userspace boundry.
 - without the "__" means this variable will only be this size within
   the kernel.

Use "__" for variables being seen by userspace.  Otherwise use the
types without "__".

Hope this helps,

greg k-h

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

* Re: [dm-devel] Re: [RFC] device-mapper ioctl interface
  2003-06-09 20:39     ` Greg KH
@ 2003-06-09 21:39       ` Daniel Phillips
  0 siblings, 0 replies; 13+ messages in thread
From: Daniel Phillips @ 2003-06-09 21:39 UTC (permalink / raw)
  To: dm-devel, Greg KH; +Cc: dm-devel, Joe Thornber, Linux Mailing List

On Monday 09 June 2003 22:39, Greg KH wrote:
> On Mon, Jun 09, 2003 at 10:03:50PM +0200, Daniel Phillips wrote:
> > On Friday 06 June 2003 19:17, Greg KH wrote:
> > > On Thu, Jun 05, 2003 at 10:39:43AM +0100, Joe Thornber wrote:
> > > > Here's the header file for the the proposed new ioctl interface for
> > > > dm.  We've tried to change as little as possible to minimise code
> > > > changes in LVM2 and EVMS.
> > >
> > > Minor comment:
> > > 	- please do not use uint_32t types in kernel header files.  Use
> > > 	  the proper __u32 type which is guarenteed to be the proper
> > > 	  size across the user/kernel boundry.
> >
> > We even have a flavor without the __'s:
> >
> >    http://lxr.linux.no/source/include/asm-i386/types.h?v=2.5.56#L47
> >
> > A little easier on the eyes, imho.
>
> But they are not the same.
>  - "__" means this variable will be the same size across the
>    kernel/userspace boundry.
>  - without the "__" means this variable will only be this size within
>    the kernel.
>
> Use "__" for variables being seen by userspace.  Otherwise use the
> types without "__".

Indeed, and (to restate the obvious) ioctl interfaces will need the __ form.

On the other hand, a quick tour through the current tree shows the __ form 
being used in a lot more places than it's strictly needed.  Although there's 
no entry in the style guide for this, perhaps we should consider one.

Regards,

Daniel


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

* Re: [dm-devel] Re: [RFC] device-mapper ioctl interface
  2003-06-06 17:17 ` Greg KH
  2003-06-06 19:53   ` [dm-devel] " Alasdair G Kergon
  2003-06-09 20:03   ` Daniel Phillips
@ 2003-06-09 22:08   ` Daniel Phillips
  2 siblings, 0 replies; 13+ messages in thread
From: Daniel Phillips @ 2003-06-09 22:08 UTC (permalink / raw)
  To: dm-devel, Greg KH, Joe Thornber; +Cc: dm-devel, Linux Mailing List

On Friday 06 June 2003 19:17, Greg KH wrote:
> On Thu, Jun 05, 2003 at 10:39:43AM +0100, Joe Thornber wrote:
> > Here's the header file for the the proposed new ioctl interface for
> > dm.  We've tried to change as little as possible to minimise code
> > changes in LVM2 and EVMS.
>
> Minor comment:
> 	- please do not use uint_32t types in kernel header files.  Use
> 	  the proper __u32 type which is guarenteed to be the proper
> 	  size across the user/kernel boundry.

I'm not sure what we were both smoking, but obviously all flavors of u32 
should be the same size regardless of where they are located.  As I 
understand it, the only interesting difference between __u32 and u32 is that 
the former is standard C while the latter is Linux's (sensible) local 
dialect.

Regards,

Daniel


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

end of thread, other threads:[~2003-06-09 21:54 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-06-05  9:39 [RFC] device-mapper ioctl interface Joe Thornber
2003-06-05 16:47 ` Kevin Corry
2003-06-05 17:00   ` [dm-devel] " Daniel Phillips
2003-06-05 17:50     ` Kevin Corry
2003-06-05 18:53       ` Daniel Phillips
2003-06-05 19:41   ` Joe Thornber
2003-06-06 16:11     ` Kevin Corry
2003-06-06 17:17 ` Greg KH
2003-06-06 19:53   ` [dm-devel] " Alasdair G Kergon
2003-06-09 20:03   ` Daniel Phillips
2003-06-09 20:39     ` Greg KH
2003-06-09 21:39       ` Daniel Phillips
2003-06-09 22:08   ` Daniel Phillips

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