Ackerley Tng <[email protected]> writes: > Tarun Sahu <[email protected]> writes: > >> static long kvm_gmem_fallocate(struct file *file, int mode, loff_t offset, >> loff_t len) >> { >> + struct inode *inode = file_inode(file); >> int ret; >> + int idx; >> >> - if (!(mode & FALLOC_FL_KEEP_SIZE)) >> - return -EOPNOTSUPP; >> + idx = srcu_read_lock(&kvm_gmem_freeze_srcu); >> + if (kvm_gmem_is_frozen(inode)) { >> + srcu_read_unlock(&kvm_gmem_freeze_srcu, idx); >> + return -EPERM; >> + } > > fallocate may eventually go to kvm_gmem_get_folio(), so that would check > kvm_gmem_is_frozen() twice. Is this meant to catch the punch hole case? > >> >> - if (mode & ~(FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE)) >> - return -EOPNOTSUPP; >> + if (!(mode & FALLOC_FL_KEEP_SIZE)) { >> + ret = -EOPNOTSUPP; >> + goto out; >> + } >> >> - if (!PAGE_ALIGNED(offset) || !PAGE_ALIGNED(len)) >> - return -EINVAL; >> + if (mode & ~(FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE)) { >> + ret = -EOPNOTSUPP; >> + goto out; >> + } >> + >> + if (!PAGE_ALIGNED(offset) || !PAGE_ALIGNED(len)) { >> + ret = -EINVAL; >> + goto out; >> + } > > There's some reordering here. Why not let the validation happen like > before, then check kvm_gmem_is_frozen()?
To align with design. "stop the fallocate call if inode is frozen, No need to go further". I dont have strict opinion on this. I am fine with taking it across punch hole as well to make it more fine grained. But it will no longer claims stop the fallocate call (allocation one is stopped in separate path: fault path) , though functionally it does the same thing. WDYT? ~Tarun
