[Cluster-devel] [PATCH] gfs2: Clean up gfs2_file_write_iter and fix O_SYNC write handling

Christoph Hellwig hch at lst.de
Thu Feb 6 16:34:11 UTC 2020


>  	if (iocb->ki_flags & IOCB_DIRECT) {
>  		struct address_space *mapping = file->f_mapping;
> +		ssize_t buffered, ret2;
>  
> +		ret = gfs2_file_direct_write(iocb, from);
> +		if (ret < 0 || !iov_iter_count(from))
>  			goto out_unlock;
>  
>  		current->backing_dev_info = inode_to_bdi(inode);
> +		buffered = iomap_file_buffered_write(iocb, from, &gfs2_iomap_ops);
>  		current->backing_dev_info = NULL;
> -		if (unlikely(ret < 0))
> +		if (unlikely(buffered <= 0))
>  			goto out_unlock;
>  
>  		/*
>  		 * We need to ensure that the page cache pages are written to
>  		 * disk and invalidated to preserve the expected O_DIRECT
> +		 * semantics.  If the writeback or invalidate fails, only report
> +		 * the direct I/O range as we don't know if the buffered pages
> +		 * made it to disk.
>  		 */
> +		iocb->ki_pos += buffered;
> +		iocb->ki_flags |= IOCB_DSYNC;

I think I'd rather add IOCB_DSYNC before calling
iomap_file_buffered_write, just in case we ever do optimizations for
synchronous I/O there.

> +		ret2 = generic_write_sync(iocb, buffered);
> +		invalidate_mapping_pages(mapping,
> +					 (iocb->ki_pos - buffered) >> PAGE_SHIFT,

This adds a line > 80 chars.

Otherwise this looks fine to me, although I'd just put the fix in the
subject line and just mention the cleanup at the end of the actual
commit log.





More information about the Cluster-devel mailing list