[PATCH] [0/23] Fix gcc 4.6 set but unused variable warnings

[PATCH] [0/23] Fix gcc 4.6 set but unused variable warnings

[Andi Kleen]

gcc 4.6 -Wall gained a new set but unused variable warning.

First it was incredibly noisy, but when looking at it in detail it
actually found some bugs.

I haven't tried to fix all occurrences, but
at least the major ones for my configuration.

If someone is interested more work would be possible
on this.

There are still some warnings left, but no flood
and there's really usually some code improvement.

In a few cases I simply shut up the compiler, but
in many other cases dead code is gone or a real
problem has been fixed.

Some of the changes need more review (especially marked
and with suitable cc), but most are straight forward.

I did not fix all bugs -- those that were too hard
are just commented with the warning left in. 
I also marked areas that need more attention in
the individual patches.

Andrew, something for your tree?

-Andi

[PATCH] [1/23] x86: percpu: Avoid warnings of unused variables in per cpu

[Andi Kleen]

Avoid hundreds of warnings with a gcc 4.6 -Wall build

Cc: tj@kernel.org
Cc: x86@kernel.org

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

---
 arch/x86/include/asm/percpu.h |    2 ++
 1 file changed, 2 insertions(+)

Index: linux-2.6.35-rc2-gcc/arch/x86/include/asm/percpu.h
===================================================================
--- linux-2.6.35-rc2-gcc.orig/arch/x86/include/asm/percpu.h
+++ linux-2.6.35-rc2-gcc/arch/x86/include/asm/percpu.h
@@ -77,6 +77,7 @@ do {							\
 	if (0) {					\
 		pto_T__ pto_tmp__;			\
 		pto_tmp__ = (val);			\
+		(void)pto_tmp__;			\
 	}						\
 	switch (sizeof(var)) {				\
 	case 1:						\
@@ -115,6 +116,7 @@ do {									\
 	if (0) {							\
 		pao_T__ pao_tmp__;					\
 		pao_tmp__ = (val);					\
+		(void)pao_tmp__;					\
 	}								\
 	switch (sizeof(var)) {						\
 	case 1:								\

[PATCH] [2/23] IRQ: Move alloc_desk_mask variables inside ifdef

[Andi Kleen]

They are only useful with CONFIG_CPUMASK_OFFSTACK

Avoids hundreds of warnings with a gcc 4.6 -Wall build.

Cc: tglx@linutronix.de
Signed-off-by: Andi Kleen <ak@linux.intel.com>

---
 include/linux/irq.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6.35-rc2-gcc/include/linux/irq.h
===================================================================
--- linux-2.6.35-rc2-gcc.orig/include/linux/irq.h
+++ linux-2.6.35-rc2-gcc/include/linux/irq.h
@@ -439,12 +439,12 @@ extern int set_irq_msi(unsigned int irq,
 static inline bool alloc_desc_masks(struct irq_desc *desc, int node,
 							bool boot)
 {
+#ifdef CONFIG_CPUMASK_OFFSTACK
 	gfp_t gfp = GFP_ATOMIC;
 
 	if (boot)
 		gfp = GFP_NOWAIT;
 
-#ifdef CONFIG_CPUMASK_OFFSTACK
 	if (!alloc_cpumask_var_node(&desc->affinity, gfp, node))
 		return false;
 

[PATCH] [3/23] x86: Avoid unused by set variables in rdmsr

[Andi Kleen]

Avoids quite a lot of warnings with a gcc 4.6 -Wall build
because this happens in a commonly used header file (apic.h)

Cc: x86@kernel.org

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

---
 arch/x86/include/asm/msr.h |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: linux-2.6.35-rc2-gcc/arch/x86/include/asm/msr.h
===================================================================
--- linux-2.6.35-rc2-gcc.orig/arch/x86/include/asm/msr.h
+++ linux-2.6.35-rc2-gcc/arch/x86/include/asm/msr.h
@@ -148,8 +148,8 @@ static inline unsigned long long native_
 #define rdmsr(msr, val1, val2)					\
 do {								\
 	u64 __val = native_read_msr((msr));			\
-	(val1) = (u32)__val;					\
-	(val2) = (u32)(__val >> 32);				\
+	(void)((val1) = (u32)__val);				\
+	(void)((val2) = (u32)(__val >> 32));			\
 } while (0)
 
 static inline void wrmsr(unsigned msr, unsigned low, unsigned high)

[PATCH] [4/23] pagemap: Avoid unused-but-set variable

[Andi Kleen]

Avoid quite a lot of warnings in header files in a gcc 4.6 -Wall builds

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

---
 include/linux/pagemap.h |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Index: linux-2.6.35-rc2-gcc/include/linux/pagemap.h
===================================================================
--- linux-2.6.35-rc2-gcc.orig/include/linux/pagemap.h
+++ linux-2.6.35-rc2-gcc/include/linux/pagemap.h
@@ -423,8 +423,10 @@ static inline int fault_in_pages_readabl
 		const char __user *end = uaddr + size - 1;
 
 		if (((unsigned long)uaddr & PAGE_MASK) !=
-				((unsigned long)end & PAGE_MASK))
+				((unsigned long)end & PAGE_MASK)) {
 		 	ret = __get_user(c, end);
+			(void)c;
+		}
 	}
 	return ret;
 }

[PATCH] [5/23] x86 boot: Set ax register in boot vga query

[Andi Kleen]

I think the ax register should be input to the
BIOS call, not be unused, but not fully sure.  hpa?

Found by gcc 4.6's new warnings.

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

---
 arch/x86/boot/video-vga.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Index: linux-2.6.35-rc2-gcc/arch/x86/boot/video-vga.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/arch/x86/boot/video-vga.c
+++ linux-2.6.35-rc2-gcc/arch/x86/boot/video-vga.c
@@ -41,13 +41,12 @@ static __videocard video_vga;
 static u8 vga_set_basic_mode(void)
 {
 	struct biosregs ireg, oreg;
-	u16 ax;
 	u8 mode;
 
 	initregs(&ireg);
 
 	/* Query current mode */
-	ax = 0x0f00;
+	ireg.ax = 0x0f00;
 	intcall(0x10, &ireg, &oreg);
 	mode = oreg.al;
 

[PATCH] [6/23] perf: Fix set but unused variables in perf

[Andi Kleen]

Just dead code I believe

Cc: peterz@infradead.org
Cc: mingo@elte.hu

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

---
 arch/x86/kernel/cpu/perf_event.c |    5 -----
 1 file changed, 5 deletions(-)

Index: linux-2.6.35-rc2-gcc/arch/x86/kernel/cpu/perf_event.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/arch/x86/kernel/cpu/perf_event.c
+++ linux-2.6.35-rc2-gcc/arch/x86/kernel/cpu/perf_event.c
@@ -953,12 +953,9 @@ static void x86_pmu_enable_event(struct
 static int x86_pmu_enable(struct perf_event *event)
 {
 	struct cpu_hw_events *cpuc = &__get_cpu_var(cpu_hw_events);
-	struct hw_perf_event *hwc;
 	int assign[X86_PMC_IDX_MAX];
 	int n, n0, ret;
 
-	hwc = &event->hw;
-
 	n0 = cpuc->n_events;
 	n = collect_events(cpuc, event, false);
 	if (n < 0)
@@ -1122,7 +1119,6 @@ static int x86_pmu_handle_irq(struct pt_
 	struct perf_sample_data data;
 	struct cpu_hw_events *cpuc;
 	struct perf_event *event;
-	struct hw_perf_event *hwc;
 	int idx, handled = 0;
 	u64 val;
 
@@ -1135,7 +1131,6 @@ static int x86_pmu_handle_irq(struct pt_
 			continue;
 
 		event = cpuc->events[idx];
-		hwc = &event->hw;
 
 		val = x86_perf_event_update(event);
 		if (val & (1ULL << (x86_pmu.cntval_bits - 1)))

[PATCH] [7/23] x86: fix set but not read variables

[Andi Kleen]

Just some dead code, no real bugs.

Found by gcc 4.6 -Wall

Cc: x86@kernel.org

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

---
 arch/x86/kernel/aperture_64.c      |    4 ++--
 arch/x86/kernel/cpu/mtrr/generic.c |    3 +--
 2 files changed, 3 insertions(+), 4 deletions(-)

Index: linux-2.6.35-rc2-gcc/arch/x86/kernel/cpu/mtrr/generic.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/arch/x86/kernel/cpu/mtrr/generic.c
+++ linux-2.6.35-rc2-gcc/arch/x86/kernel/cpu/mtrr/generic.c
@@ -433,13 +433,12 @@ static void generic_get_mtrr(unsigned in
 {
 	unsigned int mask_lo, mask_hi, base_lo, base_hi;
 	unsigned int tmp, hi;
-	int cpu;
 
 	/*
 	 * get_mtrr doesn't need to update mtrr_state, also it could be called
 	 * from any cpu, so try to print it out directly.
 	 */
-	cpu = get_cpu();
+	get_cpu();
 
 	rdmsr(MTRRphysMask_MSR(reg), mask_lo, mask_hi);
 
Index: linux-2.6.35-rc2-gcc/arch/x86/kernel/aperture_64.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/arch/x86/kernel/aperture_64.c
+++ linux-2.6.35-rc2-gcc/arch/x86/kernel/aperture_64.c
@@ -280,7 +280,7 @@ void __init early_gart_iommu_check(void)
 	 * or BIOS forget to put that in reserved.
 	 * try to update e820 to make that region as reserved.
 	 */
-	u32 agp_aper_base = 0, agp_aper_order = 0;
+	u32 agp_aper_order = 0;
 	int i, fix, slot, valid_agp = 0;
 	u32 ctl;
 	u32 aper_size = 0, aper_order = 0, last_aper_order = 0;
@@ -291,7 +291,7 @@ void __init early_gart_iommu_check(void)
 		return;
 
 	/* This is mostly duplicate of iommu_hole_init */
-	agp_aper_base = search_agp_bridge(&agp_aper_order, &valid_agp);
+	search_agp_bridge(&agp_aper_order, &valid_agp);
 
 	fix = 0;
 	for (i = 0; i < ARRAY_SIZE(bus_dev_ranges); i++) {

[PATCH] [8/23] KGDB: Remove set but unused newPC

[Andi Kleen]

I'm not fully sure this is the correct fix, maybe this
was a bug and newPC should really have a side effect.
Jason?

Found by gcc 4.6's new warnings

Cc: jason.wessel@windriver.com

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

---
 arch/x86/kernel/kgdb.c |    3 ---
 1 file changed, 3 deletions(-)

Index: linux-2.6.35-rc2-gcc/arch/x86/kernel/kgdb.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/arch/x86/kernel/kgdb.c
+++ linux-2.6.35-rc2-gcc/arch/x86/kernel/kgdb.c
@@ -458,7 +458,6 @@ int kgdb_arch_handle_exception(int e_vec
 {
 	unsigned long addr;
 	char *ptr;
-	int newPC;
 
 	switch (remcomInBuffer[0]) {
 	case 'c':
@@ -469,8 +468,6 @@ int kgdb_arch_handle_exception(int e_vec
 			linux_regs->ip = addr;
 	case 'D':
 	case 'k':
-		newPC = linux_regs->ip;
-
 		/* clear the trace bit */
 		linux_regs->flags &= ~X86_EFLAGS_TF;
 		atomic_set(&kgdb_cpu_doing_single_step, -1);

[PATCH] [9/23] PRINTK: Use stable variable to dump kmsg buffer

[Andi Kleen]

kmsg_dump takes care to sample the global variables
inside a spinlock, but then goes on to use the same
variables outside the spinlock region too.

Use the correct variable. This will make the race
window smaller.

Found by gcc 4.6's new warnings.

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

---
 kernel/printk.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Index: linux-2.6.35-rc2-gcc/kernel/printk.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/kernel/printk.c
+++ linux-2.6.35-rc2-gcc/kernel/printk.c
@@ -1520,9 +1520,9 @@ void kmsg_dump(enum kmsg_dump_reason rea
 	chars = logged_chars;
 	spin_unlock_irqrestore(&logbuf_lock, flags);
 
-	if (logged_chars > end) {
-		s1 = log_buf + log_buf_len - logged_chars + end;
-		l1 = logged_chars - end;
+	if (chars > end) {
+		s1 = log_buf + log_buf_len - chars + end;
+		l1 = chars - end;
 
 		s2 = log_buf;
 		l2 = end;
@@ -1530,8 +1530,8 @@ void kmsg_dump(enum kmsg_dump_reason rea
 		s1 = "";
 		l1 = 0;
 
-		s2 = log_buf + end - logged_chars;
-		l2 = logged_chars;
+		s2 = log_buf + end - chars;
+		l2 = chars;
 	}
 
 	if (!spin_trylock_irqsave(&dump_list_lock, flags)) {

[PATCH] [10/23] SCHED: Only allocate per cpu cpu mask buffer with offstack cpumasks

[Andi Kleen]

This will save a few bytes in the non offstack cpumask case.

Found by gcc 4.6's new warnings.

Cc: peterz@infradead.org
Cc: mingo@elte.hu
Signed-off-by: Andi Kleen <ak@linux.intel.com>

---
 kernel/sched.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Index: linux-2.6.35-rc2-gcc/kernel/sched.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/kernel/sched.c
+++ linux-2.6.35-rc2-gcc/kernel/sched.c
@@ -7482,7 +7482,7 @@ static void init_tg_rt_entry(struct task
 void __init sched_init(void)
 {
 	int i, j;
-	unsigned long alloc_size = 0, ptr;
+	unsigned long alloc_size = 0;
 
 #ifdef CONFIG_FAIR_GROUP_SCHED
 	alloc_size += 2 * nr_cpu_ids * sizeof(void **);
@@ -7494,7 +7494,10 @@ void __init sched_init(void)
 	alloc_size += num_possible_cpus() * cpumask_size();
 #endif
 	if (alloc_size) {
+#ifdef CONFIG_CPUMASK_OFFSTACK
+		unsigned long ptr;
 		ptr = (unsigned long)kzalloc(alloc_size, GFP_NOWAIT);
+#endif
 
 #ifdef CONFIG_FAIR_GROUP_SCHED
 		init_task_group.se = (struct sched_entity **)ptr;

[PATCH] [11/23] KVM: Fix KVM_SET_SIGNAL_MASK

[Andi Kleen]

Real bug fix.

When the user passed in a NULL mask pass this on from the ioctl
handler.

Found by gcc 4.6's new warnings.

Cc: avi@redhat.com
Cc: kvm@vger.kernel.org

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

---
 virt/kvm/kvm_main.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6.35-rc2-gcc/virt/kvm/kvm_main.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/virt/kvm/kvm_main.c
+++ linux-2.6.35-rc2-gcc/virt/kvm/kvm_main.c
@@ -1520,7 +1520,7 @@ out_free2:
 				goto out;
 			p = &sigset;
 		}
-		r = kvm_vcpu_ioctl_set_sigmask(vcpu, &sigset);
+		r = kvm_vcpu_ioctl_set_sigmask(vcpu, p);
 		break;
 	}
 	case KVM_GET_FPU: {

[PATCH] [12/23] BTRFS: Clean up unused variables -- bugs

[Andi Kleen]

These are all the cases where a variable is set, but not
read which are really bugs.

- Couple of incorrect error handling fixed.
- One incorrect use of a allocation policy
- Some other things

Still needs more review.

Found by gcc 4.6's new warnings

Cc: chris.mason@oracle.com
cc: linux-btrfs@vger.kernel.org


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

---
 fs/btrfs/dir-item.c    |    2 +-
 fs/btrfs/extent-tree.c |    3 +--
 fs/btrfs/extent_io.c   |    2 ++
 fs/btrfs/inode.c       |    6 +++---
 fs/btrfs/relocation.c  |    4 +++-
 fs/btrfs/tree-log.c    |    2 +-
 6 files changed, 11 insertions(+), 8 deletions(-)

Index: linux-2.6.35-rc2-gcc/fs/btrfs/extent-tree.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/btrfs/extent-tree.c
+++ linux-2.6.35-rc2-gcc/fs/btrfs/extent-tree.c
@@ -3337,8 +3337,7 @@ struct btrfs_block_rsv *btrfs_alloc_bloc
 	btrfs_init_block_rsv(block_rsv);
 
 	alloc_target = btrfs_get_alloc_profile(root, 0);
-	block_rsv->space_info = __find_space_info(fs_info,
-						  BTRFS_BLOCK_GROUP_METADATA);
+	block_rsv->space_info = __find_space_info(fs_info, alloc_target);
 
 	return block_rsv;
 }
Index: linux-2.6.35-rc2-gcc/fs/btrfs/dir-item.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/btrfs/dir-item.c
+++ linux-2.6.35-rc2-gcc/fs/btrfs/dir-item.c
@@ -427,5 +427,5 @@ int btrfs_delete_one_dir_name(struct btr
 		ret = btrfs_truncate_item(trans, root, path,
 					  item_len - sub_item_len, 1);
 	}
-	return 0;
+	return ret;
 }
Index: linux-2.6.35-rc2-gcc/fs/btrfs/extent_io.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/btrfs/extent_io.c
+++ linux-2.6.35-rc2-gcc/fs/btrfs/extent_io.c
@@ -2825,6 +2825,8 @@ int extent_prepare_write(struct extent_i
 					 NULL, 1,
 					 end_bio_extent_preparewrite, 0,
 					 0, 0);
+			if (ret && !err)
+				err = ret;
 			iocount++;
 			block_start = block_start + iosize;
 		} else {
Index: linux-2.6.35-rc2-gcc/fs/btrfs/inode.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/btrfs/inode.c
+++ linux-2.6.35-rc2-gcc/fs/btrfs/inode.c
@@ -1372,7 +1372,7 @@ int btrfs_merge_bio_hook(struct page *pa
 
 	if (map_length < length + size)
 		return 1;
-	return 0;
+	return ret;
 }
 
 /*
@@ -2672,7 +2672,7 @@ static int check_path_shared(struct btrf
 {
 	struct extent_buffer *eb;
 	int level;
-	int ret;
+	int ret = 0;
 	u64 refs;
 
 	for (level = 0; level < BTRFS_MAX_LEVEL; level++) {
@@ -2686,7 +2686,7 @@ static int check_path_shared(struct btrf
 		if (refs > 1)
 			return 1;
 	}
-	return 0;
+	return ret; /* XXX callers? */
 }
 
 /*
Index: linux-2.6.35-rc2-gcc/fs/btrfs/tree-log.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/btrfs/tree-log.c
+++ linux-2.6.35-rc2-gcc/fs/btrfs/tree-log.c
@@ -2273,7 +2273,7 @@ fail:
 	}
 	btrfs_end_log_trans(root);
 
-	return 0;
+	return err;
 }
 
 /* see comments for btrfs_del_dir_entries_in_log */
Index: linux-2.6.35-rc2-gcc/fs/btrfs/relocation.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/btrfs/relocation.c
+++ linux-2.6.35-rc2-gcc/fs/btrfs/relocation.c
@@ -3098,6 +3098,8 @@ static int add_tree_block(struct reloc_c
 		BUG_ON(item_size != sizeof(struct btrfs_extent_item_v0));
 		ret = get_ref_objectid_v0(rc, path, extent_key,
 					  &ref_owner, NULL);
+		if (ret < 0)
+			return ret;
 		BUG_ON(ref_owner >= BTRFS_MAX_LEVEL);
 		level = (int)ref_owner;
 		/* FIXME: get real generation */
@@ -4142,7 +4144,7 @@ int btrfs_reloc_clone_csums(struct inode
 		btrfs_add_ordered_sum(inode, ordered, sums);
 	}
 	btrfs_put_ordered_extent(ordered);
-	return 0;
+	return ret;
 }
 
 void btrfs_reloc_cow_block(struct btrfs_trans_handle *trans,

[PATCH] [13/23] BTRFS: Clean up unused variables -- nonbugs

[Andi Kleen]

These are all the cases where a variable is set, but not 
read which are not bugs as far as I can see, but simply
leftovers. 

Still needs more review.

Found by gcc 4.6's new warnings

Cc: chris.mason@oracle.com
Cc: linux-btrfs@vger.kernel.org

 fs/btrfs/compression.c            |    2 --
 fs/btrfs/ctree.c                  |   20 ++------------------
 fs/btrfs/dir-item.c               |    2 +-
 fs/btrfs/disk-io.c                |   10 ----------
 fs/btrfs/extent-tree.c            |    2 --
 fs/btrfs/extent_io.c              |   10 ++--------
 fs/btrfs/inode.c                  |   19 +++----------------
 fs/btrfs/ioctl.c                  |    2 --
 fs/btrfs/ordered-data.c           |    2 --
 fs/btrfs/relocation.c             |    2 ++
 fs/btrfs/root-tree.c              |    2 --
 fs/btrfs/super.c                  |    1 +
 fs/btrfs/tree-defrag.c            |    2 --
 fs/btrfs/tree-log.c               |   15 +--------------
 fs/btrfs/volumes.c                |    4 ----
 fs/btrfs/xattr.c                  |    2 --
 fs/btrfs/zlib.c                   |    5 -----

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

---
 fs/btrfs/compression.c  |    2 --
 fs/btrfs/ctree.c        |   20 ++------------------
 fs/btrfs/disk-io.c      |   11 -----------
 fs/btrfs/extent-tree.c  |    2 --
 fs/btrfs/extent_io.c    |    9 ---------
 fs/btrfs/inode.c        |   14 --------------
 fs/btrfs/ioctl.c        |    2 --
 fs/btrfs/ordered-data.c |    2 --
 fs/btrfs/root-tree.c    |    2 --
 fs/btrfs/super.c        |    6 ++----
 fs/btrfs/tree-defrag.c  |    2 --
 fs/btrfs/tree-log.c     |   15 ---------------
 fs/btrfs/volumes.c      |    4 ----
 fs/btrfs/xattr.c        |    2 --
 fs/btrfs/zlib.c         |    5 -----
 15 files changed, 4 insertions(+), 94 deletions(-)

Index: linux-2.6.35-rc2-gcc/fs/btrfs/ctree.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/btrfs/ctree.c
+++ linux-2.6.35-rc2-gcc/fs/btrfs/ctree.c
@@ -200,7 +200,6 @@ int btrfs_copy_root(struct btrfs_trans_h
 		      struct extent_buffer **cow_ret, u64 new_root_objectid)
 {
 	struct extent_buffer *cow;
-	u32 nritems;
 	int ret = 0;
 	int level;
 	struct btrfs_disk_key disk_key;
@@ -210,7 +209,6 @@ int btrfs_copy_root(struct btrfs_trans_h
 	WARN_ON(root->ref_cows && trans->transid != root->last_trans);
 
 	level = btrfs_header_level(buf);
-	nritems = btrfs_header_nritems(buf);
 	if (level == 0)
 		btrfs_item_key(buf, &disk_key, 0);
 	else
@@ -1008,7 +1006,6 @@ static noinline int balance_level(struct
 	int wret;
 	int pslot;
 	int orig_slot = path->slots[level];
-	int err_on_enospc = 0;
 	u64 orig_ptr;
 
 	if (level == 0)
@@ -1071,8 +1068,7 @@ static noinline int balance_level(struct
 	    BTRFS_NODEPTRS_PER_BLOCK(root) / 4)
 		return 0;
 
-	if (btrfs_header_nritems(mid) < 2)
-		err_on_enospc = 1;
+	btrfs_header_nritems(mid);
 
 	left = read_node_slot(root, parent, pslot - 1);
 	if (left) {
@@ -1103,8 +1099,7 @@ static noinline int balance_level(struct
 		wret = push_node_left(trans, root, left, mid, 1);
 		if (wret < 0)
 			ret = wret;
-		if (btrfs_header_nritems(mid) < 2)
-			err_on_enospc = 1;
+		btrfs_header_nritems(mid);
 	}
 
 	/*
@@ -1224,14 +1219,12 @@ static noinline int push_nodes_for_inser
 	int wret;
 	int pslot;
 	int orig_slot = path->slots[level];
-	u64 orig_ptr;
 
 	if (level == 0)
 		return 1;
 
 	mid = path->nodes[level];
 	WARN_ON(btrfs_header_generation(mid) != trans->transid);
-	orig_ptr = btrfs_node_blockptr(mid, orig_slot);
 
 	if (level < BTRFS_MAX_LEVEL - 1)
 		parent = path->nodes[level + 1];
@@ -2534,7 +2527,6 @@ static noinline int __push_leaf_left(str
 {
 	struct btrfs_disk_key disk_key;
 	struct extent_buffer *right = path->nodes[0];
-	int slot;
 	int i;
 	int push_space = 0;
 	int push_items = 0;
@@ -2546,8 +2538,6 @@ static noinline int __push_leaf_left(str
 	u32 this_item_size;
 	u32 old_left_item_size;
 
-	slot = path->slots[1];
-
 	if (empty)
 		nr = right_nritems;
 	else
@@ -3239,7 +3229,6 @@ int btrfs_truncate_item(struct btrfs_tra
 {
 	int ret = 0;
 	int slot;
-	int slot_orig;
 	struct extent_buffer *leaf;
 	struct btrfs_item *item;
 	u32 nritems;
@@ -3249,7 +3238,6 @@ int btrfs_truncate_item(struct btrfs_tra
 	unsigned int size_diff;
 	int i;
 
-	slot_orig = path->slots[0];
 	leaf = path->nodes[0];
 	slot = path->slots[0];
 
@@ -3354,7 +3342,6 @@ int btrfs_extend_item(struct btrfs_trans
 {
 	int ret = 0;
 	int slot;
-	int slot_orig;
 	struct extent_buffer *leaf;
 	struct btrfs_item *item;
 	u32 nritems;
@@ -3363,7 +3350,6 @@ int btrfs_extend_item(struct btrfs_trans
 	unsigned int old_size;
 	int i;
 
-	slot_orig = path->slots[0];
 	leaf = path->nodes[0];
 
 	nritems = btrfs_header_nritems(leaf);
@@ -3696,7 +3682,6 @@ int btrfs_insert_empty_items(struct btrf
 			    struct btrfs_key *cpu_key, u32 *data_size,
 			    int nr)
 {
-	struct extent_buffer *leaf;
 	int ret = 0;
 	int slot;
 	int i;
@@ -3713,7 +3698,6 @@ int btrfs_insert_empty_items(struct btrf
 	if (ret < 0)
 		goto out;
 
-	leaf = path->nodes[0];
 	slot = path->slots[0];
 	BUG_ON(slot < 0);
 
Index: linux-2.6.35-rc2-gcc/fs/btrfs/super.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/btrfs/super.c
+++ linux-2.6.35-rc2-gcc/fs/btrfs/super.c
@@ -61,6 +61,8 @@ static void btrfs_put_super(struct super
 
 	ret = close_ctree(root);
 	sb->s_fs_info = NULL;
+
+	(void)ret; /* FIXME: need to fix VFS to return error? */
 }
 
 enum {
@@ -434,7 +436,6 @@ static int btrfs_fill_super(struct super
 {
 	struct inode *inode;
 	struct dentry *root_dentry;
-	struct btrfs_super_block *disk_super;
 	struct btrfs_root *tree_root;
 	struct btrfs_key key;
 	int err;
@@ -456,7 +457,6 @@ static int btrfs_fill_super(struct super
 		return PTR_ERR(tree_root);
 	}
 	sb->s_fs_info = tree_root;
-	disk_super = &tree_root->fs_info->super_copy;
 
 	key.objectid = BTRFS_FIRST_FREE_OBJECTID;
 	key.type = BTRFS_INODE_ITEM_KEY;
@@ -569,7 +569,6 @@ static int btrfs_get_sb(struct file_syst
 	char *subvol_name = NULL;
 	u64 subvol_objectid = 0;
 	int error = 0;
-	int found = 0;
 
 	if (!(flags & MS_RDONLY))
 		mode |= FMODE_WRITE;
@@ -605,7 +604,6 @@ static int btrfs_get_sb(struct file_syst
 			goto error_close_devices;
 		}
 
-		found = 1;
 		btrfs_close_devices(fs_devices);
 	} else {
 		char b[BDEVNAME_SIZE];
Index: linux-2.6.35-rc2-gcc/fs/btrfs/disk-io.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/btrfs/disk-io.c
+++ linux-2.6.35-rc2-gcc/fs/btrfs/disk-io.c
@@ -338,7 +338,6 @@ static int csum_dirty_buffer(struct btrf
 	struct extent_io_tree *tree;
 	u64 start = (u64)page->index << PAGE_CACHE_SHIFT;
 	u64 found_start;
-	int found_level;
 	unsigned long len;
 	struct extent_buffer *eb;
 	int ret;
@@ -369,8 +368,6 @@ static int csum_dirty_buffer(struct btrf
 		WARN_ON(1);
 		goto err;
 	}
-	found_level = btrfs_header_level(eb);
-
 	csum_tree_block(root, eb, 0);
 err:
 	free_extent_buffer(eb);
@@ -533,11 +530,9 @@ int btrfs_congested_async(struct btrfs_f
 
 static void run_one_async_start(struct btrfs_work *work)
 {
-	struct btrfs_fs_info *fs_info;
 	struct async_submit_bio *async;
 
 	async = container_of(work, struct  async_submit_bio, work);
-	fs_info = BTRFS_I(async->inode)->root->fs_info;
 	async->submit_bio_start(async->inode, async->rw, async->bio,
 			       async->mirror_num, async->bio_flags,
 			       async->bio_offset);
@@ -850,12 +845,8 @@ struct extent_buffer *read_tree_block(st
 				      u32 blocksize, u64 parent_transid)
 {
 	struct extent_buffer *buf = NULL;
-	struct inode *btree_inode = root->fs_info->btree_inode;
-	struct extent_io_tree *io_tree;
 	int ret;
 
-	io_tree = &BTRFS_I(btree_inode)->io_tree;
-
 	buf = btrfs_find_create_tree_block(root, bytenr, blocksize);
 	if (!buf)
 		return NULL;
@@ -1377,7 +1368,6 @@ static int bio_ready_for_csum(struct bio
 	u64 start = 0;
 	struct page *page;
 	struct extent_io_tree *io_tree = NULL;
-	struct btrfs_fs_info *info = NULL;
 	struct bio_vec *bvec;
 	int i;
 	int ret;
@@ -1396,7 +1386,6 @@ static int bio_ready_for_csum(struct bio
 		buf_len = page->private >> 2;
 		start = page_offset(page) + bvec->bv_offset;
 		io_tree = &BTRFS_I(page->mapping->host)->io_tree;
-		info = BTRFS_I(page->mapping->host)->root->fs_info;
 	}
 	/* are we fully contained in this bio? */
 	if (buf_len <= length)
Index: linux-2.6.35-rc2-gcc/fs/btrfs/extent-tree.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/btrfs/extent-tree.c
+++ linux-2.6.35-rc2-gcc/fs/btrfs/extent-tree.c
@@ -5420,7 +5420,6 @@ static noinline void reada_walk_down(str
 	u64 generation;
 	u64 refs;
 	u64 flags;
-	u64 last = 0;
 	u32 nritems;
 	u32 blocksize;
 	struct btrfs_key key;
@@ -5488,7 +5487,6 @@ reada:
 					   generation);
 		if (ret)
 			break;
-		last = bytenr + blocksize;
 		nread++;
 	}
 	wc->reada_slot = slot;
Index: linux-2.6.35-rc2-gcc/fs/btrfs/root-tree.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/btrfs/root-tree.c
+++ linux-2.6.35-rc2-gcc/fs/btrfs/root-tree.c
@@ -181,7 +181,6 @@ int btrfs_insert_root(struct btrfs_trans
 int btrfs_find_dead_roots(struct btrfs_root *root, u64 objectid)
 {
 	struct btrfs_root *dead_root;
-	struct btrfs_item *item;
 	struct btrfs_root_item *ri;
 	struct btrfs_key key;
 	struct btrfs_key found_key;
@@ -214,7 +213,6 @@ again:
 			nritems = btrfs_header_nritems(leaf);
 			slot = path->slots[0];
 		}
-		item = btrfs_item_nr(leaf, slot);
 		btrfs_item_key_to_cpu(leaf, &key, slot);
 		if (btrfs_key_type(&key) != BTRFS_ROOT_ITEM_KEY)
 			goto next;
Index: linux-2.6.35-rc2-gcc/fs/btrfs/compression.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/btrfs/compression.c
+++ linux-2.6.35-rc2-gcc/fs/btrfs/compression.c
@@ -163,7 +163,6 @@ fail:
  */
 static void end_compressed_bio_read(struct bio *bio, int err)
 {
-	struct extent_io_tree *tree;
 	struct compressed_bio *cb = bio->bi_private;
 	struct inode *inode;
 	struct page *page;
@@ -187,7 +186,6 @@ static void end_compressed_bio_read(stru
 	/* ok, we're the last bio for this extent, lets start
 	 * the decompression.
 	 */
-	tree = &BTRFS_I(inode)->io_tree;
 	ret = btrfs_zlib_decompress_biovec(cb->compressed_pages,
 					cb->start,
 					cb->orig_bio->bi_io_vec,
Index: linux-2.6.35-rc2-gcc/fs/btrfs/extent_io.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/btrfs/extent_io.c
+++ linux-2.6.35-rc2-gcc/fs/btrfs/extent_io.c
@@ -1901,10 +1901,8 @@ static int submit_one_bio(int rw, struct
 	struct page *page = bvec->bv_page;
 	struct extent_io_tree *tree = bio->bi_private;
 	u64 start;
-	u64 end;
 
 	start = ((u64)page->index << PAGE_CACHE_SHIFT) + bvec->bv_offset;
-	end = start + bvec->bv_len - 1;
 
 	bio->bi_private = NULL;
 
@@ -2204,7 +2202,6 @@ static int __extent_writepage(struct pag
 	u64 last_byte = i_size_read(inode);
 	u64 block_start;
 	u64 iosize;
-	u64 unlock_start;
 	sector_t sector;
 	struct extent_state *cached_state = NULL;
 	struct extent_map *em;
@@ -2329,7 +2326,6 @@ static int __extent_writepage(struct pag
 		if (tree->ops && tree->ops->writepage_end_io_hook)
 			tree->ops->writepage_end_io_hook(page, start,
 							 page_end, NULL, 1);
-		unlock_start = page_end + 1;
 		goto done;
 	}
 
@@ -2340,7 +2336,6 @@ static int __extent_writepage(struct pag
 			if (tree->ops && tree->ops->writepage_end_io_hook)
 				tree->ops->writepage_end_io_hook(page, cur,
 							 page_end, NULL, 1);
-			unlock_start = page_end + 1;
 			break;
 		}
 		em = epd->get_extent(inode, page, pg_offset, cur,
@@ -2387,7 +2382,6 @@ static int __extent_writepage(struct pag
 
 			cur += iosize;
 			pg_offset += iosize;
-			unlock_start = cur;
 			continue;
 		}
 		/* leave this out until we have a page_mkwrite call */
@@ -2473,7 +2467,6 @@ static int extent_write_cache_pages(stru
 	pgoff_t index;
 	pgoff_t end;		/* Inclusive */
 	int scanned = 0;
-	int range_whole = 0;
 
 	pagevec_init(&pvec, 0);
 	if (wbc->range_cyclic) {
@@ -2482,8 +2475,6 @@ static int extent_write_cache_pages(stru
 	} else {
 		index = wbc->range_start >> PAGE_CACHE_SHIFT;
 		end = wbc->range_end >> PAGE_CACHE_SHIFT;
-		if (wbc->range_start == 0 && wbc->range_end == LLONG_MAX)
-			range_whole = 1;
 		scanned = 1;
 	}
 retry:
Index: linux-2.6.35-rc2-gcc/fs/btrfs/inode.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/btrfs/inode.c
+++ linux-2.6.35-rc2-gcc/fs/btrfs/inode.c
@@ -319,8 +319,6 @@ static noinline int compress_file_range(
 	struct btrfs_root *root = BTRFS_I(inode)->root;
 	struct btrfs_trans_handle *trans;
 	u64 num_bytes;
-	u64 orig_start;
-	u64 disk_num_bytes;
 	u64 blocksize = root->sectorsize;
 	u64 actual_end;
 	u64 isize = i_size_read(inode);
@@ -335,8 +333,6 @@ static noinline int compress_file_range(
 	int i;
 	int will_compress;
 
-	orig_start = start;
-
 	actual_end = min_t(u64, isize, end + 1);
 again:
 	will_compress = 0;
@@ -371,7 +367,6 @@ again:
 	total_compressed = min(total_compressed, max_uncompressed);
 	num_bytes = (end - start + blocksize) & ~(blocksize - 1);
 	num_bytes = max(blocksize,  num_bytes);
-	disk_num_bytes = num_bytes;
 	total_in = 0;
 	ret = 0;
 
@@ -467,7 +462,6 @@ again:
 		if (total_compressed >= total_in) {
 			will_compress = 0;
 		} else {
-			disk_num_bytes = total_compressed;
 			num_bytes = total_in;
 		}
 	}
@@ -757,8 +751,6 @@ static noinline int cow_file_range(struc
 	u64 disk_num_bytes;
 	u64 cur_alloc_size;
 	u64 blocksize = root->sectorsize;
-	u64 actual_end;
-	u64 isize = i_size_read(inode);
 	struct btrfs_key ins;
 	struct extent_map *em;
 	struct extent_map_tree *em_tree = &BTRFS_I(inode)->extent_tree;
@@ -769,8 +761,6 @@ static noinline int cow_file_range(struc
 	btrfs_set_trans_block_group(trans, inode);
 	trans->block_rsv = &root->fs_info->delalloc_block_rsv;
 
-	actual_end = min_t(u64, isize, end + 1);
-
 	num_bytes = (end - start + blocksize) & ~(blocksize - 1);
 	num_bytes = max(blocksize,  num_bytes);
 	disk_num_bytes = num_bytes;
@@ -2237,7 +2227,6 @@ void btrfs_orphan_cleanup(struct btrfs_r
 {
 	struct btrfs_path *path;
 	struct extent_buffer *leaf;
-	struct btrfs_item *item;
 	struct btrfs_key key, found_key;
 	struct btrfs_trans_handle *trans;
 	struct inode *inode;
@@ -2275,7 +2264,6 @@ void btrfs_orphan_cleanup(struct btrfs_r
 
 		/* pull out the item */
 		leaf = path->nodes[0];
-		item = btrfs_item_nr(leaf, path->slots[0]);
 		btrfs_item_key_to_cpu(leaf, &found_key, path->slots[0]);
 
 		/* make sure the item matches what we want */
@@ -5640,7 +5628,6 @@ static void btrfs_submit_direct(int rw,
 	struct btrfs_root *root = BTRFS_I(inode)->root;
 	struct btrfs_dio_private *dip;
 	struct bio_vec *bvec = bio->bi_io_vec;
-	u64 start;
 	int skip_sum;
 	int write = rw & (1 << BIO_RW);
 	int ret = 0;
@@ -5666,7 +5653,6 @@ static void btrfs_submit_direct(int rw,
 	dip->inode = inode;
 	dip->logical_offset = file_offset;
 
-	start = dip->logical_offset;
 	dip->bytes = 0;
 	do {
 		dip->bytes += bvec->bv_len;
Index: linux-2.6.35-rc2-gcc/fs/btrfs/ioctl.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/btrfs/ioctl.c
+++ linux-2.6.35-rc2-gcc/fs/btrfs/ioctl.c
@@ -708,7 +708,6 @@ static noinline int btrfs_ioctl_resize(s
 	char *sizestr;
 	char *devstr = NULL;
 	int ret = 0;
-	int namelen;
 	int mod = 0;
 
 	if (root->fs_info->sb->s_flags & MS_RDONLY)
@@ -722,7 +721,6 @@ static noinline int btrfs_ioctl_resize(s
 		return PTR_ERR(vol_args);
 
 	vol_args->name[BTRFS_PATH_NAME_MAX] = '\0';
-	namelen = strlen(vol_args->name);
 
 	mutex_lock(&root->fs_info->volume_mutex);
 	sizestr = vol_args->name;
Index: linux-2.6.35-rc2-gcc/fs/btrfs/ordered-data.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/btrfs/ordered-data.c
+++ linux-2.6.35-rc2-gcc/fs/btrfs/ordered-data.c
@@ -526,7 +526,6 @@ int btrfs_wait_ordered_range(struct inod
 {
 	u64 end;
 	u64 orig_end;
-	u64 wait_end;
 	struct btrfs_ordered_extent *ordered;
 	int found;
 
@@ -537,7 +536,6 @@ int btrfs_wait_ordered_range(struct inod
 		if (orig_end > INT_LIMIT(loff_t))
 			orig_end = INT_LIMIT(loff_t);
 	}
-	wait_end = orig_end;
 again:
 	/* start IO across the range first to instantiate any delalloc
 	 * extents
Index: linux-2.6.35-rc2-gcc/fs/btrfs/tree-defrag.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/btrfs/tree-defrag.c
+++ linux-2.6.35-rc2-gcc/fs/btrfs/tree-defrag.c
@@ -36,7 +36,6 @@ int btrfs_defrag_leaves(struct btrfs_tra
 	int ret = 0;
 	int wret;
 	int level;
-	int orig_level;
 	int is_extent = 0;
 	int next_key_ret = 0;
 	u64 last_ret = 0;
@@ -64,7 +63,6 @@ int btrfs_defrag_leaves(struct btrfs_tra
 		return -ENOMEM;
 
 	level = btrfs_header_level(root->node);
-	orig_level = level;
 
 	if (level == 0)
 		goto out;
Index: linux-2.6.35-rc2-gcc/fs/btrfs/tree-log.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/btrfs/tree-log.c
+++ linux-2.6.35-rc2-gcc/fs/btrfs/tree-log.c
@@ -786,7 +786,6 @@ static noinline int add_inode_ref(struct
 {
 	struct inode *dir;
 	int ret;
-	struct btrfs_key location;
 	struct btrfs_inode_ref *ref;
 	struct btrfs_dir_item *di;
 	struct inode *inode;
@@ -795,10 +794,6 @@ static noinline int add_inode_ref(struct
 	unsigned long ref_ptr;
 	unsigned long ref_end;
 
-	location.objectid = key->objectid;
-	location.type = BTRFS_INODE_ITEM_KEY;
-	location.offset = 0;
-
 	/*
 	 * it is possible that we didn't log all the parent directories
 	 * for a given inode.  If we don't find the dir, just don't
@@ -1583,7 +1578,6 @@ static int replay_one_buffer(struct btrf
 	struct btrfs_path *path;
 	struct btrfs_root *root = wc->replay_dest;
 	struct btrfs_key key;
-	u32 item_size;
 	int level;
 	int i;
 	int ret;
@@ -1601,7 +1595,6 @@ static int replay_one_buffer(struct btrf
 	nritems = btrfs_header_nritems(eb);
 	for (i = 0; i < nritems; i++) {
 		btrfs_item_key_to_cpu(eb, &key, i);
-		item_size = btrfs_item_size_nr(eb, i);
 
 		/* inode keys are done during the first stage */
 		if (key.type == BTRFS_INODE_ITEM_KEY &&
@@ -1668,7 +1661,6 @@ static noinline int walk_down_log_tree(s
 				   struct walk_control *wc)
 {
 	u64 root_owner;
-	u64 root_gen;
 	u64 bytenr;
 	u64 ptr_gen;
 	struct extent_buffer *next;
@@ -1698,7 +1690,6 @@ static noinline int walk_down_log_tree(s
 
 		parent = path->nodes[*level];
 		root_owner = btrfs_header_owner(parent);
-		root_gen = btrfs_header_generation(parent);
 
 		next = btrfs_find_create_tree_block(root, bytenr, blocksize);
 
@@ -1749,7 +1740,6 @@ static noinline int walk_up_log_tree(str
 				 struct walk_control *wc)
 {
 	u64 root_owner;
-	u64 root_gen;
 	int i;
 	int slot;
 	int ret;
@@ -1757,8 +1747,6 @@ static noinline int walk_up_log_tree(str
 	for (i = *level; i < BTRFS_MAX_LEVEL - 1 && path->nodes[i]; i++) {
 		slot = path->slots[i];
 		if (slot + 1 < btrfs_header_nritems(path->nodes[i])) {
-			struct extent_buffer *node;
-			node = path->nodes[i];
 			path->slots[i]++;
 			*level = i;
 			WARN_ON(*level == 0);
@@ -1771,7 +1759,6 @@ static noinline int walk_up_log_tree(str
 				parent = path->nodes[*level + 1];
 
 			root_owner = btrfs_header_owner(parent);
-			root_gen = btrfs_header_generation(parent);
 			wc->process_func(root, path->nodes[*level], wc,
 				 btrfs_header_generation(path->nodes[*level]));
 			if (wc->free) {
@@ -2729,7 +2716,6 @@ static int btrfs_log_inode(struct btrfs_
 	struct btrfs_key max_key;
 	struct btrfs_root *log = root->log_root;
 	struct extent_buffer *src = NULL;
-	u32 size;
 	int err = 0;
 	int ret;
 	int nritems;
@@ -2793,7 +2779,6 @@ again:
 			break;
 
 		src = path->nodes[0];
-		size = btrfs_item_size_nr(src, path->slots[0]);
 		if (ins_nr && ins_start_slot + ins_nr == path->slots[0]) {
 			ins_nr++;
 			goto next_slot;
Index: linux-2.6.35-rc2-gcc/fs/btrfs/volumes.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/btrfs/volumes.c
+++ linux-2.6.35-rc2-gcc/fs/btrfs/volumes.c
@@ -1901,7 +1901,6 @@ int btrfs_balance(struct btrfs_root *dev
 	u64 size_to_free;
 	struct btrfs_path *path;
 	struct btrfs_key key;
-	struct btrfs_chunk *chunk;
 	struct btrfs_root *chunk_root = dev_root->fs_info->chunk_root;
 	struct btrfs_trans_handle *trans;
 	struct btrfs_key found_key;
@@ -1965,9 +1964,6 @@ int btrfs_balance(struct btrfs_root *dev
 		if (found_key.objectid != key.objectid)
 			break;
 
-		chunk = btrfs_item_ptr(path->nodes[0],
-				       path->slots[0],
-				       struct btrfs_chunk);
 		/* chunk zero is special */
 		if (found_key.offset == 0)
 			break;
Index: linux-2.6.35-rc2-gcc/fs/btrfs/xattr.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/btrfs/xattr.c
+++ linux-2.6.35-rc2-gcc/fs/btrfs/xattr.c
@@ -178,7 +178,6 @@ ssize_t btrfs_listxattr(struct dentry *d
 	struct inode *inode = dentry->d_inode;
 	struct btrfs_root *root = BTRFS_I(inode)->root;
 	struct btrfs_path *path;
-	struct btrfs_item *item;
 	struct extent_buffer *leaf;
 	struct btrfs_dir_item *di;
 	int ret = 0, slot, advance;
@@ -234,7 +233,6 @@ ssize_t btrfs_listxattr(struct dentry *d
 		}
 		advance = 1;
 
-		item = btrfs_item_nr(leaf, slot);
 		btrfs_item_key_to_cpu(leaf, &found_key, slot);
 
 		/* check to make sure this item is what we want */
Index: linux-2.6.35-rc2-gcc/fs/btrfs/zlib.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/btrfs/zlib.c
+++ linux-2.6.35-rc2-gcc/fs/btrfs/zlib.c
@@ -199,8 +199,6 @@ int btrfs_zlib_compress_pages(struct add
 	int nr_pages = 0;
 	struct page *in_page = NULL;
 	struct page *out_page = NULL;
-	int out_written = 0;
-	int in_read = 0;
 	unsigned long bytes_left;
 
 	*out_pages = 0;
@@ -233,9 +231,6 @@ int btrfs_zlib_compress_pages(struct add
 	workspace->def_strm.avail_out = PAGE_CACHE_SIZE;
 	workspace->def_strm.avail_in = min(len, PAGE_CACHE_SIZE);
 
-	out_written = 0;
-	in_read = 0;
-
 	while (workspace->def_strm.total_in < len) {
 		ret = zlib_deflate(&workspace->def_strm, Z_SYNC_FLUSH);
 		if (ret != Z_OK) {

[PATCH] [14/23] NFSD: Fix initialized but not read warnings

[Andi Kleen]

Fixes at least one real minor bug: the nfs4 recovery dir sysctl
would not return its status properly.

Also I finished Al's 1e41568d7378d ("Take ima_path_check() in nfsd 
past dentry_open() in nfsd_open()") commit, it moved the IMA
code, but left the old path initializer in there.

The rest is just dead code removed I think, although I was not 
fully sure about the "is_borc" stuff. Some more review
would be still good.

Found by gcc 4.6's new warnings.

Cc: viro@zeniv.linux.org.uk
Cc: neilb@suse.de

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

---
 fs/nfsd/nfs4state.c |    2 --
 fs/nfsd/nfsctl.c    |    2 ++
 fs/nfsd/nfsproc.c   |    2 --
 fs/nfsd/vfs.c       |   11 +----------
 4 files changed, 3 insertions(+), 14 deletions(-)

Index: linux-2.6.35-rc2-gcc/fs/nfsd/nfs4state.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/nfsd/nfs4state.c
+++ linux-2.6.35-rc2-gcc/fs/nfsd/nfs4state.c
@@ -3346,11 +3346,9 @@ static inline void
 nfs4_set_lock_denied(struct file_lock *fl, struct nfsd4_lock_denied *deny)
 {
 	struct nfs4_stateowner *sop;
-	unsigned int hval;
 
 	if (fl->fl_lmops == &nfsd_posix_mng_ops) {
 		sop = (struct nfs4_stateowner *) fl->fl_owner;
-		hval = lockownerid_hashval(sop->so_id);
 		kref_get(&sop->so_ref);
 		deny->ld_sop = sop;
 		deny->ld_clientid = sop->so_client->cl_clientid;
Index: linux-2.6.35-rc2-gcc/fs/nfsd/nfsctl.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/nfsd/nfsctl.c
+++ linux-2.6.35-rc2-gcc/fs/nfsd/nfsctl.c
@@ -1310,6 +1310,8 @@ static ssize_t __write_recoverydir(struc
 			return -EINVAL;
 
 		status = nfs4_reset_recoverydir(recdir);
+		if (status)
+			return status;
 	}
 
 	return scnprintf(buf, SIMPLE_TRANSACTION_LIMIT, "%s\n",
Index: linux-2.6.35-rc2-gcc/fs/nfsd/nfsproc.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/nfsd/nfsproc.c
+++ linux-2.6.35-rc2-gcc/fs/nfsd/nfsproc.c
@@ -290,7 +290,6 @@ nfsd_proc_create(struct svc_rqst *rqstp,
 	 * gospel of sun micro
 	 */
 	if (type != S_IFREG) {
-		int	is_borc = 0;
 		if (type != S_IFBLK && type != S_IFCHR) {
 			rdev = 0;
 		} else if (type == S_IFCHR && !(attr->ia_valid & ATTR_SIZE)) {
@@ -298,7 +297,6 @@ nfsd_proc_create(struct svc_rqst *rqstp,
 			type = S_IFIFO;
 		} else {
 			/* Okay, char or block special */
-			is_borc = 1;
 			if (!rdev)
 				rdev = wanted;
 		}
Index: linux-2.6.35-rc2-gcc/fs/nfsd/vfs.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/nfsd/vfs.c
+++ linux-2.6.35-rc2-gcc/fs/nfsd/vfs.c
@@ -1632,7 +1632,7 @@ nfsd_link(struct svc_rqst *rqstp, struct
 				char *name, int len, struct svc_fh *tfhp)
 {
 	struct dentry	*ddir, *dnew, *dold;
-	struct inode	*dirp, *dest;
+	struct inode	*dirp;
 	__be32		err;
 	int		host_err;
 
@@ -1660,7 +1660,6 @@ nfsd_link(struct svc_rqst *rqstp, struct
 		goto out_nfserr;
 
 	dold = tfhp->fh_dentry;
-	dest = dold->d_inode;
 
 	host_err = mnt_want_write(tfhp->fh_export->ex_path.mnt);
 	if (host_err) {
@@ -2112,15 +2111,7 @@ nfsd_permission(struct svc_rqst *rqstp,
 	if (err == -EACCES && S_ISREG(inode->i_mode) &&
 	    acc == (NFSD_MAY_READ | NFSD_MAY_OWNER_OVERRIDE))
 		err = inode_permission(inode, MAY_EXEC);
-	if (err)
-		goto nfsd_out;
 
-	/* Do integrity (permission) checking now, but defer incrementing
-	 * IMA counts to the actual file open.
-	 */
-	path.mnt = exp->ex_path.mnt;
-	path.dentry = dentry;
-nfsd_out:
 	return err? nfserrno(err) : 0;
 }
 

[PATCH] [15/23] EXT4: Fix initialized but not read variables

[Andi Kleen]

No real bugs found, just various dead code removed I believe.
Some review would be good.

Found by gcc 4.6's new warnings

Cc: tytso@mit.edu
Cc: linux-ext4@vger.kernel.org

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

---
 fs/ext4/balloc.c   |    2 --
 fs/ext4/extents.c  |    8 ++------
 fs/ext4/inode.c    |    5 -----
 fs/ext4/mballoc.c  |   23 ++++++++---------------
 fs/ext4/namei.c    |    2 --
 fs/ext4/resize.c   |    2 --
 fs/jbd2/journal.c  |   12 ------------
 fs/jbd2/recovery.c |    7 -------
 8 files changed, 10 insertions(+), 51 deletions(-)

Index: linux-2.6.35-rc2-gcc/fs/ext4/balloc.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/ext4/balloc.c
+++ linux-2.6.35-rc2-gcc/fs/ext4/balloc.c
@@ -377,14 +377,12 @@ void ext4_add_groupblocks(handle_t *hand
 	ext4_grpblk_t bit;
 	unsigned int i;
 	struct ext4_group_desc *desc;
-	struct ext4_super_block *es;
 	struct ext4_sb_info *sbi;
 	int err = 0, ret, blk_free_count;
 	ext4_grpblk_t blocks_freed;
 	struct ext4_group_info *grp;
 
 	sbi = EXT4_SB(sb);
-	es = sbi->s_es;
 	ext4_debug("Adding block(s) %llu-%llu\n", block, block + count - 1);
 
 	ext4_get_group_no_and_offset(sb, block, &block_group, &bit);
Index: linux-2.6.35-rc2-gcc/fs/ext4/extents.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/ext4/extents.c
+++ linux-2.6.35-rc2-gcc/fs/ext4/extents.c
@@ -1083,7 +1083,6 @@ static int ext4_ext_grow_indepth(handle_
 {
 	struct ext4_ext_path *curp = path;
 	struct ext4_extent_header *neh;
-	struct ext4_extent_idx *fidx;
 	struct buffer_head *bh;
 	ext4_fsblk_t newblock;
 	int err = 0;
@@ -1144,10 +1143,10 @@ static int ext4_ext_grow_indepth(handle_
 	ext4_idx_store_pblock(curp->p_idx, newblock);
 
 	neh = ext_inode_hdr(inode);
-	fidx = EXT_FIRST_INDEX(neh);
 	ext_debug("new root: num %d(%d), lblock %d, ptr %llu\n",
 		  le16_to_cpu(neh->eh_entries), le16_to_cpu(neh->eh_max),
-		  le32_to_cpu(fidx->ei_block), idx_pblock(fidx));
+		  le32_to_cpu(EXT_FIRST_INDEX(neh)->ei_block),
+		  idx_pblock(fidx));
 
 	neh->eh_depth = cpu_to_le16(path->p_depth + 1);
 	err = ext4_ext_dirty(handle, inode, curp);
@@ -2954,7 +2953,6 @@ static int ext4_split_unwritten_extents(
 	struct ext4_extent *ex1 = NULL;
 	struct ext4_extent *ex2 = NULL;
 	struct ext4_extent *ex3 = NULL;
-	struct ext4_extent_header *eh;
 	ext4_lblk_t ee_block, eof_block;
 	unsigned int allocated, ee_len, depth;
 	ext4_fsblk_t newblock;
@@ -2971,7 +2969,6 @@ static int ext4_split_unwritten_extents(
 		eof_block = map->m_lblk + map->m_len;
 
 	depth = ext_depth(inode);
-	eh = path[depth].p_hdr;
 	ex = path[depth].p_ext;
 	ee_block = le32_to_cpu(ex->ee_block);
 	ee_len = ext4_ext_get_actual_len(ex);
@@ -3058,7 +3055,6 @@ static int ext4_split_unwritten_extents(
 			err = PTR_ERR(path);
 			goto out;
 		}
-		eh = path[depth].p_hdr;
 		ex = path[depth].p_ext;
 		if (ex2 != &newex)
 			ex2 = ex;
Index: linux-2.6.35-rc2-gcc/fs/ext4/inode.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/ext4/inode.c
+++ linux-2.6.35-rc2-gcc/fs/ext4/inode.c
@@ -3146,13 +3146,10 @@ static int ext4_da_write_begin(struct fi
 	int ret, retries = 0;
 	struct page *page;
 	pgoff_t index;
-	unsigned from, to;
 	struct inode *inode = mapping->host;
 	handle_t *handle;
 
 	index = pos >> PAGE_CACHE_SHIFT;
-	from = pos & (PAGE_CACHE_SIZE - 1);
-	to = from + len;
 
 	if (ext4_nonda_switch(inode->i_sb)) {
 		*fsdata = (void *)FALL_BACK_TO_NONDELALLOC;
@@ -5754,7 +5751,6 @@ static int ext4_expand_extra_isize(struc
 {
 	struct ext4_inode *raw_inode;
 	struct ext4_xattr_ibody_header *header;
-	struct ext4_xattr_entry *entry;
 
 	if (EXT4_I(inode)->i_extra_isize >= new_extra_isize)
 		return 0;
@@ -5762,7 +5758,6 @@ static int ext4_expand_extra_isize(struc
 	raw_inode = ext4_raw_inode(&iloc);
 
 	header = IHDR(inode, raw_inode);
-	entry = IFIRST(header);
 
 	/* No extended attributes present */
 	if (!ext4_test_inode_state(inode, EXT4_STATE_XATTR) ||
Index: linux-2.6.35-rc2-gcc/fs/ext4/mballoc.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/ext4/mballoc.c
+++ linux-2.6.35-rc2-gcc/fs/ext4/mballoc.c
@@ -1999,7 +1999,6 @@ ext4_mb_regular_allocator(struct ext4_al
 	ext4_group_t ngroups, group, i;
 	int cr;
 	int err = 0;
-	int bsbits;
 	struct ext4_sb_info *sbi;
 	struct super_block *sb;
 	struct ext4_buddy e4b;
@@ -2041,8 +2040,6 @@ ext4_mb_regular_allocator(struct ext4_al
 			ac->ac_2order = i - 1;
 	}
 
-	bsbits = ac->ac_sb->s_blocksize_bits;
-
 	/* if stream allocation is enabled, use global goal */
 	if (ac->ac_flags & EXT4_MB_STREAM_ALLOC) {
 		/* TBD: may be hot point */
@@ -2712,7 +2709,6 @@ ext4_mb_mark_diskspace_used(struct ext4_
 				handle_t *handle, unsigned int reserv_blks)
 {
 	struct buffer_head *bitmap_bh = NULL;
-	struct ext4_super_block *es;
 	struct ext4_group_desc *gdp;
 	struct buffer_head *gdp_bh;
 	struct ext4_sb_info *sbi;
@@ -2725,8 +2721,6 @@ ext4_mb_mark_diskspace_used(struct ext4_
 
 	sb = ac->ac_sb;
 	sbi = EXT4_SB(sb);
-	es = sbi->s_es;
-
 
 	err = -EIO;
 	bitmap_bh = ext4_read_block_bitmap(sb, ac->ac_b_ex.fe_group);
@@ -2849,8 +2843,8 @@ ext4_mb_normalize_request(struct ext4_al
 {
 	int bsbits, max;
 	ext4_lblk_t end;
-	loff_t size, orig_size, start_off;
-	ext4_lblk_t start, orig_start;
+	loff_t size, start_off;
+	ext4_lblk_t start;
 	struct ext4_inode_info *ei = EXT4_I(ac->ac_inode);
 	struct ext4_prealloc_space *pa;
 
@@ -2922,8 +2916,9 @@ ext4_mb_normalize_request(struct ext4_al
 		start_off = (loff_t)ac->ac_o_ex.fe_logical << bsbits;
 		size	  = ac->ac_o_ex.fe_len << bsbits;
 	}
-	orig_size = size = size >> bsbits;
-	orig_start = start = start_off >> bsbits;
+
+	/* FIXME: start_off is not used for anything. bug? */
+	/* FIXME: start may be uninitialized? */
 
 	/* don't cover already allocated blocks in selected range */
 	if (ar->pleft && start <= ar->lleft) {
@@ -3547,7 +3542,6 @@ ext4_mb_release_inode_pa(struct ext4_bud
 	ext4_group_t group;
 	ext4_grpblk_t bit;
 	unsigned long long grp_blk_start;
-	sector_t start;
 	int err = 0;
 	int free = 0;
 
@@ -3567,9 +3561,10 @@ ext4_mb_release_inode_pa(struct ext4_bud
 		if (bit >= end)
 			break;
 		next = mb_find_next_bit(bitmap_bh->b_data, end, bit);
-		start = ext4_group_first_block_no(sb, group) + bit;
 		mb_debug(1, "    free preallocated %u/%u in group %u\n",
-				(unsigned) start, (unsigned) next - bit,
+				(unsigned)
+			 ext4_group_first_block_no(sb, group) + bit,
+			 	(unsigned) next - bit,
 				(unsigned) group);
 		free += next - bit;
 
@@ -4494,7 +4489,6 @@ void ext4_free_blocks(handle_t *handle,
 	struct super_block *sb = inode->i_sb;
 	struct ext4_allocation_context *ac = NULL;
 	struct ext4_group_desc *gdp;
-	struct ext4_super_block *es;
 	unsigned long freed = 0;
 	unsigned int overflow;
 	ext4_grpblk_t bit;
@@ -4513,7 +4507,6 @@ void ext4_free_blocks(handle_t *handle,
 	}
 
 	sbi = EXT4_SB(sb);
-	es = EXT4_SB(sb)->s_es;
 	if (!(flags & EXT4_FREE_BLOCKS_VALIDATED) &&
 	    !ext4_data_block_valid(sbi, block, count)) {
 		ext4_error(sb, "Freeing blocks not in datazone - "
Index: linux-2.6.35-rc2-gcc/fs/ext4/namei.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/ext4/namei.c
+++ linux-2.6.35-rc2-gcc/fs/ext4/namei.c
@@ -1088,7 +1088,6 @@ static struct dentry *ext4_lookup(struct
 struct dentry *ext4_get_parent(struct dentry *child)
 {
 	__u32 ino;
-	struct inode *inode;
 	static const struct qstr dotdot = {
 		.name = "..",
 		.len = 2,
@@ -1097,7 +1096,6 @@ struct dentry *ext4_get_parent(struct de
 	struct buffer_head *bh;
 
 	bh = ext4_find_entry(child->d_inode, &dotdot, &de);
-	inode = NULL;
 	if (!bh)
 		return ERR_PTR(-ENOENT);
 	ino = le32_to_cpu(de->inode);
Index: linux-2.6.35-rc2-gcc/fs/ext4/resize.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/ext4/resize.c
+++ linux-2.6.35-rc2-gcc/fs/ext4/resize.c
@@ -953,7 +953,6 @@ int ext4_group_extend(struct super_block
 		      ext4_fsblk_t n_blocks_count)
 {
 	ext4_fsblk_t o_blocks_count;
-	ext4_group_t o_groups_count;
 	ext4_grpblk_t last;
 	ext4_grpblk_t add;
 	struct buffer_head *bh;
@@ -965,7 +964,6 @@ int ext4_group_extend(struct super_block
 	 * yet: we're going to revalidate es->s_blocks_count after
 	 * taking the s_resize_lock below. */
 	o_blocks_count = ext4_blocks_count(es);
-	o_groups_count = EXT4_SB(sb)->s_groups_count;
 
 	if (test_opt(sb, DEBUG))
 		printk(KERN_DEBUG "EXT4-fs: extending last group from %llu uto %llu blocks\n",
Index: linux-2.6.35-rc2-gcc/fs/jbd2/journal.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/jbd2/journal.c
+++ linux-2.6.35-rc2-gcc/fs/jbd2/journal.c
@@ -1392,13 +1392,9 @@ int jbd2_journal_check_used_features (jo
 int jbd2_journal_check_available_features (journal_t *journal, unsigned long compat,
 				      unsigned long ro, unsigned long incompat)
 {
-	journal_superblock_t *sb;
-
 	if (!compat && !ro && !incompat)
 		return 1;
 
-	sb = journal->j_superblock;
-
 	/* We can support any known requested features iff the
 	 * superblock is in version 2.  Otherwise we fail to support any
 	 * extended sb features. */
@@ -1618,7 +1614,6 @@ int jbd2_journal_flush(journal_t *journa
 
 int jbd2_journal_wipe(journal_t *journal, int write)
 {
-	journal_superblock_t *sb;
 	int err = 0;
 
 	J_ASSERT (!(journal->j_flags & JBD2_LOADED));
@@ -1627,8 +1622,6 @@ int jbd2_journal_wipe(journal_t *journal
 	if (err)
 		return err;
 
-	sb = journal->j_superblock;
-
 	if (!journal->j_tail)
 		goto no_recovery;
 
@@ -2202,8 +2195,6 @@ void jbd2_journal_init_jbd_inode(struct
 void jbd2_journal_release_jbd_inode(journal_t *journal,
 				    struct jbd2_inode *jinode)
 {
-	int writeout = 0;
-
 	if (!journal)
 		return;
 restart:
@@ -2220,9 +2211,6 @@ restart:
 		goto restart;
 	}
 
-	/* Do we need to wait for data writeback? */
-	if (journal->j_committing_transaction == jinode->i_transaction)
-		writeout = 1;
 	if (jinode->i_transaction) {
 		list_del(&jinode->i_list);
 		jinode->i_transaction = NULL;
Index: linux-2.6.35-rc2-gcc/fs/jbd2/recovery.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/jbd2/recovery.c
+++ linux-2.6.35-rc2-gcc/fs/jbd2/recovery.c
@@ -285,12 +285,10 @@ int jbd2_journal_recover(journal_t *jour
 int jbd2_journal_skip_recovery(journal_t *journal)
 {
 	int			err;
-	journal_superblock_t *	sb;
 
 	struct recovery_info	info;
 
 	memset (&info, 0, sizeof(info));
-	sb = journal->j_superblock;
 
 	err = do_one_pass(journal, &info, PASS_SCAN);
 
@@ -365,11 +363,6 @@ static int do_one_pass(journal_t *journa
 	int			tag_bytes = journal_tag_bytes(journal);
 	__u32			crc32_sum = ~0; /* Transactional Checksums */
 
-	/* Precompute the maximum metadata descriptors in a descriptor block */
-	int			MAX_BLOCKS_PER_DESC;
-	MAX_BLOCKS_PER_DESC = ((journal->j_blocksize-sizeof(journal_header_t))
-			       / tag_bytes);
-
 	/*
 	 * First thing is to establish what we expect to find in the log
 	 * (in terms of transaction IDs), and where (in terms of log

[PATCH] [15/23] EXT4: Fix initialized but not read variables

[Andi Kleen]

No real bugs found, just various dead code removed I believe.
Some review would be good.

Found by gcc 4.6's new warnings

Cc: tytso@mit.edu
Cc: linux-ext4@vger.kernel.org

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

---
 fs/ext4/balloc.c   |    2 --
 fs/ext4/extents.c  |    8 ++------
 fs/ext4/inode.c    |    5 -----
 fs/ext4/mballoc.c  |   23 ++++++++---------------
 fs/ext4/namei.c    |    2 --
 fs/ext4/resize.c   |    2 --
 fs/jbd2/journal.c  |   12 ------------
 fs/jbd2/recovery.c |    7 -------
 8 files changed, 10 insertions(+), 51 deletions(-)

Index: linux-2.6.35-rc2-gcc/fs/ext4/balloc.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/ext4/balloc.c
+++ linux-2.6.35-rc2-gcc/fs/ext4/balloc.c
@@ -377,14 +377,12 @@ void ext4_add_groupblocks(handle_t *hand
 	ext4_grpblk_t bit;
 	unsigned int i;
 	struct ext4_group_desc *desc;
-	struct ext4_super_block *es;
 	struct ext4_sb_info *sbi;
 	int err = 0, ret, blk_free_count;
 	ext4_grpblk_t blocks_freed;
 	struct ext4_group_info *grp;
 
 	sbi = EXT4_SB(sb);
-	es = sbi->s_es;
 	ext4_debug("Adding block(s) %llu-%llu\n", block, block + count - 1);
 
 	ext4_get_group_no_and_offset(sb, block, &block_group, &bit);
Index: linux-2.6.35-rc2-gcc/fs/ext4/extents.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/ext4/extents.c
+++ linux-2.6.35-rc2-gcc/fs/ext4/extents.c
@@ -1083,7 +1083,6 @@ static int ext4_ext_grow_indepth(handle_
 {
 	struct ext4_ext_path *curp = path;
 	struct ext4_extent_header *neh;
-	struct ext4_extent_idx *fidx;
 	struct buffer_head *bh;
 	ext4_fsblk_t newblock;
 	int err = 0;
@@ -1144,10 +1143,10 @@ static int ext4_ext_grow_indepth(handle_
 	ext4_idx_store_pblock(curp->p_idx, newblock);
 
 	neh = ext_inode_hdr(inode);
-	fidx = EXT_FIRST_INDEX(neh);
 	ext_debug("new root: num %d(%d), lblock %d, ptr %llu\n",
 		  le16_to_cpu(neh->eh_entries), le16_to_cpu(neh->eh_max),
-		  le32_to_cpu(fidx->ei_block), idx_pblock(fidx));
+		  le32_to_cpu(EXT_FIRST_INDEX(neh)->ei_block),
+		  idx_pblock(fidx));
 
 	neh->eh_depth = cpu_to_le16(path->p_depth + 1);
 	err = ext4_ext_dirty(handle, inode, curp);
@@ -2954,7 +2953,6 @@ static int ext4_split_unwritten_extents(
 	struct ext4_extent *ex1 = NULL;
 	struct ext4_extent *ex2 = NULL;
 	struct ext4_extent *ex3 = NULL;
-	struct ext4_extent_header *eh;
 	ext4_lblk_t ee_block, eof_block;
 	unsigned int allocated, ee_len, depth;
 	ext4_fsblk_t newblock;
@@ -2971,7 +2969,6 @@ static int ext4_split_unwritten_extents(
 		eof_block = map->m_lblk + map->m_len;
 
 	depth = ext_depth(inode);
-	eh = path[depth].p_hdr;
 	ex = path[depth].p_ext;
 	ee_block = le32_to_cpu(ex->ee_block);
 	ee_len = ext4_ext_get_actual_len(ex);
@@ -3058,7 +3055,6 @@ static int ext4_split_unwritten_extents(
 			err = PTR_ERR(path);
 			goto out;
 		}
-		eh = path[depth].p_hdr;
 		ex = path[depth].p_ext;
 		if (ex2 != &newex)
 			ex2 = ex;
Index: linux-2.6.35-rc2-gcc/fs/ext4/inode.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/ext4/inode.c
+++ linux-2.6.35-rc2-gcc/fs/ext4/inode.c
@@ -3146,13 +3146,10 @@ static int ext4_da_write_begin(struct fi
 	int ret, retries = 0;
 	struct page *page;
 	pgoff_t index;
-	unsigned from, to;
 	struct inode *inode = mapping->host;
 	handle_t *handle;
 
 	index = pos >> PAGE_CACHE_SHIFT;
-	from = pos & (PAGE_CACHE_SIZE - 1);
-	to = from + len;
 
 	if (ext4_nonda_switch(inode->i_sb)) {
 		*fsdata = (void *)FALL_BACK_TO_NONDELALLOC;
@@ -5754,7 +5751,6 @@ static int ext4_expand_extra_isize(struc
 {
 	struct ext4_inode *raw_inode;
 	struct ext4_xattr_ibody_header *header;
-	struct ext4_xattr_entry *entry;
 
 	if (EXT4_I(inode)->i_extra_isize >= new_extra_isize)
 		return 0;
@@ -5762,7 +5758,6 @@ static int ext4_expand_extra_isize(struc
 	raw_inode = ext4_raw_inode(&iloc);
 
 	header = IHDR(inode, raw_inode);
-	entry = IFIRST(header);
 
 	/* No extended attributes present */
 	if (!ext4_test_inode_state(inode, EXT4_STATE_XATTR) ||
Index: linux-2.6.35-rc2-gcc/fs/ext4/mballoc.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/ext4/mballoc.c
+++ linux-2.6.35-rc2-gcc/fs/ext4/mballoc.c
@@ -1999,7 +1999,6 @@ ext4_mb_regular_allocator(struct ext4_al
 	ext4_group_t ngroups, group, i;
 	int cr;
 	int err = 0;
-	int bsbits;
 	struct ext4_sb_info *sbi;
 	struct super_block *sb;
 	struct ext4_buddy e4b;
@@ -2041,8 +2040,6 @@ ext4_mb_regular_allocator(struct ext4_al
 			ac->ac_2order = i - 1;
 	}
 
-	bsbits = ac->ac_sb->s_blocksize_bits;
-
 	/* if stream allocation is enabled, use global goal */
 	if (ac->ac_flags & EXT4_MB_STREAM_ALLOC) {
 		/* TBD: may be hot point */
@@ -2712,7 +2709,6 @@ ext4_mb_mark_diskspace_used(struct ext4_
 				handle_t *handle, unsigned int reserv_blks)
 {
 	struct buffer_head *bitmap_bh = NULL;
-	struct ext4_super_block *es;
 	struct ext4_group_desc *gdp;
 	struct buffer_head *gdp_bh;
 	struct ext4_sb_info *sbi;
@@ -2725,8 +2721,6 @@ ext4_mb_mark_diskspace_used(struct ext4_
 
 	sb = ac->ac_sb;
 	sbi = EXT4_SB(sb);
-	es = sbi->s_es;
-
 
 	err = -EIO;
 	bitmap_bh = ext4_read_block_bitmap(sb, ac->ac_b_ex.fe_group);
@@ -2849,8 +2843,8 @@ ext4_mb_normalize_request(struct ext4_al
 {
 	int bsbits, max;
 	ext4_lblk_t end;
-	loff_t size, orig_size, start_off;
-	ext4_lblk_t start, orig_start;
+	loff_t size, start_off;
+	ext4_lblk_t start;
 	struct ext4_inode_info *ei = EXT4_I(ac->ac_inode);
 	struct ext4_prealloc_space *pa;
 
@@ -2922,8 +2916,9 @@ ext4_mb_normalize_request(struct ext4_al
 		start_off = (loff_t)ac->ac_o_ex.fe_logical << bsbits;
 		size	  = ac->ac_o_ex.fe_len << bsbits;
 	}
-	orig_size = size = size >> bsbits;
-	orig_start = start = start_off >> bsbits;
+
+	/* FIXME: start_off is not used for anything. bug? */
+	/* FIXME: start may be uninitialized? */
 
 	/* don't cover already allocated blocks in selected range */
 	if (ar->pleft && start <= ar->lleft) {
@@ -3547,7 +3542,6 @@ ext4_mb_release_inode_pa(struct ext4_bud
 	ext4_group_t group;
 	ext4_grpblk_t bit;
 	unsigned long long grp_blk_start;
-	sector_t start;
 	int err = 0;
 	int free = 0;
 
@@ -3567,9 +3561,10 @@ ext4_mb_release_inode_pa(struct ext4_bud
 		if (bit >= end)
 			break;
 		next = mb_find_next_bit(bitmap_bh->b_data, end, bit);
-		start = ext4_group_first_block_no(sb, group) + bit;
 		mb_debug(1, "    free preallocated %u/%u in group %u\n",
-				(unsigned) start, (unsigned) next - bit,
+				(unsigned)
+			 ext4_group_first_block_no(sb, group) + bit,
+			 	(unsigned) next - bit,
 				(unsigned) group);
 		free += next - bit;
 
@@ -4494,7 +4489,6 @@ void ext4_free_blocks(handle_t *handle,
 	struct super_block *sb = inode->i_sb;
 	struct ext4_allocation_context *ac = NULL;
 	struct ext4_group_desc *gdp;
-	struct ext4_super_block *es;
 	unsigned long freed = 0;
 	unsigned int overflow;
 	ext4_grpblk_t bit;
@@ -4513,7 +4507,6 @@ void ext4_free_blocks(handle_t *handle,
 	}
 
 	sbi = EXT4_SB(sb);
-	es = EXT4_SB(sb)->s_es;
 	if (!(flags & EXT4_FREE_BLOCKS_VALIDATED) &&
 	    !ext4_data_block_valid(sbi, block, count)) {
 		ext4_error(sb, "Freeing blocks not in datazone - "
Index: linux-2.6.35-rc2-gcc/fs/ext4/namei.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/ext4/namei.c
+++ linux-2.6.35-rc2-gcc/fs/ext4/namei.c
@@ -1088,7 +1088,6 @@ static struct dentry *ext4_lookup(struct
 struct dentry *ext4_get_parent(struct dentry *child)
 {
 	__u32 ino;
-	struct inode *inode;
 	static const struct qstr dotdot = {
 		.name = "..",
 		.len = 2,
@@ -1097,7 +1096,6 @@ struct dentry *ext4_get_parent(struct de
 	struct buffer_head *bh;
 
 	bh = ext4_find_entry(child->d_inode, &dotdot, &de);
-	inode = NULL;
 	if (!bh)
 		return ERR_PTR(-ENOENT);
 	ino = le32_to_cpu(de->inode);
Index: linux-2.6.35-rc2-gcc/fs/ext4/resize.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/ext4/resize.c
+++ linux-2.6.35-rc2-gcc/fs/ext4/resize.c
@@ -953,7 +953,6 @@ int ext4_group_extend(struct super_block
 		      ext4_fsblk_t n_blocks_count)
 {
 	ext4_fsblk_t o_blocks_count;
-	ext4_group_t o_groups_count;
 	ext4_grpblk_t last;
 	ext4_grpblk_t add;
 	struct buffer_head *bh;
@@ -965,7 +964,6 @@ int ext4_group_extend(struct super_block
 	 * yet: we're going to revalidate es->s_blocks_count after
 	 * taking the s_resize_lock below. */
 	o_blocks_count = ext4_blocks_count(es);
-	o_groups_count = EXT4_SB(sb)->s_groups_count;
 
 	if (test_opt(sb, DEBUG))
 		printk(KERN_DEBUG "EXT4-fs: extending last group from %llu uto %llu blocks\n",
Index: linux-2.6.35-rc2-gcc/fs/jbd2/journal.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/jbd2/journal.c
+++ linux-2.6.35-rc2-gcc/fs/jbd2/journal.c
@@ -1392,13 +1392,9 @@ int jbd2_journal_check_used_features (jo
 int jbd2_journal_check_available_features (journal_t *journal, unsigned long compat,
 				      unsigned long ro, unsigned long incompat)
 {
-	journal_superblock_t *sb;
-
 	if (!compat && !ro && !incompat)
 		return 1;
 
-	sb = journal->j_superblock;
-
 	/* We can support any known requested features iff the
 	 * superblock is in version 2.  Otherwise we fail to support any
 	 * extended sb features. */
@@ -1618,7 +1614,6 @@ int jbd2_journal_flush(journal_t *journa
 
 int jbd2_journal_wipe(journal_t *journal, int write)
 {
-	journal_superblock_t *sb;
 	int err = 0;
 
 	J_ASSERT (!(journal->j_flags & JBD2_LOADED));
@@ -1627,8 +1622,6 @@ int jbd2_journal_wipe(journal_t *journal
 	if (err)
 		return err;
 
-	sb = journal->j_superblock;
-
 	if (!journal->j_tail)
 		goto no_recovery;
 
@@ -2202,8 +2195,6 @@ void jbd2_journal_init_jbd_inode(struct
 void jbd2_journal_release_jbd_inode(journal_t *journal,
 				    struct jbd2_inode *jinode)
 {
-	int writeout = 0;
-
 	if (!journal)
 		return;
 restart:
@@ -2220,9 +2211,6 @@ restart:
 		goto restart;
 	}
 
-	/* Do we need to wait for data writeback? */
-	if (journal->j_committing_transaction == jinode->i_transaction)
-		writeout = 1;
 	if (jinode->i_transaction) {
 		list_del(&jinode->i_list);
 		jinode->i_transaction = NULL;
Index: linux-2.6.35-rc2-gcc/fs/jbd2/recovery.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/jbd2/recovery.c
+++ linux-2.6.35-rc2-gcc/fs/jbd2/recovery.c
@@ -285,12 +285,10 @@ int jbd2_journal_recover(journal_t *jour
 int jbd2_journal_skip_recovery(journal_t *journal)
 {
 	int			err;
-	journal_superblock_t *	sb;
 
 	struct recovery_info	info;
 
 	memset (&info, 0, sizeof(info));
-	sb = journal->j_superblock;
 
 	err = do_one_pass(journal, &info, PASS_SCAN);
 
@@ -365,11 +363,6 @@ static int do_one_pass(journal_t *journa
 	int			tag_bytes = journal_tag_bytes(journal);
 	__u32			crc32_sum = ~0; /* Transactional Checksums */
 
-	/* Precompute the maximum metadata descriptors in a descriptor block */
-	int			MAX_BLOCKS_PER_DESC;
-	MAX_BLOCKS_PER_DESC = ((journal->j_blocksize-sizeof(journal_header_t))
-			       / tag_bytes);

[PATCH] [16/23] XFS: Fix gcc 4.6 set but not read and unused statement warnings

[Andi Kleen]

For my configuration, that is without quota or RT.

Mostly dead code removed I think (but needs additional review) 

That is there were one or two bad error handling cases,
but they were not easily fixable, with comments 
and I left the warnings in for those for you to remember.

e.g. if there is a ENOSPC down in xfs_trans.c while
modifying the superblock it would not be handled.

Unused statements were mostly related to stub macros for disabled
features like QUOTA or RT ALLOC. I replace those with
inlines.

There were also some problems with variables used in ASSERT()
I partly moved those into the ASSERT itself and partly
used a new QASSERT that always evaluates.

Cc: xfs@oss.sgi.com

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

---
 fs/xfs/linux-2.6/xfs_sync.c |    3 +++
 fs/xfs/support/debug.h      |    4 ++++
 fs/xfs/xfs_alloc.c          |   10 +++-------
 fs/xfs/xfs_da_btree.c       |   15 +++++----------
 fs/xfs/xfs_dir2_block.c     |    6 +++---
 fs/xfs/xfs_filestream.c     |   10 ++--------
 fs/xfs/xfs_iget.c           |    3 ---
 fs/xfs/xfs_inode.c          |    4 ----
 fs/xfs/xfs_inode_item.c     |    8 ++------
 fs/xfs/xfs_log.c            |    2 --
 fs/xfs/xfs_quota.h          |   14 ++++++++++----
 fs/xfs/xfs_trans.c          |    1 +
 12 files changed, 33 insertions(+), 47 deletions(-)

Index: linux-2.6.35-rc2-gcc/fs/xfs/linux-2.6/xfs_sync.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/xfs/linux-2.6/xfs_sync.c
+++ linux-2.6.35-rc2-gcc/fs/xfs/linux-2.6/xfs_sync.c
@@ -554,6 +554,9 @@ xfs_sync_worker(
 		xfs_log_force(mp, 0);
 		xfs_reclaim_inodes(mp, 0);
 		/* dgc: errors ignored here */
+		/* ak: yes and you'll get a warning for it now when you
+		 * upgrade compilers.
+		 */
 		error = xfs_qm_sync(mp, SYNC_TRYLOCK);
 		if (xfs_log_need_covered(mp))
 			error = xfs_commit_dummy_trans(mp, 0);
Index: linux-2.6.35-rc2-gcc/fs/xfs/xfs_da_btree.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/xfs/xfs_da_btree.c
+++ linux-2.6.35-rc2-gcc/fs/xfs/xfs_da_btree.c
@@ -581,10 +581,8 @@ xfs_da_node_add(xfs_da_state_t *state, x
 	xfs_da_intnode_t *node;
 	xfs_da_node_entry_t *btree;
 	int tmp;
-	xfs_mount_t *mp;
 
 	node = oldblk->bp->data;
-	mp = state->mp;
 	ASSERT(be16_to_cpu(node->hdr.info.magic) == XFS_DA_NODE_MAGIC);
 	ASSERT((oldblk->index >= 0) && (oldblk->index <= be16_to_cpu(node->hdr.count)));
 	ASSERT(newblk->blkno != 0);
@@ -710,8 +708,6 @@ STATIC int
 xfs_da_root_join(xfs_da_state_t *state, xfs_da_state_blk_t *root_blk)
 {
 	xfs_da_intnode_t *oldroot;
-	/* REFERENCED */
-	xfs_da_blkinfo_t *blkinfo;
 	xfs_da_args_t *args;
 	xfs_dablk_t child;
 	xfs_dabuf_t *bp;
@@ -742,15 +738,14 @@ xfs_da_root_join(xfs_da_state_t *state,
 	if (error)
 		return(error);
 	ASSERT(bp != NULL);
-	blkinfo = bp->data;
 	if (be16_to_cpu(oldroot->hdr.level) == 1) {
-		ASSERT(be16_to_cpu(blkinfo->magic) == XFS_DIR2_LEAFN_MAGIC ||
-		       be16_to_cpu(blkinfo->magic) == XFS_ATTR_LEAF_MAGIC);
+		ASSERT(be16_to_cpu(bp->data->magic) == XFS_DIR2_LEAFN_MAGIC ||
+		       be16_to_cpu(bp->data->magic) == XFS_ATTR_LEAF_MAGIC);
 	} else {
-		ASSERT(be16_to_cpu(blkinfo->magic) == XFS_DA_NODE_MAGIC);
+		ASSERT(be16_to_cpu(bp->data->magic) == XFS_DA_NODE_MAGIC);
 	}
-	ASSERT(!blkinfo->forw);
-	ASSERT(!blkinfo->back);
+	ASSERT(!bp->data->forw);
+	ASSERT(!bp->data->back);
 	memcpy(root_blk->bp->data, bp->data, state->blocksize);
 	xfs_da_log_buf(args->trans, root_blk->bp, 0, state->blocksize - 1);
 	error = xfs_da_shrink_inode(args, child, bp);
Index: linux-2.6.35-rc2-gcc/fs/xfs/xfs_dir2_block.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/xfs/xfs_dir2_block.c
+++ linux-2.6.35-rc2-gcc/fs/xfs/xfs_dir2_block.c
@@ -1073,10 +1073,10 @@ xfs_dir2_sf_to_block(
 	 */
 
 	buf_len = dp->i_df.if_bytes;
-	buf = kmem_alloc(dp->i_df.if_bytes, KM_SLEEP);
+	buf = kmem_alloc(buf_len, KM_SLEEP);
 
-	memcpy(buf, sfp, dp->i_df.if_bytes);
-	xfs_idata_realloc(dp, -dp->i_df.if_bytes, XFS_DATA_FORK);
+	memcpy(buf, sfp, buf_len);
+	xfs_idata_realloc(dp, -buf_len, XFS_DATA_FORK);
 	dp->i_d.di_size = 0;
 	xfs_trans_log_inode(tp, dp, XFS_ILOG_CORE);
 	/*
Index: linux-2.6.35-rc2-gcc/fs/xfs/xfs_filestream.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/xfs/xfs_filestream.c
+++ linux-2.6.35-rc2-gcc/fs/xfs/xfs_filestream.c
@@ -140,9 +140,8 @@ _xfs_filestream_pick_ag(
 	int		flags,
 	xfs_extlen_t	minlen)
 {
-	int		streams, max_streams;
 	int		err, trylock, nscan;
-	xfs_extlen_t	longest, free, minfree, maxfree = 0;
+	xfs_extlen_t	longest, minfree, maxfree = 0;
 	xfs_agnumber_t	ag, max_ag = NULLAGNUMBER;
 	struct xfs_perag *pag;
 
@@ -174,7 +173,6 @@ _xfs_filestream_pick_ag(
 		/* Keep track of the AG with the most free blocks. */
 		if (pag->pagf_freeblks > maxfree) {
 			maxfree = pag->pagf_freeblks;
-			max_streams = atomic_read(&pag->pagf_fstrms);
 			max_ag = ag;
 		}
 
@@ -196,8 +194,6 @@ _xfs_filestream_pick_ag(
 		     (flags & XFS_PICK_LOWSPACE))) {
 
 			/* Break out, retaining the reference on the AG. */
-			free = pag->pagf_freeblks;
-			streams = atomic_read(&pag->pagf_fstrms);
 			xfs_perag_put(pag);
 			*agp = ag;
 			break;
@@ -234,8 +230,6 @@ next_ag:
 		if (max_ag != NULLAGNUMBER) {
 			xfs_filestream_get_ag(mp, max_ag);
 			TRACE_AG_PICK1(mp, max_ag, maxfree);
-			streams = max_streams;
-			free = maxfree;
 			*agp = max_ag;
 			break;
 		}
@@ -364,7 +358,7 @@ xfs_fstrm_free_func(
 	/* Drop the reference taken on the AG when the item was added. */
 	ref = xfs_filestream_put_ag(ip->i_mount, item->ag);
 
-	ASSERT(ref >= 0);
+	QASSERT(ref >= 0);
 	TRACE_FREE(ip->i_mount, ip, item->pip, item->ag,
 		xfs_filestream_peek_ag(ip->i_mount, item->ag));
 
Index: linux-2.6.35-rc2-gcc/fs/xfs/xfs_iget.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/xfs/xfs_iget.c
+++ linux-2.6.35-rc2-gcc/fs/xfs/xfs_iget.c
@@ -265,7 +265,6 @@ xfs_iget_cache_miss(
 {
 	struct xfs_inode	*ip;
 	int			error;
-	unsigned long		first_index, mask;
 	xfs_agino_t		agino = XFS_INO_TO_AGINO(mp, ino);
 
 	ip = xfs_inode_alloc(mp, ino);
@@ -302,8 +301,6 @@ xfs_iget_cache_miss(
 			BUG();
 	}
 
-	mask = ~(((XFS_INODE_CLUSTER_SIZE(mp) >> mp->m_sb.sb_inodelog)) - 1);
-	first_index = agino & mask;
 	write_lock(&pag->pag_ici_lock);
 
 	/* insert the new inode */
Index: linux-2.6.35-rc2-gcc/fs/xfs/xfs_inode.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/xfs/xfs_inode.c
+++ linux-2.6.35-rc2-gcc/fs/xfs/xfs_inode.c
@@ -925,7 +925,6 @@ xfs_iread_extents(
 	int		error;
 	xfs_ifork_t	*ifp;
 	xfs_extnum_t	nextents;
-	size_t		size;
 
 	if (unlikely(XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE)) {
 		XFS_ERROR_REPORT("xfs_iread_extents", XFS_ERRLEVEL_LOW,
@@ -933,7 +932,6 @@ xfs_iread_extents(
 		return XFS_ERROR(EFSCORRUPTED);
 	}
 	nextents = XFS_IFORK_NEXTENTS(ip, whichfork);
-	size = nextents * sizeof(xfs_bmbt_rec_t);
 	ifp = XFS_IFORK_PTR(ip, whichfork);
 
 	/*
@@ -3517,13 +3515,11 @@ xfs_iext_remove_indirect(
 	xfs_extnum_t	ext_diff;	/* extents to remove in current list */
 	xfs_extnum_t	nex1;		/* number of extents before idx */
 	xfs_extnum_t	nex2;		/* extents after idx + count */
-	int		nlists;		/* entries in indirection array */
 	int		page_idx = idx;	/* index in target extent list */
 
 	ASSERT(ifp->if_flags & XFS_IFEXTIREC);
 	erp = xfs_iext_idx_to_irec(ifp,  &page_idx, &erp_idx, 0);
 	ASSERT(erp != NULL);
-	nlists = ifp->if_real_bytes / XFS_IEXT_BUFSZ;
 	nex1 = page_idx;
 	ext_cnt = count;
 	while (ext_cnt) {
Index: linux-2.6.35-rc2-gcc/fs/xfs/xfs_inode_item.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/xfs/xfs_inode_item.c
+++ linux-2.6.35-rc2-gcc/fs/xfs/xfs_inode_item.c
@@ -220,7 +220,6 @@ xfs_inode_item_format(
 	xfs_inode_t		*ip;
 	size_t			data_bytes;
 	xfs_bmbt_rec_t		*ext_buffer;
-	int			nrecs;
 	xfs_mount_t		*mp;
 
 	ip = iip->ili_inode;
@@ -323,9 +322,8 @@ xfs_inode_item_format(
 			ASSERT(ip->i_df.if_u1.if_extents != NULL);
 			ASSERT(ip->i_d.di_nextents > 0);
 			ASSERT(iip->ili_extents_buf == NULL);
-			nrecs = ip->i_df.if_bytes /
-				(uint)sizeof(xfs_bmbt_rec_t);
-			ASSERT(nrecs > 0);
+			ASSERT((ip->i_df.if_bytes /
+				(uint)sizeof(xfs_bmbt_rec_t)) > 0);
 #ifdef XFS_NATIVE_HOST
 			if (nrecs == ip->i_d.di_nextents) {
 				/*
@@ -957,10 +955,8 @@ xfs_iflush_abort(
 	xfs_inode_t		*ip)
 {
 	xfs_inode_log_item_t	*iip = ip->i_itemp;
-	xfs_mount_t		*mp;
 
 	iip = ip->i_itemp;
-	mp = ip->i_mount;
 	if (iip) {
 		struct xfs_ail	*ailp = iip->ili_item.li_ailp;
 		if (iip->ili_item.li_flags & XFS_LI_IN_AIL) {
Index: linux-2.6.35-rc2-gcc/fs/xfs/xfs_log.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/xfs/xfs_log.c
+++ linux-2.6.35-rc2-gcc/fs/xfs/xfs_log.c
@@ -1047,7 +1047,6 @@ xlog_alloc_log(xfs_mount_t	*mp,
 	xlog_in_core_t		*iclog, *prev_iclog=NULL;
 	xfs_buf_t		*bp;
 	int			i;
-	int			iclogsize;
 	int			error = ENOMEM;
 	uint			log2_size = 0;
 
@@ -1127,7 +1126,6 @@ xlog_alloc_log(xfs_mount_t	*mp,
 	 * with different amounts of memory.  See the definition of
 	 * xlog_in_core_t in xfs_log_priv.h for details.
 	 */
-	iclogsize = log->l_iclog_size;
 	ASSERT(log->l_iclog_size >= 4096);
 	for (i=0; i < log->l_iclog_bufs; i++) {
 		*iclogp = kmem_zalloc(sizeof(xlog_in_core_t), KM_MAYFAIL);
Index: linux-2.6.35-rc2-gcc/fs/xfs/xfs_quota.h
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/xfs/xfs_quota.h
+++ linux-2.6.35-rc2-gcc/fs/xfs/xfs_quota.h
@@ -346,7 +346,13 @@ xfs_qm_vop_dqalloc(struct xfs_inode *ip,
 #define xfs_trans_mod_dquot_byino(tp, ip, fields, delta)
 #define xfs_trans_apply_dquot_deltas(tp)
 #define xfs_trans_unreserve_and_mod_dquots(tp)
-#define xfs_trans_reserve_quota_nblks(tp, ip, nblks, ninos, flags)	(0)
+
+static inline int xfs_trans_reserve_quota_nblks(struct xfs_trans *t,
+		struct xfs_inode *i, long a, long b, uint c)
+{
+	return 0;
+}
+
 #define xfs_trans_reserve_quota_bydquots(tp, mp, u, g, nb, ni, fl)	(0)
 #define xfs_qm_vop_create_dqattach(tp, ip, u, g)
 #define xfs_qm_vop_rename_dqattach(it)					(0)
@@ -355,13 +361,13 @@ xfs_qm_vop_dqalloc(struct xfs_inode *ip,
 #define xfs_qm_dqattach(ip, fl)						(0)
 #define xfs_qm_dqattach_locked(ip, fl)					(0)
 #define xfs_qm_dqdetach(ip)
-#define xfs_qm_dqrele(d)
+static inline void xfs_qm_dqrele(struct xfs_dquot *d) {}
 #define xfs_qm_statvfs(ip, s)
-#define xfs_qm_sync(mp, fl)						(0)
+static inline int xfs_qm_sync(struct xfs_mount *m, int i) { return 0; }
 #define xfs_qm_newmount(mp, a, b)					(0)
 #define xfs_qm_mount_quotas(mp)
 #define xfs_qm_unmount(mp)
-#define xfs_qm_unmount_quotas(mp)					(0)
+static inline void xfs_qm_unmount_quotas(struct xfs_mount *m) {}
 #endif /* CONFIG_XFS_QUOTA */
 
 #define xfs_trans_unreserve_quota_nblks(tp, ip, nblks, ninos, flags) \
Index: linux-2.6.35-rc2-gcc/fs/xfs/xfs_trans.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/xfs/xfs_trans.c
+++ linux-2.6.35-rc2-gcc/fs/xfs/xfs_trans.c
@@ -1120,6 +1120,7 @@ xfs_trans_unreserve_and_mod_sb(
 		error = xfs_mod_incore_sb_batch(tp->t_mountp, msb,
 			(uint)(msbp - msb), rsvd);
 		ASSERT(error == 0);
+		/* FIXME: need real error handling here, error can be ENOSPC */
 	}
 }
 
Index: linux-2.6.35-rc2-gcc/fs/xfs/xfs_alloc.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/xfs/xfs_alloc.c
+++ linux-2.6.35-rc2-gcc/fs/xfs/xfs_alloc.c
@@ -688,8 +688,6 @@ xfs_alloc_ag_vextent_near(
 	xfs_agblock_t	ltbno;		/* start bno of left side entry */
 	xfs_agblock_t	ltbnoa;		/* aligned ... */
 	xfs_extlen_t	ltdiff;		/* difference to left side entry */
-	/*REFERENCED*/
-	xfs_agblock_t	ltend;		/* end bno of left side entry */
 	xfs_extlen_t	ltlen;		/* length of left side entry */
 	xfs_extlen_t	ltlena;		/* aligned ... */
 	xfs_agblock_t	ltnew;		/* useful start bno of left side */
@@ -814,8 +812,7 @@ xfs_alloc_ag_vextent_near(
 		if ((error = xfs_alloc_get_rec(cnt_cur, &ltbno, &ltlen, &i)))
 			goto error0;
 		XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
-		ltend = ltbno + ltlen;
-		ASSERT(ltend <= be32_to_cpu(XFS_BUF_TO_AGF(args->agbp)->agf_length));
+		ASSERT(ltbno + ltlen <= be32_to_cpu(XFS_BUF_TO_AGF(args->agbp)->agf_length));
 		args->len = blen;
 		if (!xfs_alloc_fix_minleft(args)) {
 			xfs_btree_del_cursor(cnt_cur, XFS_BTREE_NOERROR);
@@ -828,7 +825,7 @@ xfs_alloc_ag_vextent_near(
 		 */
 		args->agbno = bnew;
 		ASSERT(bnew >= ltbno);
-		ASSERT(bnew + blen <= ltend);
+		ASSERT(bnew + blen <= ltbno + ltlen);
 		/*
 		 * Set up a cursor for the by-bno tree.
 		 */
@@ -1157,7 +1154,6 @@ xfs_alloc_ag_vextent_near(
 	/*
 	 * Fix up the length and compute the useful address.
 	 */
-	ltend = ltbno + ltlen;
 	args->len = XFS_EXTLEN_MIN(ltlena, args->maxlen);
 	xfs_alloc_fix_len(args);
 	if (!xfs_alloc_fix_minleft(args)) {
@@ -1170,7 +1166,7 @@ xfs_alloc_ag_vextent_near(
 	(void)xfs_alloc_compute_diff(args->agbno, rlen, args->alignment, ltbno,
 		ltlen, &ltnew);
 	ASSERT(ltnew >= ltbno);
-	ASSERT(ltnew + rlen <= ltend);
+	ASSERT(ltnew + rlen <= ltbno + ltlen);
 	ASSERT(ltnew + rlen <= be32_to_cpu(XFS_BUF_TO_AGF(args->agbp)->agf_length));
 	args->agbno = ltnew;
 	if ((error = xfs_alloc_fixup_trees(cnt_cur, bno_cur_lt, ltbno, ltlen,
Index: linux-2.6.35-rc2-gcc/fs/xfs/support/debug.h
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/xfs/support/debug.h
+++ linux-2.6.35-rc2-gcc/fs/xfs/support/debug.h
@@ -37,6 +37,9 @@ extern void assfail(char *expr, char *f,
 #ifndef DEBUG
 #define ASSERT(expr)	((void)0)
 
+/* Assert that always evaluates its input to avoid warnings */
+#define QASSERT(expr)	((void)(expr))
+
 #ifndef STATIC
 # define STATIC static noinline
 #endif
@@ -45,6 +48,7 @@ extern void assfail(char *expr, char *f,
 
 #define ASSERT(expr)	\
 	(unlikely(expr) ? (void)0 : assfail(#expr, __FILE__, __LINE__))
+#define QASSERT(expr) ASSERT(expr)
 
 #ifndef STATIC
 # define STATIC noinline

[PATCH] [16/23] XFS: Fix gcc 4.6 set but not read and unused statement warnings

[Andi Kleen]

For my configuration, that is without quota or RT.

Mostly dead code removed I think (but needs additional review) 

That is there were one or two bad error handling cases,
but they were not easily fixable, with comments 
and I left the warnings in for those for you to remember.

e.g. if there is a ENOSPC down in xfs_trans.c while
modifying the superblock it would not be handled.

Unused statements were mostly related to stub macros for disabled
features like QUOTA or RT ALLOC. I replace those with
inlines.

There were also some problems with variables used in ASSERT()
I partly moved those into the ASSERT itself and partly
used a new QASSERT that always evaluates.

Cc: xfs@oss.sgi.com

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

---
 fs/xfs/linux-2.6/xfs_sync.c |    3 +++
 fs/xfs/support/debug.h      |    4 ++++
 fs/xfs/xfs_alloc.c          |   10 +++-------
 fs/xfs/xfs_da_btree.c       |   15 +++++----------
 fs/xfs/xfs_dir2_block.c     |    6 +++---
 fs/xfs/xfs_filestream.c     |   10 ++--------
 fs/xfs/xfs_iget.c           |    3 ---
 fs/xfs/xfs_inode.c          |    4 ----
 fs/xfs/xfs_inode_item.c     |    8 ++------
 fs/xfs/xfs_log.c            |    2 --
 fs/xfs/xfs_quota.h          |   14 ++++++++++----
 fs/xfs/xfs_trans.c          |    1 +
 12 files changed, 33 insertions(+), 47 deletions(-)

Index: linux-2.6.35-rc2-gcc/fs/xfs/linux-2.6/xfs_sync.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/xfs/linux-2.6/xfs_sync.c
+++ linux-2.6.35-rc2-gcc/fs/xfs/linux-2.6/xfs_sync.c
@@ -554,6 +554,9 @@ xfs_sync_worker(
 		xfs_log_force(mp, 0);
 		xfs_reclaim_inodes(mp, 0);
 		/* dgc: errors ignored here */
+		/* ak: yes and you'll get a warning for it now when you
+		 * upgrade compilers.
+		 */
 		error = xfs_qm_sync(mp, SYNC_TRYLOCK);
 		if (xfs_log_need_covered(mp))
 			error = xfs_commit_dummy_trans(mp, 0);
Index: linux-2.6.35-rc2-gcc/fs/xfs/xfs_da_btree.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/xfs/xfs_da_btree.c
+++ linux-2.6.35-rc2-gcc/fs/xfs/xfs_da_btree.c
@@ -581,10 +581,8 @@ xfs_da_node_add(xfs_da_state_t *state, x
 	xfs_da_intnode_t *node;
 	xfs_da_node_entry_t *btree;
 	int tmp;
-	xfs_mount_t *mp;
 
 	node = oldblk->bp->data;
-	mp = state->mp;
 	ASSERT(be16_to_cpu(node->hdr.info.magic) == XFS_DA_NODE_MAGIC);
 	ASSERT((oldblk->index >= 0) && (oldblk->index <= be16_to_cpu(node->hdr.count)));
 	ASSERT(newblk->blkno != 0);
@@ -710,8 +708,6 @@ STATIC int
 xfs_da_root_join(xfs_da_state_t *state, xfs_da_state_blk_t *root_blk)
 {
 	xfs_da_intnode_t *oldroot;
-	/* REFERENCED */
-	xfs_da_blkinfo_t *blkinfo;
 	xfs_da_args_t *args;
 	xfs_dablk_t child;
 	xfs_dabuf_t *bp;
@@ -742,15 +738,14 @@ xfs_da_root_join(xfs_da_state_t *state,
 	if (error)
 		return(error);
 	ASSERT(bp != NULL);
-	blkinfo = bp->data;
 	if (be16_to_cpu(oldroot->hdr.level) == 1) {
-		ASSERT(be16_to_cpu(blkinfo->magic) == XFS_DIR2_LEAFN_MAGIC ||
-		       be16_to_cpu(blkinfo->magic) == XFS_ATTR_LEAF_MAGIC);
+		ASSERT(be16_to_cpu(bp->data->magic) == XFS_DIR2_LEAFN_MAGIC ||
+		       be16_to_cpu(bp->data->magic) == XFS_ATTR_LEAF_MAGIC);
 	} else {
-		ASSERT(be16_to_cpu(blkinfo->magic) == XFS_DA_NODE_MAGIC);
+		ASSERT(be16_to_cpu(bp->data->magic) == XFS_DA_NODE_MAGIC);
 	}
-	ASSERT(!blkinfo->forw);
-	ASSERT(!blkinfo->back);
+	ASSERT(!bp->data->forw);
+	ASSERT(!bp->data->back);
 	memcpy(root_blk->bp->data, bp->data, state->blocksize);
 	xfs_da_log_buf(args->trans, root_blk->bp, 0, state->blocksize - 1);
 	error = xfs_da_shrink_inode(args, child, bp);
Index: linux-2.6.35-rc2-gcc/fs/xfs/xfs_dir2_block.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/xfs/xfs_dir2_block.c
+++ linux-2.6.35-rc2-gcc/fs/xfs/xfs_dir2_block.c
@@ -1073,10 +1073,10 @@ xfs_dir2_sf_to_block(
 	 */
 
 	buf_len = dp->i_df.if_bytes;
-	buf = kmem_alloc(dp->i_df.if_bytes, KM_SLEEP);
+	buf = kmem_alloc(buf_len, KM_SLEEP);
 
-	memcpy(buf, sfp, dp->i_df.if_bytes);
-	xfs_idata_realloc(dp, -dp->i_df.if_bytes, XFS_DATA_FORK);
+	memcpy(buf, sfp, buf_len);
+	xfs_idata_realloc(dp, -buf_len, XFS_DATA_FORK);
 	dp->i_d.di_size = 0;
 	xfs_trans_log_inode(tp, dp, XFS_ILOG_CORE);
 	/*
Index: linux-2.6.35-rc2-gcc/fs/xfs/xfs_filestream.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/xfs/xfs_filestream.c
+++ linux-2.6.35-rc2-gcc/fs/xfs/xfs_filestream.c
@@ -140,9 +140,8 @@ _xfs_filestream_pick_ag(
 	int		flags,
 	xfs_extlen_t	minlen)
 {
-	int		streams, max_streams;
 	int		err, trylock, nscan;
-	xfs_extlen_t	longest, free, minfree, maxfree = 0;
+	xfs_extlen_t	longest, minfree, maxfree = 0;
 	xfs_agnumber_t	ag, max_ag = NULLAGNUMBER;
 	struct xfs_perag *pag;
 
@@ -174,7 +173,6 @@ _xfs_filestream_pick_ag(
 		/* Keep track of the AG with the most free blocks. */
 		if (pag->pagf_freeblks > maxfree) {
 			maxfree = pag->pagf_freeblks;
-			max_streams = atomic_read(&pag->pagf_fstrms);
 			max_ag = ag;
 		}
 
@@ -196,8 +194,6 @@ _xfs_filestream_pick_ag(
 		     (flags & XFS_PICK_LOWSPACE))) {
 
 			/* Break out, retaining the reference on the AG. */
-			free = pag->pagf_freeblks;
-			streams = atomic_read(&pag->pagf_fstrms);
 			xfs_perag_put(pag);
 			*agp = ag;
 			break;
@@ -234,8 +230,6 @@ next_ag:
 		if (max_ag != NULLAGNUMBER) {
 			xfs_filestream_get_ag(mp, max_ag);
 			TRACE_AG_PICK1(mp, max_ag, maxfree);
-			streams = max_streams;
-			free = maxfree;
 			*agp = max_ag;
 			break;
 		}
@@ -364,7 +358,7 @@ xfs_fstrm_free_func(
 	/* Drop the reference taken on the AG when the item was added. */
 	ref = xfs_filestream_put_ag(ip->i_mount, item->ag);
 
-	ASSERT(ref >= 0);
+	QASSERT(ref >= 0);
 	TRACE_FREE(ip->i_mount, ip, item->pip, item->ag,
 		xfs_filestream_peek_ag(ip->i_mount, item->ag));
 
Index: linux-2.6.35-rc2-gcc/fs/xfs/xfs_iget.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/xfs/xfs_iget.c
+++ linux-2.6.35-rc2-gcc/fs/xfs/xfs_iget.c
@@ -265,7 +265,6 @@ xfs_iget_cache_miss(
 {
 	struct xfs_inode	*ip;
 	int			error;
-	unsigned long		first_index, mask;
 	xfs_agino_t		agino = XFS_INO_TO_AGINO(mp, ino);
 
 	ip = xfs_inode_alloc(mp, ino);
@@ -302,8 +301,6 @@ xfs_iget_cache_miss(
 			BUG();
 	}
 
-	mask = ~(((XFS_INODE_CLUSTER_SIZE(mp) >> mp->m_sb.sb_inodelog)) - 1);
-	first_index = agino & mask;
 	write_lock(&pag->pag_ici_lock);
 
 	/* insert the new inode */
Index: linux-2.6.35-rc2-gcc/fs/xfs/xfs_inode.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/xfs/xfs_inode.c
+++ linux-2.6.35-rc2-gcc/fs/xfs/xfs_inode.c
@@ -925,7 +925,6 @@ xfs_iread_extents(
 	int		error;
 	xfs_ifork_t	*ifp;
 	xfs_extnum_t	nextents;
-	size_t		size;
 
 	if (unlikely(XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE)) {
 		XFS_ERROR_REPORT("xfs_iread_extents", XFS_ERRLEVEL_LOW,
@@ -933,7 +932,6 @@ xfs_iread_extents(
 		return XFS_ERROR(EFSCORRUPTED);
 	}
 	nextents = XFS_IFORK_NEXTENTS(ip, whichfork);
-	size = nextents * sizeof(xfs_bmbt_rec_t);
 	ifp = XFS_IFORK_PTR(ip, whichfork);
 
 	/*
@@ -3517,13 +3515,11 @@ xfs_iext_remove_indirect(
 	xfs_extnum_t	ext_diff;	/* extents to remove in current list */
 	xfs_extnum_t	nex1;		/* number of extents before idx */
 	xfs_extnum_t	nex2;		/* extents after idx + count */
-	int		nlists;		/* entries in indirection array */
 	int		page_idx = idx;	/* index in target extent list */
 
 	ASSERT(ifp->if_flags & XFS_IFEXTIREC);
 	erp = xfs_iext_idx_to_irec(ifp,  &page_idx, &erp_idx, 0);
 	ASSERT(erp != NULL);
-	nlists = ifp->if_real_bytes / XFS_IEXT_BUFSZ;
 	nex1 = page_idx;
 	ext_cnt = count;
 	while (ext_cnt) {
Index: linux-2.6.35-rc2-gcc/fs/xfs/xfs_inode_item.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/xfs/xfs_inode_item.c
+++ linux-2.6.35-rc2-gcc/fs/xfs/xfs_inode_item.c
@@ -220,7 +220,6 @@ xfs_inode_item_format(
 	xfs_inode_t		*ip;
 	size_t			data_bytes;
 	xfs_bmbt_rec_t		*ext_buffer;
-	int			nrecs;
 	xfs_mount_t		*mp;
 
 	ip = iip->ili_inode;
@@ -323,9 +322,8 @@ xfs_inode_item_format(
 			ASSERT(ip->i_df.if_u1.if_extents != NULL);
 			ASSERT(ip->i_d.di_nextents > 0);
 			ASSERT(iip->ili_extents_buf == NULL);
-			nrecs = ip->i_df.if_bytes /
-				(uint)sizeof(xfs_bmbt_rec_t);
-			ASSERT(nrecs > 0);
+			ASSERT((ip->i_df.if_bytes /
+				(uint)sizeof(xfs_bmbt_rec_t)) > 0);
 #ifdef XFS_NATIVE_HOST
 			if (nrecs == ip->i_d.di_nextents) {
 				/*
@@ -957,10 +955,8 @@ xfs_iflush_abort(
 	xfs_inode_t		*ip)
 {
 	xfs_inode_log_item_t	*iip = ip->i_itemp;
-	xfs_mount_t		*mp;
 
 	iip = ip->i_itemp;
-	mp = ip->i_mount;
 	if (iip) {
 		struct xfs_ail	*ailp = iip->ili_item.li_ailp;
 		if (iip->ili_item.li_flags & XFS_LI_IN_AIL) {
Index: linux-2.6.35-rc2-gcc/fs/xfs/xfs_log.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/xfs/xfs_log.c
+++ linux-2.6.35-rc2-gcc/fs/xfs/xfs_log.c
@@ -1047,7 +1047,6 @@ xlog_alloc_log(xfs_mount_t	*mp,
 	xlog_in_core_t		*iclog, *prev_iclog=NULL;
 	xfs_buf_t		*bp;
 	int			i;
-	int			iclogsize;
 	int			error = ENOMEM;
 	uint			log2_size = 0;
 
@@ -1127,7 +1126,6 @@ xlog_alloc_log(xfs_mount_t	*mp,
 	 * with different amounts of memory.  See the definition of
 	 * xlog_in_core_t in xfs_log_priv.h for details.
 	 */
-	iclogsize = log->l_iclog_size;
 	ASSERT(log->l_iclog_size >= 4096);
 	for (i=0; i < log->l_iclog_bufs; i++) {
 		*iclogp = kmem_zalloc(sizeof(xlog_in_core_t), KM_MAYFAIL);
Index: linux-2.6.35-rc2-gcc/fs/xfs/xfs_quota.h
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/xfs/xfs_quota.h
+++ linux-2.6.35-rc2-gcc/fs/xfs/xfs_quota.h
@@ -346,7 +346,13 @@ xfs_qm_vop_dqalloc(struct xfs_inode *ip,
 #define xfs_trans_mod_dquot_byino(tp, ip, fields, delta)
 #define xfs_trans_apply_dquot_deltas(tp)
 #define xfs_trans_unreserve_and_mod_dquots(tp)
-#define xfs_trans_reserve_quota_nblks(tp, ip, nblks, ninos, flags)	(0)
+
+static inline int xfs_trans_reserve_quota_nblks(struct xfs_trans *t,
+		struct xfs_inode *i, long a, long b, uint c)
+{
+	return 0;
+}
+
 #define xfs_trans_reserve_quota_bydquots(tp, mp, u, g, nb, ni, fl)	(0)
 #define xfs_qm_vop_create_dqattach(tp, ip, u, g)
 #define xfs_qm_vop_rename_dqattach(it)					(0)
@@ -355,13 +361,13 @@ xfs_qm_vop_dqalloc(struct xfs_inode *ip,
 #define xfs_qm_dqattach(ip, fl)						(0)
 #define xfs_qm_dqattach_locked(ip, fl)					(0)
 #define xfs_qm_dqdetach(ip)
-#define xfs_qm_dqrele(d)
+static inline void xfs_qm_dqrele(struct xfs_dquot *d) {}
 #define xfs_qm_statvfs(ip, s)
-#define xfs_qm_sync(mp, fl)						(0)
+static inline int xfs_qm_sync(struct xfs_mount *m, int i) { return 0; }
 #define xfs_qm_newmount(mp, a, b)					(0)
 #define xfs_qm_mount_quotas(mp)
 #define xfs_qm_unmount(mp)
-#define xfs_qm_unmount_quotas(mp)					(0)
+static inline void xfs_qm_unmount_quotas(struct xfs_mount *m) {}
 #endif /* CONFIG_XFS_QUOTA */
 
 #define xfs_trans_unreserve_quota_nblks(tp, ip, nblks, ninos, flags) \
Index: linux-2.6.35-rc2-gcc/fs/xfs/xfs_trans.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/xfs/xfs_trans.c
+++ linux-2.6.35-rc2-gcc/fs/xfs/xfs_trans.c
@@ -1120,6 +1120,7 @@ xfs_trans_unreserve_and_mod_sb(
 		error = xfs_mod_incore_sb_batch(tp->t_mountp, msb,
 			(uint)(msbp - msb), rsvd);
 		ASSERT(error == 0);
+		/* FIXME: need real error handling here, error can be ENOSPC */
 	}
 }
 
Index: linux-2.6.35-rc2-gcc/fs/xfs/xfs_alloc.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/xfs/xfs_alloc.c
+++ linux-2.6.35-rc2-gcc/fs/xfs/xfs_alloc.c
@@ -688,8 +688,6 @@ xfs_alloc_ag_vextent_near(
 	xfs_agblock_t	ltbno;		/* start bno of left side entry */
 	xfs_agblock_t	ltbnoa;		/* aligned ... */
 	xfs_extlen_t	ltdiff;		/* difference to left side entry */
-	/*REFERENCED*/
-	xfs_agblock_t	ltend;		/* end bno of left side entry */
 	xfs_extlen_t	ltlen;		/* length of left side entry */
 	xfs_extlen_t	ltlena;		/* aligned ... */
 	xfs_agblock_t	ltnew;		/* useful start bno of left side */
@@ -814,8 +812,7 @@ xfs_alloc_ag_vextent_near(
 		if ((error = xfs_alloc_get_rec(cnt_cur, &ltbno, &ltlen, &i)))
 			goto error0;
 		XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
-		ltend = ltbno + ltlen;
-		ASSERT(ltend <= be32_to_cpu(XFS_BUF_TO_AGF(args->agbp)->agf_length));
+		ASSERT(ltbno + ltlen <= be32_to_cpu(XFS_BUF_TO_AGF(args->agbp)->agf_length));
 		args->len = blen;
 		if (!xfs_alloc_fix_minleft(args)) {
 			xfs_btree_del_cursor(cnt_cur, XFS_BTREE_NOERROR);
@@ -828,7 +825,7 @@ xfs_alloc_ag_vextent_near(
 		 */
 		args->agbno = bnew;
 		ASSERT(bnew >= ltbno);
-		ASSERT(bnew + blen <= ltend);
+		ASSERT(bnew + blen <= ltbno + ltlen);
 		/*
 		 * Set up a cursor for the by-bno tree.
 		 */
@@ -1157,7 +1154,6 @@ xfs_alloc_ag_vextent_near(
 	/*
 	 * Fix up the length and compute the useful address.
 	 */
-	ltend = ltbno + ltlen;
 	args->len = XFS_EXTLEN_MIN(ltlena, args->maxlen);
 	xfs_alloc_fix_len(args);
 	if (!xfs_alloc_fix_minleft(args)) {
@@ -1170,7 +1166,7 @@ xfs_alloc_ag_vextent_near(
 	(void)xfs_alloc_compute_diff(args->agbno, rlen, args->alignment, ltbno,
 		ltlen, &ltnew);
 	ASSERT(ltnew >= ltbno);
-	ASSERT(ltnew + rlen <= ltend);
+	ASSERT(ltnew + rlen <= ltbno + ltlen);
 	ASSERT(ltnew + rlen <= be32_to_cpu(XFS_BUF_TO_AGF(args->agbp)->agf_length));
 	args->agbno = ltnew;
 	if ((error = xfs_alloc_fixup_trees(cnt_cur, bno_cur_lt, ltbno, ltlen,
Index: linux-2.6.35-rc2-gcc/fs/xfs/support/debug.h
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/xfs/support/debug.h
+++ linux-2.6.35-rc2-gcc/fs/xfs/support/debug.h
@@ -37,6 +37,9 @@ extern void assfail(char *expr, char *f,
 #ifndef DEBUG
 #define ASSERT(expr)	((void)0)
 
+/* Assert that always evaluates its input to avoid warnings */
+#define QASSERT(expr)	((void)(expr))
+
 #ifndef STATIC
 # define STATIC static noinline
 #endif
@@ -45,6 +48,7 @@ extern void assfail(char *expr, char *f,
 
 #define ASSERT(expr)	\
 	(unlikely(expr) ? (void)0 : assfail(#expr, __FILE__, __LINE__))
+#define QASSERT(expr) ASSERT(expr)
 
 #ifndef STATIC
 # define STATIC noinline

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH] [17/23] EXT3: Fix set but unused variables

[Andi Kleen]

I think it's mostly dead code, but needs more review. I was not fully sure
about the jbd case.

cc: tytso@mit.edu
cc: linux-ext4@vger.kernel.org
Signed-off-by: Andi Kleen <ak@linux.intel.com>

---
 fs/ext3/namei.c   |    3 +--
 fs/ext3/resize.c  |    2 --
 fs/jbd/journal.c  |    7 -------
 fs/jbd/recovery.c |    7 -------
 4 files changed, 1 insertion(+), 18 deletions(-)

Index: linux-2.6.35-rc2-gcc/fs/ext3/namei.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/ext3/namei.c
+++ linux-2.6.35-rc2-gcc/fs/ext3/namei.c
@@ -1447,7 +1447,6 @@ static int ext3_add_entry (handle_t *han
 	struct inode *inode)
 {
 	struct inode *dir = dentry->d_parent->d_inode;
-	unsigned long offset;
 	struct buffer_head * bh;
 	struct ext3_dir_entry_2 *de;
 	struct super_block * sb;
@@ -1469,7 +1468,7 @@ static int ext3_add_entry (handle_t *han
 		ext3_mark_inode_dirty(handle, dir);
 	}
 	blocks = dir->i_size >> sb->s_blocksize_bits;
-	for (block = 0, offset = 0; block < blocks; block++) {
+	for (block = 0; block < blocks; block++) {
 		bh = ext3_bread(handle, dir, block, 0, &retval);
 		if(!bh)
 			return retval;
Index: linux-2.6.35-rc2-gcc/fs/ext3/resize.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/ext3/resize.c
+++ linux-2.6.35-rc2-gcc/fs/ext3/resize.c
@@ -964,7 +964,6 @@ int ext3_group_extend(struct super_block
 		      ext3_fsblk_t n_blocks_count)
 {
 	ext3_fsblk_t o_blocks_count;
-	unsigned long o_groups_count;
 	ext3_grpblk_t last;
 	ext3_grpblk_t add;
 	struct buffer_head * bh;
@@ -976,7 +975,6 @@ int ext3_group_extend(struct super_block
 	 * yet: we're going to revalidate es->s_blocks_count after
 	 * taking the s_resize_lock below. */
 	o_blocks_count = le32_to_cpu(es->s_blocks_count);
-	o_groups_count = EXT3_SB(sb)->s_groups_count;
 
 	if (test_opt(sb, DEBUG))
 		printk(KERN_DEBUG "EXT3-fs: extending last group from "E3FSBLK" uto "E3FSBLK" blocks\n",
Index: linux-2.6.35-rc2-gcc/fs/jbd/journal.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/jbd/journal.c
+++ linux-2.6.35-rc2-gcc/fs/jbd/journal.c
@@ -1281,13 +1281,9 @@ int journal_check_used_features (journal
 int journal_check_available_features (journal_t *journal, unsigned long compat,
 				      unsigned long ro, unsigned long incompat)
 {
-	journal_superblock_t *sb;
-
 	if (!compat && !ro && !incompat)
 		return 1;
 
-	sb = journal->j_superblock;
-
 	/* We can support any known requested features iff the
 	 * superblock is in version 2.  Otherwise we fail to support any
 	 * extended sb features. */
@@ -1481,7 +1477,6 @@ int journal_flush(journal_t *journal)
 
 int journal_wipe(journal_t *journal, int write)
 {
-	journal_superblock_t *sb;
 	int err = 0;
 
 	J_ASSERT (!(journal->j_flags & JFS_LOADED));
@@ -1490,8 +1485,6 @@ int journal_wipe(journal_t *journal, int
 	if (err)
 		return err;
 
-	sb = journal->j_superblock;
-
 	if (!journal->j_tail)
 		goto no_recovery;
 
Index: linux-2.6.35-rc2-gcc/fs/jbd/recovery.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/jbd/recovery.c
+++ linux-2.6.35-rc2-gcc/fs/jbd/recovery.c
@@ -283,12 +283,10 @@ int journal_recover(journal_t *journal)
 int journal_skip_recovery(journal_t *journal)
 {
 	int			err;
-	journal_superblock_t *	sb;
 
 	struct recovery_info	info;
 
 	memset (&info, 0, sizeof(info));
-	sb = journal->j_superblock;
 
 	err = do_one_pass(journal, &info, PASS_SCAN);
 
@@ -321,11 +319,6 @@ static int do_one_pass(journal_t *journa
 	unsigned int		sequence;
 	int			blocktype;
 
-	/* Precompute the maximum metadata descriptors in a descriptor block */
-	int			MAX_BLOCKS_PER_DESC;
-	MAX_BLOCKS_PER_DESC = ((journal->j_blocksize-sizeof(journal_header_t))
-			       / sizeof(journal_block_tag_t));
-
 	/*
 	 * First thing is to establish what we expect to find in the log
 	 * (in terms of transaction IDs), and where (in terms of log

[PATCH] [18/23] ACPI: Fix unused but set variables in ACPI

[Andi Kleen]

Some minor improvements in error handling, but overall
it was mostly dead code.

cc: lenb@kernel.org
cc: linux-acpi@vger.kernel.org

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

---
 drivers/acpi/acpica/utmutex.c    |    5 +----
 drivers/acpi/numa.c              |    4 +---
 drivers/acpi/osl.c               |    7 +++----
 drivers/acpi/power.c             |    6 +-----
 drivers/acpi/processor_idle.c    |    3 +--
 drivers/acpi/processor_thermal.c |    2 ++
 drivers/acpi/video.c             |   10 +++++-----
 drivers/ata/libata-acpi.c        |    6 ------
 8 files changed, 14 insertions(+), 29 deletions(-)

Index: linux-2.6.35-rc2-gcc/drivers/acpi/processor_thermal.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/drivers/acpi/processor_thermal.c
+++ linux-2.6.35-rc2-gcc/drivers/acpi/processor_thermal.c
@@ -506,6 +506,8 @@ static ssize_t acpi_processor_write_limi
 	}
 
 	result = acpi_processor_apply_limit(pr);
+	if (result < 0)
+		return result;
 
 	return count;
 }
Index: linux-2.6.35-rc2-gcc/drivers/acpi/acpica/utmutex.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/drivers/acpi/acpica/utmutex.c
+++ linux-2.6.35-rc2-gcc/drivers/acpi/acpica/utmutex.c
@@ -279,13 +279,10 @@ acpi_status acpi_ut_acquire_mutex(acpi_m
 
 acpi_status acpi_ut_release_mutex(acpi_mutex_handle mutex_id)
 {
-	acpi_thread_id this_thread_id;
-
 	ACPI_FUNCTION_NAME(ut_release_mutex);
 
-	this_thread_id = acpi_os_get_thread_id();
 	ACPI_DEBUG_PRINT((ACPI_DB_MUTEX, "Thread %p releasing Mutex [%s]\n",
-			  ACPI_CAST_PTR(void, this_thread_id),
+			  ACPI_CAST_PTR(void, acpi_os_get_thread_id()),
 			  acpi_ut_get_mutex_name(mutex_id)));
 
 	if (mutex_id > ACPI_MAX_MUTEX) {
Index: linux-2.6.35-rc2-gcc/drivers/acpi/numa.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/drivers/acpi/numa.c
+++ linux-2.6.35-rc2-gcc/drivers/acpi/numa.c
@@ -255,12 +255,10 @@ acpi_parse_memory_affinity(struct acpi_s
 
 static int __init acpi_parse_srat(struct acpi_table_header *table)
 {
-	struct acpi_table_srat *srat;
-
 	if (!table)
 		return -EINVAL;
 
-	srat = (struct acpi_table_srat *)table;
+	/* Real work done in acpi_table_parse_srat below. */
 
 	return 0;
 }
Index: linux-2.6.35-rc2-gcc/drivers/acpi/osl.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/drivers/acpi/osl.c
+++ linux-2.6.35-rc2-gcc/drivers/acpi/osl.c
@@ -141,15 +141,14 @@ static struct osi_linux {
 static void __init acpi_request_region (struct acpi_generic_address *addr,
 	unsigned int length, char *desc)
 {
-	struct resource *res;
-
 	if (!addr->address || !length)
 		return;
 
+	/* Resources are never freed */
 	if (addr->space_id == ACPI_ADR_SPACE_SYSTEM_IO)
-		res = request_region(addr->address, length, desc);
+		request_region(addr->address, length, desc);
 	else if (addr->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY)
-		res = request_mem_region(addr->address, length, desc);
+		request_mem_region(addr->address, length, desc);
 }
 
 static int __init acpi_reserve_resources(void)
Index: linux-2.6.35-rc2-gcc/drivers/acpi/processor_idle.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/drivers/acpi/processor_idle.c
+++ linux-2.6.35-rc2-gcc/drivers/acpi/processor_idle.c
@@ -795,13 +795,12 @@ static inline void acpi_idle_do_entry(st
 	} else if (cx->entry_method == ACPI_CSTATE_HALT) {
 		acpi_safe_halt();
 	} else {
-		int unused;
 		/* IO port based C-state */
 		inb(cx->address);
 		/* Dummy wait op - must do something useless after P_LVL2 read
 		   because chipsets cannot guarantee that STPCLK# signal
 		   gets asserted in time to freeze execution properly. */
-		unused = inl(acpi_gbl_FADT.xpm_timer_block.address);
+		inl(acpi_gbl_FADT.xpm_timer_block.address);
 	}
 	start_critical_timings();
 }
Index: linux-2.6.35-rc2-gcc/drivers/acpi/video.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/drivers/acpi/video.c
+++ linux-2.6.35-rc2-gcc/drivers/acpi/video.c
@@ -2140,7 +2140,7 @@ acpi_video_bus_get_devices(struct acpi_v
 		status = acpi_video_bus_get_one_device(dev, video);
 		if (ACPI_FAILURE(status)) {
 			printk(KERN_WARNING PREFIX
-					"Cant attach device");
+					"Cant attach device\n");
 			continue;
 		}
 	}
@@ -2150,19 +2150,19 @@ acpi_video_bus_get_devices(struct acpi_v
 static int acpi_video_bus_put_one_device(struct acpi_video_device *device)
 {
 	acpi_status status;
-	struct acpi_video_bus *video;
-
 
 	if (!device || !device->video)
 		return -ENOENT;
 
-	video = device->video;
-
 	acpi_video_device_remove_fs(device->dev);
 
 	status = acpi_remove_notify_handler(device->dev->handle,
 					    ACPI_DEVICE_NOTIFY,
 					    acpi_video_device_notify);
+	if (ACPI_FAILURE(status)) {
+		printk(KERN_WARNING PREFIX
+		       "Cant remove video notify handler\n");
+	}
 	if (device->backlight) {
 		sysfs_remove_link(&device->backlight->dev.kobj, "device");
 		backlight_device_unregister(device->backlight);
Index: linux-2.6.35-rc2-gcc/drivers/ata/libata-acpi.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/drivers/ata/libata-acpi.c
+++ linux-2.6.35-rc2-gcc/drivers/ata/libata-acpi.c
@@ -145,12 +145,6 @@ static void ata_acpi_handle_hotplug(stru
 	struct ata_eh_info *ehi = &ap->link.eh_info;
 	int wait = 0;
 	unsigned long flags;
-	acpi_handle handle;
-
-	if (dev)
-		handle = dev->acpi_handle;
-	else
-		handle = ap->acpi_handle;
 
 	spin_lock_irqsave(ap->lock, flags);
 	/*
Index: linux-2.6.35-rc2-gcc/drivers/acpi/power.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/drivers/acpi/power.c
+++ linux-2.6.35-rc2-gcc/drivers/acpi/power.c
@@ -553,8 +553,6 @@ static int acpi_power_seq_show(struct se
 	int result = 0, state;
 	struct acpi_power_resource *resource = NULL;
 	struct list_head *node, *next;
-	struct acpi_power_reference *ref;
-
 
 	resource = seq->private;
 
@@ -579,10 +577,8 @@ static int acpi_power_seq_show(struct se
 	}
 
 	mutex_lock(&resource->resource_lock);
-	list_for_each_safe(node, next, &resource->reference) {
-		ref = container_of(node, struct acpi_power_reference, node);
+	list_for_each_safe(node, next, &resource->reference)
 		count++;
-	}
 	mutex_unlock(&resource->resource_lock);
 
 	seq_printf(seq, "system level:            S%d\n"

[PATCH] [19/23] KVM: Fix unused but set warnings

[Andi Kleen]

No real bugs in this one, the real bug I found is in a separate
patch.

Cc: avi@redhat.com
Cc: kvm@vger.kernel.org

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

---
 arch/x86/kvm/paging_tmpl.h |    1 +
 arch/x86/kvm/vmx.c         |    3 +--
 virt/kvm/assigned-dev.c    |    2 --
 3 files changed, 2 insertions(+), 4 deletions(-)

Index: linux-2.6.35-rc2-gcc/arch/x86/kvm/paging_tmpl.h
===================================================================
--- linux-2.6.35-rc2-gcc.orig/arch/x86/kvm/paging_tmpl.h
+++ linux-2.6.35-rc2-gcc/arch/x86/kvm/paging_tmpl.h
@@ -442,6 +442,7 @@ static int FNAME(page_fault)(struct kvm_
 	kvm_mmu_free_some_pages(vcpu);
 	sptep = FNAME(fetch)(vcpu, addr, &walker, user_fault, write_fault,
 			     level, &write_pt, pfn);
+	(void)sptep;
 	pgprintk("%s: shadow pte %p %llx ptwrite %d\n", __func__,
 		 sptep, *sptep, write_pt);
 
Index: linux-2.6.35-rc2-gcc/arch/x86/kvm/vmx.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/arch/x86/kvm/vmx.c
+++ linux-2.6.35-rc2-gcc/arch/x86/kvm/vmx.c
@@ -1624,10 +1624,9 @@ static void enter_pmode(struct kvm_vcpu
 static gva_t rmode_tss_base(struct kvm *kvm)
 {
 	if (!kvm->arch.tss_addr) {
-		struct kvm_memslots *slots;
 		gfn_t base_gfn;
 
-		slots = kvm_memslots(kvm);
+		kvm_memslots(kvm);
 		base_gfn = kvm->memslots->memslots[0].base_gfn +
 				 kvm->memslots->memslots[0].npages - 3;
 		return base_gfn << PAGE_SHIFT;
Index: linux-2.6.35-rc2-gcc/virt/kvm/assigned-dev.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/virt/kvm/assigned-dev.c
+++ linux-2.6.35-rc2-gcc/virt/kvm/assigned-dev.c
@@ -58,12 +58,10 @@ static int find_index_from_host_irq(stru
 static void kvm_assigned_dev_interrupt_work_handler(struct work_struct *work)
 {
 	struct kvm_assigned_dev_kernel *assigned_dev;
-	struct kvm *kvm;
 	int i;
 
 	assigned_dev = container_of(work, struct kvm_assigned_dev_kernel,
 				    interrupt_work);
-	kvm = assigned_dev->kvm;
 
 	spin_lock_irq(&assigned_dev->assigned_dev_lock);
 	if (assigned_dev->irq_requested_type & KVM_DEV_IRQ_HOST_MSIX) {

[PATCH] [20/23] MM: Fix unused but set warnings

[Andi Kleen]

No real bugs, just some dead code and some fixups.

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

---
 arch/x86/include/asm/pgtable_64.h |    4 ++--
 include/linux/highmem.h           |    6 +++++-
 include/linux/mmdebug.h           |    2 +-
 mm/filemap.c                      |    2 --
 mm/memory.c                       |    2 --
 mm/slab.c                         |    2 +-
 6 files changed, 9 insertions(+), 9 deletions(-)

Index: linux-2.6.35-rc2-gcc/mm/filemap.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/mm/filemap.c
+++ linux-2.6.35-rc2-gcc/mm/filemap.c
@@ -2238,14 +2238,12 @@ static ssize_t generic_perform_write(str
 
 	do {
 		struct page *page;
-		pgoff_t index;		/* Pagecache index for current page */
 		unsigned long offset;	/* Offset into pagecache page */
 		unsigned long bytes;	/* Bytes to write to page */
 		size_t copied;		/* Bytes copied from user */
 		void *fsdata;
 
 		offset = (pos & (PAGE_CACHE_SIZE - 1));
-		index = pos >> PAGE_CACHE_SHIFT;
 		bytes = min_t(unsigned long, PAGE_CACHE_SIZE - offset,
 						iov_iter_count(i));
 
Index: linux-2.6.35-rc2-gcc/mm/memory.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/mm/memory.c
+++ linux-2.6.35-rc2-gcc/mm/memory.c
@@ -307,7 +307,6 @@ void free_pgd_range(struct mmu_gather *t
 {
 	pgd_t *pgd;
 	unsigned long next;
-	unsigned long start;
 
 	/*
 	 * The next few lines have given us lots of grief...
@@ -351,7 +350,6 @@ void free_pgd_range(struct mmu_gather *t
 	if (addr > end - 1)
 		return;
 
-	start = addr;
 	pgd = pgd_offset(tlb->mm, addr);
 	do {
 		next = pgd_addr_end(addr, end);
Index: linux-2.6.35-rc2-gcc/arch/x86/include/asm/pgtable_64.h
===================================================================
--- linux-2.6.35-rc2-gcc.orig/arch/x86/include/asm/pgtable_64.h
+++ linux-2.6.35-rc2-gcc/arch/x86/include/asm/pgtable_64.h
@@ -126,8 +126,8 @@ static inline int pgd_large(pgd_t pgd) {
 /* x86-64 always has all page tables mapped. */
 #define pte_offset_map(dir, address) pte_offset_kernel((dir), (address))
 #define pte_offset_map_nested(dir, address) pte_offset_kernel((dir), (address))
-#define pte_unmap(pte) /* NOP */
-#define pte_unmap_nested(pte) /* NOP */
+#define pte_unmap(pte) ((void)(pte))/* NOP */
+#define pte_unmap_nested(pte) ((void)(pte)) /* NOP */
 
 #define update_mmu_cache(vma, address, ptep) do { } while (0)
 
Index: linux-2.6.35-rc2-gcc/include/linux/mmdebug.h
===================================================================
--- linux-2.6.35-rc2-gcc.orig/include/linux/mmdebug.h
+++ linux-2.6.35-rc2-gcc/include/linux/mmdebug.h
@@ -4,7 +4,7 @@
 #ifdef CONFIG_DEBUG_VM
 #define VM_BUG_ON(cond) BUG_ON(cond)
 #else
-#define VM_BUG_ON(cond) do { } while (0)
+#define VM_BUG_ON(cond) do { (void)(cond); } while (0)
 #endif
 
 #ifdef CONFIG_DEBUG_VIRTUAL
Index: linux-2.6.35-rc2-gcc/mm/slab.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/mm/slab.c
+++ linux-2.6.35-rc2-gcc/mm/slab.c
@@ -395,7 +395,7 @@ static void kmem_list3_init(struct kmem_
 #define	STATS_DEC_ACTIVE(x)	do { } while (0)
 #define	STATS_INC_ALLOCED(x)	do { } while (0)
 #define	STATS_INC_GROWN(x)	do { } while (0)
-#define	STATS_ADD_REAPED(x,y)	do { } while (0)
+#define	STATS_ADD_REAPED(x,y)	do { (void)(y); } while (0)
 #define	STATS_SET_HIGH(x)	do { } while (0)
 #define	STATS_INC_ERR(x)	do { } while (0)
 #define	STATS_INC_NODEALLOCS(x)	do { } while (0)
Index: linux-2.6.35-rc2-gcc/include/linux/highmem.h
===================================================================
--- linux-2.6.35-rc2-gcc.orig/include/linux/highmem.h
+++ linux-2.6.35-rc2-gcc/include/linux/highmem.h
@@ -72,7 +72,11 @@ static inline void *kmap_atomic(struct p
 }
 #define kmap_atomic_prot(page, idx, prot)	kmap_atomic(page, idx)
 
-#define kunmap_atomic(addr, idx)	do { pagefault_enable(); } while (0)
+static inline void kunmap_atomic(void *addr, enum km_type idx)
+{
+	pagefault_enable();
+}
+
 #define kmap_atomic_pfn(pfn, idx)	kmap_atomic(pfn_to_page(pfn), (idx))
 #define kmap_atomic_to_page(ptr)	virt_to_page(ptr)
 

[PATCH] [21/23] kernel/*: Fix unused but set warnings

[Andi Kleen]

No real bugs I believe, just some dead code.

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

---
 kernel/debug/kdb/kdb_bp.c  |    2 --
 kernel/hrtimer.c           |    3 +--
 kernel/sched_fair.c        |    3 +--
 kernel/sysctl.c            |    5 +----
 kernel/trace/ring_buffer.c |    2 --
 5 files changed, 3 insertions(+), 12 deletions(-)

Index: linux-2.6.35-rc2-gcc/kernel/sched_fair.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/kernel/sched_fair.c
+++ linux-2.6.35-rc2-gcc/kernel/sched_fair.c
@@ -1311,7 +1311,7 @@ static struct sched_group *
 find_idlest_group(struct sched_domain *sd, struct task_struct *p,
 		  int this_cpu, int load_idx)
 {
-	struct sched_group *idlest = NULL, *this = NULL, *group = sd->groups;
+	struct sched_group *idlest = NULL, *group = sd->groups;
 	unsigned long min_load = ULONG_MAX, this_load = 0;
 	int imbalance = 100 + (sd->imbalance_pct-100)/2;
 
@@ -1346,7 +1346,6 @@ find_idlest_group(struct sched_domain *s
 
 		if (local_group) {
 			this_load = avg_load;
-			this = group;
 		} else if (avg_load < min_load) {
 			min_load = avg_load;
 			idlest = group;
Index: linux-2.6.35-rc2-gcc/kernel/sysctl.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/kernel/sysctl.c
+++ linux-2.6.35-rc2-gcc/kernel/sysctl.c
@@ -1711,10 +1711,7 @@ static __init int sysctl_init(void)
 {
 	sysctl_set_parent(NULL, root_table);
 #ifdef CONFIG_SYSCTL_SYSCALL_CHECK
-	{
-		int err;
-		err = sysctl_check_table(current->nsproxy, root_table);
-	}
+	sysctl_check_table(current->nsproxy, root_table);
 #endif
 	return 0;
 }
Index: linux-2.6.35-rc2-gcc/kernel/hrtimer.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/kernel/hrtimer.c
+++ linux-2.6.35-rc2-gcc/kernel/hrtimer.c
@@ -1096,11 +1096,10 @@ EXPORT_SYMBOL_GPL(hrtimer_cancel);
  */
 ktime_t hrtimer_get_remaining(const struct hrtimer *timer)
 {
-	struct hrtimer_clock_base *base;
 	unsigned long flags;
 	ktime_t rem;
 
-	base = lock_hrtimer_base(timer, &flags);
+	lock_hrtimer_base(timer, &flags);
 	rem = hrtimer_expires_remaining(timer);
 	unlock_hrtimer_base(timer, &flags);
 
Index: linux-2.6.35-rc2-gcc/kernel/debug/kdb/kdb_bp.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/kernel/debug/kdb/kdb_bp.c
+++ linux-2.6.35-rc2-gcc/kernel/debug/kdb/kdb_bp.c
@@ -274,7 +274,6 @@ static int kdb_bp(int argc, const char *
 	int i, bpno;
 	kdb_bp_t *bp, *bp_check;
 	int diag;
-	int free;
 	char *symname = NULL;
 	long offset = 0ul;
 	int nextarg;
@@ -305,7 +304,6 @@ static int kdb_bp(int argc, const char *
 	/*
 	 * Find an empty bp structure to allocate
 	 */
-	free = KDB_MAXBPT;
 	for (bpno = 0, bp = kdb_breakpoints; bpno < KDB_MAXBPT; bpno++, bp++) {
 		if (bp->bp_free)
 			break;
Index: linux-2.6.35-rc2-gcc/kernel/trace/ring_buffer.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/kernel/trace/ring_buffer.c
+++ linux-2.6.35-rc2-gcc/kernel/trace/ring_buffer.c
@@ -3007,13 +3007,11 @@ static void rb_advance_reader(struct rin
 
 static void rb_advance_iter(struct ring_buffer_iter *iter)
 {
-	struct ring_buffer *buffer;
 	struct ring_buffer_per_cpu *cpu_buffer;
 	struct ring_buffer_event *event;
 	unsigned length;
 
 	cpu_buffer = iter->cpu_buffer;
-	buffer = cpu_buffer->buffer;
 
 	/*
 	 * Check if we are at the end of the buffer.

[PATCH] [22/23] BLOCK: Fix unused but set variables in blk-merge

[Andi Kleen]

Just some dead code.

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

---
 block/blk-merge.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Index: linux-2.6.35-rc2-gcc/block/blk-merge.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/block/blk-merge.c
+++ linux-2.6.35-rc2-gcc/block/blk-merge.c
@@ -12,7 +12,6 @@
 static unsigned int __blk_recalc_rq_segments(struct request_queue *q,
 					     struct bio *bio)
 {
-	unsigned int phys_size;
 	struct bio_vec *bv, *bvprv = NULL;
 	int cluster, i, high, highprv = 1;
 	unsigned int seg_size, nr_phys_segs;
@@ -24,7 +23,7 @@ static unsigned int __blk_recalc_rq_segm
 	fbio = bio;
 	cluster = test_bit(QUEUE_FLAG_CLUSTER, &q->queue_flags);
 	seg_size = 0;
-	phys_size = nr_phys_segs = 0;
+	nr_phys_segs = 0;
 	for_each_bio(bio) {
 		bio_for_each_segment(bv, bio, i) {
 			/*

[PATCH] [23/23] FS: Fix unused but set warnings

[Andi Kleen]

No real bugs I believe, just some dead code, and some
shut up code.

Cc: viro@zeniv.linux.org.uk

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

Index: linux-2.6.35-rc2-gcc/fs/splice.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/splice.c
+++ linux-2.6.35-rc2-gcc/fs/splice.c
@@ -597,7 +597,6 @@ ssize_t default_file_splice_read(struct
 	struct page *pages[PIPE_DEF_BUFFERS];
 	struct partial_page partial[PIPE_DEF_BUFFERS];
 	struct iovec *vec, __vec[PIPE_DEF_BUFFERS];
-	pgoff_t index;
 	ssize_t res;
 	size_t this_len;
 	int error;
@@ -621,7 +620,6 @@ ssize_t default_file_splice_read(struct
 			goto shrink_ret;
 	}
 
-	index = *ppos >> PAGE_CACHE_SHIFT;
 	offset = *ppos & ~PAGE_CACHE_MASK;
 	nr_pages = (len + offset + PAGE_CACHE_SIZE - 1) >> PAGE_CACHE_SHIFT;
 
Index: linux-2.6.35-rc2-gcc/include/linux/audit.h
===================================================================
--- linux-2.6.35-rc2-gcc.orig/include/linux/audit.h
+++ linux-2.6.35-rc2-gcc/include/linux/audit.h
@@ -544,7 +544,7 @@ extern int audit_signals;
 #define audit_putname(n) do { ; } while (0)
 #define __audit_inode(n,d) do { ; } while (0)
 #define __audit_inode_child(i,p) do { ; } while (0)
-#define audit_inode(n,d) do { ; } while (0)
+#define audit_inode(n,d) do { (void)(d); } while (0)
 #define audit_inode_child(i,p) do { ; } while (0)
 #define audit_core_dumps(i) do { ; } while (0)
 #define auditsc_get_stamp(c,t,s) (0)
Index: linux-2.6.35-rc2-gcc/fs/notify/inotify/inotify.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/fs/notify/inotify/inotify.c
+++ linux-2.6.35-rc2-gcc/fs/notify/inotify/inotify.c
@@ -570,7 +570,6 @@ void inotify_destroy(struct inotify_hand
 	while (1) {
 		struct inotify_watch *watch;
 		struct list_head *watches;
-		struct super_block *sb;
 		struct inode *inode;
 
 		mutex_lock(&ih->mutex);
@@ -580,7 +579,6 @@ void inotify_destroy(struct inotify_hand
 			break;
 		}
 		watch = list_first_entry(watches, struct inotify_watch, h_list);
-		sb = watch->inode->i_sb;
 		if (!pin_to_kill(ih, watch))
 			continue;
 
@@ -797,7 +795,6 @@ void inotify_evict_watch(struct inotify_
 int inotify_rm_wd(struct inotify_handle *ih, u32 wd)
 {
 	struct inotify_watch *watch;
-	struct super_block *sb;
 	struct inode *inode;
 
 	mutex_lock(&ih->mutex);
@@ -806,7 +803,6 @@ int inotify_rm_wd(struct inotify_handle
 		mutex_unlock(&ih->mutex);
 		return -EINVAL;
 	}
-	sb = watch->inode->i_sb;
 	if (!pin_to_kill(ih, watch))
 		return 0;
 

Re: [PATCH] [1/23] x86: percpu: Avoid warnings of unused variables in per cpu

[Tejun Heo]

On 06/10/2010 01:10 PM, Andi Kleen wrote:
> Avoid hundreds of warnings with a gcc 4.6 -Wall build
> 
> Cc: tj@kernel.org
> Cc: x86@kernel.org
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>

Acked-by: Tejun Heo <tj@kernel.org>

Can x86 or -mm please take this patch?  There isn't any pending percpu
patch currently and it seems a bit silly to start a branch for this
one.

Thanks.

-- 
tejun

Re: [PATCH] [1/23] x86: percpu: Avoid warnings of unused variables in per cpu

[Ingo Molnar]

* Tejun Heo <tj@kernel.org> wrote:

> On 06/10/2010 01:10 PM, Andi Kleen wrote:
> > Avoid hundreds of warnings with a gcc 4.6 -Wall build
> > 
> > Cc: tj@kernel.org
> > Cc: x86@kernel.org
> > 
> > Signed-off-by: Andi Kleen <ak@linux.intel.com>
> 
> Acked-by: Tejun Heo <tj@kernel.org>
> 
> Can x86 or -mm please take this patch?  There isn't any pending percpu
> patch currently and it seems a bit silly to start a branch for this
> one.

Sure, queued it up.

Thanks,

	Ingo

[tip:x86/urgent] percpu, x86: Avoid warnings of unused variables in per cpu

[tip-bot for Andi Kleen]

Commit-ID:  157a9df3b0f8042f1a6a4fe9e0e494431aaca1ae
Gitweb:     http://git.kernel.org/tip/157a9df3b0f8042f1a6a4fe9e0e494431aaca1ae
Author:     Andi Kleen <andi@firstfloor.org>
AuthorDate: Thu, 10 Jun 2010 13:10:36 +0200
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Thu, 10 Jun 2010 14:09:30 +0200

percpu, x86: Avoid warnings of unused variables in per cpu

Avoid hundreds of warnings with a gcc 4.6 -Wall build.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
Acked-by: Tejun Heo <tj@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
LKML-Reference: <4C10C930.9080900@kernel.org>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/x86/include/asm/percpu.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/x86/include/asm/percpu.h b/arch/x86/include/asm/percpu.h
index 0797e74..cd28f9a 100644
--- a/arch/x86/include/asm/percpu.h
+++ b/arch/x86/include/asm/percpu.h
@@ -77,6 +77,7 @@ do {							\
 	if (0) {					\
 		pto_T__ pto_tmp__;			\
 		pto_tmp__ = (val);			\
+		(void)pto_tmp__;			\
 	}						\
 	switch (sizeof(var)) {				\
 	case 1:						\
@@ -115,6 +116,7 @@ do {									\
 	if (0) {							\
 		pao_T__ pao_tmp__;					\
 		pao_tmp__ = (val);					\
+		(void)pao_tmp__;					\
 	}								\
 	switch (sizeof(var)) {						\
 	case 1:								\

Re: [PATCH] [11/23] KVM: Fix KVM_SET_SIGNAL_MASK

[Avi Kivity]

On 06/10/2010 02:10 PM, Andi Kleen wrote:
> Real bug fix.
>
> When the user passed in a NULL mask pass this on from the ioctl
> handler.
>
> Found by gcc 4.6's new warnings.
>    

Applied, thanks.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH] [14/23] NFSD: Fix initialized but not read warnings

[J.Bruce Fields]

On Thu, Jun 10, 2010 at 09:45:43PM +1000, Neil Brown wrote:
> 
> Hi Andi,
>  NFSD stuff should really go to Bruce Fields and linux-nfs@vger.kernel.org
> 
> Thanks,
> NeilBrown
> 
> On Thu, 10 Jun 2010 13:10:50 +0200 (CEST)
> Andi Kleen <andi@firstfloor.org> wrote:
> 
> > 
> > Fixes at least one real minor bug: the nfs4 recovery dir sysctl
> > would not return its status properly.
> > 
> > Also I finished Al's 1e41568d7378d ("Take ima_path_check() in nfsd 
> > past dentry_open() in nfsd_open()") commit, it moved the IMA
> > code, but left the old path initializer in there.
> > 
> > The rest is just dead code removed I think, although I was not 
> > fully sure about the "is_borc" stuff. Some more review
> > would be still good.

Thanks, all look good to me (though I might like to figure out how we
eneded up in this situation in a couple of cases...).

Absent any objection, I'll apply these to the nfsd tree for 2.6.36.

--b.

> > 
> > Found by gcc 4.6's new warnings.
> > 
> > Cc: viro@zeniv.linux.org.uk
> > Cc: neilb@suse.de
> > 
> > Signed-off-by: Andi Kleen <ak@linux.intel.com>
> > 
> > ---
> >  fs/nfsd/nfs4state.c |    2 --
> >  fs/nfsd/nfsctl.c    |    2 ++
> >  fs/nfsd/nfsproc.c   |    2 --
> >  fs/nfsd/vfs.c       |   11 +----------
> >  4 files changed, 3 insertions(+), 14 deletions(-)
> > 
> > Index: linux-2.6.35-rc2-gcc/fs/nfsd/nfs4state.c
> > ===================================================================
> > --- linux-2.6.35-rc2-gcc.orig/fs/nfsd/nfs4state.c
> > +++ linux-2.6.35-rc2-gcc/fs/nfsd/nfs4state.c
> > @@ -3346,11 +3346,9 @@ static inline void
> >  nfs4_set_lock_denied(struct file_lock *fl, struct nfsd4_lock_denied *deny)
> >  {
> >  	struct nfs4_stateowner *sop;
> > -	unsigned int hval;
> >  
> >  	if (fl->fl_lmops == &nfsd_posix_mng_ops) {
> >  		sop = (struct nfs4_stateowner *) fl->fl_owner;
> > -		hval = lockownerid_hashval(sop->so_id);
> >  		kref_get(&sop->so_ref);
> >  		deny->ld_sop = sop;
> >  		deny->ld_clientid = sop->so_client->cl_clientid;
> > Index: linux-2.6.35-rc2-gcc/fs/nfsd/nfsctl.c
> > ===================================================================
> > --- linux-2.6.35-rc2-gcc.orig/fs/nfsd/nfsctl.c
> > +++ linux-2.6.35-rc2-gcc/fs/nfsd/nfsctl.c
> > @@ -1310,6 +1310,8 @@ static ssize_t __write_recoverydir(struc
> >  			return -EINVAL;
> >  
> >  		status = nfs4_reset_recoverydir(recdir);
> > +		if (status)
> > +			return status;
> >  	}
> >  
> >  	return scnprintf(buf, SIMPLE_TRANSACTION_LIMIT, "%s\n",
> > Index: linux-2.6.35-rc2-gcc/fs/nfsd/nfsproc.c
> > ===================================================================
> > --- linux-2.6.35-rc2-gcc.orig/fs/nfsd/nfsproc.c
> > +++ linux-2.6.35-rc2-gcc/fs/nfsd/nfsproc.c
> > @@ -290,7 +290,6 @@ nfsd_proc_create(struct svc_rqst *rqstp,
> >  	 * gospel of sun micro
> >  	 */
> >  	if (type != S_IFREG) {
> > -		int	is_borc = 0;
> >  		if (type != S_IFBLK && type != S_IFCHR) {
> >  			rdev = 0;
> >  		} else if (type == S_IFCHR && !(attr->ia_valid & ATTR_SIZE)) {
> > @@ -298,7 +297,6 @@ nfsd_proc_create(struct svc_rqst *rqstp,
> >  			type = S_IFIFO;
> >  		} else {
> >  			/* Okay, char or block special */
> > -			is_borc = 1;
> >  			if (!rdev)
> >  				rdev = wanted;
> >  		}
> > Index: linux-2.6.35-rc2-gcc/fs/nfsd/vfs.c
> > ===================================================================
> > --- linux-2.6.35-rc2-gcc.orig/fs/nfsd/vfs.c
> > +++ linux-2.6.35-rc2-gcc/fs/nfsd/vfs.c
> > @@ -1632,7 +1632,7 @@ nfsd_link(struct svc_rqst *rqstp, struct
> >  				char *name, int len, struct svc_fh *tfhp)
> >  {
> >  	struct dentry	*ddir, *dnew, *dold;
> > -	struct inode	*dirp, *dest;
> > +	struct inode	*dirp;
> >  	__be32		err;
> >  	int		host_err;
> >  
> > @@ -1660,7 +1660,6 @@ nfsd_link(struct svc_rqst *rqstp, struct
> >  		goto out_nfserr;
> >  
> >  	dold = tfhp->fh_dentry;
> > -	dest = dold->d_inode;
> >  
> >  	host_err = mnt_want_write(tfhp->fh_export->ex_path.mnt);
> >  	if (host_err) {
> > @@ -2112,15 +2111,7 @@ nfsd_permission(struct svc_rqst *rqstp,
> >  	if (err == -EACCES && S_ISREG(inode->i_mode) &&
> >  	    acc == (NFSD_MAY_READ | NFSD_MAY_OWNER_OVERRIDE))
> >  		err = inode_permission(inode, MAY_EXEC);
> > -	if (err)
> > -		goto nfsd_out;
> >  
> > -	/* Do integrity (permission) checking now, but defer incrementing
> > -	 * IMA counts to the actual file open.
> > -	 */
> > -	path.mnt = exp->ex_path.mnt;
> > -	path.dentry = dentry;
> > -nfsd_out:
> >  	return err? nfserrno(err) : 0;
> >  }
> >  
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> 

Re: [PATCH] [19/23] KVM: Fix unused but set warnings

[Avi Kivity]

On 06/10/2010 02:10 PM, Andi Kleen wrote:
> No real bugs in this one, the real bug I found is in a separate
> patch.
>
>
> Index: linux-2.6.35-rc2-gcc/arch/x86/kvm/vmx.c
> ===================================================================
> --- linux-2.6.35-rc2-gcc.orig/arch/x86/kvm/vmx.c
> +++ linux-2.6.35-rc2-gcc/arch/x86/kvm/vmx.c
> @@ -1624,10 +1624,9 @@ static void enter_pmode(struct kvm_vcpu
>   static gva_t rmode_tss_base(struct kvm *kvm)
>   {
>   	if (!kvm->arch.tss_addr) {
> -		struct kvm_memslots *slots;
>   		gfn_t base_gfn;
>
> -		slots = kvm_memslots(kvm);
> +		kvm_memslots(kvm);
>   		base_gfn = kvm->memslots->memslots[0].base_gfn +
>   				 kvm->memslots->memslots[0].npages - 3;
>   		return base_gfn<<  PAGE_SHIFT;
>    

I think the base_gfn assignment below needs to use slots to get the rcu 
dereference correct.  I'll apply the patch without this hunk and fix it 
independently.


-- 
error compiling committee.c: too many arguments to function

Re: [PATCH] [10/23] SCHED: Only allocate per cpu cpu mask buffer with offstack cpumasks

[Peter Zijlstra]

On Thu, 2010-06-10 at 13:10 +0200, Andi Kleen wrote:
> This will save a few bytes in the non offstack cpumask case.
> 
> Found by gcc 4.6's new warnings.
> 
> Cc: peterz@infradead.org
> Cc: mingo@elte.hu
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> 
> ---
>  kernel/sched.c |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> Index: linux-2.6.35-rc2-gcc/kernel/sched.c
> ===================================================================
> --- linux-2.6.35-rc2-gcc.orig/kernel/sched.c
> +++ linux-2.6.35-rc2-gcc/kernel/sched.c
> @@ -7482,7 +7482,7 @@ static void init_tg_rt_entry(struct task
>  void __init sched_init(void)
>  {
>  	int i, j;
> -	unsigned long alloc_size = 0, ptr;
> +	unsigned long alloc_size = 0;
>  
>  #ifdef CONFIG_FAIR_GROUP_SCHED
>  	alloc_size += 2 * nr_cpu_ids * sizeof(void **);
> @@ -7494,7 +7494,10 @@ void __init sched_init(void)
>  	alloc_size += num_possible_cpus() * cpumask_size();
>  #endif
>  	if (alloc_size) {
> +#ifdef CONFIG_CPUMASK_OFFSTACK
> +		unsigned long ptr;
>  		ptr = (unsigned long)kzalloc(alloc_size, GFP_NOWAIT);
> +#endif
>  
>  #ifdef CONFIG_FAIR_GROUP_SCHED
>  		init_task_group.se = (struct sched_entity **)ptr;

This patch will actually break things.. please read the code.

I guess we could move the unsigned long into that block, but I really
don't see the point.

Re: [PATCH] [10/23] SCHED: Only allocate per cpu cpu mask buffer with offstack cpumasks

[Andi Kleen]

> This patch will actually break things.. please read the code.
> 
> I guess we could move the unsigned long into that block, but I really
> don't see the point.

How does it break things? 

ptr is not used for anything unless that define is set. gcc doesn't
lie on this.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [PATCH] [10/23] SCHED: Only allocate per cpu cpu mask buffer with offstack cpumasks

[Peter Zijlstra]

On Thu, 2010-06-10 at 16:52 +0200, Andi Kleen wrote:
> > This patch will actually break things.. please read the code.
> > 
> > I guess we could move the unsigned long into that block, but I really
> > don't see the point.
> 
> How does it break things? 
> 
> ptr is not used for anything unless that define is set. gcc doesn't
> lie on this.

        if (alloc_size) {
                ptr = (unsigned long)kzalloc(alloc_size, GFP_NOWAIT);

#ifdef CONFIG_FAIR_GROUP_SCHED
                init_task_group.se = (struct sched_entity **)ptr;
                ptr += nr_cpu_ids * sizeof(void **);

                init_task_group.cfs_rq = (struct cfs_rq **)ptr;
                ptr += nr_cpu_ids * sizeof(void **);

#endif /* CONFIG_FAIR_GROUP_SCHED */
#ifdef CONFIG_RT_GROUP_SCHED
                init_task_group.rt_se = (struct sched_rt_entity **)ptr;
                ptr += nr_cpu_ids * sizeof(void **);

                init_task_group.rt_rq = (struct rt_rq **)ptr;
                ptr += nr_cpu_ids * sizeof(void **);

#endif /* CONFIG_RT_GROUP_SCHED */
#ifdef CONFIG_CPUMASK_OFFSTACK
                for_each_possible_cpu(i) {
                        per_cpu(load_balance_tmpmask, i) = (void *)ptr;
                        ptr += cpumask_size();
                }
#endif /* CONFIG_CPUMASK_OFFSTACK */
        }

Re: [PATCH] [10/23] SCHED: Only allocate per cpu cpu mask buffer with offstack cpumasks

[Andi Kleen]

Peter Zijlstra <peterz@infradead.org> writes:

> On Thu, 2010-06-10 at 16:52 +0200, Andi Kleen wrote:
>> > This patch will actually break things.. please read the code.
>> > 
>> > I guess we could move the unsigned long into that block, but I really
>> > don't see the point.
>> 
>> How does it break things? 
>> 
>> ptr is not used for anything unless that define is set. gcc doesn't
>> lie on this.

Ok.

For the !FAIR_GROUP_SCHED || !CPUMASK_OFFSTACK case it's still wasted
memory because it is not used for anything. If you enable the kmemleak
tracer you should also get a warning about this

I updated the patch with a better ifdef to have CONFIG_UNFAIR_GROUP_SCHED
too.

-Andi

---

SCHED: Only allocate per cpu cpu mask buffer with offstack cpumasks v2

This will save a few bytes in the non offstack cpumask case.

Found by gcc 4.6's new warnings.

Cc: peterz@infradead.org
Cc: mingo@elte.hu
Signed-off-by: Andi Kleen <ak@linux.intel.com>

---
 kernel/sched.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Index: linux-2.6.35-rc2-gcc/kernel/sched.c
===================================================================
--- linux-2.6.35-rc2-gcc.orig/kernel/sched.c
+++ linux-2.6.35-rc2-gcc/kernel/sched.c
@@ -7482,7 +7482,7 @@ static void init_tg_rt_entry(struct task
 void __init sched_init(void)
 {
 	int i, j;
-	unsigned long alloc_size = 0, ptr;
+	unsigned long alloc_size = 0;
 
 #ifdef CONFIG_FAIR_GROUP_SCHED
 	alloc_size += 2 * nr_cpu_ids * sizeof(void **);
@@ -7494,7 +7494,10 @@ void __init sched_init(void)
 	alloc_size += num_possible_cpus() * cpumask_size();
 #endif
 	if (alloc_size) {
+#if defined(CONFIG_CPUMASK_OFFSTACK) || defined(CONFIG_FAIR_GROUP_SCHED)
+		unsigned long ptr;
 		ptr = (unsigned long)kzalloc(alloc_size, GFP_NOWAIT);
+#endif
 
 #ifdef CONFIG_FAIR_GROUP_SCHED
 		init_task_group.se = (struct sched_entity **)ptr;


-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [PATCH] [10/23] SCHED: Only allocate per cpu cpu mask buffer with offstack cpumasks

[Peter Zijlstra]

On Thu, 2010-06-10 at 17:06 +0200, Andi Kleen wrote:
> +#if defined(CONFIG_CPUMASK_OFFSTACK) || defined(CONFIG_FAIR_GROUP_SCHED)

Still wrong.. something like the below might work, but I really think
its not worth the trouble.

---
 kernel/sched.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/kernel/sched.c b/kernel/sched.c
index 19b3c5d..af40894 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -7689,7 +7689,9 @@ static void init_tg_rt_entry(struct task_group *tg, struct rt_rq *rt_rq,
 void __init sched_init(void)
 {
 	int i, j;
-	unsigned long alloc_size = 0, ptr;
+
+#if defined CONFIG_FAIR_GROUP_SCHED || defined CONFIG_RT_GROUP_SCHED || defined CONFIG_CPUMASK_OFFSTACK
+	unsigned long alloc_size = 0;
 
 #ifdef CONFIG_FAIR_GROUP_SCHED
 	alloc_size += 2 * nr_cpu_ids * sizeof(void **);
@@ -7701,6 +7703,8 @@ void __init sched_init(void)
 	alloc_size += num_possible_cpus() * cpumask_size();
 #endif
 	if (alloc_size) {
+		unsigned long ptr;
+	       
 		ptr = (unsigned long)kzalloc(alloc_size, GFP_NOWAIT);
 
 #ifdef CONFIG_FAIR_GROUP_SCHED
@@ -7726,6 +7730,7 @@ void __init sched_init(void)
 		}
 #endif /* CONFIG_CPUMASK_OFFSTACK */
 	}
+#endif /* {FAIR,RT}_GROUP,OFFSTACK */
 
 #ifdef CONFIG_SMP
 	init_defrootdomain();
@@ -7762,7 +7767,6 @@ void __init sched_init(void)
 #ifdef CONFIG_FAIR_GROUP_SCHED
 		init_task_group.shares = init_task_group_load;
 		INIT_LIST_HEAD(&rq->leaf_cfs_rq_list);
-#ifdef CONFIG_CGROUP_SCHED
 		/*
 		 * How much cpu bandwidth does init_task_group get?
 		 *
@@ -7783,16 +7787,13 @@ void __init sched_init(void)
 		 * directly in rq->cfs (i.e init_task_group->se[] = NULL).
 		 */
 		init_tg_cfs_entry(&init_task_group, &rq->cfs, NULL, i, 1, NULL);
-#endif
 #endif /* CONFIG_FAIR_GROUP_SCHED */
 
 		rq->rt.rt_runtime = def_rt_bandwidth.rt_runtime;
 #ifdef CONFIG_RT_GROUP_SCHED
 		INIT_LIST_HEAD(&rq->leaf_rt_rq_list);
-#ifdef CONFIG_CGROUP_SCHED
 		init_tg_rt_entry(&init_task_group, &rq->rt, NULL, i, 1, NULL);
 #endif
-#endif
 
 		for (j = 0; j < CPU_LOAD_IDX_MAX; j++)
 			rq->cpu_load[j] = 0;

Re: [PATCH] [10/23] SCHED: Only allocate per cpu cpu mask buffer with offstack cpumasks

[Andi Kleen]

On Thu, Jun 10, 2010 at 05:19:36PM +0200, Peter Zijlstra wrote:
> On Thu, 2010-06-10 at 17:06 +0200, Andi Kleen wrote:
> > +#if defined(CONFIG_CPUMASK_OFFSTACK) || defined(CONFIG_FAIR_GROUP_SCHED)
> 
> Still wrong.. something like the below might work, but I really think
> its not worth the trouble.

Yes it seems the ifdef maze in this function is too much for me.

Anyways I retract the patch, but you'll be reminded of it once
you update to gcc 4.6

-Andi

Re: [PATCH] [5/23] x86 boot: Set ax register in boot vga query

[H. Peter Anvin]

On 06/10/2010 04:10 AM, Andi Kleen wrote:
> I think the ax register should be input to the
> BIOS call, not be unused, but not fully sure.  hpa?

Quite correct.  A typo during the conversion to the "glove box" interface.

Thanks, will apply!

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH] [1/23] x86: percpu: Avoid warnings of unused variables in per cpu

[Justin P. Mattock]

On 06/10/2010 05:09 AM, Ingo Molnar wrote:
>
> * Tejun Heo<tj@kernel.org>  wrote:
>
>> On 06/10/2010 01:10 PM, Andi Kleen wrote:
>>> Avoid hundreds of warnings with a gcc 4.6 -Wall build
>>>
>>> Cc: tj@kernel.org
>>> Cc: x86@kernel.org
>>>
>>> Signed-off-by: Andi Kleen<ak@linux.intel.com>
>>
>> Acked-by: Tejun Heo<tj@kernel.org>
>>
>> Can x86 or -mm please take this patch?  There isn't any pending percpu
>> patch currently and it seems a bit silly to start a branch for this
>> one.
>
> Sure, queued it up.
>
> Thanks,
>
> 	Ingo
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>


Thanks for this.. I applied it over
here.. and now don't have all of
that spam from percpu while compiling
the kernel..

only one I see now is something about
warning: variable 'gfp' set but not used

if there's another I can test it if you
would like.

Justin P. Mattock

Re: [PATCH] [1/23] x86: percpu: Avoid warnings of unused variables in per cpu

[Andi Kleen]

> Thanks for this.. I applied it over
> here.. and now don't have all of
> that spam from percpu while compiling
> the kernel..

Which gcc version are you using?
>
> only one I see now is something about
> warning: variable 'gfp' set but not used

That's fixed in 'IRQ: Move alloc_desk_mask variables inside ifdef'

The whole series fixes a lot more warnings, however
there are still quite some left (in my config)

-Andi
-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [PATCH] [1/23] x86: percpu: Avoid warnings of unused variables in per cpu

[Justin P. Mattock]

On 06/10/2010 11:10 AM, Andi Kleen wrote:
>> Thanks for this.. I applied it over
>> here.. and now don't have all of
>> that spam from percpu while compiling
>> the kernel..
>
> Which gcc version are you using?

  gcc version 4.6.0 20100416
>>
>> only one I see now is something about
>> warning: variable 'gfp' set but not used
>
> That's fixed in 'IRQ: Move alloc_desk_mask variables inside ifdef'
>
> The whole series fixes a lot more warnings, however
> there are still quite some left (in my config)
>
> -Andi

let me go and add all of these patches, to see
if I see any more warnings..

Justin P. Mattock

Re: [PATCH] [1/23] x86: percpu: Avoid warnings of unused variables in per cpu

[Justin P. Mattock]

On 06/10/2010 11:10 AM, Andi Kleen wrote:
>> Thanks for this.. I applied it over
>> here.. and now don't have all of
>> that spam from percpu while compiling
>> the kernel..
>
> Which gcc version are you using?
>>
>> only one I see now is something about
>> warning: variable 'gfp' set but not used
>
> That's fixed in 'IRQ: Move alloc_desk_mask variables inside ifdef'
>
> The whole series fixes a lot more warnings, however
> there are still quite some left (in my config)
>
> -Andi


o.k. I applied your patches, except
patch #10 x86: percpu: Avoid warnings of unused variables

will try later on and see.

as for the warnings, I can send a seperate
post for all to see, still quit a bit
but lots nicer after your patch(s)

Justin P. Mattock	

[tip:x86/urgent] x86, setup: Set ax register in boot vga query

[tip-bot for Andi Kleen]

Commit-ID:  cf3bdc29fcbf2cb4cfa238591021d41ea8f8431f
Gitweb:     http://git.kernel.org/tip/cf3bdc29fcbf2cb4cfa238591021d41ea8f8431f
Author:     Andi Kleen <andi@firstfloor.org>
AuthorDate: Thu, 10 Jun 2010 13:10:40 +0200
Committer:  H. Peter Anvin <hpa@linux.intel.com>
CommitDate: Thu, 10 Jun 2010 15:24:29 -0700

x86, setup: Set ax register in boot vga query

Catch missing conversion to the register structure "glove box" scheme.

Found by gcc 4.6's new warnings.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
LKML-Reference: <20100610111040.F1781B1A2B@basil.firstfloor.org>
Signed-off-by: H. Peter Anvin <hpa@zytor.com>
---
 arch/x86/boot/video-vga.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/arch/x86/boot/video-vga.c b/arch/x86/boot/video-vga.c
index ed7aeff..45bc940 100644
--- a/arch/x86/boot/video-vga.c
+++ b/arch/x86/boot/video-vga.c
@@ -41,13 +41,12 @@ static __videocard video_vga;
 static u8 vga_set_basic_mode(void)
 {
 	struct biosregs ireg, oreg;
-	u16 ax;
 	u8 mode;
 
 	initregs(&ireg);
 
 	/* Query current mode */
-	ax = 0x0f00;
+	ireg.ax = 0x0f00;
 	intcall(0x10, &ireg, &oreg);
 	mode = oreg.al;
 

Re: [PATCH] [16/23] XFS: Fix gcc 4.6 set but not read and unused statement warnings

[Christoph Hellwig]

Most of this looks good to me, but I really don't like the QASSERT
macro.  If you're fine with it I'll split this up into smaller patches
and resubmit the unmodified parts under your name and redo those bits
I don't like.

Re: [PATCH] [16/23] XFS: Fix gcc 4.6 set but not read and unused statement warnings

[Christoph Hellwig]

Most of this looks good to me, but I really don't like the QASSERT
macro.  If you're fine with it I'll split this up into smaller patches
and resubmit the unmodified parts under your name and redo those bits
I don't like.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] [16/23] XFS: Fix gcc 4.6 set but not read and unused statement warnings

[Andi Kleen]

On Fri, Jun 11, 2010 at 12:20:07PM -0400, Christoph Hellwig wrote:
> Most of this looks good to me, but I really don't like the QASSERT
> macro.  If you're fine with it I'll split this up into smaller patches

Why do you not like it? 

The alternative will be likely uglier.

> and resubmit the unmodified parts under your name and redo those bits
> I don't like.

Fine for me.

-Andi
-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [PATCH] [16/23] XFS: Fix gcc 4.6 set but not read and unused statement warnings

[Andi Kleen]

On Fri, Jun 11, 2010 at 12:20:07PM -0400, Christoph Hellwig wrote:
> Most of this looks good to me, but I really don't like the QASSERT
> macro.  If you're fine with it I'll split this up into smaller patches

Why do you not like it? 

The alternative will be likely uglier.

> and resubmit the unmodified parts under your name and redo those bits
> I don't like.

Fine for me.

-Andi
-- 
ak@linux.intel.com -- Speaking for myself only.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] [16/23] XFS: Fix gcc 4.6 set but not read and unused statement warnings

[Dave Chinner]

On Thu, Jun 10, 2010 at 01:10:52PM +0200, Andi Kleen wrote:
> 
> For my configuration, that is without quota or RT.
> 
> Mostly dead code removed I think (but needs additional review) 
>
> That is there were one or two bad error handling cases,
> but they were not easily fixable, with comments 
> and I left the warnings in for those for you to remember.
> 
> e.g. if there is a ENOSPC down in xfs_trans.c while
> modifying the superblock it would not be handled.

See my comments about these below.

> Unused statements were mostly related to stub macros for disabled
> features like QUOTA or RT ALLOC. I replace those with
> inlines.

Did you compile with/without XFS_DEBUG (I don't think so)? when
changing code that affects ASSERT statements, CONFIG_XFS_DEBUG needs to
be selected to test that this code compiles. Most XFs developers
build with XFS_CONFIG_DEBUG for everything other than performance
testing, so ensuring this builds is definitely required. ;)

I'd also be interested if any fixes are needed with all options
enabled. Seems silly just to fix a few warnings in just one
particular configuration rather than all of them...

> There were also some problems with variables used in ASSERT()
> I partly moved those into the ASSERT itself and partly
> used a new QASSERT that always evaluates.

That's a pretty ugly hack for a single occurrence of a warning.
Re-arrnaging the code is a much better idea than introducing a new
ASSERT type. e.g:

-	ASSERT(ref >= 0);
+	if (ref < 0)
+		ASSERT(0);

> Cc: xfs@oss.sgi.com
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> 
> ---
>  fs/xfs/linux-2.6/xfs_sync.c |    3 +++
>  fs/xfs/support/debug.h      |    4 ++++
>  fs/xfs/xfs_alloc.c          |   10 +++-------
>  fs/xfs/xfs_da_btree.c       |   15 +++++----------
>  fs/xfs/xfs_dir2_block.c     |    6 +++---
>  fs/xfs/xfs_filestream.c     |   10 ++--------
>  fs/xfs/xfs_iget.c           |    3 ---
>  fs/xfs/xfs_inode.c          |    4 ----
>  fs/xfs/xfs_inode_item.c     |    8 ++------
>  fs/xfs/xfs_log.c            |    2 --
>  fs/xfs/xfs_quota.h          |   14 ++++++++++----
>  fs/xfs/xfs_trans.c          |    1 +
>  12 files changed, 33 insertions(+), 47 deletions(-)
> 
> Index: linux-2.6.35-rc2-gcc/fs/xfs/linux-2.6/xfs_sync.c
> ===================================================================
> --- linux-2.6.35-rc2-gcc.orig/fs/xfs/linux-2.6/xfs_sync.c
> +++ linux-2.6.35-rc2-gcc/fs/xfs/linux-2.6/xfs_sync.c
> @@ -554,6 +554,9 @@ xfs_sync_worker(
>  		xfs_log_force(mp, 0);
>  		xfs_reclaim_inodes(mp, 0);
>  		/* dgc: errors ignored here */
> +		/* ak: yes and you'll get a warning for it now when you
> +		 * upgrade compilers.
> +		 */
>  		error = xfs_qm_sync(mp, SYNC_TRYLOCK);
>  		if (xfs_log_need_covered(mp))
>  			error = xfs_commit_dummy_trans(mp, 0);

I don't think the coment is necessary - the compiler will remind us
that we are ignoring errors.

FWIW, we've now got a situation where external static code checkers
will tell us that we are not handling an error case (which is where
this code and comment came from), and now the compiler will warn us
we are assigning but not using the return value. It's a bit of a
Catch-22 situation. Hence my question is this - how are we supposed
to "safely" ignore a return value seeing as the compiler is now
making the current method rather noisy?

> Index: linux-2.6.35-rc2-gcc/fs/xfs/xfs_da_btree.c
> ===================================================================
> --- linux-2.6.35-rc2-gcc.orig/fs/xfs/xfs_da_btree.c
> +++ linux-2.6.35-rc2-gcc/fs/xfs/xfs_da_btree.c
> @@ -581,10 +581,8 @@ xfs_da_node_add(xfs_da_state_t *state, x
>  	xfs_da_intnode_t *node;
>  	xfs_da_node_entry_t *btree;
>  	int tmp;
> -	xfs_mount_t *mp;
>  
>  	node = oldblk->bp->data;
> -	mp = state->mp;
>  	ASSERT(be16_to_cpu(node->hdr.info.magic) == XFS_DA_NODE_MAGIC);
>  	ASSERT((oldblk->index >= 0) && (oldblk->index <= be16_to_cpu(node->hdr.count)));
>  	ASSERT(newblk->blkno != 0);

That'll break a CONFIG_XFS_DEBUG build as the next statement:

        if (state->args->whichfork == XFS_DATA_FORK)
                ASSERT(newblk->blkno >= mp->m_dirleafblk &&
                       newblk->blkno < mp->m_dirfreeblk);

uses mp inside ASSERT statements.

> Index: linux-2.6.35-rc2-gcc/fs/xfs/xfs_dir2_block.c
> ===================================================================
> --- linux-2.6.35-rc2-gcc.orig/fs/xfs/xfs_dir2_block.c
> +++ linux-2.6.35-rc2-gcc/fs/xfs/xfs_dir2_block.c
> @@ -1073,10 +1073,10 @@ xfs_dir2_sf_to_block(
>  	 */
>  
>  	buf_len = dp->i_df.if_bytes;
> -	buf = kmem_alloc(dp->i_df.if_bytes, KM_SLEEP);
> +	buf = kmem_alloc(buf_len, KM_SLEEP);
>  
> -	memcpy(buf, sfp, dp->i_df.if_bytes);
> -	xfs_idata_realloc(dp, -dp->i_df.if_bytes, XFS_DATA_FORK);
> +	memcpy(buf, sfp, buf_len);
> +	xfs_idata_realloc(dp, -buf_len, XFS_DATA_FORK);
>  	dp->i_d.di_size = 0;
>  	xfs_trans_log_inode(tp, dp, XFS_ILOG_CORE);
>  	/*

Just remove the buf_len variable in this case.

> Index: linux-2.6.35-rc2-gcc/fs/xfs/xfs_filestream.c
> ===================================================================
> --- linux-2.6.35-rc2-gcc.orig/fs/xfs/xfs_filestream.c
> +++ linux-2.6.35-rc2-gcc/fs/xfs/xfs_filestream.c
> @@ -140,9 +140,8 @@ _xfs_filestream_pick_ag(
>  	int		flags,
>  	xfs_extlen_t	minlen)
>  {
> -	int		streams, max_streams;
>  	int		err, trylock, nscan;
> -	xfs_extlen_t	longest, free, minfree, maxfree = 0;
> +	xfs_extlen_t	longest, minfree, maxfree = 0;
>  	xfs_agnumber_t	ag, max_ag = NULLAGNUMBER;
>  	struct xfs_perag *pag;
>  
> @@ -174,7 +173,6 @@ _xfs_filestream_pick_ag(
>  		/* Keep track of the AG with the most free blocks. */
>  		if (pag->pagf_freeblks > maxfree) {
>  			maxfree = pag->pagf_freeblks;
> -			max_streams = atomic_read(&pag->pagf_fstrms);
>  			max_ag = ag;
>  		}
>  
> @@ -196,8 +194,6 @@ _xfs_filestream_pick_ag(
>  		     (flags & XFS_PICK_LOWSPACE))) {
>  
>  			/* Break out, retaining the reference on the AG. */
> -			free = pag->pagf_freeblks;
> -			streams = atomic_read(&pag->pagf_fstrms);
>  			xfs_perag_put(pag);
>  			*agp = ag;
>  			break;
> @@ -234,8 +230,6 @@ next_ag:
>  		if (max_ag != NULLAGNUMBER) {
>  			xfs_filestream_get_ag(mp, max_ag);
>  			TRACE_AG_PICK1(mp, max_ag, maxfree);
> -			streams = max_streams;
> -			free = maxfree;
>  			*agp = max_ag;
>  			break;
>  		}
> @@ -364,7 +358,7 @@ xfs_fstrm_free_func(
>  	/* Drop the reference taken on the AG when the item was added. */
>  	ref = xfs_filestream_put_ag(ip->i_mount, item->ag);
>  
> -	ASSERT(ref >= 0);
> +	QASSERT(ref >= 0);
>  	TRACE_FREE(ip->i_mount, ip, item->pip, item->ag,
>  		xfs_filestream_peek_ag(ip->i_mount, item->ag));

These are all "unused" because they are used in debug code only. i.e.
in XFS_FILESTREAMS_TRACE configs. This is manual debug code that
needs to be converted to the trace infrastructure - the compiler may
say it is unused, but it is not dead code, so shoul dnot be removed.

See also my comment about QASSERT() above.

>  #define xfs_trans_unreserve_quota_nblks(tp, ip, nblks, ninos, flags) \
> Index: linux-2.6.35-rc2-gcc/fs/xfs/xfs_trans.c
> ===================================================================
> --- linux-2.6.35-rc2-gcc.orig/fs/xfs/xfs_trans.c
> +++ linux-2.6.35-rc2-gcc/fs/xfs/xfs_trans.c
> @@ -1120,6 +1120,7 @@ xfs_trans_unreserve_and_mod_sb(
>  		error = xfs_mod_incore_sb_batch(tp->t_mountp, msb,
>  			(uint)(msbp - msb), rsvd);
>  		ASSERT(error == 0);
> +		/* FIXME: need real error handling here, error can be ENOSPC */

That comment is wrong and hence is not needed.  By design, we should
never, ever get an error here because we've already reserved all the
space we need and this is just an accounting call.  That's what the
ASSERT(error == 0) is documenting. It's ben placed there to catch
any re-occurrence of the race condition that is documented in the
function head comment during development.  Anyway, if we do get an
error here, we cannot handle it anyway - it's too late to do
anything short of a complete shutdown as we've already written the
transaction to the log.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] [16/23] XFS: Fix gcc 4.6 set but not read and unused statement warnings

[Dave Chinner]

On Thu, Jun 10, 2010 at 01:10:52PM +0200, Andi Kleen wrote:
> 
> For my configuration, that is without quota or RT.
> 
> Mostly dead code removed I think (but needs additional review) 
>
> That is there were one or two bad error handling cases,
> but they were not easily fixable, with comments 
> and I left the warnings in for those for you to remember.
> 
> e.g. if there is a ENOSPC down in xfs_trans.c while
> modifying the superblock it would not be handled.

See my comments about these below.

> Unused statements were mostly related to stub macros for disabled
> features like QUOTA or RT ALLOC. I replace those with
> inlines.

Did you compile with/without XFS_DEBUG (I don't think so)? when
changing code that affects ASSERT statements, CONFIG_XFS_DEBUG needs to
be selected to test that this code compiles. Most XFs developers
build with XFS_CONFIG_DEBUG for everything other than performance
testing, so ensuring this builds is definitely required. ;)

I'd also be interested if any fixes are needed with all options
enabled. Seems silly just to fix a few warnings in just one
particular configuration rather than all of them...

> There were also some problems with variables used in ASSERT()
> I partly moved those into the ASSERT itself and partly
> used a new QASSERT that always evaluates.

That's a pretty ugly hack for a single occurrence of a warning.
Re-arrnaging the code is a much better idea than introducing a new
ASSERT type. e.g:

-	ASSERT(ref >= 0);
+	if (ref < 0)
+		ASSERT(0);

> Cc: xfs@oss.sgi.com
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> 
> ---
>  fs/xfs/linux-2.6/xfs_sync.c |    3 +++
>  fs/xfs/support/debug.h      |    4 ++++
>  fs/xfs/xfs_alloc.c          |   10 +++-------
>  fs/xfs/xfs_da_btree.c       |   15 +++++----------
>  fs/xfs/xfs_dir2_block.c     |    6 +++---
>  fs/xfs/xfs_filestream.c     |   10 ++--------
>  fs/xfs/xfs_iget.c           |    3 ---
>  fs/xfs/xfs_inode.c          |    4 ----
>  fs/xfs/xfs_inode_item.c     |    8 ++------
>  fs/xfs/xfs_log.c            |    2 --
>  fs/xfs/xfs_quota.h          |   14 ++++++++++----
>  fs/xfs/xfs_trans.c          |    1 +
>  12 files changed, 33 insertions(+), 47 deletions(-)
> 
> Index: linux-2.6.35-rc2-gcc/fs/xfs/linux-2.6/xfs_sync.c
> ===================================================================
> --- linux-2.6.35-rc2-gcc.orig/fs/xfs/linux-2.6/xfs_sync.c
> +++ linux-2.6.35-rc2-gcc/fs/xfs/linux-2.6/xfs_sync.c
> @@ -554,6 +554,9 @@ xfs_sync_worker(
>  		xfs_log_force(mp, 0);
>  		xfs_reclaim_inodes(mp, 0);
>  		/* dgc: errors ignored here */
> +		/* ak: yes and you'll get a warning for it now when you
> +		 * upgrade compilers.
> +		 */
>  		error = xfs_qm_sync(mp, SYNC_TRYLOCK);
>  		if (xfs_log_need_covered(mp))
>  			error = xfs_commit_dummy_trans(mp, 0);

I don't think the coment is necessary - the compiler will remind us
that we are ignoring errors.

FWIW, we've now got a situation where external static code checkers
will tell us that we are not handling an error case (which is where
this code and comment came from), and now the compiler will warn us
we are assigning but not using the return value. It's a bit of a
Catch-22 situation. Hence my question is this - how are we supposed
to "safely" ignore a return value seeing as the compiler is now
making the current method rather noisy?

> Index: linux-2.6.35-rc2-gcc/fs/xfs/xfs_da_btree.c
> ===================================================================
> --- linux-2.6.35-rc2-gcc.orig/fs/xfs/xfs_da_btree.c
> +++ linux-2.6.35-rc2-gcc/fs/xfs/xfs_da_btree.c
> @@ -581,10 +581,8 @@ xfs_da_node_add(xfs_da_state_t *state, x
>  	xfs_da_intnode_t *node;
>  	xfs_da_node_entry_t *btree;
>  	int tmp;
> -	xfs_mount_t *mp;
>  
>  	node = oldblk->bp->data;
> -	mp = state->mp;
>  	ASSERT(be16_to_cpu(node->hdr.info.magic) == XFS_DA_NODE_MAGIC);
>  	ASSERT((oldblk->index >= 0) && (oldblk->index <= be16_to_cpu(node->hdr.count)));
>  	ASSERT(newblk->blkno != 0);

That'll break a CONFIG_XFS_DEBUG build as the next statement:

        if (state->args->whichfork == XFS_DATA_FORK)
                ASSERT(newblk->blkno >= mp->m_dirleafblk &&
                       newblk->blkno < mp->m_dirfreeblk);

uses mp inside ASSERT statements.

> Index: linux-2.6.35-rc2-gcc/fs/xfs/xfs_dir2_block.c
> ===================================================================
> --- linux-2.6.35-rc2-gcc.orig/fs/xfs/xfs_dir2_block.c
> +++ linux-2.6.35-rc2-gcc/fs/xfs/xfs_dir2_block.c
> @@ -1073,10 +1073,10 @@ xfs_dir2_sf_to_block(
>  	 */
>  
>  	buf_len = dp->i_df.if_bytes;
> -	buf = kmem_alloc(dp->i_df.if_bytes, KM_SLEEP);
> +	buf = kmem_alloc(buf_len, KM_SLEEP);
>  
> -	memcpy(buf, sfp, dp->i_df.if_bytes);
> -	xfs_idata_realloc(dp, -dp->i_df.if_bytes, XFS_DATA_FORK);
> +	memcpy(buf, sfp, buf_len);
> +	xfs_idata_realloc(dp, -buf_len, XFS_DATA_FORK);
>  	dp->i_d.di_size = 0;
>  	xfs_trans_log_inode(tp, dp, XFS_ILOG_CORE);
>  	/*

Just remove the buf_len variable in this case.

> Index: linux-2.6.35-rc2-gcc/fs/xfs/xfs_filestream.c
> ===================================================================
> --- linux-2.6.35-rc2-gcc.orig/fs/xfs/xfs_filestream.c
> +++ linux-2.6.35-rc2-gcc/fs/xfs/xfs_filestream.c
> @@ -140,9 +140,8 @@ _xfs_filestream_pick_ag(
>  	int		flags,
>  	xfs_extlen_t	minlen)
>  {
> -	int		streams, max_streams;
>  	int		err, trylock, nscan;
> -	xfs_extlen_t	longest, free, minfree, maxfree = 0;
> +	xfs_extlen_t	longest, minfree, maxfree = 0;
>  	xfs_agnumber_t	ag, max_ag = NULLAGNUMBER;
>  	struct xfs_perag *pag;
>  
> @@ -174,7 +173,6 @@ _xfs_filestream_pick_ag(
>  		/* Keep track of the AG with the most free blocks. */
>  		if (pag->pagf_freeblks > maxfree) {
>  			maxfree = pag->pagf_freeblks;
> -			max_streams = atomic_read(&pag->pagf_fstrms);
>  			max_ag = ag;
>  		}
>  
> @@ -196,8 +194,6 @@ _xfs_filestream_pick_ag(
>  		     (flags & XFS_PICK_LOWSPACE))) {
>  
>  			/* Break out, retaining the reference on the AG. */
> -			free = pag->pagf_freeblks;
> -			streams = atomic_read(&pag->pagf_fstrms);
>  			xfs_perag_put(pag);
>  			*agp = ag;
>  			break;
> @@ -234,8 +230,6 @@ next_ag:
>  		if (max_ag != NULLAGNUMBER) {
>  			xfs_filestream_get_ag(mp, max_ag);
>  			TRACE_AG_PICK1(mp, max_ag, maxfree);
> -			streams = max_streams;
> -			free = maxfree;
>  			*agp = max_ag;
>  			break;
>  		}
> @@ -364,7 +358,7 @@ xfs_fstrm_free_func(
>  	/* Drop the reference taken on the AG when the item was added. */
>  	ref = xfs_filestream_put_ag(ip->i_mount, item->ag);
>  
> -	ASSERT(ref >= 0);
> +	QASSERT(ref >= 0);
>  	TRACE_FREE(ip->i_mount, ip, item->pip, item->ag,
>  		xfs_filestream_peek_ag(ip->i_mount, item->ag));

These are all "unused" because they are used in debug code only. i.e.
in XFS_FILESTREAMS_TRACE configs. This is manual debug code that
needs to be converted to the trace infrastructure - the compiler may
say it is unused, but it is not dead code, so shoul dnot be removed.

See also my comment about QASSERT() above.

>  #define xfs_trans_unreserve_quota_nblks(tp, ip, nblks, ninos, flags) \
> Index: linux-2.6.35-rc2-gcc/fs/xfs/xfs_trans.c
> ===================================================================
> --- linux-2.6.35-rc2-gcc.orig/fs/xfs/xfs_trans.c
> +++ linux-2.6.35-rc2-gcc/fs/xfs/xfs_trans.c
> @@ -1120,6 +1120,7 @@ xfs_trans_unreserve_and_mod_sb(
>  		error = xfs_mod_incore_sb_batch(tp->t_mountp, msb,
>  			(uint)(msbp - msb), rsvd);
>  		ASSERT(error == 0);
> +		/* FIXME: need real error handling here, error can be ENOSPC */

That comment is wrong and hence is not needed.  By design, we should
never, ever get an error here because we've already reserved all the
space we need and this is just an accounting call.  That's what the
ASSERT(error == 0) is documenting. It's ben placed there to catch
any re-occurrence of the race condition that is documented in the
function head comment during development.  Anyway, if we do get an
error here, we cannot handle it anyway - it's too late to do
anything short of a complete shutdown as we've already written the
transaction to the log.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] [16/23] XFS: Fix gcc 4.6 set but not read and unused statement warnings

[Andi Kleen]

On Mon, Jun 14, 2010 at 02:27:00PM +1000, Dave Chinner wrote:
> > Unused statements were mostly related to stub macros for disabled
> > features like QUOTA or RT ALLOC. I replace those with
> > inlines.
> 
> Did you compile with/without XFS_DEBUG (I don't think so)? when

No.

I merely made my own config work with relatively little warnings.

> changing code that affects ASSERT statements, CONFIG_XFS_DEBUG needs to
> be selected to test that this code compiles. Most XFs developers
> build with XFS_CONFIG_DEBUG for everything other than performance
> testing, so ensuring this builds is definitely required. ;)

Ok fair enough.

> 
> I'd also be interested if any fixes are needed with all options
> enabled. Seems silly just to fix a few warnings in just one
> particular configuration rather than all of them...

There are tons more warnings with allyesconfig I'm sure,
not only in xfs.  They will need time to work out.

This will happen over time as people eventually move
to gcc 4.6 (after it's released)

Some of the warnings are also related
to not enabling everything (e.g. no quota)

> 
> > There were also some problems with variables used in ASSERT()
> > I partly moved those into the ASSERT itself and partly
> > used a new QASSERT that always evaluates.
> 
> That's a pretty ugly hack for a single occurrence of a warning.
> Re-arrnaging the code is a much better idea than introducing a new
> ASSERT type. e.g:

I originally planned to use it for more, but then ended up
changing other code in other ways.

I still don't think it's a ugly hack, it's a relatively
simple way to handle this. But I can change this single occurrence.

> FWIW, we've now got a situation where external static code checkers
> will tell us that we are not handling an error case (which is where
> this code and comment came from), and now the compiler will warn us
> we are assigning but not using the return value. It's a bit of a
> Catch-22 situation. Hence my question is this - how are we supposed
> to "safely" ignore a return value seeing as the compiler is now
> making the current method rather noisy?

Fix the problem?  

Otherwise you can use a (void) cast, but that's a bad way
if there's a real problem.

> >  	dp->i_d.di_size = 0;
> >  	xfs_trans_log_inode(tp, dp, XFS_ILOG_CORE);
> >  	/*
> 
> Just remove the buf_len variable in this case.

I think the code looks cleaner when using buf_len

> These are all "unused" because they are used in debug code only. i.e.
> in XFS_FILESTREAMS_TRACE configs. This is manual debug code that
> needs to be converted to the trace infrastructure - the compiler may

I have no plans to do that.

> say it is unused, but it is not dead code, so shoul dnot be removed.

I did not remove anything.

> >  			(uint)(msbp - msb), rsvd);
> >  		ASSERT(error == 0);
> > +		/* FIXME: need real error handling here, error can be ENOSPC */
> 
> That comment is wrong and hence is not needed.  By design, we should
> never, ever get an error here because we've already reserved all the
> space we need and this is just an accounting call.  That's what the
> ASSERT(error == 0) is documenting. It's ben placed there to catch

Ok. But I must say ASSERT() is not really a good way to 
document things like that. Whoever wrote this gets
what he deserves now ...

> function head comment during development.  Anyway, if we do get an
> error here, we cannot handle it anyway - it's too late to do
> anything short of a complete shutdown as we've already written the
> transaction to the log.

Well I guess it should be unconditional BUG_ON then.


-Andi
-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [PATCH] [16/23] XFS: Fix gcc 4.6 set but not read and unused statement warnings

[Andi Kleen]

On Mon, Jun 14, 2010 at 02:27:00PM +1000, Dave Chinner wrote:
> > Unused statements were mostly related to stub macros for disabled
> > features like QUOTA or RT ALLOC. I replace those with
> > inlines.
> 
> Did you compile with/without XFS_DEBUG (I don't think so)? when

No.

I merely made my own config work with relatively little warnings.

> changing code that affects ASSERT statements, CONFIG_XFS_DEBUG needs to
> be selected to test that this code compiles. Most XFs developers
> build with XFS_CONFIG_DEBUG for everything other than performance
> testing, so ensuring this builds is definitely required. ;)

Ok fair enough.

> 
> I'd also be interested if any fixes are needed with all options
> enabled. Seems silly just to fix a few warnings in just one
> particular configuration rather than all of them...

There are tons more warnings with allyesconfig I'm sure,
not only in xfs.  They will need time to work out.

This will happen over time as people eventually move
to gcc 4.6 (after it's released)

Some of the warnings are also related
to not enabling everything (e.g. no quota)

> 
> > There were also some problems with variables used in ASSERT()
> > I partly moved those into the ASSERT itself and partly
> > used a new QASSERT that always evaluates.
> 
> That's a pretty ugly hack for a single occurrence of a warning.
> Re-arrnaging the code is a much better idea than introducing a new
> ASSERT type. e.g:

I originally planned to use it for more, but then ended up
changing other code in other ways.

I still don't think it's a ugly hack, it's a relatively
simple way to handle this. But I can change this single occurrence.

> FWIW, we've now got a situation where external static code checkers
> will tell us that we are not handling an error case (which is where
> this code and comment came from), and now the compiler will warn us
> we are assigning but not using the return value. It's a bit of a
> Catch-22 situation. Hence my question is this - how are we supposed
> to "safely" ignore a return value seeing as the compiler is now
> making the current method rather noisy?

Fix the problem?  

Otherwise you can use a (void) cast, but that's a bad way
if there's a real problem.

> >  	dp->i_d.di_size = 0;
> >  	xfs_trans_log_inode(tp, dp, XFS_ILOG_CORE);
> >  	/*
> 
> Just remove the buf_len variable in this case.

I think the code looks cleaner when using buf_len

> These are all "unused" because they are used in debug code only. i.e.
> in XFS_FILESTREAMS_TRACE configs. This is manual debug code that
> needs to be converted to the trace infrastructure - the compiler may

I have no plans to do that.

> say it is unused, but it is not dead code, so shoul dnot be removed.

I did not remove anything.

> >  			(uint)(msbp - msb), rsvd);
> >  		ASSERT(error == 0);
> > +		/* FIXME: need real error handling here, error can be ENOSPC */
> 
> That comment is wrong and hence is not needed.  By design, we should
> never, ever get an error here because we've already reserved all the
> space we need and this is just an accounting call.  That's what the
> ASSERT(error == 0) is documenting. It's ben placed there to catch

Ok. But I must say ASSERT() is not really a good way to 
document things like that. Whoever wrote this gets
what he deserves now ...

> function head comment during development.  Anyway, if we do get an
> error here, we cannot handle it anyway - it's too late to do
> anything short of a complete shutdown as we've already written the
> transaction to the log.

Well I guess it should be unconditional BUG_ON then.


-Andi
-- 
ak@linux.intel.com -- Speaking for myself only.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] [16/23] XFS: Fix gcc 4.6 set but not read and unused statement warnings

[Dave Chinner]

On Mon, Jun 14, 2010 at 09:43:09AM +0200, Andi Kleen wrote:
> On Mon, Jun 14, 2010 at 02:27:00PM +1000, Dave Chinner wrote:
> > > There were also some problems with variables used in ASSERT()
> > > I partly moved those into the ASSERT itself and partly
> > > used a new QASSERT that always evaluates.
> > 
> > That's a pretty ugly hack for a single occurrence of a warning.
> > Re-arrnaging the code is a much better idea than introducing a new
> > ASSERT type. e.g:
> 
> I originally planned to use it for more, but then ended up
> changing other code in other ways.
> 
> I still don't think it's a ugly hack, it's a relatively
> simple way to handle this. But I can change this single occurrence.
> 
> > FWIW, we've now got a situation where external static code checkers
> > will tell us that we are not handling an error case (which is where
> > this code and comment came from), and now the compiler will warn us
> > we are assigning but not using the return value. It's a bit of a
> > Catch-22 situation. Hence my question is this - how are we supposed
> > to "safely" ignore a return value seeing as the compiler is now
> > making the current method rather noisy?
> 
> Fix the problem?  

There is no problem. The end of the error reporting line is the
main loop of a background kernel thread - anything important is
already stashed for later reporting to a real context - so all that
is left to do is throw away the error once it propagated to the top
of the call chain....

> Otherwise you can use a (void) cast, but that's a bad way
> if there's a real problem.

Right, and that's exactly my point - we removed all the (void)
casts because the error checker flagged them as dangerous. Now the
compiler is complaining about not using the error that is
returned. So my question still stands....

> > >  			(uint)(msbp - msb), rsvd);
> > >  		ASSERT(error == 0);
> > > +		/* FIXME: need real error handling here, error can be ENOSPC */
> > 
> > That comment is wrong and hence is not needed.  By design, we should
> > never, ever get an error here because we've already reserved all the
> > space we need and this is just an accounting call.  That's what the
> > ASSERT(error == 0) is documenting. It's ben placed there to catch
> 
> Ok. But I must say ASSERT() is not really a good way to 
> document things like that. Whoever wrote this gets
> what he deserves now ...

We have historically documented code assumptions and bounds
with ASSERT() calls rather than in comments because it means
they are checked at runtime. It means we find out really quickly
when we've made some change that has had an unintended side effect,
rather than it going unnoticed until some user trips over it.

This one in specific has been there for at least 5 years - goes back
to before git was used and has proven to be useful for finding at
least one subtle race in new code introduced back in 2007
(45c34141126a89da07197d5b89c04c6847f1171a "[XFS] Apply transaction
delta counts atomically to incore counters").

FWIW, there's around 2000 asserts in the XFS code - that's about 2%
of the code - which means assumptions in the XFS code are pretty
well documented compared to other Linux filesystems...

> > function head comment during development.  Anyway, if we do get an
> > error here, we cannot handle it anyway - it's too late to do
> > anything short of a complete shutdown as we've already written the
> > transaction to the log.
> 
> Well I guess it should be unconditional BUG_ON then.

Don't be silly. A filesystem shutdown is all that is necessary,
especially as the XFS shutdown procedure has hooks to turn
corruption events into system panics if the admin wants to configure
it that way (generally useful for triggering crash dumps on
corruption events for offline triage).

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] [16/23] XFS: Fix gcc 4.6 set but not read and unused statement warnings

[Dave Chinner]

On Mon, Jun 14, 2010 at 09:43:09AM +0200, Andi Kleen wrote:
> On Mon, Jun 14, 2010 at 02:27:00PM +1000, Dave Chinner wrote:
> > > There were also some problems with variables used in ASSERT()
> > > I partly moved those into the ASSERT itself and partly
> > > used a new QASSERT that always evaluates.
> > 
> > That's a pretty ugly hack for a single occurrence of a warning.
> > Re-arrnaging the code is a much better idea than introducing a new
> > ASSERT type. e.g:
> 
> I originally planned to use it for more, but then ended up
> changing other code in other ways.
> 
> I still don't think it's a ugly hack, it's a relatively
> simple way to handle this. But I can change this single occurrence.
> 
> > FWIW, we've now got a situation where external static code checkers
> > will tell us that we are not handling an error case (which is where
> > this code and comment came from), and now the compiler will warn us
> > we are assigning but not using the return value. It's a bit of a
> > Catch-22 situation. Hence my question is this - how are we supposed
> > to "safely" ignore a return value seeing as the compiler is now
> > making the current method rather noisy?
> 
> Fix the problem?  

There is no problem. The end of the error reporting line is the
main loop of a background kernel thread - anything important is
already stashed for later reporting to a real context - so all that
is left to do is throw away the error once it propagated to the top
of the call chain....

> Otherwise you can use a (void) cast, but that's a bad way
> if there's a real problem.

Right, and that's exactly my point - we removed all the (void)
casts because the error checker flagged them as dangerous. Now the
compiler is complaining about not using the error that is
returned. So my question still stands....

> > >  			(uint)(msbp - msb), rsvd);
> > >  		ASSERT(error == 0);
> > > +		/* FIXME: need real error handling here, error can be ENOSPC */
> > 
> > That comment is wrong and hence is not needed.  By design, we should
> > never, ever get an error here because we've already reserved all the
> > space we need and this is just an accounting call.  That's what the
> > ASSERT(error == 0) is documenting. It's ben placed there to catch
> 
> Ok. But I must say ASSERT() is not really a good way to 
> document things like that. Whoever wrote this gets
> what he deserves now ...

We have historically documented code assumptions and bounds
with ASSERT() calls rather than in comments because it means
they are checked at runtime. It means we find out really quickly
when we've made some change that has had an unintended side effect,
rather than it going unnoticed until some user trips over it.

This one in specific has been there for at least 5 years - goes back
to before git was used and has proven to be useful for finding at
least one subtle race in new code introduced back in 2007
(45c34141126a89da07197d5b89c04c6847f1171a "[XFS] Apply transaction
delta counts atomically to incore counters").

FWIW, there's around 2000 asserts in the XFS code - that's about 2%
of the code - which means assumptions in the XFS code are pretty
well documented compared to other Linux filesystems...

> > function head comment during development.  Anyway, if we do get an
> > error here, we cannot handle it anyway - it's too late to do
> > anything short of a complete shutdown as we've already written the
> > transaction to the log.
> 
> Well I guess it should be unconditional BUG_ON then.

Don't be silly. A filesystem shutdown is all that is necessary,
especially as the XFS shutdown procedure has hooks to turn
corruption events into system panics if the admin wants to configure
it that way (generally useful for triggering crash dumps on
corruption events for offline triage).

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] [16/23] XFS: Fix gcc 4.6 set but not read and unused statement warnings

[Andi Kleen]

> > > function head comment during development.  Anyway, if we do get an
> > > error here, we cannot handle it anyway - it's too late to do
> > > anything short of a complete shutdown as we've already written the
> > > transaction to the log.
> > 
> > Well I guess it should be unconditional BUG_ON then.
> 
> Don't be silly. A filesystem shutdown is all that is necessary,

Without BUG_ON it will not end up in kerneloops.org and you will
never know about it.  That's standard Linux kernel development
practice.

Maybe XFS should catch up on that.

Ok in principle you could make the shutdown a WARN()

Anyways I'm out of this.

-Andi
-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [PATCH] [16/23] XFS: Fix gcc 4.6 set but not read and unused statement warnings

[Andi Kleen]

> > > function head comment during development.  Anyway, if we do get an
> > > error here, we cannot handle it anyway - it's too late to do
> > > anything short of a complete shutdown as we've already written the
> > > transaction to the log.
> > 
> > Well I guess it should be unconditional BUG_ON then.
> 
> Don't be silly. A filesystem shutdown is all that is necessary,

Without BUG_ON it will not end up in kerneloops.org and you will
never know about it.  That's standard Linux kernel development
practice.

Maybe XFS should catch up on that.

Ok in principle you could make the shutdown a WARN()

Anyways I'm out of this.

-Andi
-- 
ak@linux.intel.com -- Speaking for myself only.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] [15/23] EXT4: Fix initialized but not read variables

[tytso]

On Thu, Jun 10, 2010 at 01:10:51PM +0200, Andi Kleen wrote:
> 
> No real bugs found, just various dead code removed I believe.
> Some review would be good.
> 
> Found by gcc 4.6's new warnings
> 
> Cc: tytso@mit.edu
> Cc: linux-ext4@vger.kernel.org
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>

I've reviewed this and pulled a fixed version into the ext4 patch
queue.  You deleted the initializers for size and start in mballoc.c,
which introduced a bug (ironically you commented about start not
getting inititialized in a FIXME, when the patch deleted said
initialization right above the introduced FIXME comment):

> @@ -2922,8 +2916,9 @@ ext4_mb_normalize_request(struct ext4_al
>  		start_off = (loff_t)ac->ac_o_ex.fe_logical << bsbits;
>  		size	  = ac->ac_o_ex.fe_len << bsbits;
>  	}
> -	orig_size = size = size >> bsbits;
> -	orig_start = start = start_off >> bsbits;
> +
> +	/* FIXME: start_off is not used for anything. bug? */
> +	/* FIXME: start may be uninitialized? */
>  
>  	/* don't cover already allocated blocks in selected range */
>  	if (ar->pleft && start <= ar->lleft) {

Also you only fixed one of two uses of the variable 'fidx' which you
removed here:

> @@ -1144,10 +1143,10 @@ static int ext4_ext_grow_indepth(handle_
>  	ext4_idx_store_pblock(curp->p_idx, newblock);
>  
>  	neh = ext_inode_hdr(inode);
> -	fidx = EXT_FIRST_INDEX(neh);
>  	ext_debug("new root: num %d(%d), lblock %d, ptr %llu\n",
>  		  le16_to_cpu(neh->eh_entries), le16_to_cpu(neh->eh_max),
> -		  le32_to_cpu(fidx->ei_block), idx_pblock(fidx));
> +		  le32_to_cpu(EXT_FIRST_INDEX(neh)->ei_block),
> +		  idx_pblock(fidx));
>  
>  	neh->eh_depth = cpu_to_le16(path->p_depth + 1);
>  	err = ext4_ext_dirty(handle, inode, curp);

Other than that the patch was fine.  I've fixed these up and I'll
carry the patch in the ext4 tree.

Thanks,

					- Ted

Re: [PATCH] [17/23] EXT3: Fix set but unused variables

[tytso]

On Thu, Jun 10, 2010 at 01:10:53PM +0200, Andi Kleen wrote:
> 
> I think it's mostly dead code, but needs more review. I was not fully sure
> about the jbd case.
> 
> cc: tytso@mit.edu
> cc: linux-ext4@vger.kernel.org
> Signed-off-by: Andi Kleen <ak@linux.intel.com>

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

						- Ted

Re: [PATCH] [17/23] EXT3: Fix set but unused variables

[tytso]

On Thu, Jun 10, 2010 at 01:10:53PM +0200, Andi Kleen wrote:
> Index: linux-2.6.35-rc2-gcc/fs/jbd/recovery.c
> ===================================================================
> --- linux-2.6.35-rc2-gcc.orig/fs/jbd/recovery.c
> +++ linux-2.6.35-rc2-gcc/fs/jbd/recovery.c
> @@ -283,12 +283,10 @@ int journal_recover(journal_t *journal)
>  int journal_skip_recovery(journal_t *journal)
>  {
>  	int			err;
> -	journal_superblock_t *	sb;
>  
>  	struct recovery_info	info;
>  
>  	memset (&info, 0, sizeof(info));
> -	sb = journal->j_superblock;

Oops, spoke too soon.  This will cause a compile error if
CONFIG_JBD_DEBUG is defined.

The following pesudo-patch is required:

 #ifdef CONFIG_JBD_DEBUG
-		int dropped = info.end_transaction - be32_to_cpu(sb->s_sequence);
+		int dropped = info.end_transaction - 
+				be32_to_cpu(journal->j_superblock->s_sequence);
 #endif

						- Ted

Re: [PATCH] [16/23] XFS: Fix gcc 4.6 set but not read and unused statement warnings

[Dave Chinner]

On Mon, Jun 14, 2010 at 04:37:20PM +0200, Andi Kleen wrote:
> > > > function head comment during development.  Anyway, if we do get an
> > > > error here, we cannot handle it anyway - it's too late to do
> > > > anything short of a complete shutdown as we've already written the
> > > > transaction to the log.
> > > 
> > > Well I guess it should be unconditional BUG_ON then.
> > 
> > Don't be silly. A filesystem shutdown is all that is necessary,
> 
> Without BUG_ON it will not end up in kerneloops.org and you will
> never know about it. 

We find out about corrupted filesystems all the time from users
sending mail to the list. Even if we did panic by default on
corruption events, kerneloops.org is *useless* for reporting them
because finding out about a corruption is only the very first step
of what is usually a long and involved process that requires user
interaction to gather information necessary to find the cause of the
corruption.

Besides, if we _really_ want the machine to panic on corruption,
then we configure the machine specifically for it via setting the
relevant corruption type bit in /proc/sys/fs/xfs/panic_mask. This is
generally only used when a developer asks a user to set it to get
kernel crash dumps triggered when a corruption event occurs so we
can do remote, offline analysis of the failure.

> That's standard Linux kernel development
> practice.  Maybe XFS should catch up on that.

I find this really amusing because linux filesystems have, over the
last few years, implemented a simpler version of XFS's way of
dealing with corruption events(*). Perhaps you should catch up
with the state of the art before throwing rocks, Andi....

Cheers,

Dave.

(*) extN, fat, hpfs, jfs, nilfs2, ntfs, ocfs2 and logfs all have
configurable corruption event behaviour that default to remount-ro
and can be configured to panic the machine.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] [16/23] XFS: Fix gcc 4.6 set but not read and unused statement warnings

[Dave Chinner]

On Mon, Jun 14, 2010 at 04:37:20PM +0200, Andi Kleen wrote:
> > > > function head comment during development.  Anyway, if we do get an
> > > > error here, we cannot handle it anyway - it's too late to do
> > > > anything short of a complete shutdown as we've already written the
> > > > transaction to the log.
> > > 
> > > Well I guess it should be unconditional BUG_ON then.
> > 
> > Don't be silly. A filesystem shutdown is all that is necessary,
> 
> Without BUG_ON it will not end up in kerneloops.org and you will
> never know about it. 

We find out about corrupted filesystems all the time from users
sending mail to the list. Even if we did panic by default on
corruption events, kerneloops.org is *useless* for reporting them
because finding out about a corruption is only the very first step
of what is usually a long and involved process that requires user
interaction to gather information necessary to find the cause of the
corruption.

Besides, if we _really_ want the machine to panic on corruption,
then we configure the machine specifically for it via setting the
relevant corruption type bit in /proc/sys/fs/xfs/panic_mask. This is
generally only used when a developer asks a user to set it to get
kernel crash dumps triggered when a corruption event occurs so we
can do remote, offline analysis of the failure.

> That's standard Linux kernel development
> practice.  Maybe XFS should catch up on that.

I find this really amusing because linux filesystems have, over the
last few years, implemented a simpler version of XFS's way of
dealing with corruption events(*). Perhaps you should catch up
with the state of the art before throwing rocks, Andi....

Cheers,

Dave.

(*) extN, fat, hpfs, jfs, nilfs2, ntfs, ocfs2 and logfs all have
configurable corruption event behaviour that default to remount-ro
and can be configured to panic the machine.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] [16/23] XFS: Fix gcc 4.6 set but not read and unused statement warnings

[Andi Kleen]

> We find out about corrupted filesystems all the time from users
> sending mail to the list. Even if we did panic by default on
> corruption events, kerneloops.org is *useless* for reporting them
> because finding out about a corruption is only the very first step
> of what is usually a long and involved process that requires user
> interaction to gather information necessary to find the cause of the
> corruption.

The idea behind kerneloops.org 
is normally that any single report can be always a flake
(broken memory, hardware, flipped bit whatever).

An error becomes important and interesting when there are multiple
occurrences of it in the field.
> 
> Besides, if we _really_ want the machine to panic on corruption,

BUG_ON is not panic normally.

> then we configure the machine specifically for it via setting the
> relevant corruption type bit in /proc/sys/fs/xfs/panic_mask. This is
> generally only used when a developer asks a user to set it to get
> kernel crash dumps triggered when a corruption event occurs so we
> can do remote, offline analysis of the failure.

Especially when you're talking about desktop class systems
without ECC memory that will mean you'll spend at least some
time on errors which are simply bit flips.

> > That's standard Linux kernel development
> > practice.  Maybe XFS should catch up on that.
> 
> I find this really amusing because linux filesystems have, over the

This has really nothing to do with file systems, it's general
practice for everything (well except XFS) 

> last few years, implemented a simpler version of XFS's way of
> dealing with corruption events(*). Perhaps you should catch up
> with the state of the art before throwing rocks, Andi....

I suspect you miss quite a lot of valuable information from
your user base by not supporting kerneloops.org. On the other
hand it would likely also save you from spending time on 
flakes.

That said you don't need BUG_ON to support it (WARN etc. work
too), it's just the easiest way.

-Andi
-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [PATCH] [16/23] XFS: Fix gcc 4.6 set but not read and unused statement warnings

[Andi Kleen]

> We find out about corrupted filesystems all the time from users
> sending mail to the list. Even if we did panic by default on
> corruption events, kerneloops.org is *useless* for reporting them
> because finding out about a corruption is only the very first step
> of what is usually a long and involved process that requires user
> interaction to gather information necessary to find the cause of the
> corruption.

The idea behind kerneloops.org 
is normally that any single report can be always a flake
(broken memory, hardware, flipped bit whatever).

An error becomes important and interesting when there are multiple
occurrences of it in the field.
> 
> Besides, if we _really_ want the machine to panic on corruption,

BUG_ON is not panic normally.

> then we configure the machine specifically for it via setting the
> relevant corruption type bit in /proc/sys/fs/xfs/panic_mask. This is
> generally only used when a developer asks a user to set it to get
> kernel crash dumps triggered when a corruption event occurs so we
> can do remote, offline analysis of the failure.

Especially when you're talking about desktop class systems
without ECC memory that will mean you'll spend at least some
time on errors which are simply bit flips.

> > That's standard Linux kernel development
> > practice.  Maybe XFS should catch up on that.
> 
> I find this really amusing because linux filesystems have, over the

This has really nothing to do with file systems, it's general
practice for everything (well except XFS) 

> last few years, implemented a simpler version of XFS's way of
> dealing with corruption events(*). Perhaps you should catch up
> with the state of the art before throwing rocks, Andi....

I suspect you miss quite a lot of valuable information from
your user base by not supporting kerneloops.org. On the other
hand it would likely also save you from spending time on 
flakes.

That said you don't need BUG_ON to support it (WARN etc. work
too), it's just the easiest way.

-Andi
-- 
ak@linux.intel.com -- Speaking for myself only.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] [16/23] XFS: Fix gcc 4.6 set but not read and unused statement warnings

[Christoph Hellwig]

On Tue, Jun 15, 2010 at 09:02:45AM +0200, Andi Kleen wrote:
> I suspect you miss quite a lot of valuable information from
> your user base by not supporting kerneloops.org. On the other
> hand it would likely also save you from spending time on 
> flakes.
> 
> That said you don't need BUG_ON to support it (WARN etc. work
> too), it's just the easiest way.

Note that a XFS filesystem shutdown already gives a stack trace.
But picking up every filesystem shutdown on kerneloops.org seems
to be quite a bit too much.  It's usually due to IO errors from
the underlying device.

Re: [PATCH] [16/23] XFS: Fix gcc 4.6 set but not read and unused statement warnings

[Christoph Hellwig]

On Tue, Jun 15, 2010 at 09:02:45AM +0200, Andi Kleen wrote:
> I suspect you miss quite a lot of valuable information from
> your user base by not supporting kerneloops.org. On the other
> hand it would likely also save you from spending time on 
> flakes.
> 
> That said you don't need BUG_ON to support it (WARN etc. work
> too), it's just the easiest way.

Note that a XFS filesystem shutdown already gives a stack trace.
But picking up every filesystem shutdown on kerneloops.org seems
to be quite a bit too much.  It's usually due to IO errors from
the underlying device.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] [16/23] XFS: Fix gcc 4.6 set but not read and unused statement warnings

[Andi Kleen]

On Tue, Jun 15, 2010 at 03:40:28AM -0400, Christoph Hellwig wrote:
> On Tue, Jun 15, 2010 at 09:02:45AM +0200, Andi Kleen wrote:
> > I suspect you miss quite a lot of valuable information from
> > your user base by not supporting kerneloops.org. On the other
> > hand it would likely also save you from spending time on 
> > flakes.
> > 
> > That said you don't need BUG_ON to support it (WARN etc. work
> > too), it's just the easiest way.
> 
> Note that a XFS filesystem shutdown already gives a stack trace.
> But picking up every filesystem shutdown on kerneloops.org seems
> to be quite a bit too much.  It's usually due to IO errors from
> the underlying device.

Yes, but known race check asserts should be probably there, right?
Maybe you need a special kind of ASSERT (or shutdown) for those? 

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [PATCH] [16/23] XFS: Fix gcc 4.6 set but not read and unused statement warnings

[Andi Kleen]

On Tue, Jun 15, 2010 at 03:40:28AM -0400, Christoph Hellwig wrote:
> On Tue, Jun 15, 2010 at 09:02:45AM +0200, Andi Kleen wrote:
> > I suspect you miss quite a lot of valuable information from
> > your user base by not supporting kerneloops.org. On the other
> > hand it would likely also save you from spending time on 
> > flakes.
> > 
> > That said you don't need BUG_ON to support it (WARN etc. work
> > too), it's just the easiest way.
> 
> Note that a XFS filesystem shutdown already gives a stack trace.
> But picking up every filesystem shutdown on kerneloops.org seems
> to be quite a bit too much.  It's usually due to IO errors from
> the underlying device.

Yes, but known race check asserts should be probably there, right?
Maybe you need a special kind of ASSERT (or shutdown) for those? 

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] [17/23] EXT3: Fix set but unused variables

[Jan Kara]

> On Thu, Jun 10, 2010 at 01:10:53PM +0200, Andi Kleen wrote:
> > Index: linux-2.6.35-rc2-gcc/fs/jbd/recovery.c
> > ===================================================================
> > --- linux-2.6.35-rc2-gcc.orig/fs/jbd/recovery.c
> > +++ linux-2.6.35-rc2-gcc/fs/jbd/recovery.c
> > @@ -283,12 +283,10 @@ int journal_recover(journal_t *journal)
> >  int journal_skip_recovery(journal_t *journal)
> >  {
> >  	int			err;
> > -	journal_superblock_t *	sb;
> >  
> >  	struct recovery_info	info;
> >  
> >  	memset (&info, 0, sizeof(info));
> > -	sb = journal->j_superblock;
> 
> Oops, spoke too soon.  This will cause a compile error if
> CONFIG_JBD_DEBUG is defined.
> 
> The following pesudo-patch is required:
> 
>  #ifdef CONFIG_JBD_DEBUG
> -		int dropped = info.end_transaction - be32_to_cpu(sb->s_sequence);
> +		int dropped = info.end_transaction - 
> +				be32_to_cpu(journal->j_superblock->s_sequence);
>  #endif
  I have merged the patch with your fix to my tree.

								Honza
-- 
Jan Kara <jack@suse.cz>
SuSE CR Labs

Re: [PATCH] [4/23] pagemap: Avoid unused-but-set variable

[Andrew Morton]

On Thu, 10 Jun 2010 13:10:39 +0200 (CEST)
Andi Kleen <andi@firstfloor.org> wrote:

> 
> Avoid quite a lot of warnings in header files in a gcc 4.6 -Wall builds
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> 
> ---
>  include/linux/pagemap.h |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> Index: linux-2.6.35-rc2-gcc/include/linux/pagemap.h
> ===================================================================
> --- linux-2.6.35-rc2-gcc.orig/include/linux/pagemap.h
> +++ linux-2.6.35-rc2-gcc/include/linux/pagemap.h
> @@ -423,8 +423,10 @@ static inline int fault_in_pages_readabl
>  		const char __user *end = uaddr + size - 1;
>  
>  		if (((unsigned long)uaddr & PAGE_MASK) !=
> -				((unsigned long)end & PAGE_MASK))
> +				((unsigned long)end & PAGE_MASK)) {
>  		 	ret = __get_user(c, end);
> +			(void)c;
> +		}
>  	}
>  	return ret;
>  }

urgh.  In fact I'd urgh the whole patchset.

Problem is, anyone who looks at all these random (void) casts is going
to have a hard time working out why they're there.  This is worsened by
the long-standing practice wherein some people put unneeded (void) casts all
over the place due to being traumatised by lint 15 years ago (I think).

Wouldn't it be better to make this stuff self-documenting, so anyone
who reads the code can immediately see what it's doing, rather than
scratching their heads over random, seemingly-unneeded casts?

#define gcc_46_is_a_pita(expr)	((void)(expr))

?

Re: [PATCH] [4/23] pagemap: Avoid unused-but-set variable

[Andi Kleen]

> urgh.  In fact I'd urgh the whole patchset.
> 
> Problem is, anyone who looks at all these random (void) casts is going

There are actually not that many in the patch kit if you read it 
completely. I think that's nearly the only one outside a macro.

> to have a hard time working out why they're there.  This is worsened by
> the long-standing practice wherein some people put unneeded (void) casts all
> over the place due to being traumatised by lint 15 years ago (I think).

It's really the same reason why they used this with lints.

When I tried it first I started complaining about gcc too (I even
filled a gcc bug), then disabled it in my build,
but after some contemplation I found it really finds real bugs.

> 
> Wouldn't it be better to make this stuff self-documenting, so anyone
> who reads the code can immediately see what it's doing, rather than
> scratching their heads over random, seemingly-unneeded casts?
> 
> #define gcc_46_is_a_pita(expr)	((void)(expr))

The warning is really useful and we'll get all used to it.

So I went with the plain cast. Maybe some more comments would
be useful.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [PATCH] [8/23] KGDB: Remove set but unused newPC

[Jason Wessel]

On 06/10/2010 06:10 AM, Andi Kleen wrote:
> I'm not fully sure this is the correct fix, maybe this
> was a bug and newPC should really have a side effect.
> Jason?
>
>   
Andi,

This looks fine to me.  The newPC got there from the merge of the 32/64
kgdb specific implementation.   Definitely don't need it anymore.

I'll queue it for 2.6.36.

Thanks,
Jason.

> Found by gcc 4.6's new warnings
>
> Cc: jason.wessel@windriver.com
>
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
>
> ---
>  arch/x86/kernel/kgdb.c |    3 ---
>  1 file changed, 3 deletions(-)
>
> Index: linux-2.6.35-rc2-gcc/arch/x86/kernel/kgdb.c
> ===================================================================
> --- linux-2.6.35-rc2-gcc.orig/arch/x86/kernel/kgdb.c
> +++ linux-2.6.35-rc2-gcc/arch/x86/kernel/kgdb.c
> @@ -458,7 +458,6 @@ int kgdb_arch_handle_exception(int e_vec
>  {
>  	unsigned long addr;
>  	char *ptr;
> -	int newPC;
>  
>  	switch (remcomInBuffer[0]) {
>  	case 'c':
> @@ -469,8 +468,6 @@ int kgdb_arch_handle_exception(int e_vec
>  			linux_regs->ip = addr;
>  	case 'D':
>  	case 'k':
> -		newPC = linux_regs->ip;
> -
>  		/* clear the trace bit */
>  		linux_regs->flags &= ~X86_EFLAGS_TF;
>  		atomic_set(&kgdb_cpu_doing_single_step, -1);
>