Re: [PATCH] btrfs: remove the dead copied check in btrfs_copy_from_user()

[David Sterba <dsterba@suse.cz>]

On Thu, Aug 13, 2020 at 02:15:33PM +0800, Qu Wenruo wrote:
> There is btrfs specific check in btrfs_copy_from_user(), after
> iov_iter_copy_from_user_atomic() call, we check if the page is uptodate
> and if the copied bytes is smaller than what we expect.
> 
> However that check will never be triggered due to the following reasons:
> - PageUptodate() check conflicts with current behavior
>   Currently we ensure all pages that will go through a partial write
>   (some bytes are not covered by the write range) will be forced
>   uptodate.
> 
>   This is the common behavior to ensure we get the correct content.
>   This behavior is always true, no matter if my previous patch "btrfs:
>   refactor how we prepare pages for btrfs_buffered_write()" is applied.

Would it make sense to add an assert here? Checking for the page
up-to-date status.

> - iov_iter_copy_from_user_atomic() only returns 0 or @bytes
>   It won't return a short write.

And maybe for that one too, I'm not able to navigate through the maze of
the iov_iter_* macros.

> So we're completely fine to remove the (PageUptodate() && copied <
> count) check, as we either get copied == 0, and break the loop anyway,
> or do a proper copy.
> 
> This will revert commit 31339acd07b4 ("Btrfs: deal with short returns from
> copy_from_user").

As this is a very old patch, the changes outside of btrfs are likely to
make the piece of code redundant.