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  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, 31 Aug 2018 12:58:44 -0400
From:   "Theodore Y. Ts'o" <tytso@....edu>
To:     Andrew Morton <akpm@...ux-foundation.org>
Cc:     Souptick Joarder <jrdr.linux@...il.com>,
        konishi.ryusuke@....ntt.co.jp, viro@...iv.linux.org.uk,
        adilger.kernel@...ger.ca, axboe@...nel.dk, darrick.wong@...cle.com,
        ebiggers@...gle.com, pombredanne@...b.com, agruenba@...hat.com,
        gregkh@...uxfoundation.org, kemi.wang@...el.com,
        willy@...radead.org, linux-fsdevel@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-ext4@...r.kernel.org,
        linux-nilfs@...r.kernel.org
Subject: Re: [PATCH v2] fs: Convert return type int to vm_fault_t

On Thu, Aug 30, 2018 at 04:35:21PM -0700, Andrew Morton wrote:
> 
> The v1->v2 delta (below) reveals unchangelogged ext4 changes?

Souptick, please don't make unrelated changes in a vm_fault_t patch.

Especially please don't make whitespace changes that will cause
checkpatch.pl to whine about line lengths longer than 80 characters.
There's a *reason* for the original indentation.

> @@ -6239,8 +6237,7 @@ retry_alloc:
>  		ext4_set_inode_state(inode, EXT4_STATE_JDATA);
>  	}
>  	ext4_journal_stop(handle);
> -	if (err == -ENOSPC &&
> -		ext4_should_retry_alloc(inode->i_sb, &retries))
> +	if (err == -ENOSPC && ext4_should_retry_alloc(inode->i_sb, &retries))
>  		goto retry_alloc;
>  out_ret:
>  	ret = block_page_mkwrite_return(err);

The fact that this is not a non-trivials set of changes means anything
to make reviews harder is really not appreciated.


Also, the fact that the patch series involves multiple file system is
a massive pain.  It means I'm going to have to do a separate
regression test --- or preferably, I would ask *you* to run a file
system regression test[1] --- to make sure what is *not* a trivial
patch doesn't break things.  Also, it means that this patch series is
going to get more complicated to get into kernel, and we may have to
deal with patch conflicts if this goes in via some third party tree
(such as Andrew's tree).

[1] https:/thunk.org/gce-xfstests

One way to make life easier is to add the new function with the new
interface first, and then wait a release cycle, and then move file
systems over in independant patches.  

					- Ted

Powered by blists - more mailing lists