MS Windows mounts removable storage in "Removal optimized mode" by
default. All the writes to the media are synchronous which is achieved
by setting FUA (Force Unit Access) bit in SCSI WRITE(10,12) commands.
This prevents I/O requests aggregation in block layer dramatically
decreasing performance.
This patch brings an option to accept or ignore mentioned bit
a) via specifying module parameter "fua", or
b) through sysfs entry
/sys/devices/platform/musb_hdrc/gadget/gadget-lun-N/fua
Originally patch was written by Denis Karpov for Maemo5 platform.
Signed-off-by: Andy Shevchenko <[email protected]>
Cc: Denis Karpov <[email protected]>
Cc: Adrian Hunter <[email protected]>
Cc: David Brownell <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: [email protected]
---
drivers/usb/gadget/file_storage.c | 24 ++++++++++++++++++------
drivers/usb/gadget/storage_common.c | 26 ++++++++++++++++++++++++++
2 files changed, 44 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/gadget/file_storage.c b/drivers/usb/gadget/file_storage.c
index b49d86e..ce40bfb 100644
--- a/drivers/usb/gadget/file_storage.c
+++ b/drivers/usb/gadget/file_storage.c
@@ -93,6 +93,8 @@
* removable Default false, boolean for removable media
* luns=N Default N = number of filenames, number of
* LUNs to support
+ * fua=N Default true, boolean for accepting FUA flag
+ * in SCSI WRITE(10,12) commands
* stall Default determined according to the type of
* USB device controller (usually true),
* boolean to permit the driver to halt
@@ -111,11 +113,11 @@
* PAGE_CACHE_SIZE)
*
* If CONFIG_USB_FILE_STORAGE_TEST is not set, only the "file", "ro",
- * "removable", "luns", "stall", and "cdrom" options are available; default
+ * "removable", "luns", "fua", "stall", and "cdrom" options are available; default
* values are used for everything else.
*
* The pathnames of the backing files and the ro settings are available in
- * the attribute files "file" and "ro" in the lun<n> subdirectory of the
+ * the attribute files "file", "fua", and "ro" in the lun<n> subdirectory of the
* gadget's sysfs directory. If the "removable" option is set, writing to
* these files will simulate ejecting/loading the medium (writing an empty
* line means eject) and adjusting a write-enable tab. Changes to the ro
@@ -307,6 +309,7 @@ static struct {
int removable;
int can_stall;
+ int fua;
int cdrom;
char *transport_parm;
@@ -326,6 +329,7 @@ static struct {
.protocol_parm = "SCSI",
.removable = 0,
.can_stall = 1,
+ .fua = 1,
.cdrom = 0,
.vendor = FSG_VENDOR_ID,
.product = FSG_PRODUCT_ID,
@@ -350,6 +354,9 @@ MODULE_PARM_DESC(removable, "true to simulate removable media");
module_param_named(stall, mod_data.can_stall, bool, S_IRUGO);
MODULE_PARM_DESC(stall, "false to prevent bulk stalls");
+module_param_named(fua, mod_data.fua, bool, S_IRUGO);
+MODULE_PARM_DESC(fua, "true to obey SCSI WRITE(6,10,12) FUA bit");
+
module_param_named(cdrom, mod_data.cdrom, bool, S_IRUGO);
MODULE_PARM_DESC(cdrom, "true to emulate cdrom instead of disk");
@@ -1272,7 +1279,8 @@ static int do_write(struct fsg_dev *fsg)
curlun->sense_data = SS_INVALID_FIELD_IN_CDB;
return -EINVAL;
}
- if (fsg->cmnd[1] & 0x08) { // FUA
+ /* FUA */
+ if (curlun->fua && (fsg->cmnd[1] & 0x08)) {
spin_lock(&curlun->filp->f_lock);
curlun->filp->f_flags |= O_DSYNC;
spin_unlock(&curlun->filp->f_lock);
@@ -3126,6 +3134,7 @@ static int fsg_main_thread(void *fsg_)
/* The write permissions and store_xxx pointers are set in fsg_bind() */
static DEVICE_ATTR(ro, 0444, fsg_show_ro, NULL);
+static DEVICE_ATTR(fua, 0644, show_fua, store_fua);
static DEVICE_ATTR(file, 0444, fsg_show_file, NULL);
@@ -3330,6 +3339,7 @@ static int __init fsg_bind(struct usb_gadget *gadget)
curlun->ro = mod_data.cdrom || mod_data.ro[i];
curlun->initially_ro = curlun->ro;
curlun->removable = mod_data.removable;
+ curlun->fua = mod_data.fua;
curlun->dev.release = lun_release;
curlun->dev.parent = &gadget->dev;
curlun->dev.driver = &fsg_driver.driver;
@@ -3344,7 +3354,9 @@ static int __init fsg_bind(struct usb_gadget *gadget)
if ((rc = device_create_file(&curlun->dev,
&dev_attr_ro)) != 0 ||
(rc = device_create_file(&curlun->dev,
- &dev_attr_file)) != 0) {
+ &dev_attr_file)) != 0 ||
+ (rc = device_create_file(&curlun->dev,
+ &dev_attr_fua)) != 0) {
device_unregister(&curlun->dev);
goto out;
}
@@ -3478,8 +3490,8 @@ static int __init fsg_bind(struct usb_gadget *gadget)
if (IS_ERR(p))
p = NULL;
}
- LINFO(curlun, "ro=%d, file: %s\n",
- curlun->ro, (p ? p : "(error)"));
+ LINFO(curlun, "ro=%d, fua=%d file: %s\n",
+ curlun->ro, curlun->fua, (p ? p : "(error)"));
}
}
kfree(pathbuf);
diff --git a/drivers/usb/gadget/storage_common.c b/drivers/usb/gadget/storage_common.c
index 04c462f..ec2ff30 100644
--- a/drivers/usb/gadget/storage_common.c
+++ b/drivers/usb/gadget/storage_common.c
@@ -269,6 +269,7 @@ struct fsg_lun {
unsigned int prevent_medium_removal:1;
unsigned int registered:1;
unsigned int info_valid:1;
+ unsigned int fua:1;
u32 sense_data;
u32 sense_data_info;
@@ -689,6 +690,14 @@ static ssize_t fsg_show_ro(struct device *dev, struct device_attribute *attr,
: curlun->initially_ro);
}
+static ssize_t show_fua(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct fsg_lun *curlun = fsg_lun_from_dev(dev);
+
+ return sprintf(buf, "%u\n", curlun->fua);
+}
+
static ssize_t fsg_show_file(struct device *dev, struct device_attribute *attr,
char *buf)
{
@@ -743,6 +752,23 @@ static ssize_t fsg_store_ro(struct device *dev, struct device_attribute *attr,
return rc;
}
+static ssize_t store_fua(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct fsg_lun *curlun = fsg_lun_from_dev(dev);
+ unsigned long attr_val = 0;
+
+ if (strict_strtoul(buf, 2, &attr_val))
+ return -EINVAL;
+
+ if (!(curlun->fua))
+ fsg_lun_fsync_sub(curlun);
+
+ curlun->fua = attr_val ? 1 : 0;
+
+ return count;
+}
+
static ssize_t fsg_store_file(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
--
1.6.3.3
On Tue, 13 Jul 2010, Andy Shevchenko wrote:
> MS Windows mounts removable storage in "Removal optimized mode" by
> default. All the writes to the media are synchronous which is achieved
> by setting FUA (Force Unit Access) bit in SCSI WRITE(10,12) commands.
> This prevents I/O requests aggregation in block layer dramatically
> decreasing performance.
>
> This patch brings an option to accept or ignore mentioned bit
> a) via specifying module parameter "fua", or
> b) through sysfs entry
> /sys/devices/platform/musb_hdrc/gadget/gadget-lun-N/fua
...
> @@ -111,11 +113,11 @@
> * PAGE_CACHE_SIZE)
> *
> * If CONFIG_USB_FILE_STORAGE_TEST is not set, only the "file", "ro",
> - * "removable", "luns", "stall", and "cdrom" options are available; default
> + * "removable", "luns", "fua", "stall", and "cdrom" options are available; default
> * values are used for everything else.
> *
> * The pathnames of the backing files and the ro settings are available in
> - * the attribute files "file" and "ro" in the lun<n> subdirectory of the
> + * the attribute files "file", "fua", and "ro" in the lun<n> subdirectory of the
> * gadget's sysfs directory. If the "removable" option is set, writing to
> * these files will simulate ejecting/loading the medium (writing an empty
> * line means eject) and adjusting a write-enable tab. Changes to the ro
Please do not create lines longer than 80 columns. Reflow the text.
> @@ -307,6 +309,7 @@ static struct {
>
> int removable;
> int can_stall;
> + int fua;
> int cdrom;
>
> char *transport_parm;
> @@ -326,6 +329,7 @@ static struct {
> .protocol_parm = "SCSI",
> .removable = 0,
> .can_stall = 1,
> + .fua = 1,
> .cdrom = 0,
> .vendor = FSG_VENDOR_ID,
> .product = FSG_PRODUCT_ID,
> @@ -350,6 +354,9 @@ MODULE_PARM_DESC(removable, "true to simulate removable media");
> module_param_named(stall, mod_data.can_stall, bool, S_IRUGO);
> MODULE_PARM_DESC(stall, "false to prevent bulk stalls");
>
> +module_param_named(fua, mod_data.fua, bool, S_IRUGO);
> +MODULE_PARM_DESC(fua, "true to obey SCSI WRITE(6,10,12) FUA bit");
> +
> module_param_named(cdrom, mod_data.cdrom, bool, S_IRUGO);
> MODULE_PARM_DESC(cdrom, "true to emulate cdrom instead of disk");
This implementation is wrong. If "fua" is supposed to be per-lun
then the module parameter needs to accept an array of values, like the
"file" and "ro" parameters.
Alan Stern
MS Windows mounts removable storage in "Removal optimized mode" by
default. All the writes to the media are synchronous which is achieved
by setting FUA (Force Unit Access) bit in SCSI WRITE(10,12) commands.
This prevents I/O requests aggregation in block layer dramatically
decreasing performance.
This patch brings an option to accept or ignore mentioned bit
a) via specifying module parameter "fua", or
b) through sysfs entry
/sys/devices/platform/musb_hdrc/gadget/gadget-lun-N/fua
Patch is based on work done by Denis Karpov for Maemo5 platform.
Signed-off-by: Andy Shevchenko <[email protected]>
Cc: Denis Karpov <[email protected]>
Cc: Adrian Hunter <[email protected]>
Cc: Alan Stern <[email protected]>
Cc: David Brownell <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: [email protected]
---
drivers/usb/gadget/file_storage.c | 30 +++++++++++++++++++++++-------
drivers/usb/gadget/storage_common.c | 27 +++++++++++++++++++++++++++
2 files changed, 50 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/gadget/file_storage.c b/drivers/usb/gadget/file_storage.c
index b49d86e..45f58d9 100644
--- a/drivers/usb/gadget/file_storage.c
+++ b/drivers/usb/gadget/file_storage.c
@@ -93,6 +93,8 @@
* removable Default false, boolean for removable media
* luns=N Default N = number of filenames, number of
* LUNs to support
+ * fua=b[,b...] Default false, booleans for ignore FUA flag
+ * in SCSI WRITE(6,10,12) commands
* stall Default determined according to the type of
* USB device controller (usually true),
* boolean to permit the driver to halt
@@ -111,12 +113,12 @@
* PAGE_CACHE_SIZE)
*
* If CONFIG_USB_FILE_STORAGE_TEST is not set, only the "file", "ro",
- * "removable", "luns", "stall", and "cdrom" options are available; default
- * values are used for everything else.
+ * "removable", "luns", "fua", "stall", and "cdrom" options are available;
+ * default values are used for everything else.
*
* The pathnames of the backing files and the ro settings are available in
- * the attribute files "file" and "ro" in the lun<n> subdirectory of the
- * gadget's sysfs directory. If the "removable" option is set, writing to
+ * the attribute files "file", "fua", and "ro" in the lun<n> subdirectory of
+ * the gadget's sysfs directory. If the "removable" option is set, writing to
* these files will simulate ejecting/loading the medium (writing an empty
* line means eject) and adjusting a write-enable tab. Changes to the ro
* setting are not allowed when the medium is loaded or if CD-ROM emulation
@@ -301,8 +303,10 @@ MODULE_LICENSE("Dual BSD/GPL");
static struct {
char *file[FSG_MAX_LUNS];
int ro[FSG_MAX_LUNS];
+ int fua[FSG_MAX_LUNS];
unsigned int num_filenames;
unsigned int num_ros;
+ unsigned int num_fuas;
unsigned int nluns;
int removable;
@@ -341,6 +345,9 @@ MODULE_PARM_DESC(file, "names of backing files or devices");
module_param_array_named(ro, mod_data.ro, bool, &mod_data.num_ros, S_IRUGO);
MODULE_PARM_DESC(ro, "true to force read-only");
+module_param_array_named(fua, mod_data.fua, bool, &mod_data.num_fuas, S_IRUGO);
+MODULE_PARM_DESC(fua, "true to ignore SCSI WRITE(6,10,12) FUA bit");
+
module_param_named(luns, mod_data.nluns, uint, S_IRUGO);
MODULE_PARM_DESC(luns, "number of LUNs");
@@ -1272,7 +1279,8 @@ static int do_write(struct fsg_dev *fsg)
curlun->sense_data = SS_INVALID_FIELD_IN_CDB;
return -EINVAL;
}
- if (fsg->cmnd[1] & 0x08) { // FUA
+ /* FUA */
+ if (!curlun->fua && (fsg->cmnd[1] & 0x08)) {
spin_lock(&curlun->filp->f_lock);
curlun->filp->f_flags |= O_DSYNC;
spin_unlock(&curlun->filp->f_lock);
@@ -3126,6 +3134,7 @@ static int fsg_main_thread(void *fsg_)
/* The write permissions and store_xxx pointers are set in fsg_bind() */
static DEVICE_ATTR(ro, 0444, fsg_show_ro, NULL);
+static DEVICE_ATTR(fua, 0644, fsg_show_fua, NULL);
static DEVICE_ATTR(file, 0444, fsg_show_file, NULL);
@@ -3305,6 +3314,10 @@ static int __init fsg_bind(struct usb_gadget *gadget)
}
}
+ /* Only for removable media? */
+ dev_attr_fua.attr.mode = 0644;
+ dev_attr_fua.store = fsg_store_fua;
+
/* Find out how many LUNs there should be */
i = mod_data.nluns;
if (i == 0)
@@ -3330,6 +3343,7 @@ static int __init fsg_bind(struct usb_gadget *gadget)
curlun->ro = mod_data.cdrom || mod_data.ro[i];
curlun->initially_ro = curlun->ro;
curlun->removable = mod_data.removable;
+ curlun->fua = mod_data.fua[i];
curlun->dev.release = lun_release;
curlun->dev.parent = &gadget->dev;
curlun->dev.driver = &fsg_driver.driver;
@@ -3344,6 +3358,8 @@ static int __init fsg_bind(struct usb_gadget *gadget)
if ((rc = device_create_file(&curlun->dev,
&dev_attr_ro)) != 0 ||
(rc = device_create_file(&curlun->dev,
+ &dev_attr_fua)) != 0 ||
+ (rc = device_create_file(&curlun->dev,
&dev_attr_file)) != 0) {
device_unregister(&curlun->dev);
goto out;
@@ -3478,8 +3494,8 @@ static int __init fsg_bind(struct usb_gadget *gadget)
if (IS_ERR(p))
p = NULL;
}
- LINFO(curlun, "ro=%d, file: %s\n",
- curlun->ro, (p ? p : "(error)"));
+ LINFO(curlun, "ro=%d, fua=%d, file: %s\n",
+ curlun->ro, curlun->fua, (p ? p : "(error)"));
}
}
kfree(pathbuf);
diff --git a/drivers/usb/gadget/storage_common.c b/drivers/usb/gadget/storage_common.c
index 04c462f..80c93d7 100644
--- a/drivers/usb/gadget/storage_common.c
+++ b/drivers/usb/gadget/storage_common.c
@@ -269,6 +269,7 @@ struct fsg_lun {
unsigned int prevent_medium_removal:1;
unsigned int registered:1;
unsigned int info_valid:1;
+ unsigned int fua:1;
u32 sense_data;
u32 sense_data_info;
@@ -689,6 +690,14 @@ static ssize_t fsg_show_ro(struct device *dev, struct device_attribute *attr,
: curlun->initially_ro);
}
+static ssize_t fsg_show_fua(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct fsg_lun *curlun = fsg_lun_from_dev(dev);
+
+ return sprintf(buf, "%u\n", curlun->fua);
+}
+
static ssize_t fsg_show_file(struct device *dev, struct device_attribute *attr,
char *buf)
{
@@ -743,6 +752,24 @@ static ssize_t fsg_store_ro(struct device *dev, struct device_attribute *attr,
return rc;
}
+static ssize_t fsg_store_fua(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ ssize_t rc = count;
+ struct fsg_lun *curlun = fsg_lun_from_dev(dev);
+ int i;
+
+ if (sscanf(buf, "%d", &i) != 1)
+ return -EINVAL;
+
+ if (curlun->fua)
+ fsg_lun_fsync_sub(curlun);
+
+ curlun->fua = !!i;
+
+ return rc;
+}
+
static ssize_t fsg_store_file(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
--
1.6.3.3
On Wed, 14 Jul 2010 11:05:31 +0200, Andy Shevchenko <[email protected]> wrote:
> MS Windows mounts removable storage in "Removal optimized mode" by
> default. All the writes to the media are synchronous which is achieved
> by setting FUA (Force Unit Access) bit in SCSI WRITE(10,12) commands.
> This prevents I/O requests aggregation in block layer dramatically
> decreasing performance.
> diff --git a/drivers/usb/gadget/file_storage.c b/drivers/usb/gadget/file_storage.c
> index b49d86e..45f58d9 100644
> --- a/drivers/usb/gadget/file_storage.c
> +++ b/drivers/usb/gadget/file_storage.c
> @@ -93,6 +93,8 @@
> * removable Default false, boolean for removable media
> * luns=N Default N = number of filenames, number of
> * LUNs to support
> + * fua=b[,b...] Default false, booleans for ignore FUA flag
> + * in SCSI WRITE(6,10,12) commands
I wonder if it makes sense to make it per-LUN. I would imagine that it's great
to ignore FUA if the device has its own power supply in which case after disconnect
the data won't be lost. This is a per-device property not really per-LUN. As such
I'd make this option global for the gadget.
> @@ -743,6 +752,24 @@ static ssize_t fsg_store_ro(struct device *dev, struct device_attribute *attr,
> return rc;
> }
>+static ssize_t fsg_store_fua(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + ssize_t rc = count;
Not really needed here.
> + struct fsg_lun *curlun = fsg_lun_from_dev(dev);
> + int i;
> +
> + if (sscanf(buf, "%d", &i) != 1)
> + return -EINVAL;
Why not simple_strtol() directly?
> +
> + if (curlun->fua)
> + fsg_lun_fsync_sub(curlun);
Shouldn't that read something like:
+ if (!curlun->fua && i)
+ fsg_lun_fsync_sub(curlun);
ie. there is no sense in syncing if FUA was active (in which case all
writes were synced already, right?) or if the new value is false (since
then user does not won't syncing).
> +
> + curlun->fua = !!i;
> +
> + return rc;
Just plain:
+ return count;
> +}
> +
> static ssize_t fsg_store_file(struct device *dev, struct device_attribute *attr,
> const char *buf, size_t count)
> {
--
Best regards, _ _
| Humble Liege of Serenely Enlightened Majesty of o' \,=./ `o
| Computer Science, Michał "mina86" Nazarewicz (o o)
+----[mina86*mina86.com]---[mina86*jabber.org]----ooO--(_)--Ooo--
2010/7/14 Michał Nazarewicz <[email protected]>:
> On Wed, 14 Jul 2010 11:05:31 +0200, Andy Shevchenko
> <[email protected]> wrote:
>>
>> MS Windows mounts removable storage in "Removal optimized mode" by
>> default. All the writes to the media are synchronous which is achieved
>> by setting FUA (Force Unit Access) bit in SCSI WRITE(10,12) commands.
>> This prevents I/O requests aggregation in block layer dramatically
>> decreasing performance.
>
>> diff --git a/drivers/usb/gadget/file_storage.c
>> b/drivers/usb/gadget/file_storage.c
>> index b49d86e..45f58d9 100644
>> --- a/drivers/usb/gadget/file_storage.c
>> +++ b/drivers/usb/gadget/file_storage.c
>> @@ -93,6 +93,8 @@
>> * removable Default false, boolean for removable media
>> * luns=N Default N = number of filenames, number of
>> * LUNs to support
>> + * fua=b[,b...] Default false, booleans for ignore FUA
>> flag
>> + * in SCSI WRITE(6,10,12) commands
>
> I wonder if it makes sense to make it per-LUN. I would imagine that it's
> great
> to ignore FUA if the device has its own power supply in which case after
> disconnect
> the data won't be lost. This is a per-device property not really per-LUN.
> As such
> I'd make this option global for the gadget.
Make sense only for removable media with one partition.
Otherwise. why we have sync option per partition f.e., not per device?
>> @@ -743,6 +752,24 @@ static ssize_t fsg_store_ro(struct device *dev,
>> struct device_attribute *attr,
>> return rc;
>> }
>> +static ssize_t fsg_store_fua(struct device *dev, struct device_attribute
>> *attr,
>> + const char *buf, size_t count)
>> +{
>> + ssize_t rc = count;
>
> Not really needed here.
See below
>
>> + struct fsg_lun *curlun = fsg_lun_from_dev(dev);
>> + int i;
>> +
>> + if (sscanf(buf, "%d", &i) != 1)
>> + return -EINVAL;
>
> Why not simple_strtol() directly?
I did it in the same way like fsg_store_ro() does.
I have no objections to back to previous solution.
>> +
>> + if (curlun->fua)
>> + fsg_lun_fsync_sub(curlun);
>
> Shouldn't that read something like:
>
> + if (!curlun->fua && i)
> + fsg_lun_fsync_sub(curlun);
>
> ie. there is no sense in syncing if FUA was active (in which case all
> writes were synced already, right?) or if the new value is false (since
> then user does not won't syncing).
The idea is to sync data before switching from async mode.
Actually fua = 1 means ignorance of that flag.
--
With Best Regards,
Andy Shevchenko
On 07/14/2010 04:44 PM, ext Andy Shevchenko wrote:
> 2010/7/14 Michał Nazarewicz<[email protected]>:
>> On Wed, 14 Jul 2010 11:05:31 +0200, Andy Shevchenko
>> <[email protected]> wrote:
>>>
>>> MS Windows mounts removable storage in "Removal optimized mode" by
>>> default. All the writes to the media are synchronous which is achieved
>>> by setting FUA (Force Unit Access) bit in SCSI WRITE(10,12) commands.
>>> This prevents I/O requests aggregation in block layer dramatically
>>> decreasing performance.
>>
>>> diff --git a/drivers/usb/gadget/file_storage.c
>>> b/drivers/usb/gadget/file_storage.c
>>> index b49d86e..45f58d9 100644
>>> --- a/drivers/usb/gadget/file_storage.c
>>> +++ b/drivers/usb/gadget/file_storage.c
>>> @@ -93,6 +93,8 @@
>>> * removable Default false, boolean for removable media
>>> * luns=N Default N = number of filenames, number of
>>> * LUNs to support
>>> + * fua=b[,b...] Default false, booleans for ignore FUA
>>> flag
>>> + * in SCSI WRITE(6,10,12) commands
>>
>> I wonder if it makes sense to make it per-LUN. I would imagine that it's
>> great
>> to ignore FUA if the device has its own power supply in which case after
>> disconnect
>> the data won't be lost. This is a per-device property not really per-LUN.
>> As such
>> I'd make this option global for the gadget.
> Make sense only for removable media with one partition.
> Otherwise. why we have sync option per partition f.e., not per device?
>
by partition do you mean medium? They are different terms.
A storage medium may have 1 or more logical partitions. It is left upto the
Host/user to decide how he wants to partition the medium.
file_storage driver does not deal with partitions. only mediums.
regards,
-roger
On Wed, 14 Jul 2010 15:44:58 +0200, Andy Shevchenko <[email protected]> wrote:
>>> @@ -93,6 +93,8 @@
>>> * removable Default false, boolean for removable media
>>> * luns=N Default N = number of filenames, number of
>>> * LUNs to support
>>> + * fua=b[,b...] Default false, booleans for ignore FUA
>>> flag
>>> + * in SCSI WRITE(6,10,12) commands
>>
>> I wonder if it makes sense to make it per-LUN. I would imagine
>> that it's great to ignore FUA if the device has its own power supply
>> in which case after disconnect the data won't be lost. This is a
>> per-device property not really per-LUN. As such I'd make this option
>> global for the gadget.
> Make sense only for removable media with one partition.
> Otherwise. why we have sync option per partition f.e., not per device?
Ah, OK, I see why this is per LUN. You want to be able not to ignore
FUA if the backing storage is a removable media, right?
>>> + ssize_t rc = count;
>> Not really needed here.
> See below
This still stands. The “rc” is not needed.
>>> + if (sscanf(buf, "%d", &i) != 1)
>>> + return -EINVAL;
>> Why not simple_strtol() directly?
> I did it in the same way like fsg_store_ro() does.
> I have no objections to back to previous solution.
OK. I'd use simpre_strol() myself. Maybe even patched fsg_store_ro().
>>> +
>>> + if (curlun->fua)
>>> + fsg_lun_fsync_sub(curlun);
>> Shouldn't that read something like:
>>
>> + if (!curlun->fua && i)
>> + fsg_lun_fsync_sub(curlun);
>>
>> ie. there is no sense in syncing if FUA was active (in which case all
>> writes were synced already, right?) or if the new value is false (since
>> then user does not won't syncing).
> The idea is to sync data before switching from async mode.
But there can be a case of switching from async to async when syncing
is not necessary. That's why I proposed the &&. With fua = 1 meaning
ignore the flag my proposal would be:
+ if (!i && curlun->fua)
+ fsg_lun_fsync_sub(curlun);
> Actually fua = 1 means ignorance of that flag.
ignore_fua would be better name then I think. This also stands for
module parameter.
--
Best regards, _ _
| Humble Liege of Serenely Enlightened Majesty of o' \,=./ `o
| Computer Science, Michał "mina86" Nazarewicz (o o)
+----[mina86*mina86.com]---[mina86*jabber.org]----ooO--(_)--Ooo--
2010/7/14 Michał Nazarewicz <[email protected]>:
> On Wed, 14 Jul 2010 15:44:58 +0200, Andy Shevchenko
> <[email protected]> wrote:
>>>> + * fua=b[,b...] Default false, booleans for ignore FUA
>>>> flag
>>>> + * in SCSI WRITE(6,10,12) commands
>>>
>>> I wonder if it makes sense to make it per-LUN. I would imagine
>>> that it's great to ignore FUA if the device has its own power supply
>>> in which case after disconnect the data won't be lost. This is a
>>> per-device property not really per-LUN. As such I'd make this option
>>> global for the gadget.
>
>> Make sense only for removable media with one partition.
>> Otherwise. why we have sync option per partition f.e., not per device?
>
> Ah, OK, I see why this is per LUN. You want to be able not to ignore
> FUA if the backing storage is a removable media, right?
In instance, or vise versa.
So, the user could decide if he wants to avoid this flag for one LUN
or for another.
>>>> + if (sscanf(buf, "%d", &i) != 1)
>>>> + return -EINVAL;
>>> Why not simple_strtol() directly?
>> I did it in the same way like fsg_store_ro() does.
>> I have no objections to back to previous solution.
> OK. I'd use simpre_strol() myself. Maybe even patched fsg_store_ro().
Agree, but better to do series of patches then, I guess.
>>>> +
>>>> + if (curlun->fua)
>>>> + fsg_lun_fsync_sub(curlun);
>
>>> Shouldn't that read something like:
>>>
>>> + if (!curlun->fua && i)
>>> + fsg_lun_fsync_sub(curlun);
>>>
>>> ie. there is no sense in syncing if FUA was active (in which case all
>>> writes were synced already, right?) or if the new value is false (since
>>> then user does not won't syncing).
>
>> The idea is to sync data before switching from async mode.
>
> But there can be a case of switching from async to async when syncing
> is not necessary. That's why I proposed the &&. With fua = 1 meaning
> ignore the flag my proposal would be:
>
> + if (!i && curlun->fua)
> + fsg_lun_fsync_sub(curlun);
Makes sense.
>> Actually fua = 1 means ignorance of that flag.
> ignore_fua would be better name then I think. This also stands for
> module parameter.
I already thought about. Rather I agree with you.
--
With Best Regards,
Andy Shevchenko
> 2010/7/14 Michał Nazarewicz <[email protected]>:
>> Ah, OK, I see why this is per LUN. You want to be able not to ignore
>> FUA if the backing storage is a removable media, right?
On Wed, 14 Jul 2010 17:05:07 +0200, Andy Shevchenko <[email protected]> wrote:
> In instance, or vise versa.
I actually see the most sense in disabling FUA on devices with their
own power supply for logical units which backing file is not (on) a
removable device unless the removable device has some kind of lock
(like CD-ROM's door).
Either way, I now see the point of having this option per-LUN.
>>> Actually fua = 1 means ignorance of that flag.
>> ignore_fua would be better name then I think. This also stands for
>> module parameter.
Or even “nofua”. The other solution would be to change the meaning
to the opposite (1 meaning that FUA is not ignored).
--
Best regards, _ _
| Humble Liege of Serenely Enlightened Majesty of o' \,=./ `o
| Computer Science, Michał "mina86" Nazarewicz (o o)
+----[mina86*mina86.com]---[mina86*jabber.org]----ooO--(_)--Ooo--
Bring a strict way to get the 'ro' parameter from the user.
The idea was discussed in LKML [1].
[1] http://lkml.org/lkml/2010/7/13/143
Signed-off-by: Andy Shevchenko <[email protected]>
Cc: Alan Stern <[email protected]>
Cc: David Brownell <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: [email protected]
---
drivers/usb/gadget/storage_common.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/storage_common.c b/drivers/usb/gadget/storage_common.c
index 04c462f..7123929 100644
--- a/drivers/usb/gadget/storage_common.c
+++ b/drivers/usb/gadget/storage_common.c
@@ -723,9 +723,9 @@ static ssize_t fsg_store_ro(struct device *dev, struct device_attribute *attr,
ssize_t rc = count;
struct fsg_lun *curlun = fsg_lun_from_dev(dev);
struct rw_semaphore *filesem = dev_get_drvdata(dev);
- int i;
+ unsigned long ro;
- if (sscanf(buf, "%d", &i) != 1)
+ if (strict_strtoul(buf, 2, &ro))
return -EINVAL;
/* Allow the write-enable status to change only while the backing file
@@ -735,8 +735,8 @@ static ssize_t fsg_store_ro(struct device *dev, struct device_attribute *attr,
LDBG(curlun, "read-only status change prevented\n");
rc = -EBUSY;
} else {
- curlun->ro = !!i;
- curlun->initially_ro = !!i;
+ curlun->ro = ro;
+ curlun->initially_ro = ro;
LDBG(curlun, "read-only status set to %d\n", curlun->ro);
}
up_read(filesem);
--
1.6.3.3
MS Windows mounts removable storage in "Removal optimized mode" by
default. All the writes to the media are synchronous which is achieved
by setting FUA (Force Unit Access) bit in SCSI WRITE(10,12) commands.
This prevents I/O requests aggregation in block layer dramatically
decreasing performance.
This patch brings an option to accept or ignore mentioned bit
a) via specifying module parameter "nofua", or
b) through sysfs entry
/sys/devices/platform/musb_hdrc/gadget/gadget-lun-N/nofua
Patch is based on the work that was done by Denis Karpov for Maemo 5
platform.
Signed-off-by: Andy Shevchenko <[email protected]>
Cc: Denis Karpov <[email protected]>
Cc: Adrian Hunter <[email protected]>
Cc: Alan Stern <[email protected]>
Cc: David Brownell <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: [email protected]
---
drivers/usb/gadget/file_storage.c | 31 ++++++++++++++++++++++++-------
drivers/usb/gadget/storage_common.c | 28 ++++++++++++++++++++++++++++
2 files changed, 52 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/gadget/file_storage.c b/drivers/usb/gadget/file_storage.c
index b49d86e..20bd39e 100644
--- a/drivers/usb/gadget/file_storage.c
+++ b/drivers/usb/gadget/file_storage.c
@@ -93,6 +93,8 @@
* removable Default false, boolean for removable media
* luns=N Default N = number of filenames, number of
* LUNs to support
+ * nofua=b[,b...] Default false, booleans for ignore FUA flag
+ * in SCSI WRITE(10,12) commands
* stall Default determined according to the type of
* USB device controller (usually true),
* boolean to permit the driver to halt
@@ -111,12 +113,12 @@
* PAGE_CACHE_SIZE)
*
* If CONFIG_USB_FILE_STORAGE_TEST is not set, only the "file", "ro",
- * "removable", "luns", "stall", and "cdrom" options are available; default
- * values are used for everything else.
+ * "removable", "luns", "nofua", "stall", and "cdrom" options are available;
+ * default values are used for everything else.
*
* The pathnames of the backing files and the ro settings are available in
- * the attribute files "file" and "ro" in the lun<n> subdirectory of the
- * gadget's sysfs directory. If the "removable" option is set, writing to
+ * the attribute files "file", "nofua", and "ro" in the lun<n> subdirectory of
+ * the gadget's sysfs directory. If the "removable" option is set, writing to
* these files will simulate ejecting/loading the medium (writing an empty
* line means eject) and adjusting a write-enable tab. Changes to the ro
* setting are not allowed when the medium is loaded or if CD-ROM emulation
@@ -301,8 +303,10 @@ MODULE_LICENSE("Dual BSD/GPL");
static struct {
char *file[FSG_MAX_LUNS];
int ro[FSG_MAX_LUNS];
+ int nofua[FSG_MAX_LUNS];
unsigned int num_filenames;
unsigned int num_ros;
+ unsigned int num_nofuas;
unsigned int nluns;
int removable;
@@ -341,6 +345,10 @@ MODULE_PARM_DESC(file, "names of backing files or devices");
module_param_array_named(ro, mod_data.ro, bool, &mod_data.num_ros, S_IRUGO);
MODULE_PARM_DESC(ro, "true to force read-only");
+module_param_array_named(nofua, mod_data.nofua, bool, &mod_data.num_nofuas,
+ S_IRUGO);
+MODULE_PARM_DESC(nofua, "true to ignore SCSI WRITE(10,12) FUA bit");
+
module_param_named(luns, mod_data.nluns, uint, S_IRUGO);
MODULE_PARM_DESC(luns, "number of LUNs");
@@ -1272,7 +1280,8 @@ static int do_write(struct fsg_dev *fsg)
curlun->sense_data = SS_INVALID_FIELD_IN_CDB;
return -EINVAL;
}
- if (fsg->cmnd[1] & 0x08) { // FUA
+ /* FUA */
+ if (!curlun->nofua && (fsg->cmnd[1] & 0x08)) {
spin_lock(&curlun->filp->f_lock);
curlun->filp->f_flags |= O_DSYNC;
spin_unlock(&curlun->filp->f_lock);
@@ -3126,6 +3135,7 @@ static int fsg_main_thread(void *fsg_)
/* The write permissions and store_xxx pointers are set in fsg_bind() */
static DEVICE_ATTR(ro, 0444, fsg_show_ro, NULL);
+static DEVICE_ATTR(nofua, 0644, fsg_show_nofua, NULL);
static DEVICE_ATTR(file, 0444, fsg_show_file, NULL);
@@ -3305,6 +3315,10 @@ static int __init fsg_bind(struct usb_gadget *gadget)
}
}
+ /* Only for removable media? */
+ dev_attr_nofua.attr.mode = 0644;
+ dev_attr_nofua.store = fsg_store_nofua;
+
/* Find out how many LUNs there should be */
i = mod_data.nluns;
if (i == 0)
@@ -3330,6 +3344,7 @@ static int __init fsg_bind(struct usb_gadget *gadget)
curlun->ro = mod_data.cdrom || mod_data.ro[i];
curlun->initially_ro = curlun->ro;
curlun->removable = mod_data.removable;
+ curlun->nofua = mod_data.nofua[i];
curlun->dev.release = lun_release;
curlun->dev.parent = &gadget->dev;
curlun->dev.driver = &fsg_driver.driver;
@@ -3344,6 +3359,8 @@ static int __init fsg_bind(struct usb_gadget *gadget)
if ((rc = device_create_file(&curlun->dev,
&dev_attr_ro)) != 0 ||
(rc = device_create_file(&curlun->dev,
+ &dev_attr_nofua)) != 0 ||
+ (rc = device_create_file(&curlun->dev,
&dev_attr_file)) != 0) {
device_unregister(&curlun->dev);
goto out;
@@ -3478,8 +3495,8 @@ static int __init fsg_bind(struct usb_gadget *gadget)
if (IS_ERR(p))
p = NULL;
}
- LINFO(curlun, "ro=%d, file: %s\n",
- curlun->ro, (p ? p : "(error)"));
+ LINFO(curlun, "ro=%d, nofua=%d, file: %s\n",
+ curlun->ro, curlun->nofua, (p ? p : "(error)"));
}
}
kfree(pathbuf);
diff --git a/drivers/usb/gadget/storage_common.c b/drivers/usb/gadget/storage_common.c
index 7123929..d631b27 100644
--- a/drivers/usb/gadget/storage_common.c
+++ b/drivers/usb/gadget/storage_common.c
@@ -269,6 +269,7 @@ struct fsg_lun {
unsigned int prevent_medium_removal:1;
unsigned int registered:1;
unsigned int info_valid:1;
+ unsigned int nofua:1;
u32 sense_data;
u32 sense_data_info;
@@ -689,6 +690,14 @@ static ssize_t fsg_show_ro(struct device *dev, struct device_attribute *attr,
: curlun->initially_ro);
}
+static ssize_t fsg_show_nofua(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct fsg_lun *curlun = fsg_lun_from_dev(dev);
+
+ return sprintf(buf, "%u\n", curlun->nofua);
+}
+
static ssize_t fsg_show_file(struct device *dev, struct device_attribute *attr,
char *buf)
{
@@ -743,6 +752,25 @@ static ssize_t fsg_store_ro(struct device *dev, struct device_attribute *attr,
return rc;
}
+static ssize_t fsg_store_nofua(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct fsg_lun *curlun = fsg_lun_from_dev(dev);
+ unsigned long nofua;
+
+ if (strict_strtoul(buf, 2, &nofua))
+ return -EINVAL;
+
+ /* Sync data when switching from async mode to sync */
+ if (!nofua && curlun->nofua)
+ fsg_lun_fsync_sub(curlun);
+
+ curlun->nofua = nofua;
+
+ return count;
+}
+
static ssize_t fsg_store_file(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
--
1.6.3.3
Hi,
On Wed, Jul 14, 2010 at 04:55:33PM +0300, Roger Quadros wrote:
> file_storage driver does not deal with partitions. only mediums.
it deals with backing files, no matter if it's a medium, partition or a
file on a file system (using a loop-device).
--
balbi
On 07/16/2010 10:54 AM, ext Felipe Balbi wrote:
> Hi,
>
> On Wed, Jul 14, 2010 at 04:55:33PM +0300, Roger Quadros wrote:
>> file_storage driver does not deal with partitions. only mediums.
>
> it deals with backing files, no matter if it's a medium, partition or a
> file on a file system (using a loop-device).
>
True, you are talking from the device side of the g_files_storage where it
accesses backing files, I was talking from the USB side, where g_file_storage
exposes LUNs or Mediums and not partitions.
br,
-roger
On Thu, Jul 15, 2010 at 12:07:21PM +0300, Andy Shevchenko wrote:
> MS Windows mounts removable storage in "Removal optimized mode" by
> default. All the writes to the media are synchronous which is achieved
> by setting FUA (Force Unit Access) bit in SCSI WRITE(10,12) commands.
> This prevents I/O requests aggregation in block layer dramatically
> decreasing performance.
>
> This patch brings an option to accept or ignore mentioned bit
> a) via specifying module parameter "nofua", or
> b) through sysfs entry
> /sys/devices/platform/musb_hdrc/gadget/gadget-lun-N/nofua
Any new sysfs entry needs a matching documentation entry in the
Documentation/ABI/ directory.
Care to resend these two patches, with that documentation change added
as well?
thanks,
greg k-h
On Thu, Jul 15, 2010 at 12:07:20PM +0300, Andy Shevchenko wrote:
> Bring a strict way to get the 'ro' parameter from the user.
> The idea was discussed in LKML [1].
>
> [1] http://lkml.org/lkml/2010/7/13/143
Care to provide a bit more information as to why this is being changed
instead of just a link to an old email?
thanks,
greg k-h
Bring a strict way to get the 'ro' parameter from the user.
The patch followed by this one adds another boolean parameter. To be consistent
Michał Nazarewicz proposed to use simple_strtol() in both cases (correspondend
discussion in LKML [1]). Due to simple_strtol() doesn't return error in a good
way and we have a boolean parameter the strict_strtoul() is used.
[1] http://lkml.org/lkml/2010/7/14/169
Signed-off-by: Andy Shevchenko <[email protected]>
Cc: Alan Stern <[email protected]>
Cc: David Brownell <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: [email protected]
---
drivers/usb/gadget/storage_common.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/storage_common.c b/drivers/usb/gadget/storage_common.c
index 04c462f..7123929 100644
--- a/drivers/usb/gadget/storage_common.c
+++ b/drivers/usb/gadget/storage_common.c
@@ -723,9 +723,9 @@ static ssize_t fsg_store_ro(struct device *dev, struct device_attribute *attr,
ssize_t rc = count;
struct fsg_lun *curlun = fsg_lun_from_dev(dev);
struct rw_semaphore *filesem = dev_get_drvdata(dev);
- int i;
+ unsigned long ro;
- if (sscanf(buf, "%d", &i) != 1)
+ if (strict_strtoul(buf, 2, &ro))
return -EINVAL;
/* Allow the write-enable status to change only while the backing file
@@ -735,8 +735,8 @@ static ssize_t fsg_store_ro(struct device *dev, struct device_attribute *attr,
LDBG(curlun, "read-only status change prevented\n");
rc = -EBUSY;
} else {
- curlun->ro = !!i;
- curlun->initially_ro = !!i;
+ curlun->ro = ro;
+ curlun->initially_ro = ro;
LDBG(curlun, "read-only status set to %d\n", curlun->ro);
}
up_read(filesem);
--
1.6.3.3
MS Windows mounts removable storage in "Removal optimized mode" by
default. All the writes to the media are synchronous which is achieved
by setting FUA (Force Unit Access) bit in SCSI WRITE(10,12) commands.
This prevents I/O requests aggregation in block layer dramatically
decreasing performance.
This patch brings an option to accept or ignore mentioned bit
a) via specifying module parameter "nofua", or
b) through sysfs entry
/sys/devices/platform/musb_hdrc/gadget/gadget-lun-X/nofua
Patch is based on the work that was done by Denis Karpov for Maemo 5
platform.
Signed-off-by: Andy Shevchenko <[email protected]>
Cc: Denis Karpov <[email protected]>
Cc: Adrian Hunter <[email protected]>
Cc: Alan Stern <[email protected]>
Cc: David Brownell <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: [email protected]
---
.../sysfs-devices-platform-musb_hdrc-gadget | 12 +++++++
drivers/usb/gadget/file_storage.c | 31 +++++++++++++++----
drivers/usb/gadget/storage_common.c | 28 ++++++++++++++++++
3 files changed, 64 insertions(+), 7 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-devices-platform-musb_hdrc-gadget
diff --git a/Documentation/ABI/testing/sysfs-devices-platform-musb_hdrc-gadget b/Documentation/ABI/testing/sysfs-devices-platform-musb_hdrc-gadget
new file mode 100644
index 0000000..192d3f3
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-devices-platform-musb_hdrc-gadget
@@ -0,0 +1,12 @@
+What: /sys/devices/platform/musb_hdrc/gadget/gadget-lun-X/nofua
+Date: July 2010
+Contact: Andy Shevchenko <[email protected]>
+Description:
+ Show or set the reaction on the FUA (Force Unit Access) bit in
+ the SCSI WRITE(10,12) commands when a gadget in USB Mass
+ Storage mode.
+
+ Possible values are:
+ 1 -> ignore FUA flag
+ 0 -> follow on it
+
diff --git a/drivers/usb/gadget/file_storage.c b/drivers/usb/gadget/file_storage.c
index b49d86e..20bd39e 100644
--- a/drivers/usb/gadget/file_storage.c
+++ b/drivers/usb/gadget/file_storage.c
@@ -93,6 +93,8 @@
* removable Default false, boolean for removable media
* luns=N Default N = number of filenames, number of
* LUNs to support
+ * nofua=b[,b...] Default false, booleans for ignore FUA flag
+ * in SCSI WRITE(10,12) commands
* stall Default determined according to the type of
* USB device controller (usually true),
* boolean to permit the driver to halt
@@ -111,12 +113,12 @@
* PAGE_CACHE_SIZE)
*
* If CONFIG_USB_FILE_STORAGE_TEST is not set, only the "file", "ro",
- * "removable", "luns", "stall", and "cdrom" options are available; default
- * values are used for everything else.
+ * "removable", "luns", "nofua", "stall", and "cdrom" options are available;
+ * default values are used for everything else.
*
* The pathnames of the backing files and the ro settings are available in
- * the attribute files "file" and "ro" in the lun<n> subdirectory of the
- * gadget's sysfs directory. If the "removable" option is set, writing to
+ * the attribute files "file", "nofua", and "ro" in the lun<n> subdirectory of
+ * the gadget's sysfs directory. If the "removable" option is set, writing to
* these files will simulate ejecting/loading the medium (writing an empty
* line means eject) and adjusting a write-enable tab. Changes to the ro
* setting are not allowed when the medium is loaded or if CD-ROM emulation
@@ -301,8 +303,10 @@ MODULE_LICENSE("Dual BSD/GPL");
static struct {
char *file[FSG_MAX_LUNS];
int ro[FSG_MAX_LUNS];
+ int nofua[FSG_MAX_LUNS];
unsigned int num_filenames;
unsigned int num_ros;
+ unsigned int num_nofuas;
unsigned int nluns;
int removable;
@@ -341,6 +345,10 @@ MODULE_PARM_DESC(file, "names of backing files or devices");
module_param_array_named(ro, mod_data.ro, bool, &mod_data.num_ros, S_IRUGO);
MODULE_PARM_DESC(ro, "true to force read-only");
+module_param_array_named(nofua, mod_data.nofua, bool, &mod_data.num_nofuas,
+ S_IRUGO);
+MODULE_PARM_DESC(nofua, "true to ignore SCSI WRITE(10,12) FUA bit");
+
module_param_named(luns, mod_data.nluns, uint, S_IRUGO);
MODULE_PARM_DESC(luns, "number of LUNs");
@@ -1272,7 +1280,8 @@ static int do_write(struct fsg_dev *fsg)
curlun->sense_data = SS_INVALID_FIELD_IN_CDB;
return -EINVAL;
}
- if (fsg->cmnd[1] & 0x08) { // FUA
+ /* FUA */
+ if (!curlun->nofua && (fsg->cmnd[1] & 0x08)) {
spin_lock(&curlun->filp->f_lock);
curlun->filp->f_flags |= O_DSYNC;
spin_unlock(&curlun->filp->f_lock);
@@ -3126,6 +3135,7 @@ static int fsg_main_thread(void *fsg_)
/* The write permissions and store_xxx pointers are set in fsg_bind() */
static DEVICE_ATTR(ro, 0444, fsg_show_ro, NULL);
+static DEVICE_ATTR(nofua, 0644, fsg_show_nofua, NULL);
static DEVICE_ATTR(file, 0444, fsg_show_file, NULL);
@@ -3305,6 +3315,10 @@ static int __init fsg_bind(struct usb_gadget *gadget)
}
}
+ /* Only for removable media? */
+ dev_attr_nofua.attr.mode = 0644;
+ dev_attr_nofua.store = fsg_store_nofua;
+
/* Find out how many LUNs there should be */
i = mod_data.nluns;
if (i == 0)
@@ -3330,6 +3344,7 @@ static int __init fsg_bind(struct usb_gadget *gadget)
curlun->ro = mod_data.cdrom || mod_data.ro[i];
curlun->initially_ro = curlun->ro;
curlun->removable = mod_data.removable;
+ curlun->nofua = mod_data.nofua[i];
curlun->dev.release = lun_release;
curlun->dev.parent = &gadget->dev;
curlun->dev.driver = &fsg_driver.driver;
@@ -3344,6 +3359,8 @@ static int __init fsg_bind(struct usb_gadget *gadget)
if ((rc = device_create_file(&curlun->dev,
&dev_attr_ro)) != 0 ||
(rc = device_create_file(&curlun->dev,
+ &dev_attr_nofua)) != 0 ||
+ (rc = device_create_file(&curlun->dev,
&dev_attr_file)) != 0) {
device_unregister(&curlun->dev);
goto out;
@@ -3478,8 +3495,8 @@ static int __init fsg_bind(struct usb_gadget *gadget)
if (IS_ERR(p))
p = NULL;
}
- LINFO(curlun, "ro=%d, file: %s\n",
- curlun->ro, (p ? p : "(error)"));
+ LINFO(curlun, "ro=%d, nofua=%d, file: %s\n",
+ curlun->ro, curlun->nofua, (p ? p : "(error)"));
}
}
kfree(pathbuf);
diff --git a/drivers/usb/gadget/storage_common.c b/drivers/usb/gadget/storage_common.c
index 7123929..d631b27 100644
--- a/drivers/usb/gadget/storage_common.c
+++ b/drivers/usb/gadget/storage_common.c
@@ -269,6 +269,7 @@ struct fsg_lun {
unsigned int prevent_medium_removal:1;
unsigned int registered:1;
unsigned int info_valid:1;
+ unsigned int nofua:1;
u32 sense_data;
u32 sense_data_info;
@@ -689,6 +690,14 @@ static ssize_t fsg_show_ro(struct device *dev, struct device_attribute *attr,
: curlun->initially_ro);
}
+static ssize_t fsg_show_nofua(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct fsg_lun *curlun = fsg_lun_from_dev(dev);
+
+ return sprintf(buf, "%u\n", curlun->nofua);
+}
+
static ssize_t fsg_show_file(struct device *dev, struct device_attribute *attr,
char *buf)
{
@@ -743,6 +752,25 @@ static ssize_t fsg_store_ro(struct device *dev, struct device_attribute *attr,
return rc;
}
+static ssize_t fsg_store_nofua(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct fsg_lun *curlun = fsg_lun_from_dev(dev);
+ unsigned long nofua;
+
+ if (strict_strtoul(buf, 2, &nofua))
+ return -EINVAL;
+
+ /* Sync data when switching from async mode to sync */
+ if (!nofua && curlun->nofua)
+ fsg_lun_fsync_sub(curlun);
+
+ curlun->nofua = nofua;
+
+ return count;
+}
+
static ssize_t fsg_store_file(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
--
1.6.3.3
On Thu, 22 Jul 2010, Andy Shevchenko wrote:
> Bring a strict way to get the 'ro' parameter from the user.
>
> The patch followed by this one adds another boolean parameter. To be consistent
> Michał Nazarewicz proposed to use simple_strtol() in both cases (correspondend
> discussion in LKML [1]). Due to simple_strtol() doesn't return error in a good
> way and we have a boolean parameter the strict_strtoul() is used.
>
> [1] http://lkml.org/lkml/2010/7/14/169
>
> Signed-off-by: Andy Shevchenko <[email protected]>
> Cc: Alan Stern <[email protected]>
> Cc: David Brownell <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: [email protected]
> ---
> drivers/usb/gadget/storage_common.c | 8 ++++----
> 1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/gadget/storage_common.c b/drivers/usb/gadget/storage_common.c
> index 04c462f..7123929 100644
> --- a/drivers/usb/gadget/storage_common.c
> +++ b/drivers/usb/gadget/storage_common.c
> @@ -723,9 +723,9 @@ static ssize_t fsg_store_ro(struct device *dev, struct device_attribute *attr,
> ssize_t rc = count;
> struct fsg_lun *curlun = fsg_lun_from_dev(dev);
> struct rw_semaphore *filesem = dev_get_drvdata(dev);
> - int i;
> + unsigned long ro;
>
> - if (sscanf(buf, "%d", &i) != 1)
> + if (strict_strtoul(buf, 2, &ro))
> return -EINVAL;
>
> /* Allow the write-enable status to change only while the backing file
> @@ -735,8 +735,8 @@ static ssize_t fsg_store_ro(struct device *dev, struct device_attribute *attr,
> LDBG(curlun, "read-only status change prevented\n");
> rc = -EBUSY;
> } else {
> - curlun->ro = !!i;
> - curlun->initially_ro = !!i;
> + curlun->ro = ro;
> + curlun->initially_ro = ro;
> LDBG(curlun, "read-only status set to %d\n", curlun->ro);
> }
> up_read(filesem);
Acked-by: Alan Stern <[email protected]>
Strictly speaking, this changes the behavior when a non-binary value is
specified. For example, if somebody were to write "10" then the old
code would set the value to True and the new code would set the value
to False -- and neither would reject it as they probably should.
That's okay; people who write ambiguous or invalid values deserve what
they get.
Alan Stern
On Thu, 22 Jul 2010, Andy Shevchenko wrote:
> MS Windows mounts removable storage in "Removal optimized mode" by
> default. All the writes to the media are synchronous which is achieved
> by setting FUA (Force Unit Access) bit in SCSI WRITE(10,12) commands.
> This prevents I/O requests aggregation in block layer dramatically
> decreasing performance.
>
> This patch brings an option to accept or ignore mentioned bit
> a) via specifying module parameter "nofua", or
> b) through sysfs entry
> /sys/devices/platform/musb_hdrc/gadget/gadget-lun-X/nofua
>
> Patch is based on the work that was done by Denis Karpov for Maemo 5
> platform.
>
> Signed-off-by: Andy Shevchenko <[email protected]>
> Cc: Denis Karpov <[email protected]>
> Cc: Adrian Hunter <[email protected]>
> Cc: Alan Stern <[email protected]>
> Cc: David Brownell <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: [email protected]
> ---
> .../sysfs-devices-platform-musb_hdrc-gadget | 12 +++++++
> drivers/usb/gadget/file_storage.c | 31 +++++++++++++++----
> drivers/usb/gadget/storage_common.c | 28 ++++++++++++++++++
> 3 files changed, 64 insertions(+), 7 deletions(-)
> create mode 100644 Documentation/ABI/testing/sysfs-devices-platform-musb_hdrc-gadget
>
> diff --git a/Documentation/ABI/testing/sysfs-devices-platform-musb_hdrc-gadget b/Documentation/ABI/testing/sysfs-devices-platform-musb_hdrc-gadget
> new file mode 100644
> index 0000000..192d3f3
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-devices-platform-musb_hdrc-gadget
> @@ -0,0 +1,12 @@
> +What: /sys/devices/platform/musb_hdrc/gadget/gadget-lun-X/nofua
The name of the new file and the path given here shouldn't mention
musb_hdrc. That's what _you_ see because that's the device controller
driver you use, but other people will see something different. As a
simple test, you might try using dummy_hcd instead and see what the
name turns out to be.
Just put "..." instead of "musb_hdrc", or something like that.
> +Date: July 2010
> +Contact: Andy Shevchenko <[email protected]>
> +Description:
> + Show or set the reaction on the FUA (Force Unit Access) bit in
> + the SCSI WRITE(10,12) commands when a gadget in USB Mass
> + Storage mode.
> +
> + Possible values are:
> + 1 -> ignore FUA flag
> + 0 -> follow on it
Change this to "obey the FUA flag".
The rest is okay. Subject to these changes:
Acked-by: Alan Stern <[email protected]>
> On Thu, 22 Jul 2010, Andy Shevchenko wrote:
>> @@ -735,8 +735,8 @@ static ssize_t fsg_store_ro(struct device *dev, struct device_attribute *attr,
>> LDBG(curlun, "read-only status change prevented\n");
>> rc = -EBUSY;
>> } else {
>> - curlun->ro = !!i;
>> - curlun->initially_ro = !!i;
>> + curlun->ro = ro;
>> + curlun->initially_ro = ro;
>> LDBG(curlun, "read-only status set to %d\n", curlun->ro);
>> }
>> up_read(filesem);
On Thu, 22 Jul 2010 16:07:14 +0200, Alan Stern <[email protected]> wrote:
> Strictly speaking, this changes the behavior when a non-binary value is
> specified. For example, if somebody were to write "10" then the old
> code would set the value to True and the new code would set the value
> to False -- and neither would reject it as they probably should.
> That's okay; people who write ambiguous or invalid values deserve what
> they get.
If anyone considers that an issue, it would be solved by:
+ curlun->ro = !!ro;
+ curlun->initially_ro = !!ro;
The "!!" where there for that purpose in the first place.
--
Best regards, _ _
| Humble Liege of Serenely Enlightened Majesty of o' \,=./ `o
| Computer Science, Michał "mina86" Nazarewicz (o o)
+----[mina86*mina86.com]---[mina86*jabber.org]----ooO--(_)--Ooo--
On Thu, Jul 22, 2010 at 5:13 PM, Alan Stern <[email protected]> wrote:
>> +What: /sys/devices/platform/musb_hdrc/gadget/gadget-lun-X/nofua
>
> The name of the new file and the path given here shouldn't mention
> musb_hdrc. That's what _you_ see because that's the device controller
> driver you use, but other people will see something different. As a
> simple test, you might try using dummy_hcd instead and see what the
> name turns out to be.
>
> Just put "..." instead of "musb_hdrc", or something like that.
Thanks for point.
Actually this seems to be part of exist sysfs-devices-platform-_UDC_-gadget.
--
With Best Regards,
Andy Shevchenko
MS Windows mounts removable storage in "Removal optimized mode" by
default. All the writes to the media are synchronous which is achieved
by setting FUA (Force Unit Access) bit in SCSI WRITE(10,12) commands.
This prevents I/O requests aggregation in block layer dramatically
decreasing performance.
This patch brings an option to accept or ignore mentioned bit
a) via specifying module parameter "nofua", or
b) through sysfs entry
/sys/devices/platform/_UDC_/gadget/gadget-lunX/nofua
(_UDC_ is the name of the USB Device Controller driver)
Patch is based on the work that was done by Denis Karpov for Maemo 5
platform.
Signed-off-by: Andy Shevchenko <[email protected]>
Cc: Denis Karpov <[email protected]>
Cc: Adrian Hunter <[email protected]>
Cc: Alan Stern <[email protected]>
Cc: David Brownell <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: [email protected]
---
.../testing/sysfs-devices-platform-_UDC_-gadget | 12 +++++++
drivers/usb/gadget/file_storage.c | 31 +++++++++++++++----
drivers/usb/gadget/storage_common.c | 28 ++++++++++++++++++
3 files changed, 64 insertions(+), 7 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-devices-platform-_UDC_-gadget b/Documentation/ABI/testing/sysfs-devices-platform-_UDC_-gadget
index 3403402..d548eaa 100644
--- a/Documentation/ABI/testing/sysfs-devices-platform-_UDC_-gadget
+++ b/Documentation/ABI/testing/sysfs-devices-platform-_UDC_-gadget
@@ -7,3 +7,15 @@ Description:
0 -> resumed
(_UDC_ is the name of the USB Device Controller driver)
+
+What: /sys/devices/platform/_UDC_/gadget/gadget-lunX/nofua
+Date: July 2010
+Contact: Andy Shevchenko <[email protected]>
+Description:
+ Show or set the reaction on the FUA (Force Unit Access) bit in
+ the SCSI WRITE(10,12) commands when a gadget in USB Mass
+ Storage mode.
+
+ Possible values are:
+ 1 -> ignore the FUA flag
+ 0 -> obey the FUA flag
diff --git a/drivers/usb/gadget/file_storage.c b/drivers/usb/gadget/file_storage.c
index b49d86e..20bd39e 100644
--- a/drivers/usb/gadget/file_storage.c
+++ b/drivers/usb/gadget/file_storage.c
@@ -93,6 +93,8 @@
* removable Default false, boolean for removable media
* luns=N Default N = number of filenames, number of
* LUNs to support
+ * nofua=b[,b...] Default false, booleans for ignore FUA flag
+ * in SCSI WRITE(10,12) commands
* stall Default determined according to the type of
* USB device controller (usually true),
* boolean to permit the driver to halt
@@ -111,12 +113,12 @@
* PAGE_CACHE_SIZE)
*
* If CONFIG_USB_FILE_STORAGE_TEST is not set, only the "file", "ro",
- * "removable", "luns", "stall", and "cdrom" options are available; default
- * values are used for everything else.
+ * "removable", "luns", "nofua", "stall", and "cdrom" options are available;
+ * default values are used for everything else.
*
* The pathnames of the backing files and the ro settings are available in
- * the attribute files "file" and "ro" in the lun<n> subdirectory of the
- * gadget's sysfs directory. If the "removable" option is set, writing to
+ * the attribute files "file", "nofua", and "ro" in the lun<n> subdirectory of
+ * the gadget's sysfs directory. If the "removable" option is set, writing to
* these files will simulate ejecting/loading the medium (writing an empty
* line means eject) and adjusting a write-enable tab. Changes to the ro
* setting are not allowed when the medium is loaded or if CD-ROM emulation
@@ -301,8 +303,10 @@ MODULE_LICENSE("Dual BSD/GPL");
static struct {
char *file[FSG_MAX_LUNS];
int ro[FSG_MAX_LUNS];
+ int nofua[FSG_MAX_LUNS];
unsigned int num_filenames;
unsigned int num_ros;
+ unsigned int num_nofuas;
unsigned int nluns;
int removable;
@@ -341,6 +345,10 @@ MODULE_PARM_DESC(file, "names of backing files or devices");
module_param_array_named(ro, mod_data.ro, bool, &mod_data.num_ros, S_IRUGO);
MODULE_PARM_DESC(ro, "true to force read-only");
+module_param_array_named(nofua, mod_data.nofua, bool, &mod_data.num_nofuas,
+ S_IRUGO);
+MODULE_PARM_DESC(nofua, "true to ignore SCSI WRITE(10,12) FUA bit");
+
module_param_named(luns, mod_data.nluns, uint, S_IRUGO);
MODULE_PARM_DESC(luns, "number of LUNs");
@@ -1272,7 +1280,8 @@ static int do_write(struct fsg_dev *fsg)
curlun->sense_data = SS_INVALID_FIELD_IN_CDB;
return -EINVAL;
}
- if (fsg->cmnd[1] & 0x08) { // FUA
+ /* FUA */
+ if (!curlun->nofua && (fsg->cmnd[1] & 0x08)) {
spin_lock(&curlun->filp->f_lock);
curlun->filp->f_flags |= O_DSYNC;
spin_unlock(&curlun->filp->f_lock);
@@ -3126,6 +3135,7 @@ static int fsg_main_thread(void *fsg_)
/* The write permissions and store_xxx pointers are set in fsg_bind() */
static DEVICE_ATTR(ro, 0444, fsg_show_ro, NULL);
+static DEVICE_ATTR(nofua, 0644, fsg_show_nofua, NULL);
static DEVICE_ATTR(file, 0444, fsg_show_file, NULL);
@@ -3305,6 +3315,10 @@ static int __init fsg_bind(struct usb_gadget *gadget)
}
}
+ /* Only for removable media? */
+ dev_attr_nofua.attr.mode = 0644;
+ dev_attr_nofua.store = fsg_store_nofua;
+
/* Find out how many LUNs there should be */
i = mod_data.nluns;
if (i == 0)
@@ -3330,6 +3344,7 @@ static int __init fsg_bind(struct usb_gadget *gadget)
curlun->ro = mod_data.cdrom || mod_data.ro[i];
curlun->initially_ro = curlun->ro;
curlun->removable = mod_data.removable;
+ curlun->nofua = mod_data.nofua[i];
curlun->dev.release = lun_release;
curlun->dev.parent = &gadget->dev;
curlun->dev.driver = &fsg_driver.driver;
@@ -3344,6 +3359,8 @@ static int __init fsg_bind(struct usb_gadget *gadget)
if ((rc = device_create_file(&curlun->dev,
&dev_attr_ro)) != 0 ||
(rc = device_create_file(&curlun->dev,
+ &dev_attr_nofua)) != 0 ||
+ (rc = device_create_file(&curlun->dev,
&dev_attr_file)) != 0) {
device_unregister(&curlun->dev);
goto out;
@@ -3478,8 +3495,8 @@ static int __init fsg_bind(struct usb_gadget *gadget)
if (IS_ERR(p))
p = NULL;
}
- LINFO(curlun, "ro=%d, file: %s\n",
- curlun->ro, (p ? p : "(error)"));
+ LINFO(curlun, "ro=%d, nofua=%d, file: %s\n",
+ curlun->ro, curlun->nofua, (p ? p : "(error)"));
}
}
kfree(pathbuf);
diff --git a/drivers/usb/gadget/storage_common.c b/drivers/usb/gadget/storage_common.c
index 7123929..d631b27 100644
--- a/drivers/usb/gadget/storage_common.c
+++ b/drivers/usb/gadget/storage_common.c
@@ -269,6 +269,7 @@ struct fsg_lun {
unsigned int prevent_medium_removal:1;
unsigned int registered:1;
unsigned int info_valid:1;
+ unsigned int nofua:1;
u32 sense_data;
u32 sense_data_info;
@@ -689,6 +690,14 @@ static ssize_t fsg_show_ro(struct device *dev, struct device_attribute *attr,
: curlun->initially_ro);
}
+static ssize_t fsg_show_nofua(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct fsg_lun *curlun = fsg_lun_from_dev(dev);
+
+ return sprintf(buf, "%u\n", curlun->nofua);
+}
+
static ssize_t fsg_show_file(struct device *dev, struct device_attribute *attr,
char *buf)
{
@@ -743,6 +752,25 @@ static ssize_t fsg_store_ro(struct device *dev, struct device_attribute *attr,
return rc;
}
+static ssize_t fsg_store_nofua(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct fsg_lun *curlun = fsg_lun_from_dev(dev);
+ unsigned long nofua;
+
+ if (strict_strtoul(buf, 2, &nofua))
+ return -EINVAL;
+
+ /* Sync data when switching from async mode to sync */
+ if (!nofua && curlun->nofua)
+ fsg_lun_fsync_sub(curlun);
+
+ curlun->nofua = nofua;
+
+ return count;
+}
+
static ssize_t fsg_store_file(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
--
1.6.3.3