linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Theodore Y. Ts'o" <tytso@mit.edu>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Souptick Joarder <jrdr.linux@gmail.com>,
	konishi.ryusuke@lab.ntt.co.jp, viro@zeniv.linux.org.uk,
	adilger.kernel@dilger.ca, axboe@kernel.dk,
	darrick.wong@oracle.com, ebiggers@google.com,
	pombredanne@nexb.com, agruenba@redhat.com,
	gregkh@linuxfoundation.org, kemi.wang@intel.com,
	willy@infradead.org, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-ext4@vger.kernel.org,
	linux-nilfs@vger.kernel.org
Subject: Re: [PATCH v2] fs: Convert return type int to vm_fault_t
Date: Fri, 31 Aug 2018 12:58:44 -0400	[thread overview]
Message-ID: <20180831165844.GB3303@thunk.org> (raw)
In-Reply-To: <20180830163521.728f3ff2fd3cc93b52a5dcc0@linux-foundation.org>

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

  reply	other threads:[~2018-08-31 21:07 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-30 17:25 [PATCH v2] fs: Convert return type int to vm_fault_t Souptick Joarder
2018-08-30 23:33 ` Andrew Morton
2018-08-31  6:03   ` Souptick Joarder
2018-09-04  0:28     ` Andrew Morton
2018-09-04  2:13       ` Matthew Wilcox
2018-09-03  2:13   ` Ryusuke Konishi
2018-09-03 11:58     ` Souptick Joarder
2018-08-30 23:35 ` Andrew Morton
2018-08-31 16:58   ` Theodore Y. Ts'o [this message]
2018-09-01 16:21     ` Souptick Joarder
2018-09-03  4:49 ` Matthew Wilcox

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180831165844.GB3303@thunk.org \
    --to=tytso@mit.edu \
    --cc=adilger.kernel@dilger.ca \
    --cc=agruenba@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=axboe@kernel.dk \
    --cc=darrick.wong@oracle.com \
    --cc=ebiggers@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jrdr.linux@gmail.com \
    --cc=kemi.wang@intel.com \
    --cc=konishi.ryusuke@lab.ntt.co.jp \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nilfs@vger.kernel.org \
    --cc=pombredanne@nexb.com \
    --cc=viro@zeniv.linux.org.uk \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).