On Mon, May 07, 2018 at 01:32:09PM +0200, Miklos Szeredi wrote: > On Mon, May 7, 2018 at 1:11 PM, Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote: > > On Mon, May 07, 2018 at 10:21:06AM +0200, Miklos Szeredi wrote: > >> f_op->dedupe_file_range() gets a u64 length to dedup and returns an ssize_t > >> actual length deduped. This breaks badly on 32bit archs since the returned > >> length will be truncated and possibly overflow into the sign bit (xfs and > >> ocfs2 are affected, btrfs limits actual length to 16MiB). > >> > >> Returning s64 should be good, since clone_verify_area() makes sure that the > >> supplied length doesn't overflow. > > > > Why s64 rather than loff_t? Particularly since the next patch turns > > the paramters into loff_t. > > Next patch turns the offsets into loff_t and leaves "len" as u64. A > size is definitely not an offset, I'd consider changing the type of > "len" to loff_t a misuse. Usually a size is the size of something in memory. The length of something on storage is definitely an loff_t. Look at fallocate() for an example. You could also argue that lseek where whence is set to anything other than SEEK_SET is also being used as a length rather than an absolute offset. We also already use 'loff_t len' as an argument to vfs_dedupe_file_range_compare().
