2011/10/26 Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>:
> On Wed, 26 Oct 2011, Barry Song wrote:
>
>> 2011/10/26 Yuping Luo <lypingsh@xxxxxxxxx>
>> > +void fsg_lun_fsync_deferred_func(struct work_struct *work)
>> > +{
>> > + struct fsg_lun *curlun =
>> > + container_of(work, struct fsg_lun, fsync_deferred_work);
>> > +
>> > + fsg_lun_fsync_sub(curlun);
>> > +}
>> > +
>> > static int do_prevent_allow(struct fsg_common *common)
>> > {
>> > struct fsg_lun *curlun = common->curlun;
>> > @@ -1510,7 +1518,7 @@ static int do_prevent_allow(struct fsg_common *common)
>> > }
>> >
>> > if (curlun->prevent_medium_removal && !prevent)
>> > - fsg_lun_fsync_sub(curlun);
>> > + schedule_work(&curlun->fsync_deferred_work);
>>
>> Alan, i can't find enough information from usb mass storage spec about this.
>
> That's because PREVENT-ALLOW MEDIUM REMOVAL is a SCSI command, not a
> USB command. You'd need to look at the SCSI spec.
>
>> does ALLOW_MEDIUM_REMOVAL need to sync fs?
>
> Not really. I added the sync in there just to be safe. But you need
> to understand that one way or another, the sync has to be done before
> the storage medium is physically removed. Otherwise you'll get data
> corruption.
>
> Any sensible host OS would issue a SYNCHRONIZE CACHE command before the
> ALLOW MEDIUM REMOVAL. But Windows isn't always sensible.
>
>> if no, the
>> fsg_lun_fsync_sub(curlun) actually is needless at all. let pdflush
>> thread write-back page cache on demand is the right way.
>
> It depends on what you're using for the backing storage. If the
> backing storage is a regular file, then yes, write-back is okay.
>
> But if the backing storage is a block device (especially a removable
> block device like a flash memory card), then the act of closing the
> backing file ought to flush the page cache. (I'm not sure if this
> actually happens -- I haven't checked -- but I think it does.) When
> this happens, you'll have to wait for the flush to finish anyway.
suppose we use a file not disk partition as back device, umount or
eject should be the one requiring sync fs but not the ALLOW MEDIUM
REMOVAL. this command only unlock the device.
i would think we should handle the fsync in umount/eject, that is what
fs will do. so fsg driver doesn't need it.
if users don't umount/eject before pulling cards and don't sync
manually as well, it is normal that their data will be corrupted.
suppose users use a SD card partition as back file, will we move the
fsync to STOP command instead of "ALLOW MEDIUM REMOVAL"? are you sure
STOP command is the one following "ALLOW MEDIUM REMOVAL", if no, a
fsync in "ALLOW MEDIUM REMOVAL" is still wrong in case we write the
storage after "ALLOW MEDIUM REMOVAL".
anyway, lauching another thread to do fsync is a wrong way to handle this issue.
>
>> otherwise, our patch is wrong. if fsync must be done in the mass
>> storage spec, our patch changes the sync thing to async and will
>> destory some other part.
>
> Perhaps you can make the call to fsg_lun_fsync_sub() conditional, based
> on whether the backing storage is a regular file.
>
> Alan Stern
-barry
--
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
