Re: [PATCHv2] usb: gadget: storage: optional SCSI WRITE FUA bit

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 14 Jul 2010 15:44:58 +0200, Andy Shevchenko <andy.shevchenko@xxxxxxxxx> 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--
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux