/qubes-issues

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Copying files taking insanely long between VMs and/or USB devices #1257

Closed
bnvk opened this Issue Oct 1, 2015 · 5 comments

Comments

Assignees
No one assigned
Labels
Projects
None yet
Milestone
  Release 3.0 updates
2 participants
@bnvk @marmarek
@bnvk

bnvk commented Oct 1, 2015

I'm noticing that copying files is taking sometimes insanely long periods. Like 30+ mins to copy 7 MB from my usbdata VM to a USB device. The status bar says it's copying at 1.7kB / sec which seems quite odd for a plugged in device. This seems to happens more when the data is a large number of small files. In this case, the 7 MB is made up of 2000+ files.

Any thoughts on why this is happening @marmarek ? Is this a bug or something I can offer more info about / help to debug?
@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Oct 1, 2015

Member

On Thu, Oct 01, 2015 at 03:44:35AM -0700, Brennan Novak wrote:

I'm noticing that copying files is taking sometimes insanely long periods. Like 30+ mins to copy 7 MB from my usbdata VM to a USB device. The status bar says it's copying at 1.7kB / sec which seems quite odd for a plugged in device. This seems to happens more when the data is a large number of small files. In this case, the 7 MB is made up of 2000+ files.

Any thoughts on why this is happening @marmarek ? Is this a bug or something I can offer more info about / help to debug?

Try first copy the files off the USB device to some directory in the
same VM. I guess it's more about USB performance, not the file copy
mechanism. Maybe this particular USB stick is dying?

Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
Member

marmarek commented Oct 1, 2015

On Thu, Oct 01, 2015 at 03:44:35AM -0700, Brennan Novak wrote:

I'm noticing that copying files is taking sometimes insanely long periods. Like 30+ mins to copy 7 MB from my usbdata VM to a USB device. The status bar says it's copying at 1.7kB / sec which seems quite odd for a plugged in device. This seems to happens more when the data is a large number of small files. In this case, the 7 MB is made up of 2000+ files.

Any thoughts on why this is happening @marmarek ? Is this a bug or something I can offer more info about / help to debug?

Try first copy the files off the USB device to some directory in the
same VM. I guess it's more about USB performance, not the file copy
mechanism. Maybe this particular USB stick is dying?

Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
@bnvk

This comment has been minimized.

Show comment
Hide comment
@bnvk

bnvk Oct 2, 2015

Sorry. I miscommunicated. The case above, the data was actually
copying files to the USB device not from, so data was at AppVM
usbdata and moving to ---> USB Media.

I've also noticed significantly slow transfer times (not as bad
as 1.7kB, but still quite) when copying data between two AppVMs
when there are lots of small files which are not quite large in
size!

bnvk commented Oct 2, 2015

Sorry. I miscommunicated. The case above, the data was actually
copying files to the USB device not from, so data was at AppVM
usbdata and moving to ---> USB Media.

I've also noticed significantly slow transfer times (not as bad
as 1.7kB, but still quite) when copying data between two AppVMs
when there are lots of small files which are not quite large in
size!

@marmarek marmarek added bug C: core C: templates P: minor labels Oct 8, 2015

@marmarek marmarek added this to the Release 3.0 updates milestone Oct 8, 2015

@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Oct 8, 2015

Member

Receiving VM calls fdatasync for each file, this takes some time...
Member

marmarek commented Oct 8, 2015

Receiving VM calls fdatasync for each file, this takes some time...
@bnvk

This comment has been minimized.

Show comment
Hide comment
@bnvk

bnvk Oct 8, 2015

Interesting. Yah, it seemed to be highly correlate when copying
lots of small files! I wonder if we'd get any noticeable
performance increase by doing something like zipping or rarring
the files before peforming the fdatasync operation if the number
of files being moved is over a certain threshold. What do you
think @marmarek?

bnvk commented Oct 8, 2015

Interesting. Yah, it seemed to be highly correlate when copying
lots of small files! I wonder if we'd get any noticeable
performance increase by doing something like zipping or rarring
the files before peforming the fdatasync operation if the number
of files being moved is over a certain threshold. What do you
think @marmarek?
@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Oct 8, 2015

Member

I'm considering removing that line at all. Just need to evaluate if
there would be no downsides.

Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
Member

marmarek commented Oct 8, 2015

I'm considering removing that line at all. Just need to evaluate if
there would be no downsides.

Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

@marmarek marmarek closed this in marmarek/qubes-linux-utils@c1d42f1 Oct 11, 2015

marmarek added a commit to marmarek/qubes-linux-utils that referenced this issue Oct 11, 2015

@marmarek
qfile-unpacker: do not call fdatasync() at each file (#1257) 
POSIX  requires  that  a  read(2)  which  can be proved to occur after a
write() has returned returns the new data.
We want here only that other processes in the same VM will see the
file either fully written, or not see it at all. So ensuring that
linkat(2) is called after write is completed should be enough.

Fixes QubesOS/qubes-issues#1257

(cherry picked from commit c1d42f1)
Verified
This commit was signed with a verified signature.
@marmarek marmarek Marek Marczykowski-Górecki
GPG key ID: 063938BA42CFA724 Learn about signing commits
eb11cf6

rustybird added a commit to rustybird/qubes-linux-utils that referenced this issue Nov 18, 2015

@rustybird
qfile-unpacker: syncfs() to avoid qvm-move-to-vm data loss 
Commit QubesOS@c1d42f1 --
"qfile-unpacker: do not call fdatasync() at each file" fixing
QubesOS/qubes-issues#1257 -- increased the chance of data loss with
qvm-move-to-vm: Say it nominally succeeds, and *deletes* the files from
the source VM. Soon after, the destination VM or the system could crash,
or an external drive hosting ~/QubesIncoming/srcVM could get unplugged
by accident, all before the data had really been persisted to disk.

But reverting the commit (ignoring the performance issue) wouldn't
completely solve this:

  "Calling fsync() does not necessarily ensure that the entry in the
   directory containing the file has also reached disk. For that an
   explicit fsync() on a file descriptor for the directory is also
   needed."  - fsync(2)

It gets even worse for "slow symlinks" (whose target is too long to be
stored directly in the inode metadata), apparently they can't be synced
at all individually.

So instead, just call syncfs() once after everything has been unpacked:

  + Should prevent all data loss (if fs and disk are well behaved)
  + Allows caching and reordering -> no slowdown with many small files
  - Blocks until any unrelated writes on the filesystem finish :\
Verified
This commit was signed with a verified signature.
@rustybird rustybird Rusty Bird
GPG key ID: 469D78F47AAF2ADF Learn about signing commits
0722fb4

rustybird added a commit to rustybird/qubes-linux-utils that referenced this issue Nov 18, 2015

@rustybird
qfile-unpacker: syncfs() to avoid qvm-move-to-vm data loss 
Commit QubesOS@c1d42f1 --
"qfile-unpacker: do not call fdatasync() at each file" fixing
QubesOS/qubes-issues#1257 -- increased the chance of data loss with
qvm-move-to-vm: Say it nominally succeeds, and *deletes* the files from
the source VM. Soon after, the destination VM or the system could crash,
or an external drive hosting ~/QubesIncoming/srcVM could get unplugged
by accident, all before the data had really been persisted to disk.

But reverting the commit (ignoring the performance issue) wouldn't
completely solve this:

  "Calling fsync() does not necessarily ensure that the entry in the
   directory containing the file has also reached disk. For that an
   explicit fsync() on a file descriptor for the directory is also
   needed."  - fsync(2)

It gets even worse for "slow symlinks" (whose target is too long to be
stored directly in the inode metadata), apparently they can't be synced
at all individually.

So instead, just call syncfs() once after everything has been unpacked:

  + Should prevent all data loss (if fs and disk are well behaved)
  + Allows caching and reordering -> no slowdown with many small files
  - Blocks until any unrelated writes on the filesystem finish :\
Verified
This commit was signed with a verified signature.
@rustybird rustybird Rusty Bird
GPG key ID: 469D78F47AAF2ADF Learn about signing commits
b4ab787

rustybird added a commit to rustybird/qubes-linux-utils that referenced this issue Nov 18, 2015

@rustybird
qfile-unpacker: syncfs() to avoid qvm-move-to-vm data loss 
Commit QubesOS@c1d42f1 --
"qfile-unpacker: do not call fdatasync() at each file" fixing
QubesOS/qubes-issues#1257 -- increased the chance of data loss with
qvm-move-to-vm: Say it nominally succeeds, and *deletes* the files from
the source VM. Soon after, the destination VM or the system could crash,
or an external drive hosting ~/QubesIncoming/srcVM could get unplugged
by accident, all before the data had really been persisted to disk.

But reverting the commit (ignoring the performance issue) wouldn't
completely solve this:

  "Calling fsync() does not necessarily ensure that the entry in the
   directory containing the file has also reached disk. For that an
   explicit fsync() on a file descriptor for the directory is also
   needed."  - fsync(2)

It gets even worse for "slow symlinks" (whose target is too long to be
stored directly in the inode metadata), apparently they can't be synced
at all individually.

So instead, just call syncfs() once after everything has been unpacked:

  + Should prevent all data loss (if fs and disk are well behaved)
  + Allows caching and reordering -> no slowdown with many small files
  - Blocks until any unrelated writes on the filesystem finish :\
Verified
This commit was signed with a verified signature.
@rustybird rustybird Rusty Bird
GPG key ID: 469D78F47AAF2ADF Learn about signing commits
4f59b3d

marmarek added a commit to QubesOS/qubes-linux-utils that referenced this issue Dec 11, 2015

@rustybird @marmarek
qfile-unpacker: syncfs() to avoid qvm-move-to-vm data loss 
Commit c1d42f1 --
"qfile-unpacker: do not call fdatasync() at each file" fixing
QubesOS/qubes-issues#1257 -- increased the chance of data loss with
qvm-move-to-vm: Say it nominally succeeds, and *deletes* the files from
the source VM. Soon after, the destination VM or the system could crash,
or an external drive hosting ~/QubesIncoming/srcVM could get unplugged
by accident, all before the data had really been persisted to disk.

But reverting the commit (ignoring the performance issue) wouldn't
completely solve this:

  "Calling fsync() does not necessarily ensure that the entry in the
   directory containing the file has also reached disk. For that an
   explicit fsync() on a file descriptor for the directory is also
   needed."  - fsync(2)

It gets even worse for "slow symlinks" (whose target is too long to be
stored directly in the inode metadata), apparently they can't be synced
at all individually.

So instead, just call syncfs() once after everything has been unpacked:

  + Should prevent all data loss (if fs and disk are well behaved)
  + Allows caching and reordering -> no slowdown with many small files
  - Blocks until any unrelated writes on the filesystem finish :\

(cherry picked from commit 4f59b3d)
Verified
This commit was signed with a verified signature.
@marmarek marmarek Marek Marczykowski-Górecki
GPG key ID: 063938BA42CFA724 Learn about signing commits
f3ffcfe
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment