linux-ext4 - Re: Review of ext4-online-defrag-move-victim-files.patch

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] [day] [month] [year] [list]

Message-ID: <48D0A5DB.6020103@rs.jp.nec.com>
Date:	Wed, 17 Sep 2008 15:38:19 +0900
From:	Akira Fujita <a-fujita@...jp.nec.com>
To:	"Theodore Ts'o" <tytso@....EDU>
CC:	Takashi Sato <t-sato@...jp.nec.com>, linux-ext4@...r.kernel.org
Subject: Re: Review of ext4-online-defrag-move-victim-files.patch

Hi Ted,
Thank you for your review comments.

Theodore Ts'o wrote:
> Hi,
> 
> I've been looking over the defrag patches with an eye towards getting
> them merged.  This patch concerns me for a number of reasons.  First of
> all, it is calling into a number of private functions which had
> previously been static functions in fs/ext4/balloc.c.  Specifically,
> goal_in_my_reservation(), rsv_window_remove(), rsv_is_empty(),
> alloc_new_reservation(), try_to_extend_reservation().  
> 
> This is bad for a couple of reasons.  First of all, the functions
> weren't renamed, so it results in the namespace leakage.  In general,
> non-static functions should be prefixed by ext4_ so that we know they
> came from the ext4 filesystem code.  Secondly, these were internal
> functions were intended for use in an older set of block allocation
> functions that may be removed in the future --- they had previously only
> been used by the function ext4_old_new_blocks(), which is used only when
> the mount option nomballoc is given.  Given the superiority of the new
> multi-block allocator, it's likely that this old code will be going
> away.
> 
> One of the things which further worries me is that your patch seems to
> be making changes to the mballoc() code as well.  Given that the
> reservations code in fs/ext4/balloc.c was never intended to used at the
> same time as the multi-block allocator code in mballoc(), I suspect
> there will be a problem here if the goal of reserving blocks using the
> reservation code was to prevent some other inode using allocating those
> blocks, since the multi-block allocator does not honor reservations made
> by the reservations code, since normally the reservations code is not
> active when the mballoc code is active (the two are mutually exclusive).

Is there any good way to use block reservation with mballoc?
If not, fixing defragger not to use block reservation
in the force mode would be better.

Regards,
Akira Fujita
--
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