On Tue, Aug 25, 2020 at 10:27:35AM +1000, Dave Chinner wrote:
> > do {
> > - unsigned offset, bytes;
> > -
> > - offset = offset_in_page(pos);
> > - bytes = min_t(loff_t, PAGE_SIZE - offset, count);
> > + loff_t bytes;
> >
> > if (IS_DAX(inode))
> > - status = dax_iomap_zero(pos, offset, bytes, iomap);
> > + bytes = dax_iomap_zero(pos, length, iomap);
>
> Hmmm. everything is loff_t here, but the callers are defining length
> as u64, not loff_t. Is there a potential sign conversion problem
> here? (sure 64 bit is way beyond anything we'll pass here, but...)
I've gone back and forth on the correct type for 'length' a few times.
size_t is too small (not for zeroing, but for seek()). An unsigned type
seems right -- a length can't be negative, and we don't want to give
the impression that it can. But the return value from these functions
definitely needs to be signed so we can represent an error. So a u64
length with an loff_t return type feels like the best solution. And
the upper layers have to promise not to pass in a length that's more
than 2^63-1.
I have a patch set which starts the conversion process for all the actors
over to using u64 for the length. Obviously, that's not mixed in with
the THP patchset.
