On Mon, Dec 07, 2020 at 06:14:56PM +0100, Christoph Hellwig wrote: > > + switch (attr->propagation) { > > + case 0: > > + kattr->propagation = 0; > > + break; > > + case MS_UNBINDABLE: > > + kattr->propagation = MS_UNBINDABLE; > > + break; > > + case MS_PRIVATE: > > + kattr->propagation = MS_PRIVATE; > > + break; > > + case MS_SLAVE: > > + kattr->propagation = MS_SLAVE; > > + break; > > + case MS_SHARED: > > + kattr->propagation = MS_SHARED; > > + break; > > + default: > > + return -EINVAL; > > + } > > This can be shortened to: > > #define MOUNT_SETATTR_PROPAGATION_FLAGS \ > (MS_UNBINDABLE | MS_PRIVATE | MS_SLAVE | MS_SHARED) > > if (attr->propagation & ~MOUNT_SETATTR_PROPAGATION_FLAGS) > return -EINVAL; > if (hweight32(attr->propagation & MOUNT_SETATTR_PROPAGATION_FLAGS) > 1) > return -EINVAL; > kattr->propagation = attr->propagation; Looks good! I've applied that. > > > +asmlinkage long sys_mount_setattr(int dfd, const char __user *path, unsigned int flags, > > Overly long line. Folded after @path now. > > Otherwise looks good: > > Reviewed-by: Christoph Hellwig <hch@xxxxxx> Thanks, I've pushed out the changes to: https://git.kernel.org/brauner/h/idmapped_mounts the original v4 can now be found at: https://git.kernel.org/brauner/h/idmapped_mounts_v4 You want a v5 with the changes you requested before you continue reviewing? Otherwise I'll just let you go through v4. Thanks! Christian