[GIT PULL] ext4 update for 2.6.37

[GIT PULL] ext4 update for 2.6.37

[Theodore Ts'o]

Hi Linus,

Please pull from:

  git://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git for_linus
or
  git://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git upstream-merge

the merge is somewhat complex, since there's been a lot of work going on
in parallel with discard and zeroout changes, as well as the change
block_prepare_write/__block_write_begin.  The changes pass the xfstests
regression test suite, using both 1k and 4k block sizes --- both before
the upstream merge (the for_linus branch) and after doing a trial merge
with the head of your tree as of Wednesday evening (the upstream-merge
branch).

The changes this time around have two major features, which are
responsible for most of the new lines of code.  One is lazy inode table
initialization, which allows ext4 file systems to be mkfs'ed very
quickly.  The second is changing the I/O submission path so that it uses
the block I/O layer directly.  This makes blktraces much smaller, and
makes ext4 far more scalable.  On the boxacle "large file create"
workload, run with 48 and 192 threads on a 48-core AMD box, ext4 now has
a 3x increase in write throughput, and CPU usage has been reduced by a
factor of 3-4.  Most of this was achieved by reducing spinlock
contention on the block queue submission locks.

We also added support for run-time discard of unused blocks using the
new FITRIM ioctl (which has been run by the linux-fs mailing list as a
generic file system-independent interface).

And, of course, a lot of bug fixes and clean ups.

     		      	     	       	     - Ted

Brian King (1):
      jbd2: Fix I/O hang in jbd2_journal_release_jbd_inode

Curt Wohlgemuth (1):
      ext4: use dedicated slab caches for group_info structures

Dmitry Monakhov (1):
      ext4: optimize orphan_list handling for ext4_setattr

Eric Sandeen (10):
      ext4: stop looping in ext4_num_dirty_pages when max_pages reached
      ext4: don't bump up LONG_MAX nr_to_write by a factor of 8
      ext4: fix oops in trace_ext4_mb_release_group_pa
      ext4: don't use ext4_allocation_contexts for tracing
      ext4: queue conversion after adding to inode's completed IO list
      ext4: remove unused ext4_sb_info members
      ext4: tidy up a void argument in inode.c
      ext4: implement writeback livelock avoidance using page tagging
      ext4: update writeback_index based on last page scanned
      ext4: move ext4_mb_{get,put}_buddy_cache_lock and make them static

Kazuya Mio (1):
      ext4: fix compile error in ext4_fallocate()

Lukas Czerner (11):
      ext4: check for negative error code from sb_issue_discard
      ext4: don't hold spinlock while calling ext4_issue_discard()
      Add helper function for blkdev_issue_zeroout (sb_issue_discard)
      ext4: add support for lazy inode table initialization
      ext4: add interface to advertise ext4 features in sysfs
      ext4: use sb_issue_zeroout in setup_new_group_blocks
      ext4: use sb_issue_zeroout in ext4_ext_zeroout
      ext4: Use return value from sb_issue_discard()
      fs: Add FITRIM ioctl
      ext4: Add batched discard support for ext4
      ext4: add batched_discard into ext4 feature list

Maciej Żenczykowski (1):
      ext4: don't update sb journal_devnum when RO dev

Namhyung Kim (1):
      ext4: Check return value of sb_getblk() and friends

Nicolas Kaiser (1):
      ext4: fix unbalanced mutex unlock in error path of ext4_li_request_new

Sergey Senozhatsky (1):
      ext4: fix NULL pointer dereference in print_daily_error_info()

Theodore Ts'o (18):
      ext4: fix EOFBLOCKS_FL handling
      jbd2: Add sanity check for attempts to start handle during umount
      ext4: avoid uninitialized memory references in ext3_htree_next_block()
      ext4: use search_dirblock() in ext4_dx_find_entry()
      ext4: use KMEM_CACHE instead of kmem_cache_create
      ext4: call mpage_da_submit_io() from mpage_da_map_blocks()
      ext4: simplify ext4_writepage()
      ext4: inline ext4_writepage() into mpage_da_submit_io()
      ext4: inline walk_page_buffers() into mpage_da_submit_io
      ext4: move mpage_put_bnr_to_bhs()'s functionality to mpage_da_submit_io()
      ext4: use bio layer instead of buffer layer in mpage_da_submit_io
      ext4: fix kernel oops if the journal superblock has a non-zero j_errno
      ext4: rename {exit,init}_ext4_*() to ext4_{exit,init}_*()
      ext4: make various ext4 functions be static
      ext4: rename {ext,idx}_pblock and inline small extent functions
      ext4: move flush_completed_IO to fs/ext4/fsync.c and make it static
      ext4: rename mark_bitmap_end() to ext4_mark_bitmap_end()
      ext4,jbd2: convert tracepoints to use major/minor numbers

Toshiyuki Okajima (2):
      ext4: improve llseek error handling for overly large seek offsets
      ext4: fix potential infinite loop in ext4_da_writepages()

Wen Congyang (1):
      ext4: avoid null dereference in trace_ext4_mballoc_discard

 Documentation/filesystems/ext4.txt |   14 +
 fs/ext4/Makefile                   |    2 +-
 fs/ext4/balloc.c                   |    5 +-
 fs/ext4/block_validity.c           |    7 +-
 fs/ext4/dir.c                      |    2 +-
 fs/ext4/ext4.h                     |  110 ++++++--
 fs/ext4/ext4_extents.h             |   65 ++++-
 fs/ext4/extents.c                  |  369 ++++++++++-------------
 fs/ext4/file.c                     |   44 +++-
 fs/ext4/fsync.c                    |   83 +++++
 fs/ext4/ialloc.c                   |  136 ++++++++-
 fs/ext4/inode.c                    |  587 +++++++++++++-----------------------
 fs/ext4/mballoc.c                  |  554 ++++++++++++++++++++++------------
 fs/ext4/migrate.c                  |    2 +-
 fs/ext4/move_extent.c              |   22 +-
 fs/ext4/namei.c                    |   63 ++---
 fs/ext4/page-io.c                  |  430 ++++++++++++++++++++++++++
 fs/ext4/resize.c                   |   53 +---
 fs/ext4/super.c                    |  531 +++++++++++++++++++++++++++++++--
 fs/ext4/xattr.c                    |    4 +-
 fs/ext4/xattr.h                    |    8 +-
 fs/ioctl.c                         |   39 +++
 fs/jbd2/checkpoint.c               |   10 +
 fs/jbd2/commit.c                   |   12 +-
 fs/jbd2/journal.c                  |    4 +-
 fs/jbd2/transaction.c              |    1 +
 include/linux/blkdev.h             |    8 +
 include/linux/fs.h                 |    8 +
 include/linux/jbd2.h               |    2 +-
 include/linux/percpu_counter.h     |   10 +
 include/linux/writeback.h          |    2 +
 include/trace/events/ext4.h        |  378 ++++++++++++++---------
 include/trace/events/jbd2.h        |   78 +++--
 33 files changed, 2513 insertions(+), 1130 deletions(-)

Re: [GIT PULL] ext4 update for 2.6.37

[Markus Trippelsdorf]

On Thu, Oct 28, 2010 at 12:52:07AM -0400, Theodore Ts'o wrote:
> Hi Linus,
> 
> Please pull from:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git for_linus
> or
>   git://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git upstream-merge
> 
> the merge is somewhat complex, since there's been a lot of work going on
> in parallel with discard and zeroout changes, as well as the change
> block_prepare_write/__block_write_begin.  The changes pass the xfstests
> regression test suite, using both 1k and 4k block sizes --- both before
> the upstream merge (the for_linus branch) and after doing a trial merge
> with the head of your tree as of Wednesday evening (the upstream-merge
> branch).

This misses a trivial ifdef wrapper in fs/ext4/super.c for
!CONFIG_EXT4_FS_XATTR configurations. Something like this:

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 0348ce0..5f82585 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -4773,9 +4773,12 @@ static int __init ext4_init_fs(void)
 	if (err)
 		goto out3;
 
+#ifdef CONFIG_EXT4_FS_XATTR
 	err = ext4_init_xattr();
+#endif
 	if (err)
 		goto out2;
+
 	err = init_inodecache();
 	if (err)
 		goto out1;
-- 
Markus

-tip: origin tree build failure (was: [GIT PULL] ext4 update for 2.6.37)

[Ingo Molnar]

hi Ted,

> Theodore Ts'o (18):
>       ext4: rename {exit,init}_ext4_*() to ext4_{exit,init}_*()

Today's -tip fails to build due to upstream commit 5dabfc7 ("ext4: rename 
{exit,init}_ext4_*() to ext4_{exit,init}_*()"), on all[yes/mod]config with 
CONFIG_EXT4_FS_XATTR disabled:

  fs/ext4/super.c:4776: error: implicit declaration of function ‘ext4_init_xattr’

Commit 5dabfc7 renamed init_ext4_xattr to ext4_init_xattr but forgot to update the 
definition in fs/ext4/xattr.h. The patch below fixes it.

Thanks,

	Ingo

Signed-off-by: Ingo Molnar <mingo@elte.hu>

--- linux.orig/fs/ext4/xattr.h
+++ linux/fs/ext4/xattr.h
@@ -122,7 +122,7 @@ ext4_xattr_put_super(struct super_block 
 }
 
 static __init inline int
-init_ext4_xattr(void)
+ext4_init_xattr(void)
 {
 	return 0;
 }

Re: -tip: origin tree build failure (was: [GIT PULL] ext4 update for 2.6.37)

[Theodore Tso]

On Oct 28, 2010, at 3:56 AM, Ingo Molnar wrote:

> 
> hi Ted,
> 
>> Theodore Ts'o (18):
>>      ext4: rename {exit,init}_ext4_*() to ext4_{exit,init}_*()
> 
> Today's -tip fails to build due to upstream commit 5dabfc7 ("ext4: rename 
> {exit,init}_ext4_*() to ext4_{exit,init}_*()"), on all[yes/mod]config with 
> CONFIG_EXT4_FS_XATTR disabled:
> 
>  fs/ext4/super.c:4776: error: implicit declaration of function ‘ext4_init_xattr’
> 
> Commit 5dabfc7 renamed init_ext4_xattr to ext4_init_xattr but forgot to update the 
> definition in fs/ext4/xattr.h. The patch below fixes it.

Acked-by: "Theodore Ts'o" <tytso@mit.edu>

Thanks for catching this, my bad.

-- Ted

Re: -tip: origin tree build failure (was: [GIT PULL] ext4 update for 2.6.37)

[Linus Torvalds]

On Thu, Oct 28, 2010 at 12:56 AM, Ingo Molnar <mingo@elte.hu> wrote:
>
> Today's -tip fails to build due to upstream commit 5dabfc7 ("ext4: rename
> {exit,init}_ext4_*() to ext4_{exit,init}_*()"), on all[yes/mod]config with
> CONFIG_EXT4_FS_XATTR disabled:

Btw, could you try to write these things so that the changelog doesn't
have to be totally rewritten? Now I always end up having to move
things around and edit them to be useful from a long-term perspective,
which is kind of silly.

                              Linus

Re: -tip: origin tree build failure (was: [GIT PULL] ext4 update for 2.6.37)

[Ingo Molnar]

* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Thu, Oct 28, 2010 at 12:56 AM, Ingo Molnar <mingo@elte.hu> wrote:
> >
> > Today's -tip fails to build due to upstream commit 5dabfc7 ("ext4: rename 
> > {exit,init}_ext4_*() to ext4_{exit,init}_*()"), on all[yes/mod]config with 
> > CONFIG_EXT4_FS_XATTR disabled:
> 
> Btw, could you try to write these things so that the changelog doesn't have to be 
> totally rewritten? Now I always end up having to move things around and edit them 
> to be useful from a long-term perspective, which is kind of silly.

Heh, i was just lazy and used sfr's linux-next build bug reporting mails as a 
template ;-)

Point taken in any case, i'll read your edited changelog and will change the 
template accordingly.

Would this:

  Upstream commit 5dabfc7 ("ext4: rename {exit,init}_ext4_*() to 
  ext4_{exit,init}_*()"), breaks the build on all[yes/mod]config with
  CONFIG_EXT4_FS_XATTR disabled:

  ...

have been better?

Thanks,

	Ingo

Re: -tip: origin tree build failure (was: [GIT PULL] ext4 update for 2.6.37)

[Ted Ts'o]

Linus, if it's helpful, I have a pull request with correctly worded
changelogs here:

  git://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git for_linus

My apologies again for screwing up these configs.  The following has
been tested with allnoconfig, and I'm currently in the process of
testing allyesconfig (which I'm fairly confident will compile, or at
least if it fails, it won't be due to changes that came in via my
tree).
		     	    		       - Ted

Ingo Molnar (2):
      ext4: Fix build when !CONFIG_EXT4_FS_XATTR
      fs: build fix when !CONFIG_BLOCK

 fs/ext4/xattr.h    |    2 +-
 include/linux/fs.h |    1 +
 2 files changed, 2 insertions(+), 1 deletions(-)

Re: -tip: origin tree build failure (was: [GIT PULL] ext4 update for 2.6.37)

[Linus Torvalds]

On Thu, Oct 28, 2010 at 9:38 AM, Ingo Molnar <mingo@elte.hu> wrote:
>
> Point taken in any case, i'll read your edited changelog and will change the
> template accordingly.
>
> Would this:
>
>  Upstream commit 5dabfc7 ("ext4: rename {exit,init}_ext4_*() to
>  ext4_{exit,init}_*()"), breaks the build on all[yes/mod]config with
>  CONFIG_EXT4_FS_XATTR disabled:
>
>  ...
>
> have been better?

Yes. Except for the kernel the default git commit abbreviation is
borderline too short. Seven hex-chars can easily alias with a few more
pulls from me: git will not give aliases at the time it gives a
shorthand, but a month or two later the abbreviated commit may no
longer be unique.

So I suggest using --abbrev=12 or similar.

What I ended up writing your commit as was this:

    ext4: fix compile with CONFIG_EXT4_FS_XATTR disabled

    Commit 5dabfc78dced ("ext4: rename {exit,init}_ext4_*() to
    ext4_{exit,init}_*()") causes

      fs/ext4/super.c:4776: error: implicit declaration of function
‘ext4_init_xattr’

    when CONFIG_EXT4_FS_XATTR is disabled.

    It renamed init_ext4_xattr to ext4_init_xattr but forgot to update the
    dummy definition in fs/ext4/xattr.h.

    Signed-off-by: Ingo Molnar <mingo@elte.hu>
    Acked-by: "Theodore Ts'o" <tytso@mit.edu>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

but that's just me.

                   Linus

Re: -tip: origin tree build failure (was: [GIT PULL] ext4 update for 2.6.37)

[Ingo Molnar]

* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Thu, Oct 28, 2010 at 9:38 AM, Ingo Molnar <mingo@elte.hu> wrote:
> >
> > Point taken in any case, i'll read your edited changelog and will change the
> > template accordingly.
> >
> > Would this:
> >
> >  Upstream commit 5dabfc7 ("ext4: rename {exit,init}_ext4_*() to
> >  ext4_{exit,init}_*()"), breaks the build on all[yes/mod]config with
> >  CONFIG_EXT4_FS_XATTR disabled:
> >
> >  ...
> >
> > have been better?
> 
> Yes. Except for the kernel the default git commit abbreviation is borderline too 
> short. Seven hex-chars can easily alias with a few more pulls from me: git will 
> not give aliases at the time it gives a shorthand, but a month or two later the 
> abbreviated commit may no longer be unique.
> 
> So I suggest using --abbrev=12 or similar.

ok. A helper script i use does this:

   git log --pretty=format:"%h: %s" $@

I have added --abbrev=12. Might make sense to lengthen the %h default in upstream 
Git as well?

> What I ended up writing your commit as was this:
> 
>     ext4: fix compile with CONFIG_EXT4_FS_XATTR disabled
> 
>     Commit 5dabfc78dced ("ext4: rename {exit,init}_ext4_*() to
>     ext4_{exit,init}_*()") causes
> 
>       fs/ext4/super.c:4776: error: implicit declaration of function ‘ext4_init_xattr’
> 
>     when CONFIG_EXT4_FS_XATTR is disabled.
> 
>     It renamed init_ext4_xattr to ext4_init_xattr but forgot to update the
>     dummy definition in fs/ext4/xattr.h.
> 
>     Signed-off-by: Ingo Molnar <mingo@elte.hu>
>     Acked-by: "Theodore Ts'o" <tytso@mit.edu>
>     Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> 
> but that's just me.

Ok, this indeed is much nicer to read.

Thanks,

	Ingo

Minimum git commit abbrev length (Was Re: -tip: origin tree build failure (was: [GIT PULL] ext4 update) for 2.6.37)

[Ted Ts'o]

On Thu, Oct 28, 2010 at 07:17:01PM +0200, Ingo Molnar wrote:
> > Linus Torvalds <torvalds@linux-foundation.org> wrote:
> > Yes. Except for the kernel the default git commit abbreviation is
> > borderline too short. Seven hex-chars can easily alias with a few
> > more pulls from me: git will not give aliases at the time it gives
> > a shorthand, but a month or two later the abbreviated commit may
> > no longer be unique.
> > 
> > So I suggest using --abbrev=12 or similar.
> 
> ok. A helper script i use does this:
> 
>    git log --pretty=format:"%h: %s" $@
> 
> I have added --abbrev=12. Might make sense to lengthen the %h
> default in upstream Git as well?

Maybe the right thing to do is add a git config option which allows
for a configurable minimum git commit abbreviation length?

      		   	       	      - Ted

Re: Minimum git commit abbrev length (Was Re: -tip: origin tree build failure (was: [GIT PULL] ext4 update) for 2.6.37)

[Linus Torvalds]

[-- Attachment #1: Type: text/plain, Size: 2132 bytes --]

On Thu, Oct 28, 2010 at 10:27 AM, Ted Ts'o <tytso@mit.edu> wrote:
> On Thu, Oct 28, 2010 at 07:17:01PM +0200, Ingo Molnar wrote:
>> > Linus Torvalds <torvalds@linux-foundation.org> wrote:
>> > Yes. Except for the kernel the default git commit abbreviation is
>> > borderline too short. Seven hex-chars can easily alias with a few
>> > more pulls from me: git will not give aliases at the time it gives
>> > a shorthand, but a month or two later the abbreviated commit may
>> > no longer be unique.
>> >
>> > So I suggest using --abbrev=12 or similar.
>>
>> ok. A helper script i use does this:
>>
>>    git log --pretty=format:"%h: %s" $@
>>
>> I have added --abbrev=12. Might make sense to lengthen the %h
>> default in upstream Git as well?
>
> Maybe the right thing to do is add a git config option which allows
> for a configurable minimum git commit abbreviation length?

Yes. The default of 7 (I think) comes from fairly early in git
development, when seven hex digits was a lot (it covers about 250+
million hash values). Back then I thought that 65k revisions was a lot
(it was what we were about to hit in BK), and each revision tends to
be about 5-10 new objects or so, so a million objects was a big
number.

These days, the kernel isn't even the largest git project, and even
the kernel has about 220k revisions (_much_ bigger than the BK tree
ever was) and we are approaching two million objects. At that point,
seven hex digits is still unique for a lot of them, but when we're
talking about just two orders of magnitude difference between number
of objects and the hash size, there _will_ be hash collisions. It's no
longer even close to unrealistic - it happens all the time.

So I suspect we should both increase the default abbrev that was
unrealistically small, _and_ add a way for people to set their own
default per-project in the git config file.

Maybe something like the attached (not necessarily well-thought-out or
well-tested: I also didn't actually change the default, although I
suspect we should up it from 7 to at least 10).

                        Linus

[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 2200 bytes --]

 builtin/describe.c |    2 +-
 cache.h            |    5 +++--
 config.c           |    8 ++++++++
 environment.c      |    1 +
 4 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/builtin/describe.c b/builtin/describe.c
index 43caff2..2d98702 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -20,7 +20,7 @@ static int debug;	/* Display lots of verbose info */
 static int all;	/* Any valid ref can be used */
 static int tags;	/* Allow lightweight tags */
 static int longformat;
-static int abbrev = DEFAULT_ABBREV;
+static int abbrev = 7;	/* NOTE! Not DEFAULT_ABBREV */
 static int max_candidates = 10;
 static int found_names;
 static const char *pattern;
diff --git a/cache.h b/cache.h
index 33decd9..6c28a81 100644
--- a/cache.h
+++ b/cache.h
@@ -540,6 +540,7 @@ extern int trust_executable_bit;
 extern int trust_ctime;
 extern int quote_path_fully;
 extern int has_symlinks;
+extern int minimum_abbrev, default_abbrev;
 extern int ignore_case;
 extern int assume_unchanged;
 extern int prefer_symlink_refs;
@@ -757,8 +758,8 @@ static inline unsigned int hexval(unsigned char c)
 }
 
 /* Convert to/from hex/sha1 representation */
-#define MINIMUM_ABBREV 4
-#define DEFAULT_ABBREV 7
+#define MINIMUM_ABBREV minimum_abbrev
+#define DEFAULT_ABBREV default_abbrev
 
 struct object_context {
 	unsigned char tree[20];
diff --git a/config.c b/config.c
index 4b0a820..474361c 100644
--- a/config.c
+++ b/config.c
@@ -514,6 +514,14 @@ static int git_default_core_config(const char *var, const char *value)
 		return 0;
 	}
 
+	if (!strcmp(var, "core.abbrev")) {
+		int abbrev = git_config_int(var, value);
+		if (abbrev < minimum_abbrev || abbrev > 40)
+			return -1;
+		default_abbrev = abbrev;
+		return 0;
+	}
+
 	if (!strcmp(var, "core.loosecompression")) {
 		int level = git_config_int(var, value);
 		if (level == -1)
diff --git a/environment.c b/environment.c
index de5581f..b98003c 100644
--- a/environment.c
+++ b/environment.c
@@ -15,6 +15,7 @@ int user_ident_explicitly_given;
 int trust_executable_bit = 1;
 int trust_ctime = 1;
 int has_symlinks = 1;
+int minimum_abbrev = 4, default_abbrev = 7;
 int ignore_case;
 int assume_unchanged;
 int prefer_symlink_refs;

Re: Minimum git commit abbrev length (Was Re: -tip: origin tree build failure (was: [GIT PULL] ext4 update) for 2.6.37)

[Linus Torvalds]

On Thu, Oct 28, 2010 at 11:28 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Yes. The default of 7 (I think) comes from fairly early in git
> development, when seven hex digits was a lot (it covers about 250+
> million hash values). Back then I thought that 65k revisions was a lot
> (it was what we were about to hit in BK), and each revision tends to
> be about 5-10 new objects or so, so a million objects was a big
> number.
>
> These days, the kernel isn't even the largest git project, and even
> the kernel has about 220k revisions (_much_ bigger than the BK tree
> ever was) and we are approaching two million objects. At that point,
> seven hex digits is still unique for a lot of them, but when we're
> talking about just two orders of magnitude difference between number
> of objects and the hash size, there _will_ be hash collisions. It's no
> longer even close to unrealistic - it happens all the time.

Hmm. In fact, in the kernel, we currently have about twelve thousand
objects that end up having collisions in 7 hex digits. Even in the old
historical BK kernel tree, we have over a thousand objects that
collide (each bucket in both cases gets just two objects, there are as
of yet no multiple collisions, which is what you'd expect with a good
hash). See with

  git rev-list --objects --all | cut -c1-7 | sort | uniq -dc

and in fact git itself has a few collisions (but currently just 44
objects ending up sharing 22 SHA1 buckets in 7 digits).

With each digit, you'd expect the collisions to decrease by a factor
of 16, and that is indeed exactly what happens. For my current kernel
tree I get:

 - 7 digits: 5823 buckets with duplicates (ie 11646 objects that aren't unique)
 - 8: 406
 - 9: 30
 - 10: 1
 - 11: 0

so 12 hex digits is indeed pretty safe for the kernel, and is likely
to remain so until the kernel history grows by a factor of 16.

                        Linus

Re: -tip: origin tree build failure

[Junio C Hamano]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> Yes. Except for the kernel the default git commit abbreviation is
> borderline too short. Seven hex-chars can easily alias with a few more
> pulls from me: git will not give aliases at the time it gives a
> shorthand, but a month or two later the abbreviated commit may no
> longer be unique.
>
> So I suggest using --abbrev=12 or similar.

Would a new configuration to specify how many more letters to ensure the
uniqueness at the time of generation make sense?

By the way, I noticed that you started sending patches as attachments
lately.  What made you change your mind?

 Documentation/config.txt |    9 +++++++++
 cache.h                  |    1 +
 config.c                 |    7 +++++++
 environment.c            |    1 +
 sha1_name.c              |    4 +++-
 5 files changed, 21 insertions(+), 1 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 538ebb5..6994338 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -374,6 +374,15 @@ core.warnAmbiguousRefs::
 	If true, git will warn you if the ref name you passed it is ambiguous
 	and might match multiple refs in the .git/refs/ tree. True by default.
 
+core.abbrevguard::
+	Even though git makes sure that it uses enough hexdigits to show
+	an abbreviated object name unambiguously, as more objects are
+	added to the repository over time, a short name that used to be
+	unique will stop being unique.  Git uses this many extra hexdigits
+	that are more than necessary to make the object name currently
+	unique, in the hope that its output will stay unique a bit longer.
+	Defaults to 0.
+
 core.compression::
 	An integer -1..9, indicating a default compression level.
 	-1 is the zlib default. 0 means no compression,
diff --git a/cache.h b/cache.h
index 33decd9..931fb59 100644
--- a/cache.h
+++ b/cache.h
@@ -545,6 +545,7 @@ extern int assume_unchanged;
 extern int prefer_symlink_refs;
 extern int log_all_ref_updates;
 extern int warn_ambiguous_refs;
+extern int unique_abbrev_extra_length;
 extern int shared_repository;
 extern const char *apply_default_whitespace;
 extern const char *apply_default_ignorewhitespace;
diff --git a/config.c b/config.c
index 4b0a820..1aa72c2 100644
--- a/config.c
+++ b/config.c
@@ -489,6 +489,13 @@ static int git_default_core_config(const char *var, const char *value)
 		return 0;
 	}
 
+	if (!strcmp(var, "core.abbrevguard")) {
+		unique_abbrev_extra_length = git_config_int(var, value);
+		if (unique_abbrev_extra_length < 0)
+			unique_abbrev_extra_length = 0;
+		return 0;
+	}
+
 	if (!strcmp(var, "core.bare")) {
 		is_bare_repository_cfg = git_config_bool(var, value);
 		return 0;
diff --git a/environment.c b/environment.c
index de5581f..92e16b1 100644
--- a/environment.c
+++ b/environment.c
@@ -21,6 +21,7 @@ int prefer_symlink_refs;
 int is_bare_repository_cfg = -1; /* unspecified */
 int log_all_ref_updates = -1; /* unspecified */
 int warn_ambiguous_refs = 1;
+int unique_abbrev_extra_length;
 int repository_format_version;
 const char *git_commit_encoding;
 const char *git_log_output_encoding;
diff --git a/sha1_name.c b/sha1_name.c
index 484081d..4a226ad 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -206,7 +206,9 @@ const char *find_unique_abbrev(const unsigned char *sha1, int len)
 		if (exists
 		    ? !status
 		    : status == SHORT_NAME_NOT_FOUND) {
-			hex[len] = 0;
+			int cut_at = len + unique_abbrev_extra_length;
+			cut_at = (cut_at < 40) ? cut_at : 40;
+			hex[cut_at] = 0;
 			return hex;
 		}
 		len++;

Re: -tip: origin tree build failure

[Linus Torvalds]

On Thu, Oct 28, 2010 at 2:39 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> By the way, I noticed that you started sending patches as attachments
> lately.  What made you change your mind?

Nothing. I still hate them. But the tools I use (web interface to
gmail) are broken in this respect. There's no way to include a file,
or specify that an attachement should be inlined. And don't tell me
about IMAP - if I wanted to use IMAP, I'd be living in a padded cell.

I have a deep love/hate relationship with gmail. Many things make it
wonderful, and I'm not regretting the switch (which was initially just
a trial while traveling).

But it has two issues that I absolutely detest:
 (a) the idiotic inability to inline attachements and
 (b) the android gmail app is a total piece of shit and cannot even do
simple text messages. Crazy.
(there are other small annoyances, but they are smallish in comparison
to the above big honking bugs)

Does anybody know anybody who works on the google mail clients and
could raise these as bugs inside google?

                             Linus

Re: -tip: origin tree build failure

[Ingo Molnar]

* Len Brown <lenb@kernel.org> wrote:

> ...
> > build failure on x86, for CONFIG_ACPI=n && CONFIG_IPMI_SI=y:
> > 
> >  drivers/char/ipmi/ipmi_si_intf.c:3208: error: 'ipmi_pnp_driver' undeclared (first use in this function)
> ...
> 
> > -#ifdef CONFIG_PNP
> > +#ifdef CONFIG_ACPI
> >  	pnp_register_driver(&ipmi_pnp_driver);
> >  #endif
> ...
> 
> Hi Ingo,
> As my tree inflicted this one and I don't see your patch upstream,
> I'll include your fix with my "for -rc2" push.

Thanks!

	Ingo

Re: -tip: origin tree build failure

[Henrique de Moraes Holschuh]

On Sat, 26 Dec 2009, Len Brown wrote:
> As my tree inflicted this one and I don't see your patch upstream,
> I'll include your fix with my "for -rc2" push.

I will be sending you an alternate patch in a few minutes, anyway.  If
you already sent the pull request, just let me know as that means I need
to adjust my tree.

Instead of depending on ALSA, I made the entire feature that depends on
ALSA optional.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: -tip: origin tree build failure

[Len Brown]

...
> build failure on x86, for CONFIG_ACPI=n && CONFIG_IPMI_SI=y:
> 
>  drivers/char/ipmi/ipmi_si_intf.c:3208: error: 'ipmi_pnp_driver' undeclared (first use in this function)
...

> -#ifdef CONFIG_PNP
> +#ifdef CONFIG_ACPI
>  	pnp_register_driver(&ipmi_pnp_driver);
>  #endif
...

Hi Ingo,
As my tree inflicted this one and I don't see your patch upstream,
I'll include your fix with my "for -rc2" push.

thanks,
-Len Brown, Intel Open Source Technology Center

Re: -tip: origin tree build failure

[Geert Uytterhoeven]

On Thu, Dec 17, 2009 at 08:50, Ingo Molnar <mingo@elte.hu> wrote:
> Today's -tip failed to build because commit
> 9e368fa011d4e0aa050db348d69514900520e40b ("ipmi: add PNP discovery (ACPI
> namespace via PNPACPI)") from today's upstream kernel causes the following
> build failure on x86, for CONFIG_ACPI=n && CONFIG_IPMI_SI=y:
>
>  drivers/char/ipmi/ipmi_si_intf.c:3208: error: 'ipmi_pnp_driver' undeclared (first use in this function)
>  drivers/char/ipmi/ipmi_si_intf.c:3208: error: (Each undeclared identifier is reported only once
>  drivers/char/ipmi/ipmi_si_intf.c:3208: error: for each function it appears in.)
>  drivers/char/ipmi/ipmi_si_intf.c:3334: error: 'ipmi_pnp_driver' undeclared (first use in this function)
>
> The reason is that the ipmi_pnp_driver depends on ACPI facilities and is only
> made available under ACPI - while the registration and unregistration is made
> dependent on CONFIG_PNP:
>
>  #ifdef CONFIG_PNP
>        pnp_register_driver(&ipmi_pnp_driver);
>  #endif
>
> The solution is to only register this driver under ACPI. (Also, the CONFIG_PNP
> dependency is not needed because pnp_register_driver() is stubbed out in the
> !CONFIG_PNP case.)
>
> I've applied the patch below to tip:out-of-tree for now.

Any chance we can see this fixed in mainline soon, so allmodconfig builds again
on non-ACPI platforms? Thx!

> Thanks,
>
>        Ingo
>
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
>
> diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
> index 679cd08..176f175 100644
> --- a/drivers/char/ipmi/ipmi_si_intf.c
> +++ b/drivers/char/ipmi/ipmi_si_intf.c
> @@ -3204,7 +3204,7 @@ static __devinit int init_ipmi_si(void)
>  #ifdef CONFIG_ACPI
>        spmi_find_bmc();
>  #endif
> -#ifdef CONFIG_PNP
> +#ifdef CONFIG_ACPI
>        pnp_register_driver(&ipmi_pnp_driver);
>  #endif
>
> @@ -3330,7 +3330,7 @@ static __exit void cleanup_ipmi_si(void)
>  #ifdef CONFIG_PCI
>        pci_unregister_driver(&ipmi_pci_driver);
>  #endif
> -#ifdef CONFIG_PNP
> +#ifdef CONFIG_ACPI
>        pnp_unregister_driver(&ipmi_pnp_driver);
>  #endif
>

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: -tip: origin tree build failure

[Andi Kleen]

> No, your patch is not the correct fix. As i said, PROC_PAGE_MONITOR provides 
> the facility and mm/memory-failure.c uses that facility - and your patch does 
> not solve that fundamental dependency issue, it just fudges around the 
> dependencies.

Thanks for the report.

The filter is really part of the injector, even if it happens to be 
in the other file. I first resisted adding an ifdef to it, but it seems
we need it. I think I forgot to set to set CONFIG_EMBEDDED earlier,
that is what made the earlier build test pass even when it shouldn't have.

This updated patch should fix it. I tried to build test all applicable
combinations, if anything is still missing please let me know.

-Andi

---

HWPOISON: Add PROC_FS dependency to hwpoison injector v2

The injector filter requires stable_page_flags() which is supplied
by procfs. So make it dependent on that.

Also add ifdefs around the filter code in memory-failure.c so that
when the filter is disabled due to missing dependencies the whole
code still builds. 

Reported-by: Ingo Molnar 
Signed-off-by: Andi Kleen <ak@linux.intel.com>

---
 mm/Kconfig          |    2 +-
 mm/memory-failure.c |    9 +++++++++
 2 files changed, 10 insertions(+), 1 deletion(-)

Index: linux/mm/Kconfig
===================================================================
--- linux.orig/mm/Kconfig
+++ linux/mm/Kconfig
@@ -252,7 +252,7 @@ config MEMORY_FAILURE
 
 config HWPOISON_INJECT
 	tristate "HWPoison pages injector"
-	depends on MEMORY_FAILURE && DEBUG_KERNEL
+	depends on MEMORY_FAILURE && DEBUG_KERNEL && PROC_FS
 	select PROC_PAGE_MONITOR
 
 config NOMMU_INITIAL_TRIM_EXCESS
Index: linux/mm/memory-failure.c
===================================================================
--- linux.orig/mm/memory-failure.c
+++ linux/mm/memory-failure.c
@@ -52,6 +52,8 @@ int sysctl_memory_failure_recovery __rea
 
 atomic_long_t mce_bad_pages __read_mostly = ATOMIC_LONG_INIT(0);
 
+#if defined(CONFIG_HWPOISON_INJECT) || defined(CONFIG_HWPOISON_INJECT_MODULE)
+
 u32 hwpoison_filter_enable = 0;
 u32 hwpoison_filter_dev_major = ~0U;
 u32 hwpoison_filter_dev_minor = ~0U;
@@ -164,6 +166,13 @@ int hwpoison_filter(struct page *p)
 
 	return 0;
 }
+#else
+int hwpoison_filter(struct page *p)
+{
+	return 0;
+}
+#endif
+
 EXPORT_SYMBOL_GPL(hwpoison_filter);
 
 /*

Re: -tip: origin tree build failure

[Ingo Molnar]

* Andi Kleen <andi@firstfloor.org> wrote:

> Ingo Molnar <mingo@elte.hu> writes:
> 
> > * Ingo Molnar <mingo@elte.hu> wrote:
> >
> >>  config MEMORY_FAILURE
> >>  	depends on MMU
> >>  	depends on ARCH_SUPPORTS_MEMORY_FAILURE
> >> +	select PROC_PAGE_MONITOR
> >>  	bool "Enable recovery from hardware memory errors"
> >
> > It also needs to depend on PROC_FS in that case - as per the updated patch 
> > below.
> 
> Thanks for the report.
> 
> MEMORY_FAILURE itself doesn't depend on the page flags, just the injector 
> which has a separate config option. It already has a select 
> PROC_PAGE_MONITOR, but the proc dependency is indeed missing.
> 
> I think the correct fix is the appended patch.

No, your patch is not the correct fix. As i said, PROC_PAGE_MONITOR provides 
the facility and mm/memory-failure.c uses that facility - and your patch does 
not solve that fundamental dependency issue, it just fudges around the 
dependencies.

So your patch can still produce the same kind of build failure (with other 
Kconfig variations):

 mm/built-in.o: In function `hwpoison_filter':
 (.text+0x430ba): undefined reference to `stable_page_flags'

This should be cleaned up for real - there's no reason why stable_page_flags() 
(which is an MM facility) should live in fs/proc/. That's the root of the 
problem.

Also a few more (very small) nits:

> HWPOISON: Add PROC_FS dependency to hwpoison injector

It would be really useful if hwpoison commit titles were in lower case, like 
most other commits in the kernel do? Upper-case is used in special cases - IMO 
this case does not qualify.

> The injector filter requires stable_page_flags() which is supplied by 
> procfs. So make it dependent on that.
> 
> Reported by Ingo Molnar

FYI, we have a Reported-by tag that can be (and should be) used in such cases.

Thanks,

	Ingo

Re: -tip: origin tree build failure

[Myron Stowe]

On Thu, 2009-12-17 at 08:50 +0100, Ingo Molnar wrote:
> Today's -tip failed to build because commit 
> 9e368fa011d4e0aa050db348d69514900520e40b ("ipmi: add PNP discovery (ACPI 
> namespace via PNPACPI)") from today's upstream kernel causes the following 
> build failure on x86, for CONFIG_ACPI=n && CONFIG_IPMI_SI=y:
> 
>  drivers/char/ipmi/ipmi_si_intf.c:3208: error: 'ipmi_pnp_driver' undeclared (first use in this function)
>  drivers/char/ipmi/ipmi_si_intf.c:3208: error: (Each undeclared identifier is reported only once
>  drivers/char/ipmi/ipmi_si_intf.c:3208: error: for each function it appears in.)
>  drivers/char/ipmi/ipmi_si_intf.c:3334: error: 'ipmi_pnp_driver' undeclared (first use in this function)
> 
> The reason is that the ipmi_pnp_driver depends on ACPI facilities and is only 
> made available under ACPI - while the registration and unregistration is made 
> dependent on CONFIG_PNP:
> 
>  #ifdef CONFIG_PNP
>  	pnp_register_driver(&ipmi_pnp_driver);
>  #endif
> 
> The solution is to only register this driver under ACPI. (Also, the CONFIG_PNP 
> dependency is not needed because pnp_register_driver() is stubbed out in the 
> !CONFIG_PNP case.)

Yes, sorry we missed this case.
> 
> I've applied the patch below to tip:out-of-tree for now.

Looks good to us.  Thanks!

Myron
> 
> Thanks,
> 
> 	Ingo
> 
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
> 
> diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
> index 679cd08..176f175 100644
> --- a/drivers/char/ipmi/ipmi_si_intf.c
> +++ b/drivers/char/ipmi/ipmi_si_intf.c
> @@ -3204,7 +3204,7 @@ static __devinit int init_ipmi_si(void)
>  #ifdef CONFIG_ACPI
>  	spmi_find_bmc();
>  #endif
> -#ifdef CONFIG_PNP
> +#ifdef CONFIG_ACPI
>  	pnp_register_driver(&ipmi_pnp_driver);
>  #endif
>  
> @@ -3330,7 +3330,7 @@ static __exit void cleanup_ipmi_si(void)
>  #ifdef CONFIG_PCI
>  	pci_unregister_driver(&ipmi_pci_driver);
>  #endif
> -#ifdef CONFIG_PNP
> +#ifdef CONFIG_ACPI
>  	pnp_unregister_driver(&ipmi_pnp_driver);
>  #endif
>  
> 


-- 
Myron Stowe                             HP Open Source Linux Lab (OSLL)

Re: -tip: origin tree build failure

[Andi Kleen]

Ingo Molnar <mingo@elte.hu> writes:

> * Ingo Molnar <mingo@elte.hu> wrote:
>
>>  config MEMORY_FAILURE
>>  	depends on MMU
>>  	depends on ARCH_SUPPORTS_MEMORY_FAILURE
>> +	select PROC_PAGE_MONITOR
>>  	bool "Enable recovery from hardware memory errors"
>
> It also needs to depend on PROC_FS in that case - as per the updated patch 
> below.

Thanks for the report.

MEMORY_FAILURE itself doesn't depend on the page flags, just the
injector which has a separate config option. It already has a select
PROC_PAGE_MONITOR, but the proc dependency is indeed missing.

I think the correct fix is the appended patch.

-Andi

---

HWPOISON: Add PROC_FS dependency to hwpoison injector

The injector filter requires stable_page_flags() which is supplied
by procfs. So make it dependent on that.

Reported by Ingo Molnar

Signed-off-by: Andi Kleen <ak@linux.intel.com>

---
 mm/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux/mm/Kconfig
===================================================================
--- linux.orig/mm/Kconfig
+++ linux/mm/Kconfig
@@ -252,7 +252,7 @@ config MEMORY_FAILURE
 
 config HWPOISON_INJECT
 	tristate "HWPoison pages injector"
-	depends on MEMORY_FAILURE && DEBUG_KERNEL
+	depends on MEMORY_FAILURE && DEBUG_KERNEL && PROC_FS
 	select PROC_PAGE_MONITOR
 
 config NOMMU_INITIAL_TRIM_EXCESS

Re: -tip: origin tree build failure

[Ingo Molnar]

* Ingo Molnar <mingo@elte.hu> wrote:

>  config MEMORY_FAILURE
>  	depends on MMU
>  	depends on ARCH_SUPPORTS_MEMORY_FAILURE
> +	select PROC_PAGE_MONITOR
>  	bool "Enable recovery from hardware memory errors"

It also needs to depend on PROC_FS in that case - as per the updated patch 
below.

	Ingo

----------------->
>From 896358c0554c56398d40191dda24da8c4b8029c4 Mon Sep 17 00:00:00 2001
From: Ingo Molnar <mingo@elte.hu>
Date: Thu, 17 Dec 2009 10:40:41 +0100
Subject: [PATCH] hwpoison: fix build failure

Today's -tip fails to build due to upstream commit
1a9b5b7fe0c5dad8a635288882d36785dea742f9 ("mm: export stable
page flags")  (authored and merged yesterday) causing the
following build failure on x86  when CONFIG_PROC_PAGE_MONITOR is
disabled:

 mm/built-in.o: In function `hwpoison_filter':
  (.text+0x39fbf): undefined reference to `stable_page_flags'

The bug is that the stable_page_flags() API is only available
under  CONFIG_PROC_PAGE_MONITOR, but utilized in
mm/memory-failure.c unconditionally.

I've applied the patch below to -tip for now, which expresses
this dependency  in the Kconfig. (Eventually a cleaner solution
would be to factor such ABI  details out of procfs, they dont
belong there.)

Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Matt Mackall <mpm@selenic.com>
Cc: Nick Piggin <npiggin@suse.de>
Cc: Christoph Lameter <cl@linux-foundation.org>
Cc: Fr??d??ric Weisbecker <fweisbec@gmail.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Wu Fengguang <fengguang.wu@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
LKML-Reference: <20091217094041.GA24708@elte.hu>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 mm/Kconfig |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/mm/Kconfig b/mm/Kconfig
index 43ea8c3..021bc12 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -243,6 +243,8 @@ config ARCH_SUPPORTS_MEMORY_FAILURE
 config MEMORY_FAILURE
 	depends on MMU
 	depends on ARCH_SUPPORTS_MEMORY_FAILURE
+	depends on PROC_FS
+	select PROC_PAGE_MONITOR
 	bool "Enable recovery from hardware memory errors"
 	help
 	  Enables code to recover from some memory failures on systems

Re: -tip: origin tree build failure

[Ingo Molnar]

* Ingo Molnar <mingo@elte.hu> wrote:

> Add a Kconfig dependency on SOUND to remedy this.

ok, that should have been CONFIG_SND. Updated patch below.

	Ingo

------------------->
>From f9bf1de0fcb6dddf0145d472b13a21ab8e7723cd Mon Sep 17 00:00:00 2001
From: Ingo Molnar <mingo@elte.hu>
Date: Thu, 17 Dec 2009 07:08:12 +0100
Subject: [PATCH] acpi: Build CONFIG_SOUND=m build failure in CONFIG_THINKPAD_ACPI

-tip testing found that commit 0d204c34e85d1d63e5fdd3e3192747daf0ee7ec1
("thinkpad-acpi: basic ALSA mixer support (v2)") from today's upstream
kernel causes the following build failure on x86, for
CONFIG_THINKPAD_ACPI=y && CONFIG_SOUND=m:

  drivers/built-in.o: In function `volume_alsa_notify_change':
  thinkpad_acpi.c:(.text+0x2d5c2d): undefined reference to `snd_ctl_notify'
  thinkpad_acpi.c:(.text+0x2d5c47): undefined reference to `snd_ctl_notify'

This build fails because thinkpad_acpi.c uses sound facilities
unconditionally, without expressing its dependency on the sound
subsystem.

Add a Kconfig dependency on SOUND to remedy this.

Cc: Len Brown <len.brown@intel.com>
Cc: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
Cc: Lorne Applebaum <lorne.applebaum@gmail.com>
Cc: Matthew Garrett <mjg@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 drivers/platform/x86/Kconfig |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index f8bec62..dbd4ef1 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -216,6 +216,7 @@ config THINKPAD_ACPI
 	depends on ACPI
 	depends on INPUT
 	depends on RFKILL || RFKILL = n
+	depends on SND
 	select BACKLIGHT_LCD_SUPPORT
 	select BACKLIGHT_CLASS_DEVICE
 	select HWMON

Re: -tip: origin tree build failure

[Henrique de Moraes Holschuh]

Thank you!

Acked-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>

On Thu, 17 Dec 2009 07:17 +0100, "Ingo Molnar" <mingo@elte.hu> wrote:
> Today's -tip failed to build because commit 
> 0d204c34e85d1d63e5fdd3e3192747daf0ee7ec1 ("thinkpad-acpi: basic ALSA
> mixer 
> support (v2)") from today's upstream kernel causes the following build
> failure 
> on x86, for CONFIG_THINKPAD_ACPI=y && CONFIG_SOUND=m:
...

>  	depends on RFKILL || RFKILL = n
> +       depends on SOUND
>  	select BACKLIGHT_LCD_SUPPORT
>  	select BACKLIGHT_CLASS_DEVICE
>  	select HWMON

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

-tip: origin tree build failure

[Ingo Molnar]

Today's -tip fails to build due to upstream commit 
1a9b5b7fe0c5dad8a635288882d36785dea742f9 ("mm: export stable page flags") 
(authored and merged yesterday) causing the following build failure on x86 
when CONFIG_PROC_PAGE_MONITOR is disabled:

 mm/built-in.o: In function `hwpoison_filter':
  (.text+0x39fbf): undefined reference to `stable_page_flags'

The bug is that the stable_page_flags() API is only available under 
CONFIG_PROC_PAGE_MONITOR, but utilized in mm/memory-failure.c unconditionally.

I've applied the patch below to -tip for now, which expresses this dependency 
in the Kconfig. (Eventually a cleaner solution would be to factor such ABI 
details out of procfs, they dont belong there.)

A technical critique: as i pointed it out in the past on lkml, this whole 
ad-hoc exporting of MM details via /proc is ill advised. It's a limiting 
interface for instrumentation (we could do so much more and could integrate it 
so much better with a similar amount of intrusion), and it also creates 
needless ABI dependencies with user-space tooling.

This should be prototyped in debugfs and sufficiently integrated with other 
instrumentation frameworks in the kernel. I made a few suggestions and we even 
wrote patches in the past to help that out:

 3383e37: tracing, page-allocator: Add a postprocessing script for page-allocator-related ftrace events
 c33b359: tracing, page-allocator: Add trace event for page traffic related to the buddy lists
 0d524fb: tracing, mm: Add trace events for anti-fragmentation falling back to other migratetypes
 b9a2817: tracing, page-allocator: Add trace events for page allocation and page freeing
 eb46710: tracing/mm: rename 'trigger' file to 'dump_range'
 1487a7a: tracing/mm: fix mapcount trace record field
 dcac8cd: tracing/mm: add page frame snapshot trace

Unfortunately nothing of that was pursued from the MM and hwpoison side and 
now this inferior version goes upstream, and as two separate ABIs straight 
away. (the /proc ABI and the memory-failure filter ABI) Not very well designed 
and not very helpful IMO, and this will come back to haunt us in the future.

We know how to do this correctly, why not embrace, utilize and expand those 
instrumentation facilities?

Also, a workflow observation - i tried to figure out the history of the bug 
but the Git timestamps show this weirdness:

 commit 1a9b5b7fe0c5dad8a635288882d36785dea742f9
 Author:     Wu Fengguang <fengguang.wu@intel.com>
 AuthorDate: Wed Dec 16 12:19:59 2009 +0100
 Commit:     Andi Kleen <ak@linux.intel.com>
 CommitDate: Wed Dec 16 12:19:59 2009 +0100

that's precisely the same author and commit date, but author and committer are 
different - how is that possible? No combination of git-am, git-rebase and git 
commit --amend can achieve this kind of timestamp.

I checked the other 'HWPOISON' commits as well that were merged yesterday - 
they have a similarly damaged history.

Thanks,

	Ingo

Signed-off-by: Ingo Molnar <mingo@elte.hu>

diff --git a/mm/Kconfig b/mm/Kconfig
index 43ea8c3..d7bd560 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -243,6 +243,7 @@ config ARCH_SUPPORTS_MEMORY_FAILURE
 config MEMORY_FAILURE
 	depends on MMU
 	depends on ARCH_SUPPORTS_MEMORY_FAILURE
+	select PROC_PAGE_MONITOR
 	bool "Enable recovery from hardware memory errors"
 	help
 	  Enables code to recover from some memory failures on systems

-tip: origin tree build failure

[Ingo Molnar]

Today's -tip failed to build because commit 
9e368fa011d4e0aa050db348d69514900520e40b ("ipmi: add PNP discovery (ACPI 
namespace via PNPACPI)") from today's upstream kernel causes the following 
build failure on x86, for CONFIG_ACPI=n && CONFIG_IPMI_SI=y:

 drivers/char/ipmi/ipmi_si_intf.c:3208: error: 'ipmi_pnp_driver' undeclared (first use in this function)
 drivers/char/ipmi/ipmi_si_intf.c:3208: error: (Each undeclared identifier is reported only once
 drivers/char/ipmi/ipmi_si_intf.c:3208: error: for each function it appears in.)
 drivers/char/ipmi/ipmi_si_intf.c:3334: error: 'ipmi_pnp_driver' undeclared (first use in this function)

The reason is that the ipmi_pnp_driver depends on ACPI facilities and is only 
made available under ACPI - while the registration and unregistration is made 
dependent on CONFIG_PNP:

 #ifdef CONFIG_PNP
 	pnp_register_driver(&ipmi_pnp_driver);
 #endif

The solution is to only register this driver under ACPI. (Also, the CONFIG_PNP 
dependency is not needed because pnp_register_driver() is stubbed out in the 
!CONFIG_PNP case.)

I've applied the patch below to tip:out-of-tree for now.

Thanks,

	Ingo

Signed-off-by: Ingo Molnar <mingo@elte.hu>

diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
index 679cd08..176f175 100644
--- a/drivers/char/ipmi/ipmi_si_intf.c
+++ b/drivers/char/ipmi/ipmi_si_intf.c
@@ -3204,7 +3204,7 @@ static __devinit int init_ipmi_si(void)
 #ifdef CONFIG_ACPI
 	spmi_find_bmc();
 #endif
-#ifdef CONFIG_PNP
+#ifdef CONFIG_ACPI
 	pnp_register_driver(&ipmi_pnp_driver);
 #endif
 
@@ -3330,7 +3330,7 @@ static __exit void cleanup_ipmi_si(void)
 #ifdef CONFIG_PCI
 	pci_unregister_driver(&ipmi_pci_driver);
 #endif
-#ifdef CONFIG_PNP
+#ifdef CONFIG_ACPI
 	pnp_unregister_driver(&ipmi_pnp_driver);
 #endif
 

-tip: origin tree build failure

[Ingo Molnar]

Today's -tip failed to build because commit 
0d204c34e85d1d63e5fdd3e3192747daf0ee7ec1 ("thinkpad-acpi: basic ALSA mixer 
support (v2)") from today's upstream kernel causes the following build failure 
on x86, for CONFIG_THINKPAD_ACPI=y && CONFIG_SOUND=m:

  drivers/built-in.o: In function `volume_alsa_notify_change':
  thinkpad_acpi.c:(.text+0x2d5c2d): undefined reference to `snd_ctl_notify'
  thinkpad_acpi.c:(.text+0x2d5c47): undefined reference to `snd_ctl_notify'

I've applied the patch below to tip:out-of-tree.

Thanks,

	Ingo

----------------->
acpi: Build CONFIG_SOUND=m build failure in CONFIG_THINKPAD_ACPI=y

Today's -tip failed to build because thinkpad_acpi.c uses sound facilities 
unconditionally, without expressing its dependency on the sound subsystem:

  drivers/built-in.o: In function `volume_alsa_notify_change':
  thinkpad_acpi.c:(.text+0x2d5c2d): undefined reference to `snd_ctl_notify'
  thinkpad_acpi.c:(.text+0x2d5c47): undefined reference to `snd_ctl_notify'

Add a Kconfig dependency on SOUND to remedy this.

Cc: Len Brown <len.brown@intel.com>
Cc: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
Cc: Lorne Applebaum <lorne.applebaum@gmail.com>
Cc: Matthew Garrett <mjg@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 drivers/platform/x86/Kconfig |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index f8bec62..60083e9 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -216,6 +216,7 @@ config THINKPAD_ACPI
 	depends on ACPI
 	depends on INPUT
 	depends on RFKILL || RFKILL = n
+	depends on SOUND
 	select BACKLIGHT_LCD_SUPPORT
 	select BACKLIGHT_CLASS_DEVICE
 	select HWMON