Re: usb-storage: fsync() take too much time when handing ALLOW_MEDIUM_REMOVAL

[Yuping Luo <lypingsh@xxxxxxxxx>]

On Sat, Oct 29, 2011 at 5:48 AM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
> On Fri, 28 Oct 2011, Felipe Balbi wrote:
>
>> On Fri, Oct 28, 2011 at 03:22:16PM -0400, Alan Stern wrote:
>> > On Fri, 28 Oct 2011, Alan Stern wrote:
>> >
>> > > The important questions are:
>> > >
>> > >   When are the right times to flush the page cache?
>> > >
>> > >   Why should the MSC gadget have to worry about this in the
>> > >   first place?  That is, why doesn't the rest of the kernel
>> > >   take care of this automatically?
>> >
>> > I just went back and looked at the SCSI spec.  The description of START
>> > STOP UNIT says this:
>> >
>> >     Targets that contain cache memory shall implicitly perform a
>> >     SYNCHRONIZE CACHE command for the entire medium prior to
>> >     executing the STOP UNIT command.
>> >
>> > That's pretty clear.  Currently the do_start_stop() routine just calls
>> > fsg_lun_close(); it looks like we should put an fsg_lun_fsync_sub()
>> > call there too if the backing storage is not a regular file (and maybe
>> > even if it _is_ a regular file, if we want to obey the spec strictly).
>> >
>> > Then there won't be any need for fsg_lun_fsync_sub() in
>> > do_prevent_allow().
>>
>> Makes sense to me. Care to send a patch to be queued for this -rc cycle?
>
> If Yuping or Barry would like to do it, that's fine with me.  Otherwise
> I can send a patch.
>
> Alan Stern
>
>
just revert my previous change, and add following, will test with
windows os and linux.

diff --git a/drivers/usb/gadget/f_mass_storage.c
b/drivers/usb/gadget/f_mass_storage.c
index 52583a2..2f79416 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -1497,8 +1497,6 @@ static int do_prevent_allow(struct fsg_common *common)
                return -EINVAL;
        }

-       if (curlun->prevent_medium_removal && !prevent)
-               fsg_lun_fsync_sub(curlun);
        curlun->prevent_medium_removal = prevent;
        return 0;
 }
diff --git a/drivers/usb/gadget/file_storage.c
b/drivers/usb/gadget/file_storage.c
index 3ac4f51..9a7f7f5 100644
--- a/drivers/usb/gadget/file_storage.c
+++ b/drivers/usb/gadget/file_storage.c
@@ -1880,8 +1880,6 @@ static int do_prevent_allow(struct fsg_dev *fsg)
                return -EINVAL;
        }

-       if (curlun->prevent_medium_removal && !prevent)
-               fsg_lun_fsync_sub(curlun);
        curlun->prevent_medium_removal = prevent;
        return 0;
 }
diff --git a/drivers/usb/gadget/storage_common.c
b/drivers/usb/gadget/storage_common.c
index c7f291a..2cef9de 100644
--- a/drivers/usb/gadget/storage_common.c
+++ b/drivers/usb/gadget/storage_common.c
@@ -765,6 +765,7 @@ static void fsg_lun_close(struct fsg_lun *curlun)
 {
        if (curlun->filp) {
                LDBG(curlun, "close backing file\n");
+               fsg_lun_fsync_sub(curlun);
                fput(curlun->filp);
                curlun->filp = NULL;
        }

Thanks
Yuping Luo
--
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