On Mon, Jan 15, 2018 at 11:10 AM, Amir Goldstein <amir73il@xxxxxxxxx> wrote: > On Mon, Jan 15, 2018 at 11:29 AM, Miklos Szeredi <miklos@xxxxxxxxxx> wrote: >> On Mon, Jan 15, 2018 at 8:33 AM, Chengguang Xu <cgxu519@xxxxxxxxxx> wrote: >>> Currently syncfs(2) call on overlayfs just simply call sync_filesystem() >>> on upper_sb to synchronize whole dirty inodes in upper filesystem >>> regardless of the overlay ownership of the inode. In the use case of >>> container, when multiple containers using the same underlying upper >>> filesystem, it has some shortcomings as below. >>> >>> (1) Performance >>> Synchronization is probably heavy because it actually syncs unnecessary >>> inodes for target overlayfs. >>> >>> (2) Interference >>> Unplanned synchronization will probably impact IO performance of >>> irrelative container processes on the other overlayfs. >>> >>> This patch iterates upper inode list in overlayfs to only sync target >>> dirty inodes and wait for completion. By doing this, It is able to reduce >>> cost of synchronization and will not seriously impact IO performance of >>> irrelative processes. In special case, when having very few dirty inodes >>> and a lot of clean upper inodes in overlayfs, then iteration may waste >>> time so that synchronization is slower than before, but we think the >>> potential for performance improvement far surpass the potential for >>> performance regression in most cases. >>> >>> Signed-off-by: Chengguang Xu <cgxu519@xxxxxxxxxx> >>> --- >>> Changes since v3: >>> - Introduce upper_inode list to organize ovl indoe which has upper inode. >> >> Where is the inode removed from the list? Should be done in >> ovl_destroy_inode(). >> >> Problem is: we can drop the overlay inode even when the underlying >> inode is dirty, so we need to sync the upper inode before letting it >> go in ovl_destroy_inode(). >> > > Isn't that a bit counter to the lazy inode writeback approach? Not really. The dcache still keeps inodes and cache in memory, until there's memory pressure to get rid of the dentry itself. So this is about syncing the upper inode when the overlay dentry is flushed out of the dcache, which should not interfere with lazy writeback in general. > Won't this cause a change of behavior, e.g. by forcing data sync before > returning from close of file after a large write? No, file keeps a ref on dentry, which keeps a ref on inode + underlying dentries. Thanks, Miklos -- To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
