On Wed, Oct 17, 2018 at 01:36:52AM -0700, Christoph Hellwig wrote: > > /* Update inode timestamps and remove security privileges when remapping. */ > > @@ -2023,7 +2034,8 @@ loff_t vfs_dedupe_file_range_one(struct file *src_file, loff_t src_pos, > > { > > loff_t ret; > > > > - WARN_ON_ONCE(remap_flags & ~(REMAP_FILE_DEDUP)); > > + WARN_ON_ONCE(remap_flags & ~(REMAP_FILE_DEDUP | > > + REMAP_FILE_CAN_SHORTEN)); > > I guess this is where you could actually use REMAP_FILE_VALID_FLAGS.. > > > /* REMAP_FILE flags taken care of by the vfs. */ > > -#define REMAP_FILE_ADVISORY (0) > > +#define REMAP_FILE_ADVISORY (REMAP_FILE_CAN_SHORTEN) > > And btw, they are not 'taken care of by the VFS', they need to be > taken care of by the fs (possibly using helpers) to take affect, > but they can be safely ignored. Ok, I'll update the comment. > > + if (!IS_ALIGNED(count, bs)) { > > + if (remap_flags & REMAP_FILE_CAN_SHORTEN) > > + count = ALIGN_DOWN(count, bs); > > + else > > + return -EINVAL; > > if (!(remap_flags & REMAP_FILE_CAN_SHORTEN)) > return -EINVAL; > count = ALIGN_DOWN(count, bs); Seeing as we return EINVAL on shortened count and !CAN_SHORTEN below this, I think this can be simplified further: if (pos_in + count == size_in) { bcount = ALIGN(size_in, bs) - pos_in; } else { if (!IS_ALIGNED(count, bs)) count = ALIGN_DOWN(count, bs); bcount = count; } --D