Re: [f2fs-dev] [PATCH v2] f2fs: add F2FS_IOC_SEC_TRIM_FILE ioctl

From: Eric Biggers
Date: Thu Jun 11 2020 - 12:27:25 EST


On Thu, Jun 11, 2020 at 12:16:52PM +0900, Daeho Jeong wrote:
> + for (index = pg_start; index < pg_end;) {
> + struct dnode_of_data dn;
> + unsigned int end_offset;
> +
> + set_new_dnode(&dn, inode, NULL, NULL, 0);
> + ret = f2fs_get_dnode_of_data(&dn, index, LOOKUP_NODE);
> + if (ret)
> + goto out;
> +
> + end_offset = ADDRS_PER_PAGE(dn.node_page, inode);
> + if (pg_end < end_offset + index)
> + end_offset = pg_end - index;
> +
> + for (; dn.ofs_in_node < end_offset;
> + dn.ofs_in_node++, index++) {
> + struct block_device *cur_bdev;
> + block_t blkaddr = f2fs_data_blkaddr(&dn);
> +
> + if (__is_valid_data_blkaddr(blkaddr)) {
> + if (!f2fs_is_valid_blkaddr(F2FS_I_SB(inode),
> + blkaddr, DATA_GENERIC_ENHANCE)) {
> + ret = -EFSCORRUPTED;
> + goto out;
> + }
> + } else
> + continue;
> +
> + cur_bdev = f2fs_target_device(sbi, blkaddr, NULL);
> + if (f2fs_is_multi_device(sbi)) {
> + int i = f2fs_target_device_index(sbi, blkaddr);
> +
> + blkaddr -= FDEV(i).start_blk;
> + }
> +
> + if (len) {
> + if (prev_bdev == cur_bdev &&
> + blkaddr == prev_block + len) {
> + len++;
> + } else {
> + ret = f2fs_secure_erase(prev_bdev,
> + prev_block, len, flags);
> + if (ret)
> + goto out;
> +
> + len = 0;
> + }
> + }
> +
> + if (!len) {
> + prev_bdev = cur_bdev;
> + prev_block = blkaddr;
> + len = 1;
> + }
> + }
> +
> + f2fs_put_dnode(&dn);
> + }

This loop processes the entire file, which may be very large. So it could take
a very long time to execute.

It should at least use the following to make the task killable and preemptible:

if (fatal_signal_pending(current)) {
err = -EINTR;
goto out;
}
cond_resched();

Also, perhaps this ioctl should be made incremental, i.e. take in an
(offset, length) like pwrite()?

- Eric