linux-kernel - Re: [PATCH] Set the initial TRIM information as TRIMMED

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.LFD.2.00.1112051106070.4911@dhcp-27-109.brq.redhat.com>
Date:	Mon, 5 Dec 2011 11:17:35 +0100 (CET)
From:	Lukas Czerner <lczerner@...hat.com>
To:	Kyungmin Park <kmpark@...radead.org>
cc:	tytso@....edu, tm@....ma, linux-ext4@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Set the initial TRIM information as TRIMMED

On Thu, 1 Dec 2011, Kyungmin Park wrote:

> From: Kyungmin Park <kyungmin.park@...sung.com>
> 
> Now trim information doesn't stored at disk so every boot time. it's cleared.
> and do the trim all disk groups.
> But assume that it's already trimmed at previous time so don't need to trim it again. So set the intial state as trimmed.

Hi,

I am sorry, but from the code and comments I have seen, I do not think
it does make sense to change that behaviour. I agree that discarding the
whole file system after the mount might not be necessarily needed, but
on the other hand, you can never assume that the blocks were already
trimmed, since it most likely will not be true.

I think that the bigger problem is "running fitrim at boot time" which
does not make sense to me, because you'll be perfectly fine leaving this
up to the cron (or something similar). Also when you're booting you need
to be done ASAP so why to bother with other unnecessary operations ?

The funniest thing about this patch is (no offense, really) that you've
solved the problem of "fitrim slowing down the boot process" by changing
kernel logic to assume something which will most likely be false, instead
of just *not* doing boot time fitrim at the first place, because this is
exactly what will happen with this patch.

Thanks!
-Lukas

> 
> Signed-off-by: Kyungmin Park <kyungmin.park@...sung.com>
> ---
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index e2d8be8..97ef342 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -1098,6 +1098,12 @@ int ext4_mb_init_group(struct super_block *sb, ext4_group_t group)
>  		goto err;
>  	}
>  	mark_page_accessed(page);
> +
> +	/*
> +	 * TRIM information is not stored at disk so set the initial
> +	 * state as trimmed. Since previous time it's already trimmed all
> +	 */
> +	EXT4_MB_GRP_SET_TRIMMED(this_grp);
>  err:
>  	ext4_mb_put_buddy_page_lock(&e4b);
>  	return ret;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/