linux-ext4 - Re: [PATCH] ext4: don't re-try to remove the entry from es tree when we encounter a ENOMEM in ext4_ext

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]

Date:	Tue, 30 Jul 2013 07:50:34 +0800
From:	Zheng Liu <gnehzuil.liu@...il.com>
To:	Theodore Ts'o <tytso@....edu>
Cc:	linux-ext4@...r.kernel.org, Zheng Liu <wenqing.lz@...bao.com>
Subject: Re: [PATCH] ext4: don't re-try to remove the entry from es tree when
 we encounter a ENOMEM in ext4_ext_truncate

On Mon, Jul 29, 2013 at 11:42:39AM -0400, Theodore Ts'o wrote:
> On Thu, Jul 25, 2013 at 07:56:37PM +0800, Zheng Liu wrote:
> > From: Zheng Liu <wenqing.lz@...bao.com>
> > 
> > ext4_es_remove_extent returns ENOMEM only if we need to split an entry
> > and insert a part into es tree.  After applied this commit (e15f742c),
> > we have retried to do this.  So we don't need to do this again in
> > ext4_ext_truncate().
> > 
> > Signed-off-by: Zheng Liu <wenqing.lz@...bao.com>
> > Cc: "Theodore Ts'o" <tytso@....edu>
> 
> Actually, we still need to do this, since the retry loop in
> __es_remove_extent() tries to shrink the extent status tree for the
> inode in question, and only retries if we were able to free up some
> memory.  (We only do it for the inode we're working on, since we have
> it locked already.)  So __es_remove_extent() can still return ENOMEM,
> and so callers of ext4_es_insert_extent() and ext4_es_remove_extent()
> still need to check for ENOMEM and try to do something sane if
> possible.
> 
> The problem with truncate is that the VFS assumes truncate() will
> always succeed (the method function is returns a void, so there isn't
> even a way to propagate an error code back p to the VFS), so we really
> do need to do a retry in ext4's truncate code.
> 
> For other code paths, like for example fallocate(), it's completely
> fair game for it to return ENOMEM, although we need to make sure that
> we've gotten the error handling correct.  
> 
> For the writeback paths, where the application which performed the
> write may have exited already and we have dirty pages in the page
> cache, retrying an ENOMEM after calling congestion_wait() is something
> that *does* make sense.
> 
> This is why I didn't add an unconditional retry loop to the low-level
> extent_status tree code, since where we can return ENOMEM, it's better
> to do that, since that way applications can start failing fast in OOM
> conditions.  Whether or not we want do that is going to depend on the
> higher level code paths.

Got it, thanks for your explanation.

                                                - Zheng
--
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