> > 在 2018年1月15日，下午6:19，Miklos Szeredi <miklos@xxxxxxxxxx> 写道： > > 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. > Hi Miklos, Amir Thanks for your comments for the patch. If the speed of ovl inode destruction is not serious problem, I would like to implement like below, what do you think? In the process of ovl inode destruction: (I will introduce evict operation and do below oprations there) 1. start syncing inode. 2. add current inode to sb->s_inodes_wb list 3. take sb->s_sync_lock and wait until all inode in the wait list finishing syncing. #Of course, we can only wait for specific inode, but is that really helping much? Maybe in the worst case, inode destruction will take time for the first inode, but I think it is not common for all inodes. The purpose of adding inode to sb->s_inodes_wb list is for data consistency of sync result, because I’m afraid that inode destruction will let dirty inode escaping from sync waiting list. Thanks, Chengguang.-- 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
