[PATCH 0/3] Export UBI map/unmap/is_mapped in userspace v2

[PATCH 0/3] Export UBI map/unmap/is_mapped in userspace v2

[Corentin Chary]

Hi,x
These are three patch to make a usespace program able to use map/unmap 
operation and to query if a LEB is mapped.
We are trying to do an UBI based userspace filesystem (using fuse) and we 
needed these, but it could also be used for testing purpose.
CONFIG_MTD_UBI_DEBUG_USERSPACE_IO is needed to enable theses functions.

The patch can also be download here:
http://xf.iksaif.net/dev/uffs/linux/ubi/


Thanks

--
Corentin Chary
http://xf.iksaif.net

[PATCH 1/3] UBI: map operation via ioctl

[Corentin Chary]

From bafefa6ed036173c65b7f51c6a4a4ce5004ad09f Mon Sep 17 00:00:00 2001
From: Corentin Chary <corentincj@iksaif.net>
Date: Mon, 5 Jan 2009 14:44:11 +0100

UBI: map operation via ioctl

Signed-off-by: Corentin Chary <corentincj@iksaif.net>
---
 drivers/mtd/ubi/cdev.c |   13 +++++++++++++
 include/mtd/ubi-user.h |   16 ++++++++++++++++
 2 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/ubi/cdev.c b/drivers/mtd/ubi/cdev.c
index 98cf31e..db47121 100644
--- a/drivers/mtd/ubi/cdev.c
+++ b/drivers/mtd/ubi/cdev.c
@@ -518,6 +518,19 @@ static int vol_cdev_ioctl(struct inode *inode, struct file *file,
 		err = ubi_wl_flush(ubi);
 		break;
 	}
+	case UBI_IOMAP:
+	{
+		struct ubi_map_req req;
+		err = copy_from_user(&req, argp,
+				sizeof(struct ubi_map_req));
+		if (err) {
+			err = -EFAULT;
+			break;
+		}
+		err = ubi_leb_map(desc, req.lnum, req.dtype);
+		break;
+	}
+
 #endif
 
 	default:
diff --git a/include/mtd/ubi-user.h b/include/mtd/ubi-user.h
index ccdc562..8222f12 100644
--- a/include/mtd/ubi-user.h
+++ b/include/mtd/ubi-user.h
@@ -133,6 +133,9 @@
 #define UBI_IOCEBER _IOW(UBI_VOL_IOC_MAGIC, 1, int32_t)
 /* An atomic eraseblock change command */
 #define UBI_IOCEBCH _IOW(UBI_VOL_IOC_MAGIC, 2, int32_t)
+/* Map an eraseblock */
+#define UBI_IOMAP   _IOW(UBI_VOL_IOC_MAGIC, 3, struct ubi_map_req)
+
 
 /* Maximum MTD device name length supported by UBI */
 #define MAX_UBI_MTD_NAME_LEN 127
@@ -319,4 +322,17 @@ struct ubi_leb_change_req {
 	int8_t  padding[7];
 } __attribute__ ((packed));
 
+/**
+ * struct ubi_map_req - a data structure used in map
+ *                      eraseblock requests.
+ * @lnum: logical eraseblock number to change
+ * @dtype: data type (%UBI_LONGTERM, %UBI_SHORTTERM, %UBI_UNKNOWN)
+ * @padding: reserved for future, not used, has to be zeroed
+ */
+struct ubi_map_req {
+	int32_t lnum;
+	int8_t  dtype;
+	int8_t  padding[3];
+} __attribute__ ((packed));
+
 #endif /* __UBI_USER_H__ */
-- 
1.6.0.6

[PATCH 2/3] UBI: unmap operation via ioctl

[Corentin Chary]

From 4c15079a77b4a871fc96f2267222626a7e91b86a Mon Sep 17 00:00:00 2001
From: Corentin Chary <corentincj@iksaif.net>
Date: Mon, 5 Jan 2009 14:46:19 +0100

[PATCH 2/3] UBI: unmap operation via ioctl

Signed-off-by: Corentin Chary <corentincj@iksaif.net>
---
 drivers/mtd/ubi/cdev.c |   12 ++++++++++++
 include/mtd/ubi-user.h |    3 ++-
 2 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/ubi/cdev.c b/drivers/mtd/ubi/cdev.c
index db47121..7a9080c 100644
--- a/drivers/mtd/ubi/cdev.c
+++ b/drivers/mtd/ubi/cdev.c
@@ -530,6 +530,18 @@ static int vol_cdev_ioctl(struct inode *inode, struct file *file,
 		err = ubi_leb_map(desc, req.lnum, req.dtype);
 		break;
 	}
+	case UBI_IOUNMAP:
+	{
+		int32_t lnum;
+
+		err = get_user(lnum, (__user int32_t *)argp);
+		if (err) {
+			err = -EFAULT;
+			break;
+		}
+		err = ubi_leb_unmap(desc, lnum);
+		break;
+	}
 
 #endif
 
diff --git a/include/mtd/ubi-user.h b/include/mtd/ubi-user.h
index 8222f12..bcff3a0 100644
--- a/include/mtd/ubi-user.h
+++ b/include/mtd/ubi-user.h
@@ -135,7 +135,8 @@
 #define UBI_IOCEBCH _IOW(UBI_VOL_IOC_MAGIC, 2, int32_t)
 /* Map an eraseblock */
 #define UBI_IOMAP   _IOW(UBI_VOL_IOC_MAGIC, 3, struct ubi_map_req)
-
+/* Unmap an eraseblock */
+#define UBI_IOUNMAP _IOW(UBI_VOL_IOC_MAGIC, 4, int32_t)
 
 /* Maximum MTD device name length supported by UBI */
 #define MAX_UBI_MTD_NAME_LEN 127
-- 
1.6.0.6


-- 
--
Corentin Chary
http://xf.iksaif.net

[PATCH 3/3] UBI: is_mapped operation via ioctl

[Corentin Chary]

From dbccb9a0ed4d9b9b45cbad2d13432bad6e9ca886 Mon Sep 17 00:00:00 2001
From: Corentin Chary <corentincj@iksaif.net>
Date: Mon, 5 Jan 2009 14:48:59 +0100

UBI: is_mapped operation via ioctl

Signed-off-by: Corentin Chary <corentincj@iksaif.net>
---
 drivers/mtd/ubi/cdev.c |   12 ++++++++++++
 include/mtd/ubi-user.h |    2 ++
 2 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/ubi/cdev.c b/drivers/mtd/ubi/cdev.c
index 7a9080c..1bf21eb 100644
--- a/drivers/mtd/ubi/cdev.c
+++ b/drivers/mtd/ubi/cdev.c
@@ -542,6 +542,18 @@ static int vol_cdev_ioctl(struct inode *inode, struct file *file,
 		err = ubi_leb_unmap(desc, lnum);
 		break;
 	}
+	case UBI_IOISMAP:
+	{
+		int32_t lnum;
+
+		err = get_user(lnum, (__user int32_t *)argp);
+		if (err) {
+			err = -EFAULT;
+			break;
+		}
+		err = ubi_is_mapped(desc, lnum);
+		break;
+	}
 
 #endif
 
diff --git a/include/mtd/ubi-user.h b/include/mtd/ubi-user.h
index bcff3a0..a65c4ab 100644
--- a/include/mtd/ubi-user.h
+++ b/include/mtd/ubi-user.h
@@ -137,6 +137,8 @@
 #define UBI_IOMAP   _IOW(UBI_VOL_IOC_MAGIC, 3, struct ubi_map_req)
 /* Unmap an eraseblock */
 #define UBI_IOUNMAP _IOW(UBI_VOL_IOC_MAGIC, 4, int32_t)
+/* Check whether an eraseblock is mapped */
+#define UBI_IOISMAP _IOR(UBI_VOL_IOC_MAGIC, 5, int32_t)
 
 /* Maximum MTD device name length supported by UBI */
 #define MAX_UBI_MTD_NAME_LEN 127
-- 
1.6.0.6

Re: [PATCH 0/3] Export UBI map/unmap/is_mapped in userspace v2

[Artem Bityutskiy]

On Wed, 2009-01-07 at 10:22 +0100, Corentin Chary wrote:
> Hi,x
> These are three patch to make a usespace program able to use map/unmap 
> operation and to query if a LEB is mapped.
> We are trying to do an UBI based userspace filesystem (using fuse) and we 
> needed these, but it could also be used for testing purpose.
> CONFIG_MTD_UBI_DEBUG_USERSPACE_IO is needed to enable theses functions.
> 
> The patch can also be download here:
> http://xf.iksaif.net/dev/uffs/linux/ubi/

The requested URL /dev/uffs/linux/ubi/ was not found on this server.

-- 
Best regards,
Artem Bityutskiy (Битюцкий Артём)

Re: [PATCH 0/3] Export UBI map/unmap/is_mapped in userspace v2

[Corentin Chary]

On Wed, Jan 7, 2009 at 10:32 AM, Artem Bityutskiy
<dedekind@infradead.org> wrote:
> On Wed, 2009-01-07 at 10:22 +0100, Corentin Chary wrote:
>> Hi,x
>> These are three patch to make a usespace program able to use map/unmap
>> operation and to query if a LEB is mapped.
>> We are trying to do an UBI based userspace filesystem (using fuse) and we
>> needed these, but it could also be used for testing purpose.
>> CONFIG_MTD_UBI_DEBUG_USERSPACE_IO is needed to enable theses functions.
>>
>> The patch can also be download here:
>> http://xf.iksaif.net/dev/uffs/linux/ubi/
>
> The requested URL /dev/uffs/linux/ubi/ was not found on this server.

Was uploading, works now

-- 
Corentin Chary
http://xf.iksaif.net

Re: [PATCH 0/3] Export UBI map/unmap/is_mapped in userspace v2

[Artem Bityutskiy]

On Wed, 2009-01-07 at 10:22 +0100, Corentin Chary wrote:
> Hi,x
> These are three patch to make a usespace program able to use map/unmap 
> operation and to query if a LEB is mapped.
> We are trying to do an UBI based userspace filesystem (using fuse) and we 
> needed these, but it could also be used for testing purpose.
> CONFIG_MTD_UBI_DEBUG_USERSPACE_IO is needed to enable theses functions.
> 
> The patch can also be download here:
> http://xf.iksaif.net/dev/uffs/linux/ubi/

Pushed to the master branch of ubi-2.6.git. I tweaked your patches and
re-named ioctls - please check.

Thanks.

-- 
Best regards,
Artem Bityutskiy (Битюцкий Артём)

Re: [PATCH 0/3] Export UBI map/unmap/is_mapped in userspace v2

[Corentin Chary]

On Wed, Jan 7, 2009 at 1:15 PM, Artem Bityutskiy <dedekind@infradead.org> wrote:
> On Wed, 2009-01-07 at 10:22 +0100, Corentin Chary wrote:
>> Hi,x
>> These are three patch to make a usespace program able to use map/unmap
>> operation and to query if a LEB is mapped.
>> We are trying to do an UBI based userspace filesystem (using fuse) and we
>> needed these, but it could also be used for testing purpose.
>> CONFIG_MTD_UBI_DEBUG_USERSPACE_IO is needed to enable theses functions.
>>
>> The patch can also be download here:
>> http://xf.iksaif.net/dev/uffs/linux/ubi/
>
> Pushed to the master branch of ubi-2.6.git. I tweaked your patches and
> re-named ioctls - please check.
>
> Thanks.


Checked, seems ok to me.
Thanks

-- 
Corentin Chary
http://xf.iksaif.net

Re: [PATCH 0/3] Export UBI map/unmap/is_mapped in userspace v2

[Artem Bityutskiy]

On Wed, 7 Jan 2009, Corentin Chary wrote:

> Hi,x
> These are three patch to make a usespace program able to use map/unmap
> operation and to query if a LEB is mapped.
> We are trying to do an UBI based userspace filesystem (using fuse) and we
> needed these, but it could also be used for testing purpose.
> CONFIG_MTD_UBI_DEBUG_USERSPACE_IO is needed to enable theses functions.

Hi,

I'm not sure whether you'll ever implement your user-space stuff
or not, but I was thinking that it is probably bad to make your
life difficult by requireing this CONFIG_MTD_UBI_DEBUG_USERSPACE_IO
option.

I suggest you to add yet another ioctl which enables/disables direct
user-space writes. And make them disabled by default. Then you may
remove the CONFIG_MTD_UBI_DEBUG_USERSPACE_IO option. Your FS will
only need to enable the volume direct I/O before it starts writing
anything.

Artem.

Re: [PATCH 0/3] Export UBI map/unmap/is_mapped in userspace v2

[Corentin Chary]

On Sun, Jan 18, 2009 at 3:47 PM, Artem Bityutskiy
<dedekind@infradead.org> wrote:
>
>
> On Wed, 7 Jan 2009, Corentin Chary wrote:
>
>> Hi,x
>> These are three patch to make a usespace program able to use map/unmap
>> operation and to query if a LEB is mapped.
>> We are trying to do an UBI based userspace filesystem (using fuse) and we
>> needed these, but it could also be used for testing purpose.
>> CONFIG_MTD_UBI_DEBUG_USERSPACE_IO is needed to enable theses functions.
>
> Hi,
>
> I'm not sure whether you'll ever implement your user-space stuff
> or not, but I was thinking that it is probably bad to make your
> life difficult by requireing this CONFIG_MTD_UBI_DEBUG_USERSPACE_IO
> option.
>
> I suggest you to add yet another ioctl which enables/disables direct
> user-space writes. And make them disabled by default. Then you may
> remove the CONFIG_MTD_UBI_DEBUG_USERSPACE_IO option. Your FS will
> only need to enable the volume direct I/O before it starts writing
> anything.
>
> Artem.

Hi,
Indeed I think it's the way to go. We'll send a patch ASAP.

Thanks for your advice

-- 
Corentin Chary
http://xf.iksaif.net

Re: [PATCH 0/3] Export UBI map/unmap/is_mapped in userspace v2

[Corentin Chary]

Hi Artem,
Here is a draft (won't apply, won't compile).
Do you think this would be ok ?
Thanks

---
diff --cc drivers/mtd/ubi/cdev.c
index 9ddbade,f9631eb..0000000
--- a/drivers/mtd/ubi/cdev.c
+++ b/drivers/mtd/ubi/cdev.c
@@@ -263,8 -259,8 +259,6 @@@ static ssize_t vol_cdev_read(struct fil
  	return err ? err : count_save - count;
  }

--#ifdef CONFIG_MTD_UBI_DEBUG_USERSPACE_IO
--
  /*
   * This function allows to directly write to dynamic UBI volumes, without
   * issuing the volume update operation. Available only as a debugging feature.
@@@ -279,8 -275,7 +273,10 @@@ static ssize_t vol_cdev_direct_write(st
  	int lnum, off, len, tbuf_size, err = 0;
  	size_t count_save = count;
  	char *tbuf;
+
++	if (!vol->direct_ioctl)
++	  return -EPERM;
 +
  	dbg_gen("requested: write %zd bytes to offset %lld of volume %u",
  		count, *offp, vol->vol_id);

@@@ -347,10 -339,10 +340,6 @@@
  	return err ? err : count_save - count;
  }

--#else
--#define vol_cdev_direct_write(file, buf, count, offp) (-EPERM)
--#endif /* CONFIG_MTD_UBI_DEBUG_USERSPACE_IO */
--
  static ssize_t vol_cdev_write(struct file *file, const char __user *buf,
  			      size_t count, loff_t *offp)
  {
@@@ -560,8 -551,7 +548,20 @@@ static long vol_cdev_ioctl(struct file
  		err = ubi_is_mapped(desc, lnum);
  		break;
  	}
- #endif
+
++	case UBI_IOVOLDIRIO:
++	{
++		int32_t directio;
++
++		err = get_user(directio, (__user int32_t *)argp);
++		if (err) {
++			err = -EFAULT;
++			break;
++		}
++		err = desc->vol->direct_ioctl = (directio > 0);
++		break;
++	}
 +
  	default:
  		err = -ENOTTY;
  		break;
diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h
index 381f0e1..25a77eb 100644
--- a/drivers/mtd/ubi/ubi.h
+++ b/drivers/mtd/ubi/ubi.h
@@ -206,6 +206,7 @@ struct ubi_volume_desc;
  * @upd_marker: %1 if the update marker is set for this volume
  * @updating: %1 if the volume is being updated
  * @changing_leb: %1 if the atomic LEB change ioctl command is in progress
+ * @direct_ioctl: %1 if direct write operation are enabled for users
(via ioctl())
  *
  * @gluebi_desc: gluebi UBI volume descriptor
  * @gluebi_refcount: reference count of the gluebi MTD device
@@ -253,6 +254,7 @@ struct ubi_volume {
 	unsigned int upd_marker:1;
 	unsigned int updating:1;
 	unsigned int changing_leb:1;
+	unsigned int direct_ioctl:1;

 #ifdef CONFIG_MTD_UBI_GLUEBI
 	/*
diff --git a/include/mtd/ubi-user.h b/include/mtd/ubi-user.h
index 82113e1..1fa7a28 100644
--- a/include/mtd/ubi-user.h
+++ b/include/mtd/ubi-user.h
@@ -175,6 +175,8 @@
 #define UBI_IOCEBUNMAP _IOW(UBI_VOL_IOC_MAGIC, 4, int32_t)
 /* Check if LEB is mapped command */
 #define UBI_IOCEBISMAP _IOR(UBI_VOL_IOC_MAGIC, 5, int32_t)
+/* Set direct-io availability */
+#define UBI_IOVOLDIRIO _IOW(UBI_VOL_IOC_MAGIC, 6, int32_t)

 /* Maximum MTD device name length supported by UBI */
 #define MAX_UBI_MTD_NAME_LEN 127

Re: [PATCH 0/3] Export UBI map/unmap/is_mapped in userspace v2

[Artem Bityutskiy]

Hi,

Some obvious things are commented below.

On Mon, 2009-01-19 at 10:38 +0100, Corentin Chary wrote:
> --#ifdef CONFIG_MTD_UBI_DEBUG_USERSPACE_IO
> --
>   /*
>    * This function allows to directly write to dynamic UBI volumes, without
>    * issuing the volume update operation. Available only as a debugging feature.
> @@@ -279,8 -275,7 +273,10 @@@ static ssize_t vol_cdev_direct_write(st
>   	int lnum, off, len, tbuf_size, err = 0;
>   	size_t count_save = count;
>   	char *tbuf;
> +
> ++	if (!vol->direct_ioctl)
> ++	  return -EPERM;
>  +
I suggest calling this vol->direct_writes instead.

>   	dbg_gen("requested: write %zd bytes to offset %lld of volume %u",
>   		count, *offp, vol->vol_id);
> 
> @@@ -347,10 -339,10 +340,6 @@@
>   	return err ? err : count_save - count;
>   }
> 
> --#else
> --#define vol_cdev_direct_write(file, buf, count, offp) (-EPERM)
> --#endif /* CONFIG_MTD_UBI_DEBUG_USERSPACE_IO */
> --
>   static ssize_t vol_cdev_write(struct file *file, const char __user *buf,
>   			      size_t count, loff_t *offp)
>   {
> @@@ -560,8 -551,7 +548,20 @@@ static long vol_cdev_ioctl(struct file
>   		err = ubi_is_mapped(desc, lnum);
>   		break;
>   	}
> - #endif
> +
> ++	case UBI_IOVOLDIRIO:
Please, use a consistent name. All ioctl names start with UBI_IOC =
UBI ioctl.

I used "direct I/O" term in one of my previous mails. Please, do not use
i, because this term is reserved for file-system direct I/O, which
bypasses page cache. So, let's avoid any possible confusion.

Also, I suggest to make this ioctl more generic. You effectively want
to change some volume property. And in future we may want to be able
changing other properties, for example, cleaning or setting the
"corrupted" flag for the volume. Or changing the WL threshold. Or
changing the amount of PEBs reserved for wear-levelling. Etc.

So, let's add UBI_IOCSETPROP ioctl (UBI ioctl set property) instead.
For now, we'll have only one property - direct writes. Let's make this
ioctl accept a pointer to

struct ubi_set_prop_req {
	uint8_t property;
	uint8_t padding[7];
	uint64_t value;
}

and introduce constants

enum {
	UBI_PROP_DIRECT_WRITE = 0,
};

So, to enable direct writes you'll have to set req->property to
UBI_PROP_DIRECT_WRITE and value to 1.

> ++	{
> ++		int32_t directio;
> ++
> ++		err = get_user(directio, (__user int32_t *)argp);
> ++		if (err) {
> ++			err = -EFAULT;
> ++			break;
> ++		}
> ++		err = desc->vol->direct_ioctl = (directio > 0);
> ++		break;

You have to serialize the ioctl. Please, use the volumes_mutex for this:

mutex_lock(&ubi->volumes_mutex);
desc->vol->direct_ioctl = !!req->property;
mutex_unlock(&ubi->volumes_mutex);

Also, do not forget to validate the 'struct ubi_set_prop_req' object.

> ++	}
>  +
>   	default:
>   		err = -ENOTTY;
>   		break;
> diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h
> index 381f0e1..25a77eb 100644
> --- a/drivers/mtd/ubi/ubi.h
> +++ b/drivers/mtd/ubi/ubi.h
> @@ -206,6 +206,7 @@ struct ubi_volume_desc;
>   * @upd_marker: %1 if the update marker is set for this volume
>   * @updating: %1 if the volume is being updated
>   * @changing_leb: %1 if the atomic LEB change ioctl command is in progress
> + * @direct_ioctl: %1 if direct write operation are enabled for users
> (via ioctl())
>   *
>   * @gluebi_desc: gluebi UBI volume descriptor
>   * @gluebi_refcount: reference count of the gluebi MTD device
> @@ -253,6 +254,7 @@ struct ubi_volume {
>  	unsigned int upd_marker:1;
>  	unsigned int updating:1;
>  	unsigned int changing_leb:1;
> +	unsigned int direct_ioctl:1;

Also, find the comment for the 'volumes_mutex' and add that it protects
this flag to the comment.

> 
>  #ifdef CONFIG_MTD_UBI_GLUEBI
>  	/*
> diff --git a/include/mtd/ubi-user.h b/include/mtd/ubi-user.h
> index 82113e1..1fa7a28 100644
> --- a/include/mtd/ubi-user.h
> +++ b/include/mtd/ubi-user.h
> @@ -175,6 +175,8 @@
>  #define UBI_IOCEBUNMAP _IOW(UBI_VOL_IOC_MAGIC, 4, int32_t)
>  /* Check if LEB is mapped command */
>  #define UBI_IOCEBISMAP _IOR(UBI_VOL_IOC_MAGIC, 5, int32_t)
> +/* Set direct-io availability */
> +#define UBI_IOVOLDIRIO _IOW(UBI_VOL_IOC_MAGIC, 6, int32_t)

Also, please document this ioctl in the big commentary at the beginning
of ubi-user.h.

Thanks.

-- 
Best regards,
Artem Bityutskiy (Битюцкий Артём)

Re: [PATCH 0/3] Export UBI map/unmap/is_mapped in userspace v2

[Artem Bityutskiy]

On Mon, 2009-01-19 at 12:09 +0200, Artem Bityutskiy wrote:
> Hi,
> 
> Some obvious things are commented below.
> 

Err, and also do not forget to zap the CONFIG_MTD_UBI_DEBUG_USERSPACE_IO
from UBI Kconfig.

-- 
Best regards,
Artem Bityutskiy (Битюцкий Артём)

Re: [PATCH 0/3] Export UBI map/unmap/is_mapped in userspace v2

[Corentin Chary]

Ok, thanks for your comments.
We'll do that and send the result next week.

On Mon, Jan 19, 2009 at 11:42 AM, Artem Bityutskiy
<dedekind@infradead.org> wrote:
> On Mon, 2009-01-19 at 12:09 +0200, Artem Bityutskiy wrote:
>> Hi,
>>
>> Some obvious things are commented below.
>>
>
> Err, and also do not forget to zap the CONFIG_MTD_UBI_DEBUG_USERSPACE_IO
> from UBI Kconfig.
>
> --
> Best regards,
> Artem Bityutskiy (Битюцкий Артём)
>
>



-- 
Corentin Chary
http://xf.iksaif.net