[Devel] [PATCH rh7 v2 2/3] ve/fs/sync: per containter sync and syncfs

[Vladimir Davydov]

On Tue, Feb 02, 2016 at 06:51:55PM +0300, Andrey Ryabinin wrote:
...
> @@ -1427,17 +1440,8 @@ static void wait_sb_inodes(struct super_block *sb)
>  	iput(old_inode);
>  }
>  
> -/**
> - * writeback_inodes_sb_nr -	writeback dirty inodes from given super_block
> - * @sb: the superblock
> - * @nr: the number of pages to write
> - * @reason: reason why some writeback work initiated
> - *
> - * Start writeback on some inodes on this super_block. No guarantees are made
> - * on how many (if any) will be written, and this function does not wait
> - * for IO completion of submitted IO.
> - */
> -void writeback_inodes_sb_nr(struct super_block *sb,
> +static void writeback_inodes_sb_ub_nr(struct super_block *sb,
> +			    struct user_beancounter *ub,
>  			    unsigned long nr,
>  			    enum wb_reason reason)
>  {
> @@ -1449,6 +1453,7 @@ void writeback_inodes_sb_nr(struct super_block *sb,
>  		.done			= &done,
>  		.nr_pages		= nr,
>  		.reason			= reason,
> +		.ub			= ub,
>  	};
>  
>  	if (sb->s_bdi == &noop_backing_dev_info)
> @@ -1457,8 +1462,32 @@ void writeback_inodes_sb_nr(struct super_block *sb,
>  	bdi_queue_work(sb->s_bdi, &work);
>  	wait_for_completion(&done);
>  }
> +
> +/**
> + * writeback_inodes_sb_nr -	writeback dirty inodes from given super_block
> + * @sb: the superblock
> + * @nr: the number of pages to write
> + * @reason: reason why some writeback work initiated
> + *
> + * Start writeback on some inodes on this super_block. No guarantees are made
> + * on how many (if any) will be written, and this function does not wait
> + * for IO completion of submitted IO.
> + */

Can we please not move the comment around?

> +void writeback_inodes_sb_nr(struct super_block *sb,
> +			    unsigned long nr,
> +			    enum wb_reason reason)
> +{
> +
> +	writeback_inodes_sb_ub_nr(sb, NULL, nr, reason);
> +}
>  EXPORT_SYMBOL(writeback_inodes_sb_nr);
>  
> +void writeback_inodes_sb_ub(struct super_block *sb, struct user_beancounter *ub,
> +			enum wb_reason reason)
> +{
> +	return writeback_inodes_sb_ub_nr(sb, ub, get_nr_dirty_pages(), reason);
> +}
> +
>  /**
>   * writeback_inodes_sb	-	writeback dirty inodes from given super_block
>   * @sb: the superblock
> @@ -1513,14 +1542,7 @@ int try_to_writeback_inodes_sb(struct super_block *sb, enum wb_reason reason)
>  }
>  EXPORT_SYMBOL(try_to_writeback_inodes_sb);
>  
> -/**
> - * sync_inodes_sb	-	sync sb inode pages
> - * @sb: the superblock
> - *
> - * This function writes and waits on any dirty inode belonging to this
> - * super_block.
> - */
> -void sync_inodes_sb(struct super_block *sb)
> +void sync_inodes_sb_ub(struct super_block *sb, struct user_beancounter *ub)
>  {
>  	DECLARE_COMPLETION_ONSTACK(done);
>  	struct wb_writeback_work work = {
> @@ -1531,6 +1553,7 @@ void sync_inodes_sb(struct super_block *sb)
>  		.done		= &done,
>  		.reason		= WB_REASON_SYNC,
>  		.for_sync	= 1,
> +		.ub		= ub,
>  	};
>  
>  	/* Nothing to do? */
> @@ -1541,7 +1564,19 @@ void sync_inodes_sb(struct super_block *sb)
>  	bdi_queue_work(sb->s_bdi, &work);
>  	wait_for_completion(&done);
>  
> -	wait_sb_inodes(sb);
> +	wait_sb_inodes(sb, ub);
> +}
> +
> +/**
> + * sync_inodes_sb	-	sync sb inode pages
> + * @sb: the superblock
> + *
> + * This function writes and waits on any dirty inode belonging to this
> + * super_block.
> + */

Again, let's keep our changeset as small as possible and avoid moving
comments.

> +void sync_inodes_sb(struct super_block *sb)
> +{
> +	sync_inodes_sb_ub(sb, NULL);
>  }
>  EXPORT_SYMBOL(sync_inodes_sb);
>  
...
> @@ -225,7 +231,7 @@ int ve_fsync_behavior(void)
>  SYSCALL_DEFINE0(sync)
>  {
>  	struct ve_struct *ve = get_exec_env();
> -	struct user_beancounter *ub;
> +	struct user_beancounter *ub, *sync_ub = NULL;
>  	int nowait = 0, wait = 1;
>  
>  	ub = get_exec_ub();
> @@ -246,15 +252,18 @@ SYSCALL_DEFINE0(sync)
>  		fsb = __ve_fsync_behavior(ve);
>  		if (fsb == FSYNC_NEVER)
>  			goto skip;
> +
> +		if (fsb == FSYNC_FILTERED)
> +			sync_ub = get_io_ub();
>  	}
>  
> -	wakeup_flusher_threads(0, WB_REASON_SYNC);
> -	iterate_supers(sync_inodes_one_sb, NULL);
> -	iterate_supers(sync_fs_one_sb, &nowait);
> -	iterate_supers(sync_fs_one_sb, &wait);
> +	wakeup_flusher_threads_ub(0, ub, WB_REASON_SYNC);

s/ub,/sync_ub,

?

> +	iterate_supers(sync_inodes_one_sb, sync_ub);
> +	sync_filesystems(sync_ub, nowait);
> +	sync_filesystems(sync_ub, wait);
>  	iterate_bdevs(fdatawrite_one_bdev, NULL);
>  	iterate_bdevs(fdatawait_one_bdev, NULL);

If sync_ub != NULL, you sync inodes and bdevs twice, here and in
sync_filesystems_ve. Better move all the iterate_supers/bdevs from here
to sync_filesystems under !ub branch?