lkml.org 
[lkml]   [2010]   [Jul]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCHv2] usb: gadget: storage: optional SCSI WRITE FUA bit
On Wed, 14 Jul 2010 11:05:31 +0200, Andy Shevchenko <andy.shevchenko@gmail.com> 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--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2010-07-14 14:39    [W:0.150 / U:1.716 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site