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

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

 



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.

> 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

--
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