On 6/12/14, 9:14 PM, JP Abgrall wrote:
> This provides an interface for issuing secure trims to parallel
> the existing interface for non-secure trim.
Which does what? A bit of digging through git history tells
me, but an idiot reader like myself doesn't know what a
"secure trim" is. ;) Would be nice to have that info,
and the reason for elevating it from a block-level ioctl
to an fs-level ioctl in the commit log.
IOWS: your commit log says what, but not why.
Also:
You're adding a new high-level IOCTL, so let's get a bit more
visibility than just linux-ext4; linux-fsdevel cc'd.
one other ext4-specific note below.
> Signed-off-by: Geremy Condra <gcondra@xxxxxxxxxx>
> Signed-off-by: JP Abgrall <jpa@xxxxxxxxxx>
> ---
> fs/ext4/ext4.h | 3 ++-
> fs/ext4/ioctl.c | 14 +++++++++++++-
> fs/ext4/mballoc.c | 29 +++++++++++++++++++----------
> include/uapi/linux/fs.h | 1 +
> 4 files changed, 35 insertions(+), 12 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 7cc5a0e..cf2ddad 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -2082,7 +2082,8 @@ extern int ext4_mb_add_groupinfo(struct super_block *sb,
> ext4_group_t i, struct ext4_group_desc *desc);
> extern int ext4_group_add_blocks(handle_t *handle, struct super_block *sb,
> ext4_fsblk_t block, unsigned long count);
> -extern int ext4_trim_fs(struct super_block *, struct fstrim_range *);
> +extern int ext4_trim_fs(struct super_block *, struct fstrim_range *,
> + bool secure);
>
> /* inode.c */
> struct buffer_head *ext4_getblk(handle_t *, struct inode *,
> diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
> index 0f2252e..0a0b483 100644
> --- a/fs/ext4/ioctl.c
> +++ b/fs/ext4/ioctl.c
> @@ -580,11 +580,13 @@ resizefs_out:
> return err;
> }
>
> + case SFITRIM:
> case FITRIM:
> {
> struct request_queue *q = bdev_get_queue(sb->s_bdev);
> struct fstrim_range range;
> int ret = 0;
> + bool secure_trim = cmd == SFITRIM;
>
> if (!capable(CAP_SYS_ADMIN))
> return -EPERM;
> @@ -592,13 +594,23 @@ resizefs_out:
> if (!blk_queue_discard(q))
> return -EOPNOTSUPP;
>
> + if (secure_trim && !blk_queue_secdiscard(q))
> + return -EOPNOTSUPP;
> +
> + if (EXT4_HAS_RO_COMPAT_FEATURE(sb,
> + EXT4_FEATURE_RO_COMPAT_BIGALLOC)) {
> + ext4_msg(sb, KERN_ERR,
> + "FITRIM not supported with bigalloc");
> + return -EOPNOTSUPP;
> + }
> +
This last conditional is unrelated to the patch; if BIGALLOC has another
incomplete part of its implementation, please send it as a standalone patch,
not buried in this one.
Thanks,
-Eric
> if (copy_from_user(&range, (struct fstrim_range __user *)arg,
> sizeof(range)))
> return -EFAULT;
>
> range.minlen = max((unsigned int)range.minlen,
> q->limits.discard_granularity);
> - ret = ext4_trim_fs(sb, &range);
> + ret = ext4_trim_fs(sb, &range, secure_trim);
> if (ret < 0)
> return ret;
>
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 59e3162..b470efe 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -2734,16 +2734,18 @@ int ext4_mb_release(struct super_block *sb)
> }
>
> static inline int ext4_issue_discard(struct super_block *sb,
> - ext4_group_t block_group, ext4_grpblk_t cluster, int count)
> + ext4_group_t block_group, ext4_grpblk_t cluster, int count,
> + bool secure)
> {
> ext4_fsblk_t discard_block;
> + unsigned long flags = secure ? BLKDEV_DISCARD_SECURE : 0;
>
> discard_block = (EXT4_C2B(EXT4_SB(sb), cluster) +
> ext4_group_first_block_no(sb, block_group));
> count = EXT4_C2B(EXT4_SB(sb), count);
> trace_ext4_discard_blocks(sb,
> (unsigned long long) discard_block, count);
> - return sb_issue_discard(sb, discard_block, count, GFP_NOFS, 0);
> + return sb_issue_discard(sb, discard_block, count, GFP_NOFS, flags);
> }
>
> /*
> @@ -2765,7 +2767,7 @@ static void ext4_free_data_callback(struct super_block *sb,
> if (test_opt(sb, DISCARD)) {
> err = ext4_issue_discard(sb, entry->efd_group,
> entry->efd_start_cluster,
> - entry->efd_count);
> + entry->efd_count, 0);
> if (err && err != -EOPNOTSUPP)
> ext4_msg(sb, KERN_WARNING, "discard request in"
> " group:%d block:%d count:%d failed"
> @@ -4804,7 +4806,8 @@ do_more:
> * them with group lock_held
> */
> if (test_opt(sb, DISCARD)) {
> - err = ext4_issue_discard(sb, block_group, bit, count);
> + err = ext4_issue_discard(sb, block_group, bit, count,
> + 0);
> if (err && err != -EOPNOTSUPP)
> ext4_msg(sb, KERN_WARNING, "discard request in"
> " group:%d block:%d count:%lu failed"
> @@ -5011,13 +5014,15 @@ error_return:
> * @count: number of blocks to TRIM
> * @group: alloc. group we are working with
> * @e4b: ext4 buddy for the group
> + * @secure: false to issue a standard discard, true for secure discard
> *
> * Trim "count" blocks starting at "start" in the "group". To assure that no
> * one will allocate those blocks, mark it as used in buddy bitmap. This must
> * be called with under the group lock.
> */
> static int ext4_trim_extent(struct super_block *sb, int start, int count,
> - ext4_group_t group, struct ext4_buddy *e4b)
> + ext4_group_t group, struct ext4_buddy *e4b,
> + bool secure)
> __releases(bitlock)
> __acquires(bitlock)
> {
> @@ -5038,7 +5043,7 @@ __acquires(bitlock)
> */
> mb_mark_used(e4b, &ex);
> ext4_unlock_group(sb, group);
> - ret = ext4_issue_discard(sb, group, start, count);
> + ret = ext4_issue_discard(sb, group, start, count, secure);
> ext4_lock_group(sb, group);
> mb_free_blocks(NULL, e4b, start, ex.fe_len);
> return ret;
> @@ -5051,6 +5056,7 @@ __acquires(bitlock)
> * @start: first group block to examine
> * @max: last group block to examine
> * @minblocks: minimum extent block count
> + * @secure: false for standard discard, true for secure discard
> *
> * ext4_trim_all_free walks through group's buddy bitmap searching for free
> * extents. When the free block is found, ext4_trim_extent is called to TRIM
> @@ -5065,7 +5071,7 @@ __acquires(bitlock)
> static ext4_grpblk_t
> ext4_trim_all_free(struct super_block *sb, ext4_group_t group,
> ext4_grpblk_t start, ext4_grpblk_t max,
> - ext4_grpblk_t minblocks)
> + ext4_grpblk_t minblocks, bool secure)
> {
> void *bitmap;
> ext4_grpblk_t next, count = 0, free_count = 0;
> @@ -5098,7 +5104,8 @@ ext4_trim_all_free(struct super_block *sb, ext4_group_t group,
>
> if ((next - start) >= minblocks) {
> ret = ext4_trim_extent(sb, start,
> - next - start, group, &e4b);
> + next - start, group, &e4b,
> + secure);
> if (ret && ret != -EOPNOTSUPP)
> break;
> ret = 0;
> @@ -5140,6 +5147,7 @@ out:
> * ext4_trim_fs() -- trim ioctl handle function
> * @sb: superblock for filesystem
> * @range: fstrim_range structure
> + * @secure: false for standard discard, true for secure discard
> *
> * start: First Byte to trim
> * len: number of Bytes to trim from start
> @@ -5148,7 +5156,8 @@ out:
> * start to start+len. For each such a group ext4_trim_all_free function
> * is invoked to trim all free space.
> */
> -int ext4_trim_fs(struct super_block *sb, struct fstrim_range *range)
> +int ext4_trim_fs(struct super_block *sb, struct fstrim_range *range,
> + bool secure)
> {
> struct ext4_group_info *grp;
> ext4_group_t group, first_group, last_group;
> @@ -5204,7 +5213,7 @@ int ext4_trim_fs(struct super_block *sb, struct fstrim_range *range)
>
> if (grp->bb_free >= minlen) {
> cnt = ext4_trim_all_free(sb, group, first_cluster,
> - end, minlen);
> + end, minlen, secure);
> if (cnt < 0) {
> ret = cnt;
> break;
> diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
> index ca1a11b..efbd8f8 100644
> --- a/include/uapi/linux/fs.h
> +++ b/include/uapi/linux/fs.h
> @@ -156,6 +156,7 @@ struct inodes_stat_t {
> #define FIFREEZE _IOWR('X', 119, int) /* Freeze */
> #define FITHAW _IOWR('X', 120, int) /* Thaw */
> #define FITRIM _IOWR('X', 121, struct fstrim_range) /* Trim */
> +#define SFITRIM _IOWR('X', 122, struct fstrim_range) /* Secure trim */
>
> #define FS_IOC_GETFLAGS _IOR('f', 1, long)
> #define FS_IOC_SETFLAGS _IOW('f', 2, long)
>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
