linux-kernel - Re: -mm merge plans for 2.6.21

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:	Fri, 09 Feb 2007 23:18:39 +0100
From:	Guillaume Chazarain <guichaz@...oo.fr>
To:	Andrew Morton <akpm@...ux-foundation.org>
CC:	linux-kernel@...r.kernel.org
Subject: Re: -mm merge plans for 2.6.21

Andrew Morton a écrit :
> factor-outstanding-i-o-error-handling.patch
> factor-outstanding-i-o-error-handling-tidy.patch
>   

I still like them. But the original problem is still present.

> sync_sb_inodes-propagate-errors.patch
>   

You explained in http://lkml.org/lkml/2007/1/2/238 that the mapping 
flags should be set at the lowest level, but with this change I have a 
hard time choosing a place to stick it. I don't like when a function 
both sets the mapping flags and returns an error code, I think it should 
be mutually exclusive, so that we know what to do (propagate the return 
code?) and what to expect (are the mapping flags up to date?), which 
seemed to be the case before this patch. For instance there is no point 
in propagating an error return code up to sys_sync() as it can only drop it.

The call trace that cleared the flags, the origin of the problem, is:

void do_sync(1)
void sync_inodes(1)
void __sync_inodes(1)
void sync_inodes_sb(sb, 1)
void sync_sb_inodes(sb, WB_SYNC_ALL)
int __writeback_single_inode(inode, WB_SYNC_ALL)
int __sync_single_inode(inode, WB_SYNC_ALL)
int filemap_fdatawait(mapping)
int wait_on_page_writeback_range(mapping)
int test_and_clear(...)

re-setting the flag at a too low level, would mean it is still set after 
a msync() or fsync() that could return the status to its caller. My 
interpretation is that low level functions up to 
__writeback_single_inode() can be used by fsync() and the like that can 
return the error to their caller, unlike high level functions starting 
at sync_sb_inodes() that don't need a return value as their caller can 
do nothing with it. So re-setting the flag in sync_sb_inodes() just 
after __writeback_single_inode() looks right to me, and I propose the 
exact same patch as the first time.

> block_write_full_page-handle-enospc.patch
>   

It seems to me that __block_write_full_page is always called more or 
less directly from __mpage_writepage, and the latter handles enospc in 
the mapping flags. So I am not sure this patch is needed.

Thanks.

-- 
Guillaume

View attachment "sync_sb_inodes_handle_error.patch" of type "text/x-patch" (2200 bytes)