2010-06-10 11:10:43

by Andi Kleen

[permalink] [raw]
Subject: [PATCH] [0/23] Fix gcc 4.6 set but unused variable warnings


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


2010-06-10 11:10:50

by Andi Kleen

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


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

Found by gcc 4.6's new warnings.

Cc: [email protected]
Cc: [email protected]
Signed-off-by: Andi Kleen <[email protected]>

---
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;

2010-06-10 11:10:56

by Andi Kleen

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


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: [email protected]
Cc: [email protected]

Signed-off-by: Andi Kleen <[email protected]>

---
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;
}

2010-06-10 11:11:01

by Andi Kleen

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


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

Signed-off-by: Andi Kleen <[email protected]>

---
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)

2010-06-10 11:11:06

by Andi Kleen

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


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

Cc: [email protected]
Cc: [email protected]

Signed-off-by: Andi Kleen <[email protected]>

---
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) {

2010-06-10 11:11:09

by Andi Kleen

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


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

Cc: [email protected]

Signed-off-by: Andi Kleen <[email protected]>

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;

2010-06-10 11:11:37

by Andi Kleen

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


Just some dead code.

Signed-off-by: Andi Kleen <[email protected]>

---
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) {
/*

2010-06-10 11:11:54

by Andi Kleen

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


No real bugs I believe, just some dead code.

Signed-off-by: Andi Kleen <[email protected]>

---
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.

2010-06-10 11:11:59

by Andi Kleen

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


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

cc: [email protected]
cc: [email protected]

Signed-off-by: Andi Kleen <[email protected]>

---
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"

2010-06-10 11:12:05

by Andi Kleen

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


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

cc: [email protected]
cc: [email protected]
Signed-off-by: Andi Kleen <[email protected]>

---
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

2010-06-10 11:12:38

by Andi Kleen

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


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: [email protected]

Signed-off-by: Andi Kleen <[email protected]>

---
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

2010-06-10 11:12:42

by Andi Kleen

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


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: [email protected]
Cc: [email protected]

Signed-off-by: Andi Kleen <[email protected]>

---
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

2010-06-10 11:13:16

by Andi Kleen

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


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: [email protected]
Cc: [email protected]

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 <[email protected]>

---
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) {

2010-06-10 11:13:22

by Andi Kleen

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


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: [email protected]
cc: [email protected]


Signed-off-by: Andi Kleen <[email protected]>

---
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,

2010-06-10 11:10:47

by Andi Kleen

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


They are only useful with CONFIG_CPUMASK_OFFSTACK

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

Cc: [email protected]
Signed-off-by: Andi Kleen <[email protected]>

---
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;

2010-06-10 11:13:47

by Andi Kleen

[permalink] [raw]
Subject: [PATCH] [11/23] KVM: Fix KVM_SET_SIGNAL_MASK


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: [email protected]
Cc: [email protected]

Signed-off-by: Andi Kleen <[email protected]>

---
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: {

2010-06-10 11:10:46

by Andi Kleen

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


Avoid hundreds of warnings with a gcc 4.6 -Wall build

Cc: [email protected]
Cc: [email protected]

Signed-off-by: Andi Kleen <[email protected]>

---
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: \

2010-06-10 11:10:44

by Andi Kleen

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


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: [email protected]

Signed-off-by: Andi Kleen <[email protected]>

---
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)

2010-06-10 11:14:13

by Andi Kleen

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


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 <[email protected]>

---
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)) {

2010-06-10 11:14:33

by Andi Kleen

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


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: [email protected]

Signed-off-by: Andi Kleen <[email protected]>

---
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);

2010-06-10 11:14:50

by Andi Kleen

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


Just some dead code, no real bugs.

Found by gcc 4.6 -Wall

Cc: [email protected]

Signed-off-by: Andi Kleen <[email protected]>

---
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++) {

2010-06-10 11:15:10

by Andi Kleen

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


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: [email protected]
Signed-off-by: Andi Kleen <[email protected]>

---
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;

2010-06-10 11:15:06

by Andi Kleen

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


Just dead code I believe

Cc: [email protected]
Cc: [email protected]

Signed-off-by: Andi Kleen <[email protected]>

---
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)))

2010-06-10 11:15:49

by Andi Kleen

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


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

Signed-off-by: Andi Kleen <[email protected]>

---
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;
}

2010-06-10 11:16:23

by Tejun Heo

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

On 06/10/2010 01:10 PM, Andi Kleen wrote:
> Avoid hundreds of warnings with a gcc 4.6 -Wall build
>
> Cc: [email protected]
> Cc: [email protected]
>
> Signed-off-by: Andi Kleen <[email protected]>

Acked-by: Tejun Heo <[email protected]>

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

2010-06-10 12:10:11

by Ingo Molnar

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


* Tejun Heo <[email protected]> wrote:

> On 06/10/2010 01:10 PM, Andi Kleen wrote:
> > Avoid hundreds of warnings with a gcc 4.6 -Wall build
> >
> > Cc: [email protected]
> > Cc: [email protected]
> >
> > Signed-off-by: Andi Kleen <[email protected]>
>
> Acked-by: Tejun Heo <[email protected]>
>
> 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

2010-06-10 12:25:23

by Andi Kleen

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

Commit-ID: 157a9df3b0f8042f1a6a4fe9e0e494431aaca1ae
Gitweb: http://git.kernel.org/tip/157a9df3b0f8042f1a6a4fe9e0e494431aaca1ae
Author: Andi Kleen <[email protected]>
AuthorDate: Thu, 10 Jun 2010 13:10:36 +0200
Committer: Ingo Molnar <[email protected]>
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 <[email protected]>
Acked-by: Tejun Heo <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Andrew Morton <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
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: \

2010-06-10 14:17:26

by Avi Kivity

[permalink] [raw]
Subject: Re: [PATCH] [11/23] KVM: Fix KVM_SET_SIGNAL_MASK

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

2010-06-10 14:38:09

by Avi Kivity

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

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

2010-06-10 14:44:09

by Peter Zijlstra

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

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: [email protected]
> Cc: [email protected]
> Signed-off-by: Andi Kleen <[email protected]>
>
> ---
> 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.

2010-06-10 14:52:47

by Andi Kleen

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

> 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

--
[email protected] -- Speaking for myself only.

2010-06-10 14:55:57

by Peter Zijlstra

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

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 */
}

2010-06-10 15:06:21

by Andi Kleen

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

Peter Zijlstra <[email protected]> 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: [email protected]
Cc: [email protected]
Signed-off-by: Andi Kleen <[email protected]>

---
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;


--
[email protected] -- Speaking for myself only.

2010-06-10 15:19:48

by Peter Zijlstra

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

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;

2010-06-10 15:34:14

by Andi Kleen

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

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

2010-06-10 17:13:28

by H. Peter Anvin

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

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.

2010-06-10 17:43:46

by Justin P. Mattock

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

On 06/10/2010 05:09 AM, Ingo Molnar wrote:
>
> * Tejun Heo<[email protected]> wrote:
>
>> On 06/10/2010 01:10 PM, Andi Kleen wrote:
>>> Avoid hundreds of warnings with a gcc 4.6 -Wall build
>>>
>>> Cc: [email protected]
>>> Cc: [email protected]
>>>
>>> Signed-off-by: Andi Kleen<[email protected]>
>>
>> Acked-by: Tejun Heo<[email protected]>
>>
>> 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 [email protected]
> 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

2010-06-10 18:10:16

by Andi Kleen

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

> 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
--
[email protected] -- Speaking for myself only.

2010-06-10 18:26:37

by Justin P. Mattock

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

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

2010-06-10 20:10:50

by Justin P. Mattock

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

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

2010-06-10 23:43:13

by Andi Kleen

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

Commit-ID: cf3bdc29fcbf2cb4cfa238591021d41ea8f8431f
Gitweb: http://git.kernel.org/tip/cf3bdc29fcbf2cb4cfa238591021d41ea8f8431f
Author: Andi Kleen <[email protected]>
AuthorDate: Thu, 10 Jun 2010 13:10:40 +0200
Committer: H. Peter Anvin <[email protected]>
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 <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: H. Peter Anvin <[email protected]>
---
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;

2010-06-11 16:20:15

by Christoph Hellwig

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

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.

2010-06-11 16:36:50

by Andi Kleen

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

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
--
[email protected] -- Speaking for myself only.

2010-06-14 04:27:08

by Dave Chinner

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

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: [email protected]
>
> Signed-off-by: Andi Kleen <[email protected]>
>
> ---
> 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
[email protected]

2010-06-14 07:43:16

by Andi Kleen

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

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
--
[email protected] -- Speaking for myself only.

2010-06-14 13:38:13

by Dave Chinner

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

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
[email protected]

2010-06-14 14:37:27

by Andi Kleen

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

> > > 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
--
[email protected] -- Speaking for myself only.

2010-06-14 17:20:33

by Theodore Ts'o

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

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: [email protected]
> Cc: [email protected]
>
> Signed-off-by: Andi Kleen <[email protected]>

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

2010-06-14 17:21:17

by Theodore Ts'o

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

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: [email protected]
> cc: [email protected]
> Signed-off-by: Andi Kleen <[email protected]>

Acked-by: "Theodore Ts'o" <[email protected]>

- Ted

2010-06-14 17:27:22

by Theodore Ts'o

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

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

2010-06-14 22:25:05

by Dave Chinner

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

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
[email protected]

2010-06-15 07:02:48

by Andi Kleen

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

> 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
--
[email protected] -- Speaking for myself only.

2010-06-15 07:40:41

by Christoph Hellwig

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

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.

2010-06-15 07:46:47

by Andi Kleen

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

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

--
[email protected] -- Speaking for myself only.

2010-06-15 14:01:35

by Jan Kara

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

> 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 <[email protected]>
SuSE CR Labs

2010-06-18 23:29:30

by Andrew Morton

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

On Thu, 10 Jun 2010 13:10:39 +0200 (CEST)
Andi Kleen <[email protected]> wrote:

>
> Avoid quite a lot of warnings in header files in a gcc 4.6 -Wall builds
>
> Signed-off-by: Andi Kleen <[email protected]>
>
> ---
> 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))

?

2010-06-19 07:44:16

by Andi Kleen

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

> 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

--
[email protected] -- Speaking for myself only.

2010-07-30 12:00:18

by Jason Wessel

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



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: [email protected]
>
> Signed-off-by: Andi Kleen <[email protected]>
>
> ---
> 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);
>