----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/130084/#review103186 -----------------------------------------------------------
src/ioslaves/file/file_unix.cpp (line 112) <https://git.reviewboard.kde.org/r/130084/#comment68663> typo src/ioslaves/file/file_unix.cpp (line 138) <https://git.reviewboard.kde.org/r/130084/#comment68664> why are you testing hotpluggable? what's the exact definition in this context? technically, any sata drive is hotpluggable ... src/ioslaves/file/file_unix.cpp (line 324) <https://git.reviewboard.kde.org/r/130084/#comment68665> i'd use some parens here to make it more readable. - Oswald Buddenhagen On April 17, 2017, 1:27 p.m., KJ Tsanaktsidis wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/130084/ > ----------------------------------------------------------- > > (Updated April 17, 2017, 1:27 p.m.) > > > Review request for KDE Frameworks, Oswald Buddenhagen and Thiago Macieira. > > > Repository: kio > > > Description > ------- > > When copying a large-ish file (~1-2GB) from very fast storage to very slow > storage (e.g. an NVME SSD to a cheap USB flash drive) on a machine with lots > of RAM, Dolphin displays a progress bar which finishes in a fraction of a > second (i.e. as fast as it takes to read the source file into the Linux page > cache). Unmounting the drive then of course takes a long time, with only an > indeterminate spinner. > > This patch adds an option to force fsync during copy jobs, so that the copy > progress bar measures how long it will take to actually copy the file to the > destination. > > I've added two flags - Fsync and FsyncCrossFilesystem - to the JobEnum flag. > The former will cause all copy operations to fsync during the copy loop, > whilst the latter will only fsync copies that are across different > filesystems. > > If this patch gets OK'd, I have another patch which adds support for this > into the appropriate places in Dolphin. I would think that at least > FsyncCrossFilesystem should be the default, but Fsync always might be a > little heavy handed. At the least fsync'ing cross-filesystem copies ensures > that the unmount won't take forever. > > > Diffs > ----- > > src/ioslaves/file/CMakeLists.txt b9132ced9d4a08b2cf9f9bbbaa3bd43f026c6469 > src/ioslaves/file/ConfigureChecks.cmake > 5a83d1b9fbe90c851c774e3b467468d93b5a2bd4 > src/ioslaves/file/config-kioslave-file.h.cmake > 372f79d01ad4597aae0b2ae62627648fe7680b64 > src/ioslaves/file/file_unix.cpp 3c1b9927e3dd2d0134f77caec6e6b24a0356d26f > > Diff: https://git.reviewboard.kde.org/r/130084/diff/ > > > Testing > ------- > > Tested the patch with KDE/Dolphin on Arch Linux, which is version 5.32.0. The > diff applies cleanly to master so I assume there shouldn't be any issues > there, but I've not actually checked that. As advertised, copying a file to > USB flash storage now displays an accurate progress bar. > > I experimented with how often fsync should be called on my hardware, and I > found calling it every ~1M copied caused no decrease in copy performance > whilst still providing accurate progress info. That is the setting I've gone > with in this patch. I'm open to suggestions on how this could be tuned better > though. > > > Thanks, > > KJ Tsanaktsidis > >
