Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1360285 > unrolled thread

[GIT PULL] f2fs updates for v4.6

Back to article view | Back to linux.kernel

Contents

  [GIT PULL] f2fs updates for v4.6 Jaegeuk Kim <jaegeuk@kernel.org> - 2016-03-18 02:00 +0100
    Re: [GIT PULL] f2fs updates for v4.6 Linus Torvalds <torvalds@linux-foundation.org> - 2016-03-18 03:40 +0100
      Re: [GIT PULL] f2fs updates for v4.6 Linus Torvalds <torvalds@linux-foundation.org> - 2016-03-18 03:40 +0100
      Re: [GIT PULL] f2fs updates for v4.6 Jaegeuk Kim <jaegeuk@kernel.org> - 2016-03-18 06:10 +0100

#1360285 — [GIT PULL] f2fs updates for v4.6

Hi Linus,

Could you please pull the below patches?

Note that, I did cherry-pick one patch and add another patch to resolve the
conflict against recent crypto changes.

 - cherry-picked
    4bedd9f487276290c49de9b0aad8cbf7ff847543
    f2fs: Use skcipher
 - add
    1288a901a4c85aeea183937f8e8343d6187690ac
    fs crypto: resolve conflicts on crypto changes

The following changes since commit 4de8ebeff8ddefaceeb7fc6a9b1a514fc9624509:

  Merge tag 'trace-fixes-v4.5-rc5' of git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace (2016-02-22 14:09:18 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git tags/for-f2fs-4.6

for you to fetch changes up to bccf535404a3bb97031d1961ad3e229a65e9fa5b:

  f2fs: submit node page write bios when really required (2016-03-17 16:36:37 -0700)

----------------------------------------------------------------
= New Features =
 - uplift filesystem encryption into fs/crypto/
 - give sysfs entries to control memroy consumption

= Enhancement ==
 - aio performance by preallocating blocks in ->write_iter
 - use writepages lock for only WB_SYNC_ALL
 - avoid redundant inline_data conversion
 - enhance forground GC
 - use wait_for_stable_page as possible
 - speed up SEEK_DATA and fiiemap

= Bug Fixes =
 - corner case in terms of -ENOSPC for inline_data
 - hung task caused by long latency in shrinker
 - corruption between atomic write and f2fs_trace_pid
 - avoid garbage lengths in dentries
 - revoke atomicly written pages if an error occurs

In addition, there are various minor bug fixes and clean-ups.

----------------------------------------------------------------
Arnd Bergmann (1):
      f2fs: add missing argument to f2fs_setxattr stub

Chao Yu (33):
      f2fs: relocate is_merged_page
      f2fs: flush dirty nat entries when exceeding threshold
      f2fs: export dirty_nats_ratio in sysfs
      f2fs: correct search area in get_new_segment
      f2fs: enhance foreground GC
      f2fs: simplify f2fs_map_blocks
      f2fs: simplify __allocate_data_blocks
      f2fs: remove unneeded pointer conversion
      f2fs: introduce get_next_page_offset to speed up SEEK_DATA
      f2fs: speed up handling holes in fiemap
      f2fs: introduce f2fs_submit_merged_bio_cond
      f2fs: split drop_inmem_pages from commit_inmem_pages
      f2fs: support revoking atomic written pages
      f2fs crypto: make sure the encryption info is initialized on opendir(2)
      f2fs crypto: handle unexpected lack of encryption keys
      f2fs crypto: avoid unneeded memory allocation when {en/de}crypting symlink
      f2fs: introduce f2fs_journal struct to wrap journal info
      f2fs: enhance IO path with block plug
      f2fs: split journal cache from curseg cache
      f2fs: reorder nat cache lock in cache_nat_entry
      f2fs: detect error of update_dent_inode in ->rename
      f2fs: fix to delete old dirent in converted inline directory in ->rename
      f2fs: fix the wrong stat count of calling gc
      f2fs: show more info about superblock recovery
      f2fs: try to flush inode after merging inline data
      f2fs: trace old block address for CoWed page
      f2fs: fix incorrect upper bound when iterating inode mapping tree
      f2fs crypto: fix incorrect positioning for GCing encrypted data page
      f2fs: introduce f2fs_update_data_blkaddr for cleanup
      f2fs: introduce f2fs_flush_merged_bios for cleanup
      f2fs: fix to avoid deadlock when merging inline data
      f2fs: clean up opened code with f2fs_update_dentry
      f2fs: fix to avoid unneeded unlock_new_inode

Fan Li (2):
      f2fs: avoid unnecessary search while finding victim in gc
      f2fs: modify the readahead method in ra_node_page()

Herbert Xu (1):
      f2fs: Use skcipher

Hou Pengyang (2):
      f2fs: reconstruct the code to free an extent_node
      f2fs: improve shrink performance of extent nodes

Jaegeuk Kim (40):
      f2fs: remove needless condition check
      f2fs: use writepages->lock for WB_SYNC_ALL
      f2fs: fix to overcome inline_data floods
      f2fs: do f2fs_balance_fs when block is allocated
      f2fs: avoid multiple node page writes due to inline_data
      f2fs: don't need to sync node page at every time
      f2fs: avoid needless sync_inode_page when reading inline_data
      f2fs: don't need to call set_page_dirty for io error
      f2fs: use wait_for_stable_page to avoid contention
      f2fs: use wq_has_sleeper for cp_wait wait_queue
      f2fs: move extent_node list operations being coupled with rbtree operation
      f2fs: don't set cached_en if it will be freed
      f2fs: give scheduling point in shrinking path
      f2fs: wait on page's writeback in writepages path
      f2fs: flush bios to handle cp_error in put_super
      f2fs: fix conflict on page->private usage
      f2fs: move dio preallocation into f2fs_file_write_iter
      f2fs: preallocate blocks for buffered aio writes
      f2fs: increase i_size to avoid missing data
      f2fs crypto: replace some BUG_ON()'s with error checks
      f2fs crypto: fix spelling typo in comment
      f2fs crypto: f2fs_page_crypto() doesn't need a encryption context
      f2fs crypto: check for too-short encrypted file names
      f2fs crypto: add missing locking for keyring_key access
      f2fs: use correct errno
      f2fs crypto: sync with ext4's fname padding
      f2fs: avoid garbage lenghs in dentries
      f2fs: remain last victim segment number ascending order
      fs crypto: add basic definitions for per-file encryption
      fs crypto: add crypto.c for encrypt/decrypt functions
      fs crypto: add policy.c to handle contexts
      fs crypto: add keyinfo.c to handle permissions
      fs crypto: add fname.c to support filename encryption
      fs crypto: resolve conflicts on crypto changes
      fs crypto: add Makefile and Kconfig
      fs crypto: add dentry revalidation facility in crypto
      f2fs crypto: migrate into vfs's crypto engine
      f2fs crypto: sync ext4_lookup and ext4_file_open
      f2fs: declare static functions
      f2fs: submit node page write bios when really required

Keith Mok (1):
      f2fs: use cryptoapi crc32 functions

Liu Xue (1):
      f2fs: recovery missing dot dentries in root directory

Shawn Lin (3):
      f2fs: slightly reorganize read_raw_super_block
      f2fs: move sanity checking of cp into get_valid_checkpoint
      f2fs: reuse read_inline_data for f2fs_convert_inline_page

Sheng Yong (1):
      f2fs: fix endianness of on-disk summary_footer

Shuoran Liu (1):
      f2fs: introduce lifetime write IO statistics

Yang Shi (1):
      f2fs: mutex can't be used by down_write_nest_lock()

Yunlei He (3):
      f2fs: delete unnecessary wait for page writeback
      f2fs: fix missing skip pages info
      f2fs: avoid hungtask problem caused by losing wake_up

 Documentation/ABI/testing/sysfs-fs-f2fs    |  14 +
 fs/Kconfig                                 |   2 +
 fs/Makefile                                |   1 +
 fs/crypto/Kconfig                          |  18 +
 fs/crypto/Makefile                         |   3 +
 fs/crypto/crypto.c                         | 556 +++++++++++++++++++++++++++++
 fs/{f2fs/crypto_fname.c => crypto/fname.c} | 277 +++++++-------
 fs/crypto/keyinfo.c                        | 273 ++++++++++++++
 fs/crypto/policy.c                         | 229 ++++++++++++
 fs/f2fs/Kconfig                            |  12 +-
 fs/f2fs/Makefile                           |   2 -
 fs/f2fs/checkpoint.c                       |  77 ++--
 fs/f2fs/crypto.c                           | 491 -------------------------
 fs/f2fs/crypto_key.c                       | 254 -------------
 fs/f2fs/crypto_policy.c                    | 209 -----------
 fs/f2fs/data.c                             | 428 +++++++++++-----------
 fs/f2fs/dir.c                              |  94 ++---
 fs/f2fs/extent_cache.c                     | 176 ++++-----
 fs/f2fs/f2fs.h                             | 315 +++++++---------
 fs/f2fs/f2fs_crypto.h                      | 151 --------
 fs/f2fs/file.c                             | 114 +++---
 fs/f2fs/gc.c                               | 245 +++++++------
 fs/f2fs/inline.c                           |  43 +--
 fs/f2fs/inode.c                            |  15 +-
 fs/f2fs/namei.c                            | 168 +++++----
 fs/f2fs/node.c                             | 223 ++++++++----
 fs/f2fs/node.h                             |  26 +-
 fs/f2fs/recovery.c                         |  14 +-
 fs/f2fs/segment.c                          | 386 ++++++++++++--------
 fs/f2fs/segment.h                          |   5 +-
 fs/f2fs/super.c                            | 204 +++++++----
 fs/f2fs/trace.c                            |   6 +-
 fs/f2fs/xattr.c                            |   6 +-
 fs/f2fs/xattr.h                            |   3 +-
 include/linux/dcache.h                     |   2 +
 include/linux/f2fs_fs.h                    |  34 +-
 include/linux/fs.h                         |   8 +
 include/linux/fscrypto.h                   | 433 ++++++++++++++++++++++
 include/trace/events/f2fs.h                |  12 +-
 include/uapi/linux/fs.h                    |  18 +
 40 files changed, 3152 insertions(+), 2395 deletions(-)
 create mode 100644 fs/crypto/Kconfig
 create mode 100644 fs/crypto/Makefile
 create mode 100644 fs/crypto/crypto.c
 rename fs/{f2fs/crypto_fname.c => crypto/fname.c} (54%)
 create mode 100644 fs/crypto/keyinfo.c
 create mode 100644 fs/crypto/policy.c
 delete mode 100644 fs/f2fs/crypto.c
 delete mode 100644 fs/f2fs/crypto_key.c
 delete mode 100644 fs/f2fs/crypto_policy.c
 delete mode 100644 fs/f2fs/f2fs_crypto.h
 create mode 100644 include/linux/fscrypto.h

[toc] | [next] | [standalone]

#1360313

On Thu, Mar 17, 2016 at 5:58 PM, Jaegeuk Kim <jaegeuk@kernel.org> wrote:
>
> Note that, I did cherry-pick one patch and add another patch to resolve the
> conflict against recent crypto changes.

*please* don't do things like this!

Now I have a conflict _anyway_, and because you added extra commits it
ends up being just messier. Git notices that you renamed
fs/f2fs/crypto_fname.c into fs/crypto/fname.c, and sees that different
people did different things, so now your "merge resolution" conflicts
with the changes I already merged to the original file from the crypto
tree.

Yes, yes, since you also did that "f2fs: Use skcipher", the resolution
now is to just throw the extra merge resolution away, but it's
literally just messier and more work for me, because it makes even
less sense than it would have done to just do the merge resolution
directly.

I do *not* want maintainers of two different trees trying to resolve
things against each other ahead of time. It makes no sense, and it
makes the history make no sense.

Now, if a merge ends up being really painful, I might ask you to help
resolve a merge, but quite frankly, that is very very rare.

I'd much *much* rather see you just do stuff that makes sense for your
tree. In particular, you really should have done that "move file" as a
single file move commit, and just left it at that. Instead, your tree
does something insane:

 - first it cherry-picks the change from the crypto tree

 - then it *copies* the old file to fs/crypto/fname.c, and changes the
naming to match.

 - at this point nothing actually *uses* the new file or new naming at all.

 - then it "resolves the merge conflict" by re-doing *AGAIN* that
crypto change that it shouldn't have cared about in the first place

 - then it removes the old file and switches to the new naming in the users

That makes no sense what-so-ever.

What would have been *sensible* would have been to split it up in a
*natural* way:

 - move the file to a new subdirectory in one commit (together with
the Makefile/Kconfig stuff that enables it)

 - change the naming to match the new location in another.

or even just do it all in one single step. And at no point worry about
the fact that the crypto tree did something unrelated to your work.

You should aim to make the changes make sense in *your* tree. Not copy
and create files that aren't used until the next stage. Not pre-merge
with another completely unrelated tree.

Now you actually did extra work apparently to "help", but what it
caused was for me to have to try to figure out those crazy steps, and
it made the history just uglier.

And note that even if I didn't have a conflict (git didn't notice that
there were a few other files that moved too, because the renaming made
the file contents too different), I would still have preferred to
simply do the conflict resolution at the time of the merge, rather
than have it pre-done.

Put another way: you doing the "pre-merge" resolution ends up making
my merge simpler from a purely technical standpoint, but it also makes
the history make very little sense. I'd much rather have the history
make sense, and then in the actual *merge* when the f2fs tree really
meets the crypto tree updates, the resolution fixes things up (and
then the resolution makes sense within those confines too).

And never add files that aren't even used. Now we literally have this
insane situation that commit 7690143802f1 ("fs crypto: add crypto.c
for encrypt/decrypt functions") adds a new directory, and a new file
in it, but there is absolutely nothing that references that new file.
So it's all completely empty work.

Then later, you add the Makefile and Kconfig rules to start building
the files, but still nothing *uses* it.

And then, after that, you have one commit that removes the old code,
and switches over to the new code.

Not only does none of this make ANY SENSE WHAT-SO-EVER, but it's a
complete nightmare if something breaks. The point you *notice* that
things break is when you switch over the new code, but that isn't
actually when the new code happens, so now you have this situation
where the commit that introduces the breakage basically silently
introduces all that old code that was never actually used - but it's
not shown in that commit!

This is why I think you should have just renamed the files one by one,
and actually started using them as you go along. See? Not only would
it make more sense, but when a problem happens, you actually see what
causes it, instead of it being a "oh, we switched over to that
completely different code that had never been used by anything at all
before".

                 Linus

[toc] | [prev] | [next] | [standalone]

#1360315

On Thu, Mar 17, 2016 at 7:32 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
\>
> Then later, you add the Makefile and Kconfig rules to start building
> the files, but still nothing *uses* it.
>
> And then, after that, you have one commit that removes the old code,
> and switches over to the new code.

Oh, and after I actually finished the merge and did the test-compile,
I notice that much of this work has been rebased in the last hour or
two.

No. Just no.

I'm unpulling. None of this makes any sense. I can't take stuff like this.

              Linus

[toc] | [prev] | [next] | [standalone]

#1360338

On Thu, Mar 17, 2016 at 07:32:34PM -0700, Linus Torvalds wrote:
> On Thu, Mar 17, 2016 at 5:58 PM, Jaegeuk Kim <jaegeuk@kernel.org> wrote:
> >
> > Note that, I did cherry-pick one patch and add another patch to resolve the
> > conflict against recent crypto changes.
> 
> *please* don't do things like this!

My apologies.
I'll definitely keep in mind.

And, thank you for explaining what I did so insanely.
I can understand how I made a mistake which makes the git history much uglier
as well as unnecessary commits which make it much harder to investigate any
problem.

Let me undo my recent unnecessary extra work and add a single commit that
migrates the crypto codes.
Then, let me repost a pull request later again.
Sorry for the confusion.

Thanks,

> 
> Now I have a conflict _anyway_, and because you added extra commits it
> ends up being just messier. Git notices that you renamed
> fs/f2fs/crypto_fname.c into fs/crypto/fname.c, and sees that different
> people did different things, so now your "merge resolution" conflicts
> with the changes I already merged to the original file from the crypto
> tree.
> 
> Yes, yes, since you also did that "f2fs: Use skcipher", the resolution
> now is to just throw the extra merge resolution away, but it's
> literally just messier and more work for me, because it makes even
> less sense than it would have done to just do the merge resolution
> directly.
> 
> I do *not* want maintainers of two different trees trying to resolve
> things against each other ahead of time. It makes no sense, and it
> makes the history make no sense.
> 
> Now, if a merge ends up being really painful, I might ask you to help
> resolve a merge, but quite frankly, that is very very rare.
> 
> I'd much *much* rather see you just do stuff that makes sense for your
> tree. In particular, you really should have done that "move file" as a
> single file move commit, and just left it at that. Instead, your tree
> does something insane:
> 
>  - first it cherry-picks the change from the crypto tree
> 
>  - then it *copies* the old file to fs/crypto/fname.c, and changes the
> naming to match.
> 
>  - at this point nothing actually *uses* the new file or new naming at all.
> 
>  - then it "resolves the merge conflict" by re-doing *AGAIN* that
> crypto change that it shouldn't have cared about in the first place
> 
>  - then it removes the old file and switches to the new naming in the users
> 
> That makes no sense what-so-ever.
> 
> What would have been *sensible* would have been to split it up in a
> *natural* way:
> 
>  - move the file to a new subdirectory in one commit (together with
> the Makefile/Kconfig stuff that enables it)
> 
>  - change the naming to match the new location in another.
> 
> or even just do it all in one single step. And at no point worry about
> the fact that the crypto tree did something unrelated to your work.
> 
> You should aim to make the changes make sense in *your* tree. Not copy
> and create files that aren't used until the next stage. Not pre-merge
> with another completely unrelated tree.
> 
> Now you actually did extra work apparently to "help", but what it
> caused was for me to have to try to figure out those crazy steps, and
> it made the history just uglier.
> 
> And note that even if I didn't have a conflict (git didn't notice that
> there were a few other files that moved too, because the renaming made
> the file contents too different), I would still have preferred to
> simply do the conflict resolution at the time of the merge, rather
> than have it pre-done.
> 
> Put another way: you doing the "pre-merge" resolution ends up making
> my merge simpler from a purely technical standpoint, but it also makes
> the history make very little sense. I'd much rather have the history
> make sense, and then in the actual *merge* when the f2fs tree really
> meets the crypto tree updates, the resolution fixes things up (and
> then the resolution makes sense within those confines too).
> 
> And never add files that aren't even used. Now we literally have this
> insane situation that commit 7690143802f1 ("fs crypto: add crypto.c
> for encrypt/decrypt functions") adds a new directory, and a new file
> in it, but there is absolutely nothing that references that new file.
> So it's all completely empty work.
> 
> Then later, you add the Makefile and Kconfig rules to start building
> the files, but still nothing *uses* it.
> 
> And then, after that, you have one commit that removes the old code,
> and switches over to the new code.
> 
> Not only does none of this make ANY SENSE WHAT-SO-EVER, but it's a
> complete nightmare if something breaks. The point you *notice* that
> things break is when you switch over the new code, but that isn't
> actually when the new code happens, so now you have this situation
> where the commit that introduces the breakage basically silently
> introduces all that old code that was never actually used - but it's
> not shown in that commit!
> 
> This is why I think you should have just renamed the files one by one,
> and actually started using them as you go along. See? Not only would
> it make more sense, but when a problem happens, you actually see what
> causes it, instead of it being a "oh, we switched over to that
> completely different code that had never been used by anything at all
> before".
> 
>                  Linus

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web