Messages in this thread | | | From | Daeho Jeong <> | Date | Fri, 12 Jun 2020 09:00:58 +0900 | Subject | Re: [f2fs-dev] [PATCH v2] f2fs: add F2FS_IOC_SEC_TRIM_FILE ioctl |
| |
For the incremental way of erasing, we might as well support the (offset, length) option in a unit of 4KiB.
So, you might use this ioctl like the below. Does it work for you? struct f2fs_sec_trim { u64 startblk; u64 blklen; u32 flags; };
sectrim.startblk = 0; sectrim.blklen = 256; // 1MiB sectrim.flags = F2FS_TRIM_FILE_DISCARD | F2FS_TRIM_FILE_ZEROOUT; ret = ioctl(fd, F2FS_SEC_TRIM_FILE, §rim);
2020년 6월 12일 (금) 오전 8:00, Eric Biggers <ebiggers@kernel.org>님이 작성:
> > On Fri, Jun 12, 2020 at 07:49:12AM +0900, Daeho Jeong wrote: > > 2020년 6월 12일 (금) 오전 1:27, Eric Biggers <ebiggers@kernel.org>님이 작성: > > > > > > 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(); > > > > > > > Good point! > > > > > Also, perhaps this ioctl should be made incremental, i.e. take in an > > > (offset, length) like pwrite()? > > > > > > - Eric > > > > Discard and Zeroing will be treated in a unit of block, which is 4KB > > in F2FS case. > > Do you really need the (offset, length) option here even if the unit > > is a 4KB block? I guess this option might make the user even > > inconvenienced to use this ioctl, because they have to bear 4KB > > alignment in mind. > > The ioctl as currently proposed always erases the entire file, which could be > gigabytes. That could take a very long time. > > I'm suggesting considering making it possible to call the ioctl multiple times > to process the file incrementally, like you would do with write() or pwrite(). > > That implies that for each ioctl call, the length would need to be specified > unless it's hardcoded to 4KiB which would be very inefficient. Users would > probably want to process a larger amount at a time, like 1 MiB, right? > > Likewise the offset would need to be specified as well, unless it were to be > taken implicitly from f_pos. > > - Eric
| |