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
| ||
|
Message-ID: <20130729235034.GC3648@gmail.com> 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
Powered by blists - more mailing lists