Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next] net: remove unnecessary return's
From: Julia Lawall @ 2014-02-13 21:55 UTC (permalink / raw)
  To: Joe Perches; +Cc: Stephen Hemminger, David Miller, netdev
In-Reply-To: <1392273125.2214.25.camel@joe-AO722>

On Wed, 12 Feb 2014, Joe Perches wrote:

> On Wed, 2014-02-12 at 20:51 -0800, Stephen Hemminger wrote:
> > One of my pet coding style peeves is the practice of
> > adding extra return; at the end of function.
> > Kill several instances of this in network code.
> > I suppose some coccinelle wizardy could do this automatically.
> 
> Maybe, but grep version 2.5.4 will show most of them.
> 
> $ grep-2.5.4 -rP --include=*.[ch] "return;\n}" *
> [...]
> 
> Fixing them has to make sure that there's no
> label before the close brace.
> 
> gcc has to have a statement before the close brace
> of a void return after a label.
> 
> label:
> }
> 
> must be:
> 
> label:
> 	;
> }
> 
> to compile.

The patch below converts label: return; } to label: ; }.  I have only 
scanned through the patches, not patched the code and looked at the 
results, so I am not sure that it is completely correct.  On the other 
hand, I'm also not sure that label: ; } is better than label: return; }, 
either.  If people think it is, then I can cheack the results in more 
detail.

julia


diff -u -p a/arch/arm/mach-iop13xx/io.c b/arch/arm/mach-iop13xx/io.c
--- a/arch/arm/mach-iop13xx/io.c
+++ b/arch/arm/mach-iop13xx/io.c
@@ -80,7 +80,7 @@ static void __iop13xx_iounmap(volatile v
 	__iounmap(addr);
 
 skip:
-	return;
+	;
 }
 
 void __init iop13xx_init_early(void)
diff -u -p a/arch/arm/mach-pxa/balloon3.c b/arch/arm/mach-pxa/balloon3.c
--- a/arch/arm/mach-pxa/balloon3.c
+++ b/arch/arm/mach-pxa/balloon3.c
@@ -269,7 +269,7 @@ static void __init balloon3_lcd_init(voi
 err2:
 	gpio_free(BALLOON3_GPIO_RUN_BACKLIGHT);
 err:
-	return;
+	;
 }
 #else
 static inline void balloon3_lcd_init(void) {}
diff -u -p a/arch/arm/mach-pxa/vpac270.c b/arch/arm/mach-pxa/vpac270.c
--- a/arch/arm/mach-pxa/vpac270.c
+++ b/arch/arm/mach-pxa/vpac270.c
@@ -577,7 +577,7 @@ static void __init vpac270_lcd_init(void
 err2:
 	gpio_free(GPIO81_VPAC270_BKL_ON);
 err:
-	return;
+	;
 }
 #else
 static inline void vpac270_lcd_init(void) {}
diff -u -p a/arch/avr32/boards/atstk1000/atstk1002.c b/arch/avr32/boards/atstk1000/atstk1002.c
--- a/arch/avr32/boards/atstk1000/atstk1002.c
+++ b/arch/avr32/boards/atstk1000/atstk1002.c
@@ -236,7 +236,7 @@ err_set_clk:
 err_pll:
 	clk_put(gclk);
 err_gclk:
-	return;
+	;
 }
 #else
 static void __init atstk1002_setup_extdac(void)
diff -u -p a/arch/avr32/boards/atstk1000/atstk1003.c b/arch/avr32/boards/atstk1000/atstk1003.c
--- a/arch/avr32/boards/atstk1000/atstk1003.c
+++ b/arch/avr32/boards/atstk1000/atstk1003.c
@@ -103,7 +103,7 @@ err_set_clk:
 err_pll:
 	clk_put(gclk);
 err_gclk:
-	return;
+	;
 }
 #else
 static void __init atstk1003_setup_extdac(void)
diff -u -p a/arch/avr32/boards/atstk1000/atstk1004.c b/arch/avr32/boards/atstk1000/atstk1004.c
--- a/arch/avr32/boards/atstk1000/atstk1004.c
+++ b/arch/avr32/boards/atstk1000/atstk1004.c
@@ -108,7 +108,7 @@ err_set_clk:
 err_pll:
 	clk_put(gclk);
 err_gclk:
-	return;
+	;
 }
 #else
 static void __init atstk1004_setup_extdac(void)
diff -u -p a/arch/avr32/kernel/process.c b/arch/avr32/kernel/process.c
--- a/arch/avr32/kernel/process.c
+++ b/arch/avr32/kernel/process.c
@@ -105,7 +105,7 @@ static void dump_mem(const char *str, co
 	}
 
 out:
-	return;
+	;
 }
 
 static inline int valid_stack_ptr(struct thread_info *tinfo, unsigned long p)
diff -u -p a/arch/mips/kvm/kvm_mips.c b/arch/mips/kvm/kvm_mips.c
--- a/arch/mips/kvm/kvm_mips.c
+++ b/arch/mips/kvm/kvm_mips.c
@@ -260,7 +260,7 @@ void kvm_arch_commit_memory_region(struc
 		}
 	}
 out:
-	return;
+	;
 }
 
 void kvm_arch_flush_shadow_all(struct kvm *kvm)
diff -u -p a/arch/mips/lantiq/irq.c b/arch/mips/lantiq/irq.c
--- a/arch/mips/lantiq/irq.c
+++ b/arch/mips/lantiq/irq.c
@@ -328,7 +328,7 @@ asmlinkage void plat_irq_dispatch(void)
 	pr_alert("Spurious IRQ: CAUSE=0x%08x\n", read_c0_status());
 
 out:
-	return;
+	;
 }
 
 static int icu_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hw)
diff -u -p a/arch/powerpc/boot/wii.c b/arch/powerpc/boot/wii.c
--- a/arch/powerpc/boot/wii.c
+++ b/arch/powerpc/boot/wii.c
@@ -134,7 +134,7 @@ static void platform_fixups(void)
 	}
 
 out:
-	return;
+	;
 }
 
 void platform_init(unsigned long r3, unsigned long r4, unsigned long r5)
diff -u -p a/arch/s390/kernel/debug.c b/arch/s390/kernel/debug.c
--- a/arch/s390/kernel/debug.c
+++ b/arch/s390/kernel/debug.c
@@ -750,7 +750,7 @@ debug_unregister(debug_info_t * id)
 	mutex_unlock(&debug_mutex);
 
 out:
-	return;
+	;
 }
 EXPORT_SYMBOL(debug_unregister);
 
diff -u -p a/arch/um/os-Linux/irq.c b/arch/um/os-Linux/irq.c
--- a/arch/um/os-Linux/irq.c
+++ b/arch/um/os-Linux/irq.c
@@ -116,7 +116,7 @@ void os_free_irq_by_cb(int (*test)(struc
 		i++;
 	}
  out:
-	return;
+	;
 }
 
 int os_get_pollfd(int i)
diff -u -p a/arch/x86/lib/insn.c b/arch/x86/lib/insn.c
--- a/arch/x86/lib/insn.c
+++ b/arch/x86/lib/insn.c
@@ -173,7 +173,7 @@ vex_end:
 	prefixes->got = 1;
 
 err_out:
-	return;
+	;
 }
 
 /**
@@ -226,7 +226,7 @@ end:
 	opcode->got = 1;
 
 err_out:
-	return;
+	;
 }
 
 /**
@@ -264,7 +264,7 @@ void insn_get_modrm(struct insn *insn)
 	modrm->got = 1;
 
 err_out:
-	return;
+	;
 }
 
 
@@ -316,7 +316,7 @@ void insn_get_sib(struct insn *insn)
 	insn->sib.got = 1;
 
 err_out:
-	return;
+	;
 }
 
 
@@ -380,7 +380,7 @@ out:
 	insn->displacement.got = 1;
 
 err_out:
-	return;
+	;
 }
 
 /* Decode moffset16/32/64. Return 0 if failed */
@@ -559,7 +559,7 @@ done:
 	insn->immediate.got = 1;
 
 err_out:
-	return;
+	;
 }
 
 /**
diff -u -p a/crypto/tcrypt.c b/crypto/tcrypt.c
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -401,7 +401,7 @@ out_nooutbuf:
 out_noaxbuf:
 	testmgr_free_buf(xbuf);
 out_noxbuf:
-	return;
+	;
 }
 
 static void test_cipher_speed(const char *algo, int enc, unsigned int sec,
diff -u -p a/drivers/atm/iphase.c b/drivers/atm/iphase.c
--- a/drivers/atm/iphase.c
+++ b/drivers/atm/iphase.c
@@ -693,7 +693,7 @@ static void ia_tx_poll (IADEV *iadev) {
     }
     ia_que_tx(iadev);
 out:
-    return;
+    ;
 }
 #if 0
 static void ia_eeprom_put (IADEV *iadev, u32 addr, u_short val)
diff -u -p a/drivers/atm/nicstar.c b/drivers/atm/nicstar.c
--- a/drivers/atm/nicstar.c
+++ b/drivers/atm/nicstar.c
@@ -1056,7 +1056,7 @@ static void push_rxbufs(ns_dev * card, s
 	}
 
 out:
-	return;
+	;
 }
 
 static irqreturn_t ns_irq_handler(int irq, void *dev_id)
diff -u -p a/drivers/block/cpqarray.c b/drivers/block/cpqarray.c
--- a/drivers/block/cpqarray.c
+++ b/drivers/block/cpqarray.c
@@ -1795,7 +1795,7 @@ err_2:
 err_1:
   	kfree(id_ldrive);
 err_0:
-	return;
+	;
 }
 
 static void __exit cpqarray_exit(void)
diff -u -p a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -2789,7 +2789,7 @@ channel_handler(ipmi_smi_t intf, struct
 		}
 	}
  out:
-	return;
+	;
 }
 
 static void ipmi_poll(ipmi_smi_t intf)
@@ -3924,7 +3924,7 @@ void ipmi_smi_msg_received(ipmi_smi_t
 
 	tasklet_schedule(&intf->recv_tasklet);
  out:
-	return;
+	;
 }
 EXPORT_SYMBOL(ipmi_smi_msg_received);
 
diff -u -p a/drivers/char/ipmi/ipmi_poweroff.c b/drivers/char/ipmi/ipmi_poweroff.c
--- a/drivers/char/ipmi/ipmi_poweroff.c
+++ b/drivers/char/ipmi/ipmi_poweroff.c
@@ -310,7 +310,7 @@ static void ipmi_poweroff_atca(ipmi_user
 	if (atca_oem_poweroff_hook)
 		atca_oem_poweroff_hook(user);
  out:
-	return;
+	;
 }
 
 /*
@@ -434,7 +434,7 @@ static void ipmi_poweroff_cpi1(ipmi_user
 		goto out;
 
  out:
-	return;
+	;
 }
 
 /*
diff -u -p a/drivers/char/ipmi/ipmi_watchdog.c b/drivers/char/ipmi/ipmi_watchdog.c
--- a/drivers/char/ipmi/ipmi_watchdog.c
+++ b/drivers/char/ipmi/ipmi_watchdog.c
@@ -1098,7 +1098,7 @@ static void ipmi_unregister_watchdog(int
 	watchdog_user = NULL;
 
  out:
-	return;
+	;
 }
 
 #ifdef HAVE_DIE_NMI
diff -u -p a/drivers/clk/clk.c b/drivers/clk/clk.c
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -523,7 +523,7 @@ unlock_out:
 	clk_enable_unlock(flags);
 
 out:
-	return;
+	;
 }
 
 static bool clk_ignore_unused;
diff -u -p a/drivers/gpu/drm/gma500/oaktrail_hdmi.c b/drivers/gpu/drm/gma500/oaktrail_hdmi.c
--- a/drivers/gpu/drm/gma500/oaktrail_hdmi.c
+++ b/drivers/gpu/drm/gma500/oaktrail_hdmi.c
@@ -736,7 +736,7 @@ void oaktrail_hdmi_setup(struct drm_devi
 free:
 	kfree(hdmi_dev);
 out:
-	return;
+	;
 }
 
 void oaktrail_hdmi_teardown(struct drm_device *dev)
diff -u -p a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -974,7 +974,7 @@ mapped:
 	}
 
 ignore:
-	return;
+	;
 
 }
 
diff -u -p a/drivers/hid/hid-wacom.c b/drivers/hid/hid-wacom.c
--- a/drivers/hid/hid-wacom.c
+++ b/drivers/hid/hid-wacom.c
@@ -154,7 +154,7 @@ static void wacom_set_image(struct hid_d
 				HID_FEATURE_REPORT);
 
 err:
-	return;
+	;
 }
 
 static void wacom_leds_set_brightness(struct led_classdev *led_dev,
diff -u -p a/drivers/ide/ide-park.c b/drivers/ide/ide-park.c
--- a/drivers/ide/ide-park.c
+++ b/drivers/ide/ide-park.c
@@ -55,7 +55,7 @@ static void issue_park_cmd(ide_drive_t *
 	elv_add_request(q, rq, ELEVATOR_INSERT_FRONT);
 
 out:
-	return;
+	;
 }
 
 ide_startstop_t ide_do_park_unpark(ide_drive_t *drive, struct request *rq)
diff -u -p a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c
--- a/drivers/infiniband/core/mad.c
+++ b/drivers/infiniband/core/mad.c
@@ -1552,7 +1552,7 @@ vendor_check:
 	}
 
 out:
-	return;
+	;
 }
 
 static struct ib_mad_agent_private *
diff -u -p a/drivers/infiniband/hw/cxgb4/ev.c b/drivers/infiniband/hw/cxgb4/ev.c
--- a/drivers/infiniband/hw/cxgb4/ev.c
+++ b/drivers/infiniband/hw/cxgb4/ev.c
@@ -172,7 +172,7 @@ done:
 		wake_up(&chp->wait);
 	c4iw_qp_rem_ref(&qhp->ibqp);
 out:
-	return;
+	;
 }
 
 int c4iw_ev_handler(struct c4iw_dev *dev, u32 qid)
diff -u -p a/drivers/infiniband/hw/ipath/ipath_file_ops.c b/drivers/infiniband/hw/ipath/ipath_file_ops.c
--- a/drivers/infiniband/hw/ipath/ipath_file_ops.c
+++ b/drivers/infiniband/hw/ipath/ipath_file_ops.c
@@ -2613,5 +2613,5 @@ void ipath_user_remove(struct ipath_devd
 		atomic_set(&user_setup, 0);
 	}
 bail:
-	return;
+	;
 }
diff -u -p a/drivers/infiniband/hw/ipath/ipath_intr.c b/drivers/infiniband/hw/ipath/ipath_intr.c
--- a/drivers/infiniband/hw/ipath/ipath_intr.c
+++ b/drivers/infiniband/hw/ipath/ipath_intr.c
@@ -446,7 +446,7 @@ static void handle_e_ibstatuschanged(str
 skip_ibchange:
 	dd->ipath_lastibcstat = ibcs;
 done:
-	return;
+	;
 }
 
 static void handle_supp_msgs(struct ipath_devdata *dd,
diff -u -p a/drivers/infiniband/hw/ipath/ipath_rc.c b/drivers/infiniband/hw/ipath/ipath_rc.c
--- a/drivers/infiniband/hw/ipath/ipath_rc.c
+++ b/drivers/infiniband/hw/ipath/ipath_rc.c
@@ -709,7 +709,7 @@ queue_ack:
 	}
 	spin_unlock_irqrestore(&qp->s_lock, flags);
 done:
-	return;
+	;
 }
 
 /**
@@ -835,7 +835,7 @@ void ipath_restart_rc(struct ipath_qp *q
 	ipath_schedule_send(qp);
 
 bail:
-	return;
+	;
 }
 
 static inline void update_last_psn(struct ipath_qp *qp, u32 psn)
@@ -1312,7 +1312,7 @@ ack_err:
 ack_done:
 	spin_unlock_irqrestore(&qp->s_lock, flags);
 bail:
-	return;
+	;
 }
 
 /**
@@ -1965,5 +1965,5 @@ send_ack:
 unlock:
 	spin_unlock_irqrestore(&qp->s_lock, flags);
 done:
-	return;
+	;
 }
diff -u -p a/drivers/infiniband/hw/ipath/ipath_sdma.c b/drivers/infiniband/hw/ipath/ipath_sdma.c
--- a/drivers/infiniband/hw/ipath/ipath_sdma.c
+++ b/drivers/infiniband/hw/ipath/ipath_sdma.c
@@ -355,7 +355,7 @@ resched_noprint:
 unlock:
 	spin_unlock_irqrestore(&dd->ipath_sdma_lock, flags);
 done:
-	return;
+	;
 }
 
 /*
@@ -633,7 +633,7 @@ void ipath_restart_sdma(struct ipath_dev
 	ipath_ib_piobufavail(dd->verbs_dev);
 
 bail:
-	return;
+	;
 }
 
 static inline void make_sdma_desc(struct ipath_devdata *dd,
diff -u -p a/drivers/infiniband/hw/ipath/ipath_uc.c b/drivers/infiniband/hw/ipath/ipath_uc.c
--- a/drivers/infiniband/hw/ipath/ipath_uc.c
+++ b/drivers/infiniband/hw/ipath/ipath_uc.c
@@ -543,5 +543,5 @@ void ipath_uc_rcv(struct ipath_ibdev *de
 	qp->r_psn++;
 	qp->r_state = opcode;
 done:
-	return;
+	;
 }
diff -u -p a/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c b/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c
--- a/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c
+++ b/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c
@@ -2969,7 +2969,7 @@ static void build_kernel_pbes(struct ib_
 		}
 	}
 mr_tbl_done:
-	return;
+	;
 }
 
 struct ib_mr *ocrdma_reg_kernel_mr(struct ib_pd *ibpd,
diff -u -p a/drivers/infiniband/hw/qib/qib_iba6120.c b/drivers/infiniband/hw/qib/qib_iba6120.c
--- a/drivers/infiniband/hw/qib/qib_iba6120.c
+++ b/drivers/infiniband/hw/qib/qib_iba6120.c
@@ -1122,7 +1122,7 @@ static void handle_6120_errors(struct qi
 			qib_stats.sps_hdrfull++;
 	}
 done:
-	return;
+	;
 }
 
 /**
diff -u -p a/drivers/infiniband/hw/qib/qib_iba7220.c b/drivers/infiniband/hw/qib/qib_iba7220.c
--- a/drivers/infiniband/hw/qib/qib_iba7220.c
+++ b/drivers/infiniband/hw/qib/qib_iba7220.c
@@ -1221,7 +1221,7 @@ static void handle_7220_errors(struct qi
 			qib_stats.sps_hdrfull++;
 	}
 done:
-	return;
+	;
 }
 
 /* enable/disable chip from delivering interrupts */
diff -u -p a/drivers/infiniband/hw/qib/qib_iba7322.c b/drivers/infiniband/hw/qib/qib_iba7322.c
--- a/drivers/infiniband/hw/qib/qib_iba7322.c
+++ b/drivers/infiniband/hw/qib/qib_iba7322.c
@@ -1741,7 +1741,7 @@ static noinline void handle_7322_errors(
 	}
 
 done:
-	return;
+	;
 }
 
 static void qib_error_tasklet(unsigned long data)
@@ -2016,7 +2016,7 @@ static noinline void handle_7322_p_error
 	if (ppd->state_wanted & ppd->lflags)
 		wake_up_interruptible(&ppd->state_wait);
 done:
-	return;
+	;
 }
 
 /* enable/disable chip from delivering interrupts */
diff -u -p a/drivers/infiniband/hw/qib/qib_rc.c b/drivers/infiniband/hw/qib/qib_rc.c
--- a/drivers/infiniband/hw/qib/qib_rc.c
+++ b/drivers/infiniband/hw/qib/qib_rc.c
@@ -768,7 +768,7 @@ queue_ack:
 unlock:
 	spin_unlock_irqrestore(&qp->s_lock, flags);
 done:
-	return;
+	;
 }
 
 /**
@@ -1601,7 +1601,7 @@ ack_err:
 ack_done:
 	spin_unlock_irqrestore(&qp->s_lock, flags);
 bail:
-	return;
+	;
 }
 
 /**
diff -u -p a/drivers/infiniband/hw/qib/qib_sd7220.c b/drivers/infiniband/hw/qib/qib_sd7220.c
--- a/drivers/infiniband/hw/qib/qib_sd7220.c
+++ b/drivers/infiniband/hw/qib/qib_sd7220.c
@@ -160,7 +160,7 @@ void qib_sd7220_clr_ibpar(struct qib_dev
 		QLOGIC_IB_HWE_IB_UC_MEMORYPARITYERR);
 	qib_read_kreg32(dd, kr_scratch);
 bail:
-	return;
+	;
 }
 
 /*
diff -u -p a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -1926,7 +1926,7 @@ static void srpt_handle_new_iu(struct sr
 
 	srpt_post_recv(ch->sport->sdev, recv_ioctx);
 out:
-	return;
+	;
 }
 
 static void srpt_process_rcv_completion(struct ib_cq *cq,
diff -u -p a/drivers/mfd/88pm860x-core.c b/drivers/mfd/88pm860x-core.c
--- a/drivers/mfd/88pm860x-core.c
+++ b/drivers/mfd/88pm860x-core.c
@@ -1037,7 +1037,7 @@ static void device_8607_init(struct pm86
 	device_power_init(chip, pdata);
 	device_codec_init(chip, pdata);
 out:
-	return;
+	;
 }
 
 static void device_8606_init(struct pm860x_chip *chip,
diff -u -p a/drivers/mfd/ab3100-core.c b/drivers/mfd/ab3100-core.c
--- a/drivers/mfd/ab3100-core.c
+++ b/drivers/mfd/ab3100-core.c
@@ -624,7 +624,7 @@ static void ab3100_setup_debugfs(struct
  exit_destroy_dir:
 	debugfs_remove(ab3100_dir);
  exit_no_debugfs:
-	return;
+	;
 }
 static inline void ab3100_remove_debugfs(void)
 {
diff -u -p a/drivers/mtd/maps/vmu-flash.c b/drivers/mtd/maps/vmu-flash.c
--- a/drivers/mtd/maps/vmu-flash.c
+++ b/drivers/mtd/maps/vmu-flash.c
@@ -592,7 +592,7 @@ fail_mpart:
 		MAPLE_FUNC_MEMCARD);
 	kfree(part_cur->name);
 fail_name:
-	return;
+	;
 }
 
 /* Handles very basic info about the flash, queries for details */
diff -u -p a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
--- a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
@@ -1071,7 +1071,7 @@ void gpmi_begin(struct gpmi_nand_data *t
 	udelay(dll_wait_time_in_us);
 
 err_out:
-	return;
+	;
 }
 
 void gpmi_end(struct gpmi_nand_data *this)
diff -u -p a/drivers/mtd/sm_ftl.c b/drivers/mtd/sm_ftl.c
--- a/drivers/mtd/sm_ftl.c
+++ b/drivers/mtd/sm_ftl.c
@@ -1221,7 +1221,7 @@ error3:
 error2:
 	kfree(ftl);
 error1:
-	return;
+	;
 }
 
 /* main interface: device {surprise,} removal */
diff -u -p a/drivers/net/ethernet/atheros/atl1e/atl1e_main.c b/drivers/net/ethernet/atheros/atl1e/atl1e_main.c
--- a/drivers/net/ethernet/atheros/atl1e/atl1e_main.c
+++ b/drivers/net/ethernet/atheros/atl1e/atl1e_main.c
@@ -1396,7 +1396,7 @@ static inline void atl1e_rx_checksum(str
 	}
 
 hw_xsum :
-	return;
+	;
 }
 
 static struct atl1e_rx_page *atl1e_get_rx_page(struct atl1e_adapter *adapter,
diff -u -p a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -9815,7 +9815,7 @@ static void bnx2x_period_task(struct wor
 
 	bnx2x_release_phy_lock(bp);
 period_task_exit:
-	return;
+	;
 }
 
 /*
diff -u -p a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c
@@ -352,7 +352,7 @@ op_err:
 op_done:
 	bnx2x_vfop_end(bp, vf, vfop);
 op_pending:
-	return;
+	;
 }
 
 static int bnx2x_vfop_qctor_cmd(struct bnx2x *bp,
@@ -447,7 +447,7 @@ op_done:
 		bnx2x_vfop_default(state);
 	}
 op_pending:
-	return;
+	;
 }
 
 static int bnx2x_vfop_qdtor_cmd(struct bnx2x *bp,
@@ -670,7 +670,7 @@ op_done:
 	bnx2x_vfop_credit(bp, vfop, obj);
 	bnx2x_vfop_end(bp, vf, vfop);
 op_pending:
-	return;
+	;
 }
 
 struct bnx2x_vfop_vlan_mac_flags {
@@ -1083,7 +1083,7 @@ op_done:
 		bnx2x_vfop_default(state);
 	}
 op_pending:
-	return;
+	;
 }
 
 static int bnx2x_vfop_qflr_cmd(struct bnx2x *bp,
@@ -1159,7 +1159,7 @@ op_done:
 	kfree(args->mc);
 	bnx2x_vfop_end(bp, vf, vfop);
 op_pending:
-	return;
+	;
 }
 
 int bnx2x_vfop_mcast_cmd(struct bnx2x *bp,
@@ -1240,7 +1240,7 @@ op_done:
 		bnx2x_vfop_default(state);
 	}
 op_pending:
-	return;
+	;
 }
 
 static void bnx2x_vf_prep_rx_mode(struct bnx2x *bp, u8 qid,
@@ -2912,7 +2912,7 @@ op_done:
 	bnx2x_vfop_end(bp, vf, vfop);
 op_pending:
 	/* Not supported at the moment; Exists for macros only */
-	return;
+	;
 }
 
 int bnx2x_vfop_close_cmd(struct bnx2x *bp,
@@ -3014,7 +3014,7 @@ op_done:
 		bnx2x_vfop_default(state);
 	}
 op_pending:
-	return;
+	;
 }
 
 int bnx2x_vfop_release_cmd(struct bnx2x *bp,
diff -u -p a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c
@@ -2013,7 +2013,7 @@ void bnx2x_vf_mbx(struct bnx2x *bp, stru
 mbx_error:
 	bnx2x_vf_release(bp, vf, false); /* non blocking */
 mbx_done:
-	return;
+	;
 }
 
 /* propagate local bulletin board to vf */
diff -u -p a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -1216,7 +1216,7 @@ static void be_set_rx_mode(struct net_de
 		be_cmd_rx_filter(adapter, IFF_ALLMULTI, ON);
 	}
 done:
-	return;
+	;
 }
 
 static int be_set_vf_mac(struct net_device *netdev, int vf, u8 *mac)
diff -u -p a/drivers/net/ethernet/ibm/ehea/ehea_main.c b/drivers/net/ethernet/ibm/ehea/ehea_main.c
--- a/drivers/net/ethernet/ibm/ehea/ehea_main.c
+++ b/drivers/net/ethernet/ibm/ehea/ehea_main.c
@@ -2820,7 +2820,7 @@ static void ehea_rereg_mrs(void)
 		}
 	pr_info("re-initializing driver complete\n");
 out:
-	return;
+	;
 }
 
 static void ehea_tx_watchdog(struct net_device *dev)
diff -u -p a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c
--- a/drivers/net/ethernet/intel/e1000/e1000_main.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
@@ -487,7 +487,7 @@ static void e1000_power_down_phy(struct
 		msleep(1);
 	}
 out:
-	return;
+	;
 }
 
 static void e1000_down_and_stop(struct e1000_adapter *adapter)
@@ -3457,7 +3457,7 @@ rx_ring_summary:
 			readl(adapter->hw.hw_addr + i+8));
 	}
 exit:
-	return;
+	;
 }
 
 /**
diff -u -p a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -569,7 +569,7 @@ rx_ring_summary:
 	}
 
 exit:
-	return;
+	;
 }
 
 /**
diff -u -p a/drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c
@@ -655,7 +655,7 @@ static void ixgbe_set_fiber_fixed_speed(
 		goto out;
 	}
 out:
-	return;
+	;
 }
 
 /**
diff -u -p a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -705,7 +705,7 @@ rx_ring_summary:
 	}
 
 exit:
-	return;
+	;
 }
 
 static void ixgbe_release_hw_control(struct ixgbe_adapter *adapter)
diff -u -p a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
--- a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
+++ b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
@@ -3187,7 +3187,7 @@ static void myri10ge_set_multicast_list(
 	return;
 
 abort:
-	return;
+	;
 }
 
 static int myri10ge_set_mac_address(struct net_device *dev, void *addr)
diff -u -p a/drivers/net/ethernet/neterion/vxge/vxge-config.c b/drivers/net/ethernet/neterion/vxge/vxge-config.c
--- a/drivers/net/ethernet/neterion/vxge/vxge-config.c
+++ b/drivers/net/ethernet/neterion/vxge/vxge-config.c
@@ -1126,7 +1126,7 @@ static void __vxge_hw_blockpool_destroy(
 	}
 	ret = 0;
 exit:
-	return;
+	;
 }
 
 /*
@@ -2311,7 +2311,7 @@ static void vxge_hw_blockpool_block_add(
 
 	req_out = blockpool->req_out;
 exit:
-	return;
+	;
 }
 
 static inline void
@@ -4651,7 +4651,7 @@ static void __vxge_hw_vp_terminate(struc
 	vpath->sw_stats = NULL;
 
 exit:
-	return;
+	;
 }
 
 /*
diff -u -p a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -2265,7 +2265,7 @@ static void rtl_work_func_t(struct work_
 		_rtl8152_set_rx_mode(tp->netdev);
 
 out1:
-	return;
+	;
 }
 
 static int rtl8152_open(struct net_device *netdev)
diff -u -p a/drivers/net/wimax/i2400m/rx.c b/drivers/net/wimax/i2400m/rx.c
--- a/drivers/net/wimax/i2400m/rx.c
+++ b/drivers/net/wimax/i2400m/rx.c
@@ -409,7 +409,7 @@ void i2400m_rx_ctl(struct i2400m *i2400m
 	} else		/* an ack to a CMD, GET or SET */
 		i2400m_rx_ctl_ack(i2400m, payload, size);
 error_check:
-	return;
+	;
 }
 
 
@@ -456,7 +456,7 @@ void i2400m_rx_trace(struct i2400m *i240
 		dev_err(dev, "error sending trace to userspace: %d\n",
 			result);
 error_check:
-	return;
+	;
 }
 
 
diff -u -p a/drivers/net/wireless/ath/ar5523/ar5523.c b/drivers/net/wireless/ath/ar5523/ar5523.c
--- a/drivers/net/wireless/ath/ar5523/ar5523.c
+++ b/drivers/net/wireless/ath/ar5523/ar5523.c
@@ -660,7 +660,7 @@ static void ar5523_rx_refill_work(struct
 
 	} while (true);
 done:
-	return;
+	;
 }
 
 static void ar5523_cancel_rx_bufs(struct ar5523 *ar)
diff -u -p a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -675,7 +675,7 @@ static void wcn36xx_bss_info_changed(str
 		}
 	}
 out:
-	return;
+	;
 }
 
 /* this is required when using IEEE80211_HW_HAS_RATE_CONTROL */
diff -u -p a/drivers/net/wireless/b43legacy/dma.c b/drivers/net/wireless/b43legacy/dma.c
--- a/drivers/net/wireless/b43legacy/dma.c
+++ b/drivers/net/wireless/b43legacy/dma.c
@@ -1397,7 +1397,7 @@ static void dma_rx(struct b43legacy_dmar
 
 	b43legacy_rx(ring->dev, skb, rxhdr);
 drop:
-	return;
+	;
 }
 
 void b43legacy_dma_rx(struct b43legacy_dmaring *ring)
diff -u -p a/drivers/net/wireless/cw1200/txrx.c b/drivers/net/wireless/cw1200/txrx.c
--- a/drivers/net/wireless/cw1200/txrx.c
+++ b/drivers/net/wireless/cw1200/txrx.c
@@ -1223,7 +1223,7 @@ void cw1200_rx_cb(struct cw1200_common *
 
 drop:
 	/* TODO: update failure counters */
-	return;
+	;
 }
 
 /* ******************************************************************** */
diff -u -p a/drivers/net/wireless/iwlegacy/common.c b/drivers/net/wireless/iwlegacy/common.c
--- a/drivers/net/wireless/iwlegacy/common.c
+++ b/drivers/net/wireless/iwlegacy/common.c
@@ -1871,7 +1871,7 @@ il_set_ht_add_station(struct il_priv *il
 
 	il->stations[idx].sta.station_flags = sta_flags;
 done:
-	return;
+	;
 }
 
 /**
diff -u -p a/drivers/net/wireless/libertas/debugfs.c b/drivers/net/wireless/libertas/debugfs.c
--- a/drivers/net/wireless/libertas/debugfs.c
+++ b/drivers/net/wireless/libertas/debugfs.c
@@ -796,7 +796,7 @@ void lbs_debugfs_init_one(struct lbs_pri
 	lbs_debug_init(priv);
 #endif
 exit:
-	return;
+	;
 }
 
 void lbs_debugfs_remove_one(struct lbs_private *priv)
diff -u -p a/drivers/net/wireless/orinoco/orinoco_usb.c b/drivers/net/wireless/orinoco/orinoco_usb.c
--- a/drivers/net/wireless/orinoco/orinoco_usb.c
+++ b/drivers/net/wireless/orinoco/orinoco_usb.c
@@ -510,7 +510,7 @@ static void ezusb_req_queue_run(struct e
 	spin_unlock_irqrestore(&upriv->req_lock, flags);
 
  done:
-	return;
+	;
 }
 
 static void ezusb_req_enqueue_run(struct ezusb_priv *upriv,
@@ -532,7 +532,7 @@ static void ezusb_req_enqueue_run(struct
 	ezusb_req_queue_run(upriv);
 
  done:
-	return;
+	;
 }
 
 static void ezusb_request_out_callback(struct urb *urb)
diff -u -p a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -4103,7 +4103,7 @@ static void wl1271_bss_info_changed_ap(s
 	}
 
 out:
-	return;
+	;
 }
 
 static int wlcore_set_bssid(struct wl1271 *wl, struct wl12xx_vif *wlvif,
@@ -4418,7 +4418,7 @@ static void wl1271_bss_info_changed_sta(
 	}
 
 out:
-	return;
+	;
 }
 
 static void wl1271_op_bss_info_changed(struct ieee80211_hw *hw,
diff -u -p a/drivers/net/wireless/wl3501_cs.c b/drivers/net/wireless/wl3501_cs.c
--- a/drivers/net/wireless/wl3501_cs.c
+++ b/drivers/net/wireless/wl3501_cs.c
@@ -806,7 +806,7 @@ static void wl3501_esbq_req_free(struct
 	if (this->esbq_req_tail >= this->esbq_req_end)
 		this->esbq_req_tail = this->esbq_req_start;
 out:
-	return;
+	;
 }
 
 static int wl3501_esbq_confirm(struct wl3501_card *this)
diff -u -p a/drivers/power/88pm860x_battery.c b/drivers/power/88pm860x_battery.c
--- a/drivers/power/88pm860x_battery.c
+++ b/drivers/power/88pm860x_battery.c
@@ -527,7 +527,7 @@ static void pm860x_init_battery(struct p
 	info->last_capacity = info->start_soc;
 	dev_dbg(info->dev, "init soc : %d\n", info->last_capacity);
 out:
-	return;
+	;
 }
 
 static void set_temp_threshold(struct pm860x_battery_info *info,
diff -u -p a/drivers/s390/net/ctcm_fsms.c b/drivers/s390/net/ctcm_fsms.c
--- a/drivers/s390/net/ctcm_fsms.c
+++ b/drivers/s390/net/ctcm_fsms.c
@@ -1054,7 +1054,7 @@ static void ctcm_chx_txretry(fsm_instanc
 		}
 	}
 done:
-	return;
+	;
 }
 
 /**
@@ -1590,7 +1590,7 @@ void ctcmpc_chx_rxidle(fsm_instance *fi,
 
 	fsm_event(priv->fsm, DEV_EVENT_RXUP, dev);
 done:
-	return;
+	;
 }
 
 /*
@@ -1734,7 +1734,7 @@ static void ctcmpc_chx_attnbusy(fsm_inst
 				CTCM_FUNTAIL, dev->name, ch->id);
 
 done:
-	return;
+	;
 }
 
 /*
@@ -1847,7 +1847,7 @@ static void ctcmpc_chx_send_sweep(fsm_in
 		ctcm_ccw_check_rc(wch, rc, "send sweep");
 
 done:
-	return;
+	;
 }
 
 
diff -u -p a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -1177,7 +1177,7 @@ static void qeth_notify_skbs(struct qeth
 			skb = skb_queue_next(&buf->skb_list, skb);
 	}
 out:
-	return;
+	;
 }
 
 static void qeth_release_skbs(struct qeth_qdio_out_buffer *buf)
@@ -3576,7 +3576,7 @@ static void qeth_qdio_cq_handler(struct
 		card->perf_stats.cq_time += delta_t;
 	}
 out:
-	return;
+	;
 }
 
 void qeth_qdio_input_handler(struct ccw_device *ccwdev, unsigned int qdio_err,
@@ -4709,7 +4709,7 @@ out_offline:
 	if (ddev_offline == 1)
 		ccw_device_set_offline(ddev);
 out:
-	return;
+	;
 }
 
 static inline void qeth_qdio_establish_cq(struct qeth_card *card,
diff -u -p a/drivers/scsi/bfa/bfad_debugfs.c b/drivers/scsi/bfa/bfad_debugfs.c
--- a/drivers/scsi/bfa/bfad_debugfs.c
+++ b/drivers/scsi/bfa/bfad_debugfs.c
@@ -519,7 +519,7 @@ bfad_debugfs_init(struct bfad_port_s *po
 	}
 
 err:
-	return;
+	;
 }
 
 inline void
diff -u -p a/drivers/scsi/csiostor/csio_hw.c b/drivers/scsi/csiostor/csio_hw.c
--- a/drivers/scsi/csiostor/csio_hw.c
+++ b/drivers/scsi/csiostor/csio_hw.c
@@ -2148,7 +2148,7 @@ csio_hw_configure(struct csio_hw *hw)
 	} /* if not master */
 
 out:
-	return;
+	;
 }
 
 /*
@@ -2218,7 +2218,7 @@ csio_hw_initialize(struct csio_hw *hw)
 free_and_out:
 	mempool_free(mbp, hw->mb_mempool);
 out:
-	return;
+	;
 }
 
 #define PF_INTR_MASK (PFSW | PFCIM)
diff -u -p a/drivers/scsi/fnic/fnic_fcs.c b/drivers/scsi/fnic/fnic_fcs.c
--- a/drivers/scsi/fnic/fnic_fcs.c
+++ b/drivers/scsi/fnic/fnic_fcs.c
@@ -424,7 +424,7 @@ static void fnic_fcoe_process_vlan_resp(
 	sol_time = jiffies + msecs_to_jiffies(FCOE_CTLR_START_DELAY);
 	mod_timer(&fnic->fip_timer, round_jiffies(sol_time));
 out:
-	return;
+	;
 }
 
 static void fnic_fcoe_start_fcf_disc(struct fnic *fnic)
diff -u -p a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -1179,7 +1179,7 @@ force:
 	qla25xx_write_risc_sema_reg(vha, RISC_SEMAPHORE_FORCE_SET);
 
 acquired:
-	return;
+	;
 }
 
 /**
diff -u -p a/drivers/scsi/qla2xxx/qla_nx2.c b/drivers/scsi/qla2xxx/qla_nx2.c
--- a/drivers/scsi/qla2xxx/qla_nx2.c
+++ b/drivers/scsi/qla2xxx/qla_nx2.c
@@ -1497,7 +1497,7 @@ exit_read_template_error:
 	vfree(vha->reset_tmplt.buff);
 
 exit_read_reset_template:
-	return;
+	;
 }
 
 void
diff -u -p a/drivers/scsi/qla4xxx/ql4_83xx.c b/drivers/scsi/qla4xxx/ql4_83xx.c
--- a/drivers/scsi/qla4xxx/ql4_83xx.c
+++ b/drivers/scsi/qla4xxx/ql4_83xx.c
@@ -962,7 +962,7 @@ exit_read_template_error:
 	vfree(ha->reset_tmplt.buff);
 
 exit_read_reset_template:
-	return;
+	;
 }
 
 /**
diff -u -p a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
--- a/drivers/scsi/qla4xxx/ql4_os.c
+++ b/drivers/scsi/qla4xxx/ql4_os.c
@@ -1819,7 +1819,7 @@ free_stats:
 	dma_free_coherent(&ha->pdev->dev, stats_size, ql_iscsi_stats,
 			  iscsi_stats_dma);
 exit_get_stats:
-	return;
+	;
 }
 
 static enum blk_eh_timer_return qla4xxx_eh_cmd_timed_out(struct scsi_cmnd *sc)
diff -u -p a/drivers/scsi/sym53c8xx_2/sym_hipd.c b/drivers/scsi/sym53c8xx_2/sym_hipd.c
--- a/drivers/scsi/sym53c8xx_2/sym_hipd.c
+++ b/drivers/scsi/sym53c8xx_2/sym_hipd.c
@@ -4637,7 +4637,7 @@ out_clrack:
 	OUTL_DSP(np, SCRIPTA_BA(np, clrack));
 	return;
 out_stuck:
-	return;
+	;
 }
 
 /*
@@ -5090,7 +5090,7 @@ static void sym_alloc_lcb_tags (struct s
 
 	return;
 fail:
-	return;
+	;
 }
 
 /*
diff -u -p a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -2740,7 +2740,7 @@ static void ufshcd_async_scan(void *data
 	scsi_scan_host(hba->host);
 	pm_runtime_put_sync(hba->dev);
 out:
-	return;
+	;
 }
 
 static struct scsi_host_template ufshcd_driver_template = {
diff -u -p a/drivers/staging/lustre/lnet/selftest/brw_test.c b/drivers/staging/lustre/lnet/selftest/brw_test.c
--- a/drivers/staging/lustre/lnet/selftest/brw_test.c
+++ b/drivers/staging/lustre/lnet/selftest/brw_test.c
@@ -355,7 +355,7 @@ brw_client_done_rpc(sfw_test_unit_t *tsu
 	}
 
 out:
-	return;
+	;
 }
 
 void
diff -u -p a/drivers/staging/lustre/lustre/llite/llite_close.c b/drivers/staging/lustre/lustre/llite/llite_close.c
--- a/drivers/staging/lustre/lustre/llite/llite_close.c
+++ b/drivers/staging/lustre/lustre/llite/llite_close.c
@@ -201,7 +201,7 @@ void ll_ioepoch_close(struct inode *inod
 	ll_done_writing_attr(inode, op_data);
 
 out:
-	return;
+	;
 }
 
 /**
diff -u -p a/drivers/staging/lustre/lustre/llite/llite_close.c b/drivers/staging/lustre/lustre/llite/llite_close.c
--- a/drivers/staging/lustre/lustre/llite/llite_close.c
+++ b/drivers/staging/lustre/lustre/llite/llite_close.c
@@ -201,7 +201,7 @@ void ll_ioepoch_close(struct inode *inod
 	ll_done_writing_attr(inode, op_data);
 
 out:
-	return;
+	;
 }
 
 /**
(sfw_test_unit_t *tsu
 	}
 
 out:
-	return;
+	;
 }
 
 void
diff -u -p a/drivers/staging/octeon-usb/octeon-hcd.c b/drivers/staging/octeon-usb/octeon-hcd.c
--- a/drivers/staging/octeon-usb/octeon-hcd.c
+++ b/drivers/staging/octeon-usb/octeon-hcd.c
@@ -2041,7 +2041,7 @@ static void __cvmx_usb_perform_complete(
 					 transaction->urb);
 	kfree(transaction);
 done:
-	return;
+	;
 }
 
 
diff -u -p a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
--- a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
@@ -7700,7 +7700,7 @@ void survey_timer_hdl(struct adapter *pa
 
 
 exit_survey_timer_hdl:
-	return;
+	;
 }
 
 void link_timer_hdl(struct adapter *padapter)
diff -u -p a/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c b/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c
--- a/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c
+++ b/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c
@@ -199,7 +199,7 @@ void rtl8188e_HalDmWatchDog(struct adapt
 skip_dm:
 	/*  Check GPIO to determine current RF on/off and Pbc status. */
 	/*  Check Hardware Radio ON/OFF or not */
-	return;
+	;
 }
 
 void rtl8188e_init_dm_priv(struct adapter *Adapter)
diff -u -p a/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c b/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c
--- a/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c
+++ b/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c
@@ -955,7 +955,7 @@ static void ReadEFuseByIC(struct adapter
 	Hal_EfuseReadEFuse88E(Adapter, _offset, _size_byte, pbuf, bPseudoTest);
 
 exit:
-	return;
+	;
 }
 
 static void ReadEFuse_Pseudo(struct adapter *Adapter, u8 efuseType, u16 _offset, u16 _size_byte, u8 *pbuf, bool bPseudoTest)
diff -u -p a/drivers/staging/rtl8188eu/os_dep/osdep_service.c b/drivers/staging/rtl8188eu/os_dep/osdep_service.c
--- a/drivers/staging/rtl8188eu/os_dep/osdep_service.c
+++ b/drivers/staging/rtl8188eu/os_dep/osdep_service.c
@@ -249,7 +249,7 @@ void rtw_free_netdev(struct net_device *
 	free_netdev(netdev);
 
 RETURN:
-	return;
+	;
 }
 
 int rtw_change_ifname(struct adapter *padapter, const char *ifname)
diff -u -p a/drivers/staging/rtl8712/rtl8712_cmd.c b/drivers/staging/rtl8712/rtl8712_cmd.c
--- a/drivers/staging/rtl8712/rtl8712_cmd.c
+++ b/drivers/staging/rtl8712/rtl8712_cmd.c
@@ -475,5 +475,5 @@ void r8712_event_handle(struct _adapter
 		event_callback(padapter, (u8 *)peventbuf);
 	pevt_priv->evt_done_cnt++;
 _abort_event_:
-	return;
+	;
 }
diff -u -p a/drivers/staging/rts5139/rts51x_transport.c b/drivers/staging/rts5139/rts51x_transport.c
--- a/drivers/staging/rts5139/rts51x_transport.c
+++ b/drivers/staging/rts5139/rts51x_transport.c
@@ -732,5 +732,5 @@ void rts51x_invoke_transport(struct scsi
 	 * by issuing a port reset.  If that fails, try a class-specific
 	 * device reset. */
 Handle_Errors:
-	return;
+	;
 }
diff -u -p a/drivers/staging/tidspbridge/core/io_sm.c b/drivers/staging/tidspbridge/core/io_sm.c
--- a/drivers/staging/tidspbridge/core/io_sm.c
+++ b/drivers/staging/tidspbridge/core/io_sm.c
@@ -788,7 +788,7 @@ void io_cancel_chnl(struct io_mgr *hio_m
 
 	sm_interrupt_dsp(pio_mgr->bridge_context, MBX_PCPY_CLASS);
 func_end:
-	return;
+	;
 }
 
 
@@ -899,7 +899,7 @@ void io_dpc(unsigned long ref_data)
 	} while (serviced != requested);
 	pio_mgr->dpc_sched = requested;
 func_end:
-	return;
+	;
 }
 
 /*
@@ -964,7 +964,7 @@ void io_request_chnl(struct io_mgr *io_m
 	} else {
 	}
 func_end:
-	return;
+	;
 }
 
 /*
@@ -1136,7 +1136,7 @@ static void input_chnl(struct io_mgr *pi
 		notify_chnl_complete(pchnl, chnl_packet_obj);
 	}
 func_end:
-	return;
+	;
 }
 
 /*
@@ -1271,7 +1271,7 @@ static void notify_chnl_complete(struct
 	/* Notify that IO is complete */
 	ntfy_notify(pchnl->ntfy_obj, DSP_STREAMIOCOMPLETION);
 func_end:
-	return;
+	;
 }
 
 /*
@@ -1355,7 +1355,7 @@ static void output_chnl(struct io_mgr *p
 		ntfy_notify(pchnl->ntfy_obj, DSP_STREAMDONE);
 
 func_end:
-	return;
+	;
 }
 
 /*
diff -u -p a/drivers/staging/tidspbridge/rmgr/node.c b/drivers/staging/tidspbridge/rmgr/node.c
--- a/drivers/staging/tidspbridge/rmgr/node.c
+++ b/drivers/staging/tidspbridge/rmgr/node.c
@@ -2473,7 +2473,7 @@ static void delete_node(struct node_obje
 	kfree(hnode);
 	hnode = NULL;
 func_end:
-	return;
+	;
 }
 
 /*
diff -u -p a/drivers/staging/wlan-ng/hfa384x_usb.c b/drivers/staging/wlan-ng/hfa384x_usb.c
--- a/drivers/staging/wlan-ng/hfa384x_usb.c
+++ b/drivers/staging/wlan-ng/hfa384x_usb.c
@@ -3541,7 +3541,7 @@ static void hfa384x_usbin_rx(wlandevice_
 	}
 
 done:
-	return;
+	;
 }
 
 /*----------------------------------------------------------------
diff -u -p a/drivers/tty/amiserial.c b/drivers/tty/amiserial.c
--- a/drivers/tty/amiserial.c
+++ b/drivers/tty/amiserial.c
@@ -332,7 +332,7 @@ static void receive_chars(struct serial_
 		tty_insert_flip_char(&info->tport, 0, TTY_OVERRUN);
 	tty_flip_buffer_push(&info->tport);
 out:
-	return;
+	;
 }
 
 static void transmit_chars(struct serial_state *info)
diff -u -p a/drivers/tty/nozomi.c b/drivers/tty/nozomi.c
--- a/drivers/tty/nozomi.c
+++ b/drivers/tty/nozomi.c
@@ -470,7 +470,7 @@ static void read_mem32(u32 *buf, const v
 		ptr++;
 	}
 out:
-	return;
+	;
 }
 
 /*
diff -u -p a/drivers/tty/serial/68328serial.c b/drivers/tty/serial/68328serial.c
--- a/drivers/tty/serial/68328serial.c
+++ b/drivers/tty/serial/68328serial.c
@@ -338,7 +338,7 @@ static void transmit_chars(struct m68k_s
 
 clear_and_return:
 	/* Clear interrupt (should be auto)*/
-	return;
+	;
 }
 
 /*
diff -u -p a/drivers/usb/gadget/mv_udc_core.c b/drivers/usb/gadget/mv_udc_core.c
--- a/drivers/usb/gadget/mv_udc_core.c
+++ b/drivers/usb/gadget/mv_udc_core.c
@@ -1589,7 +1589,7 @@ static void ch9clearfeature(struct mv_ud
 	if (udc_prime_status(udc, EP_DIR_IN, 0, true))
 		ep0_stall(udc);
 out:
-	return;
+	;
 }
 
 static void ch9setfeature(struct mv_udc *udc, struct usb_ctrlrequest *setup)
@@ -1641,7 +1641,7 @@ static void ch9setfeature(struct mv_udc
 	if (udc_prime_status(udc, EP_DIR_IN, 0, true))
 		ep0_stall(udc);
 out:
-	return;
+	;
 }
 
 static void handle_setup_packet(struct mv_udc *udc, u8 ep_num,
diff -u -p a/drivers/usb/mon/mon_main.c b/drivers/usb/mon/mon_main.c
--- a/drivers/usb/mon/mon_main.c
+++ b/drivers/usb/mon/mon_main.c
@@ -302,7 +302,7 @@ static void mon_bus_init(struct usb_bus
 	return;
 
 err_alloc:
-	return;
+	;
 }
 
 static void mon_bus0_init(void)
diff -u -p a/drivers/usb/wusbcore/wa-nep.c b/drivers/usb/wusbcore/wa-nep.c
--- a/drivers/usb/wusbcore/wa-nep.c
+++ b/drivers/usb/wusbcore/wa-nep.c
@@ -251,7 +251,7 @@ static void wa_nep_cb(struct urb *urb)
 		wa_reset_all(wa);
 	}
 out:
-	return;
+	;
 }
 
 /*
diff -u -p a/drivers/usb/wusbcore/wa-xfer.c b/drivers/usb/wusbcore/wa-xfer.c
--- a/drivers/usb/wusbcore/wa-xfer.c
+++ b/drivers/usb/wusbcore/wa-xfer.c
@@ -2610,7 +2610,7 @@ static void wa_dti_cb(struct urb *urb)
 		}
 	}
 out:
-	return;
+	;
 }
 
 /*
diff -u -p a/drivers/video/aty/aty128fb.c b/drivers/video/aty/aty128fb.c
--- a/drivers/video/aty/aty128fb.c
+++ b/drivers/video/aty/aty128fb.c
@@ -1863,7 +1863,7 @@ static void aty128_bl_init(struct aty128
 	return;
 
 error:
-	return;
+	;
 }
 
 static void aty128_bl_exit(struct backlight_device *bd)
diff -u -p a/drivers/video/aty/atyfb_base.c b/drivers/video/aty/atyfb_base.c
--- a/drivers/video/aty/atyfb_base.c
+++ b/drivers/video/aty/atyfb_base.c
@@ -2260,7 +2260,7 @@ static void aty_bl_init(struct atyfb_par
 	return;
 
 error:
-	return;
+	;
 }
 
 #ifdef CONFIG_PCI
diff -u -p a/drivers/video/nvidia/nv_backlight.c b/drivers/video/nvidia/nv_backlight.c
--- a/drivers/video/nvidia/nv_backlight.c
+++ b/drivers/video/nvidia/nv_backlight.c
@@ -135,7 +135,7 @@ void nvidia_bl_init(struct nvidia_par *p
 	return;
 
 error:
-	return;
+	;
 }
 
 void nvidia_bl_exit(struct nvidia_par *par)
diff -u -p a/drivers/video/riva/fbdev.c b/drivers/video/riva/fbdev.c
--- a/drivers/video/riva/fbdev.c
+++ b/drivers/video/riva/fbdev.c
@@ -379,7 +379,7 @@ static void riva_bl_init(struct riva_par
 	return;
 
 error:
-	return;
+	;
 }
 
 static void riva_bl_exit(struct fb_info *info)
diff -u -p a/fs/buffer.c b/fs/buffer.c
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -1557,7 +1557,7 @@ void block_invalidatepage(struct page *p
 	if (offset == 0)
 		try_to_release_page(page, 0);
 out:
-	return;
+	;
 }
 EXPORT_SYMBOL(block_invalidatepage);
 
diff -u -p a/fs/coredump.c b/fs/coredump.c
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -676,7 +676,7 @@ fail_unlock:
 fail_creds:
 	put_cred(cred);
 fail:
-	return;
+	;
 }
 
 /*
diff -u -p a/fs/dlm/recover.c b/fs/dlm/recover.c
--- a/fs/dlm/recover.c
+++ b/fs/dlm/recover.c
@@ -808,7 +808,7 @@ static void recover_lvb(struct dlm_rsb *
 		memset(r->res_lvbptr, 0, lvblen);
 	}
  out:
-	return;
+	;
 }
 
 /* All master rsb's flagged RECOVER_CONVERT need to be looked at.  The locks
diff -u -p a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c
--- a/fs/ecryptfs/crypto.c
+++ b/fs/ecryptfs/crypto.c
@@ -1860,7 +1860,7 @@ static void ecryptfs_encode_for_filename
 		block_num++;
 	}
 out:
-	return;
+	;
 }
 
 static size_t ecryptfs_max_decoded_size(size_t encoded_size)
@@ -1925,7 +1925,7 @@ ecryptfs_decode_from_filename(unsigned c
 	}
 	(*dst_size) = dst_byte_offset;
 out:
-	return;
+	;
 }
 
 /**
diff -u -p a/fs/ext3/inode.c b/fs/ext3/inode.c
--- a/fs/ext3/inode.c
+++ b/fs/ext3/inode.c
@@ -3525,7 +3525,7 @@ void ext3_dirty_inode(struct inode *inod
 	}
 	ext3_journal_stop(handle);
 out:
-	return;
+	;
 }
 
 #if 0
diff -u -p a/fs/ext4/inode.c b/fs/ext4/inode.c
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -4959,7 +4959,7 @@ void ext4_dirty_inode(struct inode *inod
 
 	ext4_journal_stop(handle);
 out:
-	return;
+	;
 }
 
 #if 0
diff -u -p a/fs/hpfs/dnode.c b/fs/hpfs/dnode.c
--- a/fs/hpfs/dnode.c
+++ b/fs/hpfs/dnode.c
@@ -64,7 +64,7 @@ void hpfs_del_pos(struct inode *inode, l
 	return;
 	not_f:
 	/*printk("HPFS: warning: position pointer %p->%08x not found\n", pos, (int)*pos);*/
-	return;
+	;
 }
 
 static void for_all_poss(struct inode *inode, void (*f)(loff_t *, loff_t, loff_t),
diff -u -p a/fs/jbd/transaction.c b/fs/jbd/transaction.c
--- a/fs/jbd/transaction.c
+++ b/fs/jbd/transaction.c
@@ -1682,7 +1682,7 @@ __journal_try_to_free_buffer(journal_t *
 	}
 	spin_unlock(&journal->j_list_lock);
 out:
-	return;
+	;
 }
 
 /**
diff -u -p a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
--- a/fs/jbd2/transaction.c
+++ b/fs/jbd2/transaction.c
@@ -1830,7 +1830,7 @@ __journal_try_to_free_buffer(journal_t *
 	}
 	spin_unlock(&journal->j_list_lock);
 out:
-	return;
+	;
 }
 
 /**
diff -u -p a/fs/ocfs2/dlm/dlmthread.c b/fs/ocfs2/dlm/dlmthread.c
--- a/fs/ocfs2/dlm/dlmthread.c
+++ b/fs/ocfs2/dlm/dlmthread.c
@@ -435,7 +435,7 @@ blocked:
 	}
 
 leave:
-	return;
+	;
 }
 
 /* must have NO locks when calling this with res !=NULL * */
diff -u -p a/fs/ocfs2/dlmglue.c b/fs/ocfs2/dlmglue.c
--- a/fs/ocfs2/dlmglue.c
+++ b/fs/ocfs2/dlmglue.c
@@ -1766,7 +1766,7 @@ void ocfs2_open_unlock(struct inode *ino
 				     DLM_LOCK_EX);
 
 out:
-	return;
+	;
 }
 
 static int ocfs2_flock_handle_signal(struct ocfs2_lock_res *lockres,
diff -u -p a/fs/ocfs2/inode.c b/fs/ocfs2/inode.c
--- a/fs/ocfs2/inode.c
+++ b/fs/ocfs2/inode.c
@@ -1050,7 +1050,7 @@ bail_unlock_nfs_sync:
 bail_unblock:
 	ocfs2_unblock_signals(&oldset);
 bail:
-	return;
+	;
 }
 
 static void ocfs2_clear_inode(struct inode *inode)
diff -u -p a/fs/proc/base.c b/fs/proc/base.c
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -2734,7 +2734,7 @@ static void proc_flush_task_mnt(struct v
 out_put_leader:
 	dput(leader);
 out:
-	return;
+	;
 }
 
 /**
diff -u -p a/fs/reiserfs/inode.c b/fs/reiserfs/inode.c
--- a/fs/reiserfs/inode.c
+++ b/fs/reiserfs/inode.c
@@ -3029,7 +3029,7 @@ static void reiserfs_invalidatepage(stru
 		/* maybe should BUG_ON(!ret); - neilb */
 	}
       out:
-	return;
+	;
 }
 
 static int reiserfs_set_page_dirty(struct page *page)
diff -u -p a/kernel/audit.c b/kernel/audit.c
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -1424,7 +1424,7 @@ static void audit_log_vformat(struct aud
 out_va_end:
 	va_end(args2);
 out:
-	return;
+	;
 }
 
 /**
diff -u -p a/mm/highmem.c b/mm/highmem.c
--- a/mm/highmem.c
+++ b/mm/highmem.c
@@ -408,7 +408,7 @@ void set_page_address(struct page *page,
 		spin_unlock_irqrestore(&pas->lock, flags);
 	}
 done:
-	return;
+	;
 }
 
 void __init page_address_init(void)
diff -u -p a/mm/hugetlb_cgroup.c b/mm/hugetlb_cgroup.c
--- a/mm/hugetlb_cgroup.c
+++ b/mm/hugetlb_cgroup.c
@@ -135,7 +135,7 @@ static void hugetlb_cgroup_move_parent(i
 
 	set_hugetlb_cgroup(page, parent);
 out:
-	return;
+	;
 }
 
 /*
diff -u -p a/net/batman-adv/network-coding.c b/net/batman-adv/network-coding.c
--- a/net/batman-adv/network-coding.c
+++ b/net/batman-adv/network-coding.c
@@ -1572,7 +1572,7 @@ free_skb:
 free_nc_path:
 	batadv_nc_path_free_ref(nc_path);
 out:
-	return;
+	;
 }
 
 /**
diff -u -p a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -423,7 +423,7 @@ void batadv_interface_rx(struct net_devi
 dropped:
 	kfree_skb(skb);
 out:
-	return;
+	;
 }
 
 /**
diff -u -p a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -1216,7 +1216,7 @@ void fib_select_default(struct fib_resul
 		fib_result_assign(res, last_resort);
 	tb->tb_default = last_idx;
 out:
-	return;
+	;
 }
 
 #ifdef CONFIG_IP_ROUTE_MULTIPATH
diff -u -p a/net/ipv4/igmp.c b/net/ipv4/igmp.c
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -1346,7 +1346,7 @@ void ip_mc_inc_group(struct in_device *i
 	if (!in_dev->dead)
 		ip_rt_multicast_event(in_dev);
 out:
-	return;
+	;
 }
 EXPORT_SYMBOL(ip_mc_inc_group);
 
diff -u -p a/net/ipv4/tcp_metrics.c b/net/ipv4/tcp_metrics.c
--- a/net/ipv4/tcp_metrics.c
+++ b/net/ipv4/tcp_metrics.c
@@ -1172,5 +1172,5 @@ cleanup_subsys:
 	unregister_pernet_subsys(&tcp_net_metrics_ops);
 
 cleanup:
-	return;
+	;
 }
diff -u -p a/net/ipv6/ah6.c b/net/ipv6/ah6.c
--- a/net/ipv6/ah6.c
+++ b/net/ipv6/ah6.c
@@ -207,7 +207,7 @@ static void ipv6_rearrange_destopt(struc
 	}
 	/* Note: ok if len == 0 */
 bad:
-	return;
+	;
 }
 #else
 static void ipv6_rearrange_destopt(struct ipv6hdr *iph, struct ipv6_opt_hdr *destopt) {}
diff -u -p a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -1007,7 +1007,7 @@ static void ndisc_recv_rs(struct sk_buff
 		neigh_release(neigh);
 	}
 out:
-	return;
+	;
 }
 
 static void ndisc_ra_useropt(struct sk_buff *ra, struct nd_opt_hdr *opt)
diff -u -p a/net/ipx/af_ipx.c b/net/ipx/af_ipx.c
--- a/net/ipx/af_ipx.c
+++ b/net/ipx/af_ipx.c
@@ -125,7 +125,7 @@ static void ipx_remove_socket(struct soc
 	spin_unlock_bh(&intrfc->if_sklist_lock);
 	ipxitf_put(intrfc);
 out:
-	return;
+	;
 }
 
 static void ipx_destroy_socket(struct sock *sk)
diff -u -p a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -1347,7 +1347,7 @@ static void l2tp_tunnel_destruct(struct
 	if (sk->sk_destruct)
 		(*sk->sk_destruct)(sk);
 end:
-	return;
+	;
 }
 
 /* When the tunnel is closed, all the attached sessions need to go too.
diff -u -p a/security/tomoyo/audit.c b/security/tomoyo/audit.c
--- a/security/tomoyo/audit.c
+++ b/security/tomoyo/audit.c
@@ -399,7 +399,7 @@ void tomoyo_write_log2(struct tomoyo_req
 	}
 	wake_up(&tomoyo_log_wait);
 out:
-	return;
+	;
 }
 
 /**
diff -u -p a/sound/oss/vidc.c b/sound/oss/vidc.c
--- a/sound/oss/vidc.c
+++ b/sound/oss/vidc.c
@@ -502,7 +502,7 @@ mem_failed:
 mixer_failed:
 	sound_unload_audiodev(adev);
 audio_failed:
-	return;
+	;
 }
 
 static int __init probe_vidc(struct address_info *hw_config)

^ permalink raw reply

* [PATCH regression] dma debug: account for cachelines and read-only mappings in overlap tracking
From: Dan Williams @ 2014-02-13 21:58 UTC (permalink / raw)
  To: akpm
  Cc: Wei Liu, Eric Dumazet, Konrad Rzeszutek Wilk, netdev,
	linux-kernel, Sander Eikelenboom, Francois Romieu, Dave Jones

While debug_dma_assert_idle() checks if a given *page* is actively
undergoing dma the valid granularity of a dma mapping is a *cacheline*.
Sander's testing shows that the warning message "DMA-API: exceeded 7
overlapping mappings of pfn..." is falsely triggering.  The test is
simply mapping multiple cachelines in a given page.

Ultimately we want overlap tracking to be valid as it is a real api
violation, so we need to track active mappings by cachelines.  Update
the active dma tracking to use the page-frame-relative cacheline of the
mapping as the key, and update debug_dma_assert_idle() to check for all
possible mapped cachelines for a given page.

However, the need to track active mappings is only relevant when the
dma-mapping is writable by the device.  In fact it is fairly standard
for read-only mappings to have hundreds or thousands of overlapping
mappings at once.  Limiting the overlap tracking to writable
(!DMA_TO_DEVICE) eliminates this class of false-positive overlap
reports.

Note, the radix gang lookup is sub-optimal.  It would be best if it
stopped fetching entries once the search passed a page boundary.
Nevertheless, this implementation does not perturb the original net_dma
failing case.  That is to say the extra overhead does not show up in
terms of making the failing case pass due to a timing change.

References:
http://marc.info/?l=linux-netdev&m=139232263419315&w=2
http://marc.info/?l=linux-netdev&m=139217088107122&w=2

Reported-by: Sander Eikelenboom <linux@eikelenboom.it>
Reported-by: Dave Jones <davej@redhat.com>
Tested-by: Dave Jones <davej@redhat.com>
Tested-by: Sander Eikelenboom <linux@eikelenboom.it>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Francois Romieu <romieu@fr.zoreil.com>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 lib/dma-debug.c |  125 +++++++++++++++++++++++++++++++++++--------------------
 1 files changed, 79 insertions(+), 46 deletions(-)

diff --git a/lib/dma-debug.c b/lib/dma-debug.c
index 2defd1308b04..320619db176b 100644
--- a/lib/dma-debug.c
+++ b/lib/dma-debug.c
@@ -424,111 +424,132 @@ void debug_dma_dump_mappings(struct device *dev)
 EXPORT_SYMBOL(debug_dma_dump_mappings);
 
 /*
- * For each page mapped (initial page in the case of
- * dma_alloc_coherent/dma_map_{single|page}, or each page in a
- * scatterlist) insert into this tree using the pfn as the key. At
+ * For each mapping (initial cacheline in the case of
+ * dma_alloc_coherent/dma_map_page, initial cacheline in each page of a
+ * scatterlist, or the cacheline specified in dma_map_single) insert
+ * into this tree using the cacheline as the key. At
  * dma_unmap_{single|sg|page} or dma_free_coherent delete the entry.  If
- * the pfn already exists at insertion time add a tag as a reference
+ * the entry already exists at insertion time add a tag as a reference
  * count for the overlapping mappings.  For now, the overlap tracking
- * just ensures that 'unmaps' balance 'maps' before marking the pfn
- * idle, but we should also be flagging overlaps as an API violation.
+ * just ensures that 'unmaps' balance 'maps' before marking the
+ * cacheline idle, but we should also be flagging overlaps as an API
+ * violation.
  *
  * Memory usage is mostly constrained by the maximum number of available
  * dma-debug entries in that we need a free dma_debug_entry before
- * inserting into the tree.  In the case of dma_map_{single|page} and
- * dma_alloc_coherent there is only one dma_debug_entry and one pfn to
- * track per event.  dma_map_sg(), on the other hand,
- * consumes a single dma_debug_entry, but inserts 'nents' entries into
- * the tree.
+ * inserting into the tree.  In the case of dma_map_page and
+ * dma_alloc_coherent there is only one dma_debug_entry and one
+ * dma_active_cacheline entry to track per event.  dma_map_sg(), on the
+ * other hand, consumes a single dma_debug_entry, but inserts 'nents'
+ * entries into the tree.
  *
  * At any time debug_dma_assert_idle() can be called to trigger a
- * warning if the given page is in the active set.
+ * warning if any cachelines in the given page are in the active set.
  */
-static RADIX_TREE(dma_active_pfn, GFP_NOWAIT);
+static RADIX_TREE(dma_active_cacheline, GFP_NOWAIT);
 static DEFINE_SPINLOCK(radix_lock);
-#define ACTIVE_PFN_MAX_OVERLAP ((1 << RADIX_TREE_MAX_TAGS) - 1)
+#define ACTIVE_CLN_MAX_OVERLAP ((1 << RADIX_TREE_MAX_TAGS) - 1)
+#define CACHELINE_PER_PAGE_SHIFT (PAGE_SHIFT - L1_CACHE_SHIFT)
+#define CACHELINES_PER_PAGE (1 << CACHELINE_PER_PAGE_SHIFT)
 
-static int active_pfn_read_overlap(unsigned long pfn)
+unsigned long to_cln(struct dma_debug_entry *entry)
+{
+	return (entry->pfn << CACHELINE_PER_PAGE_SHIFT) +
+		(entry->offset >> L1_CACHE_SHIFT);
+}
+
+static int active_cln_read_overlap(unsigned long cln)
 {
 	int overlap = 0, i;
 
 	for (i = RADIX_TREE_MAX_TAGS - 1; i >= 0; i--)
-		if (radix_tree_tag_get(&dma_active_pfn, pfn, i))
+		if (radix_tree_tag_get(&dma_active_cacheline, cln, i))
 			overlap |= 1 << i;
 	return overlap;
 }
 
-static int active_pfn_set_overlap(unsigned long pfn, int overlap)
+static int active_cln_set_overlap(unsigned long cln, int overlap)
 {
 	int i;
 
-	if (overlap > ACTIVE_PFN_MAX_OVERLAP || overlap < 0)
+	if (overlap > ACTIVE_CLN_MAX_OVERLAP || overlap < 0)
 		return overlap;
 
 	for (i = RADIX_TREE_MAX_TAGS - 1; i >= 0; i--)
 		if (overlap & 1 << i)
-			radix_tree_tag_set(&dma_active_pfn, pfn, i);
+			radix_tree_tag_set(&dma_active_cacheline, cln, i);
 		else
-			radix_tree_tag_clear(&dma_active_pfn, pfn, i);
+			radix_tree_tag_clear(&dma_active_cacheline, cln, i);
 
 	return overlap;
 }
 
-static void active_pfn_inc_overlap(unsigned long pfn)
+static void active_cln_inc_overlap(unsigned long cln)
 {
-	int overlap = active_pfn_read_overlap(pfn);
+	int overlap = active_cln_read_overlap(cln);
 
-	overlap = active_pfn_set_overlap(pfn, ++overlap);
+	overlap = active_cln_set_overlap(cln, ++overlap);
 
 	/* If we overflowed the overlap counter then we're potentially
 	 * leaking dma-mappings.  Otherwise, if maps and unmaps are
 	 * balanced then this overflow may cause false negatives in
-	 * debug_dma_assert_idle() as the pfn may be marked idle
+	 * debug_dma_assert_idle() as the cln may be marked idle
 	 * prematurely.
 	 */
-	WARN_ONCE(overlap > ACTIVE_PFN_MAX_OVERLAP,
-		  "DMA-API: exceeded %d overlapping mappings of pfn %lx\n",
-		  ACTIVE_PFN_MAX_OVERLAP, pfn);
+	WARN_ONCE(overlap > ACTIVE_CLN_MAX_OVERLAP,
+		  "DMA-API: exceeded %d overlapping mappings of cln %lx\n",
+		  ACTIVE_CLN_MAX_OVERLAP, cln);
 }
 
-static int active_pfn_dec_overlap(unsigned long pfn)
+static int active_cln_dec_overlap(unsigned long cln)
 {
-	int overlap = active_pfn_read_overlap(pfn);
+	int overlap = active_cln_read_overlap(cln);
 
-	return active_pfn_set_overlap(pfn, --overlap);
+	return active_cln_set_overlap(cln, --overlap);
 }
 
-static int active_pfn_insert(struct dma_debug_entry *entry)
+static int active_cln_insert(struct dma_debug_entry *entry)
 {
 	unsigned long flags;
 	int rc;
 
+	/* If the device is not writing memory then we don't have any
+	 * concerns about the cpu consuming stale data.  This mitigates
+	 * legitimate usages of overlapping mappings.
+	 */
+	if (entry->direction == DMA_TO_DEVICE)
+		return 0;
+
 	spin_lock_irqsave(&radix_lock, flags);
-	rc = radix_tree_insert(&dma_active_pfn, entry->pfn, entry);
+	rc = radix_tree_insert(&dma_active_cacheline, to_cln(entry), entry);
 	if (rc == -EEXIST)
-		active_pfn_inc_overlap(entry->pfn);
+		active_cln_inc_overlap(to_cln(entry));
 	spin_unlock_irqrestore(&radix_lock, flags);
 
 	return rc;
 }
 
-static void active_pfn_remove(struct dma_debug_entry *entry)
+static void active_cln_remove(struct dma_debug_entry *entry)
 {
 	unsigned long flags;
 
+	/* ...mirror the insert case */
+	if (entry->direction == DMA_TO_DEVICE)
+		return;
+
 	spin_lock_irqsave(&radix_lock, flags);
 	/* since we are counting overlaps the final put of the
-	 * entry->pfn will occur when the overlap count is 0.
-	 * active_pfn_dec_overlap() returns -1 in that case
+	 * cacheline will occur when the overlap count is 0.
+	 * active_cln_dec_overlap() returns -1 in that case
 	 */
-	if (active_pfn_dec_overlap(entry->pfn) < 0)
-		radix_tree_delete(&dma_active_pfn, entry->pfn);
+	if (active_cln_dec_overlap(to_cln(entry)) < 0)
+		radix_tree_delete(&dma_active_cacheline, to_cln(entry));
 	spin_unlock_irqrestore(&radix_lock, flags);
 }
 
 /**
  * debug_dma_assert_idle() - assert that a page is not undergoing dma
- * @page: page to lookup in the dma_active_pfn tree
+ * @page: page to lookup in the dma_active_cacheline tree
  *
  * Place a call to this routine in cases where the cpu touching the page
  * before the dma completes (page is dma_unmapped) will lead to data
@@ -536,22 +557,34 @@ static void active_pfn_remove(struct dma_debug_entry *entry)
  */
 void debug_dma_assert_idle(struct page *page)
 {
+	unsigned long cln = page_to_pfn(page) << CACHELINE_PER_PAGE_SHIFT;
+	static struct dma_debug_entry *ents[CACHELINES_PER_PAGE];
+	struct dma_debug_entry *entry = NULL;
+	void **results = (void **) &ents;
+	unsigned int nents, i;
 	unsigned long flags;
-	struct dma_debug_entry *entry;
 
 	if (!page)
 		return;
 
 	spin_lock_irqsave(&radix_lock, flags);
-	entry = radix_tree_lookup(&dma_active_pfn, page_to_pfn(page));
+	nents = radix_tree_gang_lookup(&dma_active_cacheline, results, cln,
+				       CACHELINES_PER_PAGE);
+	for (i = 0; i < nents; i++) {
+		if (to_cln(ents[i]) == cln) {
+			entry = ents[i];
+			break;
+		} else if (to_cln(ents[i]) >= cln + CACHELINES_PER_PAGE)
+			break;
+	}
 	spin_unlock_irqrestore(&radix_lock, flags);
 
 	if (!entry)
 		return;
 
 	err_printk(entry->dev, entry,
-		   "DMA-API: cpu touching an active dma mapped page "
-		   "[pfn=0x%lx]\n", entry->pfn);
+		   "DMA-API: cpu touching an active dma mapped page [cln=0x%lx]\n",
+		   to_cln(entry));
 }
 
 /*
@@ -568,9 +601,9 @@ static void add_dma_entry(struct dma_debug_entry *entry)
 	hash_bucket_add(bucket, entry);
 	put_hash_bucket(bucket, &flags);
 
-	rc = active_pfn_insert(entry);
+	rc = active_cln_insert(entry);
 	if (rc == -ENOMEM) {
-		pr_err("DMA-API: pfn tracking ENOMEM, dma-debug disabled\n");
+		pr_err("DMA-API: cacheline tracking ENOMEM, dma-debug disabled\n");
 		global_disable = true;
 	}
 
@@ -631,7 +664,7 @@ static void dma_entry_free(struct dma_debug_entry *entry)
 {
 	unsigned long flags;
 
-	active_pfn_remove(entry);
+	active_cln_remove(entry);
 
 	/*
 	 * add to beginning of the list - this way the entries are

^ permalink raw reply related

* Re: [PATCH net-next] net: remove unnecessary return's
From: Dave Jones @ 2014-02-13 22:00 UTC (permalink / raw)
  To: Julia Lawall; +Cc: Joe Perches, Stephen Hemminger, David Miller, netdev
In-Reply-To: <alpine.DEB.2.02.1402132252290.1967@localhost6.localdomain6>

On Thu, Feb 13, 2014 at 10:55:23PM +0100, Julia Lawall wrote:

 > The patch below converts label: return; } to label: ; }.  I have only 
 > scanned through the patches, not patched the code and looked at the 
 > results, so I am not sure that it is completely correct.  On the other 
 > hand, I'm also not sure that label: ; } is better than label: return; }, 
 > either.  If people think it is, then I can cheack the results in more 
 > detail.

Why not delete the label, and just replace the goto with a return if
the label is at the end of the function ?

	Dave

^ permalink raw reply

* Re: [PATCH regression] dma debug: account for cachelines and read-only mappings in overlap tracking
From: Andrew Morton @ 2014-02-13 22:05 UTC (permalink / raw)
  To: Dan Williams
  Cc: Wei Liu, Eric Dumazet, Konrad Rzeszutek Wilk, netdev,
	linux-kernel, Sander Eikelenboom, Francois Romieu, Dave Jones
In-Reply-To: <20140213215652.22950.12180.stgit@viggo.jf.intel.com>

On Thu, 13 Feb 2014 13:58:00 -0800 Dan Williams <dan.j.williams@intel.com> wrote:

> While debug_dma_assert_idle() checks if a given *page* is actively
> undergoing dma the valid granularity of a dma mapping is a *cacheline*.
> Sander's testing shows that the warning message "DMA-API: exceeded 7
> overlapping mappings of pfn..." is falsely triggering.  The test is
> simply mapping multiple cachelines in a given page.
> 
> Ultimately we want overlap tracking to be valid as it is a real api
> violation, so we need to track active mappings by cachelines.  Update
> the active dma tracking to use the page-frame-relative cacheline of the
> mapping as the key, and update debug_dma_assert_idle() to check for all
> possible mapped cachelines for a given page.
> 
> However, the need to track active mappings is only relevant when the
> dma-mapping is writable by the device.  In fact it is fairly standard
> for read-only mappings to have hundreds or thousands of overlapping
> mappings at once.  Limiting the overlap tracking to writable
> (!DMA_TO_DEVICE) eliminates this class of false-positive overlap
> reports.
> 
> Note, the radix gang lookup is sub-optimal.  It would be best if it
> stopped fetching entries once the search passed a page boundary.
> Nevertheless, this implementation does not perturb the original net_dma
> failing case.  That is to say the extra overhead does not show up in
> terms of making the failing case pass due to a timing change.
> 
> References:
> http://marc.info/?l=linux-netdev&m=139232263419315&w=2
> http://marc.info/?l=linux-netdev&m=139217088107122&w=2
> 
> ...
>
> --- a/lib/dma-debug.c
> +++ b/lib/dma-debug.c
> @@ -424,111 +424,132 @@ void debug_dma_dump_mappings(struct device *dev)
>  EXPORT_SYMBOL(debug_dma_dump_mappings);
>  
>  /*
> - * For each page mapped (initial page in the case of
> - * dma_alloc_coherent/dma_map_{single|page}, or each page in a
> - * scatterlist) insert into this tree using the pfn as the key. At
> + * For each mapping (initial cacheline in the case of
> + * dma_alloc_coherent/dma_map_page, initial cacheline in each page of a
> + * scatterlist, or the cacheline specified in dma_map_single) insert
> + * into this tree using the cacheline as the key. At
>   * dma_unmap_{single|sg|page} or dma_free_coherent delete the entry.  If
> - * the pfn already exists at insertion time add a tag as a reference
> + * the entry already exists at insertion time add a tag as a reference
>   * count for the overlapping mappings.  For now, the overlap tracking
> - * just ensures that 'unmaps' balance 'maps' before marking the pfn
> - * idle, but we should also be flagging overlaps as an API violation.
> + * just ensures that 'unmaps' balance 'maps' before marking the
> + * cacheline idle, but we should also be flagging overlaps as an API
> + * violation.
>   *
>   * Memory usage is mostly constrained by the maximum number of available
>   * dma-debug entries in that we need a free dma_debug_entry before
> - * inserting into the tree.  In the case of dma_map_{single|page} and
> - * dma_alloc_coherent there is only one dma_debug_entry and one pfn to
> - * track per event.  dma_map_sg(), on the other hand,
> - * consumes a single dma_debug_entry, but inserts 'nents' entries into
> - * the tree.
> + * inserting into the tree.  In the case of dma_map_page and
> + * dma_alloc_coherent there is only one dma_debug_entry and one
> + * dma_active_cacheline entry to track per event.  dma_map_sg(), on the
> + * other hand, consumes a single dma_debug_entry, but inserts 'nents'
> + * entries into the tree.
>   *
>   * At any time debug_dma_assert_idle() can be called to trigger a
> - * warning if the given page is in the active set.
> + * warning if any cachelines in the given page are in the active set.
>   */
> -static RADIX_TREE(dma_active_pfn, GFP_NOWAIT);
> +static RADIX_TREE(dma_active_cacheline, GFP_NOWAIT);
>  static DEFINE_SPINLOCK(radix_lock);
> -#define ACTIVE_PFN_MAX_OVERLAP ((1 << RADIX_TREE_MAX_TAGS) - 1)
> +#define ACTIVE_CLN_MAX_OVERLAP ((1 << RADIX_TREE_MAX_TAGS) - 1)
> +#define CACHELINE_PER_PAGE_SHIFT (PAGE_SHIFT - L1_CACHE_SHIFT)
> +#define CACHELINES_PER_PAGE (1 << CACHELINE_PER_PAGE_SHIFT)
>  
> -static int active_pfn_read_overlap(unsigned long pfn)
> +unsigned long to_cln(struct dma_debug_entry *entry)
> +{
> +	return (entry->pfn << CACHELINE_PER_PAGE_SHIFT) +
> +		(entry->offset >> L1_CACHE_SHIFT);
> +}

"cln" is ugly and isn't a well-known kernel abbreviation.  We typically
spell these things out, so "cacheline".  But I think you mean
"cacheline number", and that is too long to spell out.

So I guess "cln" just became a well-known kernel abbreviation.

> ....
>
>  void debug_dma_assert_idle(struct page *page)
>  {
> +	unsigned long cln = page_to_pfn(page) << CACHELINE_PER_PAGE_SHIFT;

This worries me.  Are you sure we cannot overflow the ulong here under
any circumstances?  32GB PAE with sparsemem or whatever?

^ permalink raw reply

* Re: [PATCH net-next] net: remove unnecessary return's
From: Julia Lawall @ 2014-02-13 22:06 UTC (permalink / raw)
  To: Dave Jones; +Cc: Joe Perches, Stephen Hemminger, David Miller, netdev
In-Reply-To: <20140213220021.GA1174@redhat.com>

On Thu, 13 Feb 2014, Dave Jones wrote:

> On Thu, Feb 13, 2014 at 10:55:23PM +0100, Julia Lawall wrote:
> 
>  > The patch below converts label: return; } to label: ; }.  I have only 
>  > scanned through the patches, not patched the code and looked at the 
>  > results, so I am not sure that it is completely correct.  On the other 
>  > hand, I'm also not sure that label: ; } is better than label: return; }, 
>  > either.  If people think it is, then I can cheack the results in more 
>  > detail.
> 
> Why not delete the label, and just replace the goto with a return if
> the label is at the end of the function ?

Good point :)

julia

^ permalink raw reply

* [PATCH v2 1/2] net: core: introduce netif_skb_dev_features
From: Florian Westphal @ 2014-02-13 22:09 UTC (permalink / raw)
  To: netdev; +Cc: Florian Westphal

Will be used by upcoming ipv4 forward path change that needs to
determine feature mask using skb->dst->dev instead of skb->dev.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
Changes since v1:
 no more 'illegal_highdma' discards 'const' qualifier from pointer target type'

 include/linux/netdevice.h |  7 ++++++-
 net/core/dev.c            | 22 ++++++++++++----------
 2 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 440a02e..21d4e6b 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3068,7 +3068,12 @@ void netdev_change_features(struct net_device *dev);
 void netif_stacked_transfer_operstate(const struct net_device *rootdev,
 					struct net_device *dev);
 
-netdev_features_t netif_skb_features(struct sk_buff *skb);
+netdev_features_t netif_skb_dev_features(struct sk_buff *skb,
+					 const struct net_device *dev);
+static inline netdev_features_t netif_skb_features(struct sk_buff *skb)
+{
+	return netif_skb_dev_features(skb, skb->dev);
+}
 
 static inline bool net_gso_ok(netdev_features_t features, int gso_type)
 {
diff --git a/net/core/dev.c b/net/core/dev.c
index 3721db7..afc4e47 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2420,7 +2420,7 @@ EXPORT_SYMBOL(netdev_rx_csum_fault);
  * 2. No high memory really exists on this machine.
  */
 
-static int illegal_highdma(struct net_device *dev, struct sk_buff *skb)
+static int illegal_highdma(const struct net_device *dev, struct sk_buff *skb)
 {
 #ifdef CONFIG_HIGHMEM
 	int i;
@@ -2495,34 +2495,36 @@ static int dev_gso_segment(struct sk_buff *skb, netdev_features_t features)
 }
 
 static netdev_features_t harmonize_features(struct sk_buff *skb,
-	netdev_features_t features)
+					    const struct net_device *dev,
+					    netdev_features_t features)
 {
 	if (skb->ip_summed != CHECKSUM_NONE &&
 	    !can_checksum_protocol(features, skb_network_protocol(skb))) {
 		features &= ~NETIF_F_ALL_CSUM;
-	} else if (illegal_highdma(skb->dev, skb)) {
+	} else if (illegal_highdma(dev, skb)) {
 		features &= ~NETIF_F_SG;
 	}
 
 	return features;
 }
 
-netdev_features_t netif_skb_features(struct sk_buff *skb)
+netdev_features_t netif_skb_dev_features(struct sk_buff *skb,
+					 const struct net_device *dev)
 {
 	__be16 protocol = skb->protocol;
-	netdev_features_t features = skb->dev->features;
+	netdev_features_t features = dev->features;
 
-	if (skb_shinfo(skb)->gso_segs > skb->dev->gso_max_segs)
+	if (skb_shinfo(skb)->gso_segs > dev->gso_max_segs)
 		features &= ~NETIF_F_GSO_MASK;
 
 	if (protocol == htons(ETH_P_8021Q) || protocol == htons(ETH_P_8021AD)) {
 		struct vlan_ethhdr *veh = (struct vlan_ethhdr *)skb->data;
 		protocol = veh->h_vlan_encapsulated_proto;
 	} else if (!vlan_tx_tag_present(skb)) {
-		return harmonize_features(skb, features);
+		return harmonize_features(skb, dev, features);
 	}
 
-	features &= (skb->dev->vlan_features | NETIF_F_HW_VLAN_CTAG_TX |
+	features &= (dev->vlan_features | NETIF_F_HW_VLAN_CTAG_TX |
 					       NETIF_F_HW_VLAN_STAG_TX);
 
 	if (protocol == htons(ETH_P_8021Q) || protocol == htons(ETH_P_8021AD))
@@ -2530,9 +2532,9 @@ netdev_features_t netif_skb_features(struct sk_buff *skb)
 				NETIF_F_GEN_CSUM | NETIF_F_HW_VLAN_CTAG_TX |
 				NETIF_F_HW_VLAN_STAG_TX;
 
-	return harmonize_features(skb, features);
+	return harmonize_features(skb, dev, features);
 }
-EXPORT_SYMBOL(netif_skb_features);
+EXPORT_SYMBOL(netif_skb_dev_features);
 
 int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
 			struct netdev_queue *txq)
-- 
1.8.1.5

^ permalink raw reply related

* [PATCH v6 2/2] net: ip, ipv6: handle gso skbs in forwarding path
From: Florian Westphal @ 2014-02-13 22:09 UTC (permalink / raw)
  To: netdev; +Cc: Florian Westphal
In-Reply-To: <1392329352-31606-1-git-send-email-fw@strlen.de>

Marcelo Ricardo Leitner reported problems when the forwarding link path
has a lower mtu than the incoming one if the inbound interface supports GRO.

Given:
Host <mtu1500> R1 <mtu1200> R2

Host sends tcp stream which is routed via R1 and R2.  R1 performs GRO.

In this case, the kernel will fail to send ICMP fragmentation needed
messages (or pkt too big for ipv6), as GSO packets currently bypass dstmtu
checks in forward path. Instead, Linux tries to send out packets exceeding
the mtu.

When locking route MTU on Host (i.e., no ipv4 DF bit set), R1 does
not fragment the packets when forwarding, and again tries to send out
packets exceeding R1-R2 link mtu.

This alters the forwarding dstmtu checks to take the individual gso
segment lengths into account.

For ipv6, we send out pkt too big error for gso if the individual
segments are too big.

For ipv4, we either send icmp fragmentation needed, or, if the DF bit
is not set, perform software segmentation and let the output path
create fragments when the packet is leaving the machine.
It is not 100% correct as the error message will contain the headers of
the GRO skb instead of the original/segmented one, but it seems to
work fine in my (limited) tests.

Eric Dumazet suggested to simply shrink mss via ->gso_size to avoid
sofware segmentation.

However it turns out that skb_segment() assumes skb nr_frags is related
to mss size so we would BUG there.  I don't want to mess with it considering
Herbert and Eric disagree on what the correct behavior should be.

Hannes Frederic Sowa notes that when we would shrink gso_size
skb_segment would then also need to deal with the case where
SKB_MAX_FRAGS would be exceeded.

This uses sofware segmentation in the forward path when we hit ipv4
non-DF packets and the outgoing link mtu is too small.  Its not perfect,
but given the lack of bug reports wrt. GRO fwd being broken this is a
rare case anyway.  Also its not like this could not be improved later
once the dust settles.

Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
Reported-by: Marcelo Ricardo Leitner <mleitner@redhat.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
Changes since v5; propagated Herberts Ack from v5 to changelog.
[ resend is due to error in earlier patch in series ]

Changes since V4:
 - use new netif_skb_dev_features instead of netif_skb_features
   as we need dst->dev and not skb->dev (spotted by
   Hannes Frederic Sowa)

Changes since V3:
 - use ip_dst_mtu_maybe_forward instead of dst_mtu
 - add comment wrt. DF bit not being set

Changes since V2:
 - make this thing apply to current -net tree
 - kill unused variables in ip_forward/ip6_output

Changes since V1:
 suggestions from Eric Dumazet:
  - skip more expensive computation for small packets in fwd path
  - use netif_skb_features() feature mask and remove GSO flags
    instead of using 0 feature set.

 include/linux/skbuff.h | 17 ++++++++++++
 net/ipv4/ip_forward.c  | 71 ++++++++++++++++++++++++++++++++++++++++++++++++--
 net/ipv6/ip6_output.c  | 17 ++++++++++--
 3 files changed, 101 insertions(+), 4 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index f589c9a..3ebbbe7 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2916,5 +2916,22 @@ static inline bool skb_head_is_locked(const struct sk_buff *skb)
 {
 	return !skb->head_frag || skb_cloned(skb);
 }
+
+/**
+ * skb_gso_network_seglen - Return length of individual segments of a gso packet
+ *
+ * @skb: GSO skb
+ *
+ * skb_gso_network_seglen is used to determine the real size of the
+ * individual segments, including Layer3 (IP, IPv6) and L4 headers (TCP/UDP).
+ *
+ * The MAC/L2 header is not accounted for.
+ */
+static inline unsigned int skb_gso_network_seglen(const struct sk_buff *skb)
+{
+	unsigned int hdr_len = skb_transport_header(skb) -
+			       skb_network_header(skb);
+	return hdr_len + skb_gso_transport_seglen(skb);
+}
 #endif	/* __KERNEL__ */
 #endif	/* _LINUX_SKBUFF_H */
diff --git a/net/ipv4/ip_forward.c b/net/ipv4/ip_forward.c
index e9f1217..f3869c1 100644
--- a/net/ipv4/ip_forward.c
+++ b/net/ipv4/ip_forward.c
@@ -39,6 +39,71 @@
 #include <net/route.h>
 #include <net/xfrm.h>
 
+static bool ip_may_fragment(const struct sk_buff *skb)
+{
+	return unlikely((ip_hdr(skb)->frag_off & htons(IP_DF)) == 0) ||
+	       !skb->local_df;
+}
+
+static bool ip_exceeds_mtu(const struct sk_buff *skb, unsigned int mtu)
+{
+	if (skb->len <= mtu || skb->local_df)
+		return false;
+
+	if (skb_is_gso(skb) && skb_gso_network_seglen(skb) <= mtu)
+		return false;
+
+	return true;
+}
+
+static bool ip_gso_exceeds_dst_mtu(const struct sk_buff *skb)
+{
+	unsigned int mtu;
+
+	if (skb->local_df || !skb_is_gso(skb))
+		return false;
+
+	mtu = ip_dst_mtu_maybe_forward(skb_dst(skb), true);
+
+	/* if seglen > mtu, do software segmentation for IP fragmentation on
+	 * output.  DF bit cannot be set since ip_forward would have sent
+	 * icmp error.
+	 */
+	return skb_gso_network_seglen(skb) > mtu;
+}
+
+/* called if GSO skb needs to be fragmented on forward */
+static int ip_forward_finish_gso(struct sk_buff *skb)
+{
+	struct dst_entry *dst = skb_dst(skb);
+	netdev_features_t features;
+	struct sk_buff *segs;
+	int ret = 0;
+
+	features = netif_skb_dev_features(skb, dst->dev);
+	segs = skb_gso_segment(skb, features & ~NETIF_F_GSO_MASK);
+	if (IS_ERR(segs)) {
+		kfree_skb(skb);
+		return -ENOMEM;
+	}
+
+	consume_skb(skb);
+
+	do {
+		struct sk_buff *nskb = segs->next;
+		int err;
+
+		segs->next = NULL;
+		err = dst_output(segs);
+
+		if (err && ret == 0)
+			ret = err;
+		segs = nskb;
+	} while (segs);
+
+	return ret;
+}
+
 static int ip_forward_finish(struct sk_buff *skb)
 {
 	struct ip_options *opt	= &(IPCB(skb)->opt);
@@ -49,6 +114,9 @@ static int ip_forward_finish(struct sk_buff *skb)
 	if (unlikely(opt->optlen))
 		ip_forward_options(skb);
 
+	if (ip_gso_exceeds_dst_mtu(skb))
+		return ip_forward_finish_gso(skb);
+
 	return dst_output(skb);
 }
 
@@ -91,8 +159,7 @@ int ip_forward(struct sk_buff *skb)
 
 	IPCB(skb)->flags |= IPSKB_FORWARDED;
 	mtu = ip_dst_mtu_maybe_forward(&rt->dst, true);
-	if (unlikely(skb->len > mtu && !skb_is_gso(skb) &&
-		     (ip_hdr(skb)->frag_off & htons(IP_DF))) && !skb->local_df) {
+	if (!ip_may_fragment(skb) && ip_exceeds_mtu(skb, mtu)) {
 		IP_INC_STATS(dev_net(rt->dst.dev), IPSTATS_MIB_FRAGFAILS);
 		icmp_send(skb, ICMP_DEST_UNREACH, ICMP_FRAG_NEEDED,
 			  htonl(mtu));
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index ef02b26..070a2fa 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -342,6 +342,20 @@ static unsigned int ip6_dst_mtu_forward(const struct dst_entry *dst)
 	return mtu;
 }
 
+static bool ip6_pkt_too_big(const struct sk_buff *skb, unsigned int mtu)
+{
+	if (skb->len <= mtu || skb->local_df)
+		return false;
+
+	if (IP6CB(skb)->frag_max_size && IP6CB(skb)->frag_max_size > mtu)
+		return true;
+
+	if (skb_is_gso(skb) && skb_gso_network_seglen(skb) <= mtu)
+		return false;
+
+	return true;
+}
+
 int ip6_forward(struct sk_buff *skb)
 {
 	struct dst_entry *dst = skb_dst(skb);
@@ -466,8 +480,7 @@ int ip6_forward(struct sk_buff *skb)
 	if (mtu < IPV6_MIN_MTU)
 		mtu = IPV6_MIN_MTU;
 
-	if ((!skb->local_df && skb->len > mtu && !skb_is_gso(skb)) ||
-	    (IP6CB(skb)->frag_max_size && IP6CB(skb)->frag_max_size > mtu)) {
+	if (ip6_pkt_too_big(skb, mtu)) {
 		/* Again, force OUTPUT device used as source address */
 		skb->dev = dst->dev;
 		icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, mtu);
-- 
1.8.1.5

^ permalink raw reply related

* Re: [PATCH] macvlan: unregister net device when netdev_upper_dev_link() fails
From: David Miller @ 2014-02-13 22:13 UTC (permalink / raw)
  To: xiyou.wangcong; +Cc: netdev, kaber, cwang
In-Reply-To: <1392162690-6647-2-git-send-email-xiyou.wangcong@gmail.com>

From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Tue, 11 Feb 2014 15:51:29 -0800

> From: Cong Wang <cwang@twopensource.com>
> 
> rtnl_newlink() doesn't unregister it for us on failure.
> 
> Cc: Patrick McHardy <kaber@trash.net>
> Cc: David S. Miller <davem@davemloft.net>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> Signed-off-by: Cong Wang <cwang@twopensource.com>

Applied.

^ permalink raw reply

* Re: [PATCH] net: correct error path in rtnl_newlink()
From: David Miller @ 2014-02-13 22:13 UTC (permalink / raw)
  To: xiyou.wangcong; +Cc: netdev, eric.dumazet, cwang
In-Reply-To: <1392162690-6647-3-git-send-email-xiyou.wangcong@gmail.com>

From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Tue, 11 Feb 2014 15:51:30 -0800

> From: Cong Wang <cwang@twopensource.com>
> 
> I saw the following BUG when ->newlink() fails in rtnl_newlink():
> 
> [   40.240058] kernel BUG at net/core/dev.c:6438!
> 
> this is due to free_netdev() is not supposed to be called before
> netdev is completely unregistered, therefore it is not correct
> to call free_netdev() here, at least for ops->newlink!=NULL case,
> many drivers call it in ->destructor so that rtnl_unlock() will
> take care of it, we probably don't need to do anything here.
> 
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> Signed-off-by: Cong Wang <cwang@twopensource.com>

Applied.

^ permalink raw reply

* Re: [PATCH v3 0/2] sctp: fix a problem with net_namespace
From: David Miller @ 2014-02-13 22:13 UTC (permalink / raw)
  To: wangweidong1; +Cc: nhorman, vyasevich, dborkman, sergei.shtylyov, netdev
In-Reply-To: <1392169484-8256-1-git-send-email-wangweidong1@huawei.com>

From: Wang Weidong <wangweidong1@huawei.com>
Date: Wed, 12 Feb 2014 09:44:42 +0800

> fix a problem with net_namespace, and optimize
> the sctp_sysctl_net_register.
> 
> v2 -> v3:
>   -patch2: add empty line after declaration as potined out by Sergei.
> 
> v1 -> v2:
>   -patch1: add Neil's ACK.
> 
> Wang Weidong (2):
>   sctp: fix a missed .data initialization
>   sctp: optimize the sctp_sysctl_net_register

Series applied, thanks.

^ permalink raw reply

* Re: [PATCH net] bonding: Fix deadlock in bonding driver when using netpoll
From: David Miller @ 2014-02-13 22:13 UTC (permalink / raw)
  To: dingtianhong; +Cc: fubar, vfalico, andy, netdev
In-Reply-To: <52FAF350.80005@huawei.com>

From: Ding Tianhong <dingtianhong@huawei.com>
Date: Wed, 12 Feb 2014 12:06:40 +0800

> The bonding driver take write locks and spin locks that are shared
> by the tx path in enslave processing and notification processing,
> If the netconsole is in use, the bonding can call printk which puts
> us in the netpoll tx path, if the netconsole is attached to the bonding
> driver, result in deadlock.
> 
> So add protection for these place, by checking the netpoll_block_tx
> state, we can defer the sending of the netconsole frames until a later
> time using the retransmit feature of netpoll_send_skb that is triggered
> on the return code NETDEV_TX_BUSY.
> 
> Cc: Jay Vosburgh <fubar@us.ibm.com>
> Cc: Veaceslav Falico <vfalico@redhat.com>
> Cc: Andy Gospodarek <andy@greyhouse.net>
> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>

Applied.

^ permalink raw reply

* Re: [PATCH v2 1/2] net: core: introduce netif_skb_dev_features
From: David Miller @ 2014-02-13 22:17 UTC (permalink / raw)
  To: fw; +Cc: netdev
In-Reply-To: <1392329352-31606-1-git-send-email-fw@strlen.de>

From: Florian Westphal <fw@strlen.de>
Date: Thu, 13 Feb 2014 23:09:11 +0100

> Will be used by upcoming ipv4 forward path change that needs to
> determine feature mask using skb->dst->dev instead of skb->dev.
> 
> Signed-off-by: Florian Westphal <fw@strlen.de>

Applied and queued up for -stable.

^ permalink raw reply

* Re: [PATCH v6 2/2] net: ip, ipv6: handle gso skbs in forwarding path
From: David Miller @ 2014-02-13 22:17 UTC (permalink / raw)
  To: fw; +Cc: netdev
In-Reply-To: <1392329352-31606-2-git-send-email-fw@strlen.de>

From: Florian Westphal <fw@strlen.de>
Date: Thu, 13 Feb 2014 23:09:12 +0100

> Marcelo Ricardo Leitner reported problems when the forwarding link path
> has a lower mtu than the incoming one if the inbound interface supports GRO.
 ...
> However it turns out that skb_segment() assumes skb nr_frags is related
> to mss size so we would BUG there.  I don't want to mess with it considering
> Herbert and Eric disagree on what the correct behavior should be.
> 
> Hannes Frederic Sowa notes that when we would shrink gso_size
> skb_segment would then also need to deal with the case where
> SKB_MAX_FRAGS would be exceeded.
> 
> This uses sofware segmentation in the forward path when we hit ipv4
> non-DF packets and the outgoing link mtu is too small.  Its not perfect,
> but given the lack of bug reports wrt. GRO fwd being broken this is a
> rare case anyway.  Also its not like this could not be improved later
> once the dust settles.
> 
> Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
> Reported-by: Marcelo Ricardo Leitner <mleitner@redhat.com>
> Signed-off-by: Florian Westphal <fw@strlen.de>

Also applied and queued up for -stable, thanks.

^ permalink raw reply

* Re: [RFC PATCH v2 tip 0/7] 64-bit BPF insn set and tracing filters
From: Daniel Borkmann @ 2014-02-13 22:22 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Ingo Molnar, David S. Miller, Steven Rostedt, Peter Zijlstra,
	H. Peter Anvin, Thomas Gleixner, Masami Hiramatsu, Tom Zanussi,
	Jovi Zhangwei, Eric Dumazet, Linus Torvalds, Andrew Morton,
	Frederic Weisbecker, Arnaldo Carvalho de Melo, Pekka Enberg,
	Arjan van de Ven, Christoph Hellwig, linux-kernel, netdev
In-Reply-To: <52FD2908.8000009@redhat.com>

On 02/13/2014 09:20 PM, Daniel Borkmann wrote:
> On 02/07/2014 02:20 AM, Alexei Starovoitov wrote:
> ...
>> Hi Daniel,
>
> Thanks for your answer and sorry for the late reply.
>
>> Thank you for taking a look. Good questions. I had the same concerns.
>> Old BPF was carefully extended in specific places.
>> End result may look big at first glance, but every extension has specific
>> reason behind it. I tried to explain the reasoning in Documentation/bpf_jit.txt
>>
>> I'm planning to write an on-the-fly converter from old BPF to BPF64
>> when BPF64 manages to demonstrate that it is equally safe.
>> It is straight forward to convert. Encoding is very similar.
>> Core concepts are the same.
>> Try diff include/uapi/linux/filter.h include/linux/bpf.h
>> to see how much is reused.
>>
>> I believe that old BPF outlived itself and BPF64 should
>> replace it in all current use cases plus a lot more.
>> It just cannot happen at once.
>> BPF64 can come in. bpf32->bpf64 converter functioning.
>> JIT from bpf64->aarch64 and may be sparc64 needs to be in place.
>> Then old bpf can fade away.
>
> Do you see a possibility to integrate your work step by step? That is,
> to first integrate the interpreter part only; meaning, to detect "old"
> BPF programs e.g. coming from SO_ATTACH_FILTER et al and run them in
> compatibility mode while extended BPF is fully integrated and replaces
> the old engine in net/core/filter.c. Maybe, "old" programs can be
> transformed transparently to the new representation and then would be
> good to execute in eBPF. If possible, in such a way that in the first
> step JIT compilers won't need any upgrades. Once that is resolved,
> JIT compilers could successively migrate, arch by arch, to compile the
> new code? And last but not least the existing tools as well for handling
> eBPF. I think, if possible, that would be great. Also, I unfortunately
> haven't looked into your code too deeply yet due to time constraints,
> but I'm wondering e.g. for accessing some skb fields we currently use
> the "hack" to "overload" load instructions with negative arguments. Do
> we have a sort of "meta" instruction that is extendible in eBPF to avoid
> such things in future?
>
>>> First of all, I think it's very interesting work ! I'm just a bit concerned
>>> that this _huge_ patchset with 64 bit BPF, or however we call it, will line
>>
>> Huge?
>> kernel is only 2k
>> the rest is 6k of userspace LLVM backend where most of it is llvm's
>> boilerplate code. GCC backend for BPF is 3k.
>> The goal is to have both GCC and LLVM backends to be upstreamed
>> when kernel pieces are agreed upon.
>> For comparison existing tools/net/bpf* is 2.5k
>> but here with 6k we get optimizing compiler from C and assembler.
>>
>>> up in one row next to the BPF code we currently have and next to new
>>> nftables
>>> engine and we will end up with three such engines which do quite similar
>>> things and are all exposed to user space thus they need to be maintained
>>> _forever_, adding up legacy even more. What would be the long-term future
>>> use
>>> cases where the 64 bit engine comes into place compared to the current BPF
>>> engine? What are the concrete killer features? I didn't went through your
>>
>> killer features vs old bpf are:
>> - zero-cost function calls
>> - 32-bit vs 64-bit
>> - optimizing compiler that can compile C into BPF64
>>
>> Why call kernel function from BPF?
>> So that BPF instruction set has to be extended only once and JITs are
>> written only once.
>> Over the years many extensions crept into old BPF as 'negative offsets'.
>> but JITs don't support all of them and assume bpf input as 'skb' only.
>> seccomp is using old bpf, but, because of these limitations, cannot use JIT.
>> BPF64 allows seccomp to be JITed, since bpf input is generalized
>> as 'struct bpf_context'.
>> New 'negative offset' extension for old bpf would mean implementing it in
>> JITs of all architectures? Painful, but doable. We can do better.

I'm very curious, do you also have any performance numbers, e.g. for
networking by taking JIT'ed/non-JIT'ed BPF filters and compare them against
JIT'ed/non-JIT'ed eBPF filters to see how many pps we gain or loose e.g.
for a scenario with a middle box running cls_bpf .. or some other macro/
micro benchmark just to get a picture where both stand in terms of
performance? Who knows, maybe it would outperform nftables engine as
well? ;-) How would that look on a 32bit arch with eBPF that is 64bit?

>> Fixed instruction set that allows zero-overhead calls into kernel functions
>> is much more flexible and extendable in a clean way.
>> Take a look at kernel/trace/bpf_trace_callbacks.c
>> It is a customization of generic BPF64 core for 'tracing filters'.
>> The set of functions for networking and definition of 'bpf_context'
>> will be different.
>> So BPF64 for tracing need X extensions, BPF64 for networking needs Y
>> extensions, but core framework stays the same and JIT stays the same.
>>
>> How to do zero-overhead call?
>> Map BPF registers to native registers one to one
>> and have compatible calling convention between BPF and native.
>> Then BPF asm code:
>> mov R1, 1
>> mov R2, 2
>> call foo
>> will be JITed into x86-64:
>> mov rdi, 1
>> mov rsi, 2
>> call foo
>> That makes BPF64 calls into kernel as fast as possible.
>> Especially for networking we don't want overhead of FFI mechanisms.
>>
>> That's why A and X regs and lack of callee-saved regs make old BPF
>> impractical to support generic function calls.
>>
>> BPF64 defines R1-R5 as function arguments and R6-R9 as
>> callee-saved, so kernel can natively call into JIT-ed BPF and back
>> with no extra argument shuffling.
>> gcc/llvm backends know that R6-R9 will be preserved while BPF is
>> calling into kernel functions and can make proper optimizations.
>> R6-R9 map to rbx-r15 on x86-64. On aarch64 we have
>> even more freedom of mapping.
>>
>>> code
>>> in detail, but although we might/could have _some_ performance benefits but
>>> at
>>> the _huge_ cost of adding complexity. The current BPF I find okay to debug
>>> and
>>> to follow, but how would be debug'ability of 64 bit programs end up, as you
>>> mention, it becomes "unbearably complex"?
>>
>> "unbearably complex" was the reference to x86 static analyzer :)
>> It's difficult to reconstruct and verify control and data flow of x86 asm code.
>> Binary compilers do that (like transmeta and others), but that's not suitable
>> for kernel.
>>
>> Both old bpf asm and bpf64 asm code I find equivalent in readability.
>>
>> clang dropmon.c ...|llc -filetype=asm
>> will produce the following bpf64 asm code:
>>          mov     r6, r1
>>          ldd     r1, 8(r6)
>>          std     -8(r10), r1
>>          mov     r7, 0
>>          mov     r3, r10
>>          addi    r3, -8
>>          mov     r1, r6
>>          mov     r2, r7
>>          call    bpf_table_lookup
>>          jeqi    r0, 0 goto .LBB0_2
>>
>> which corresponds to C:
>> void dropmon(struct bpf_context *ctx)
>> {       void *loc;
>>          uint64_t *drop_cnt;
>>          loc = (void *)ctx->arg2;
>>          drop_cnt = bpf_table_lookup(ctx, 0, &loc);
>>          if (drop_cnt) ...
>>
>> I think restricted C is easier to program and debug.
>> Which is another killer feature of bpf64.
>>
>> Interesting use case would be if some kernel subsystem
>> decides to generate BPF64 insns on the fly and JIT them.
>> Sort of self-modifieable kernel code.
>> It's certainly easier to generate BPF64 binary with macroses
>> from linux/bpf.h instead of x86 binary...
>> I may be dreaming here :)
>>
>>> Did you instead consider to
>>> replace
>>> the current BPF engine instead, and add a sort of built-in compatibility
>>> mode for current BPF programs? I think that this would be the way better
>>> option to go with instead of adding a new engine next to the other. For
>>> maintainability, trying to replace the old one might be harder to do on the
>>> short term but better to maintain on the long run for everyone, no?
>>
>> Exactly. I think on-the-fly converter from bpf32->bpf64 is this built-in
>> compatibility layer. I completely agree that replacing bpf32 is hard
>> short term, since it will raise too many concerns about
>> stability/safety, but long term it's a way to go.
>
> Yes, I agree.
>
>> I'm open to all suggestions on how to make it more generic, useful,
>> faster.
>>
>> Thank you for feedback.
>
> Thank you, must have been really fun to implement this. :)
>
>> Regards,
>> Alexei
>>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: RTL8153 fails to get link after applying c7de7dec2 to 3.8 kernel
From: Grant Grundler @ 2014-02-13 22:22 UTC (permalink / raw)
  To: hayeswang; +Cc: Inki Yoo, netdev, David Miller
In-Reply-To: <FD21003FF7E540DCA7F8F39A2BC046B4@realtek.com.tw>

On Wed, Feb 12, 2014 at 6:55 PM, hayeswang <hayeswang@realtek.com> wrote:
...
> According to the information from our FAE about the dangles for samsung,
> the default values of some device power are disabled or off. I don't know
> the history about this. However, it would cause the dangle no link, because
> the ecm driver wouldn't enable them. That is, the dangle couldn't use the
> ECM mode.

That makes sense. How can I check that?

> I think you should create a udev rule to change the configuration
> to 1 when the dangle is plugged. Then, it could load the vendor mode driver.

Which "configuration" are you referring to?
Something like this? /sys/devices/12110000.usb/usb3/3-2/bConfigurationValue

I'm happy to try any command line you give me.

If that command works, I can write the udev rule for future testing
with these devices.

> I don't think so. The reason is as above. Change the configuration of the
> device to 1 (vendor mode) is what you have to do.

Ok...just need one more bit of guidance from you and this will be
resolved I think.

cheers,
grant

^ permalink raw reply

* [PATCH net-next] tcp: add pacing_rate information into tcp_info
From: Eric Dumazet @ 2014-02-13 22:27 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

From: Eric Dumazet <edumazet@google.com>

Add two new fields to struct tcp_info, to report sk_pacing_rate
and sk_max_pacing_rate to monitoring applications, as ss from iproute2.

User exported fields are 64bit, even if kernel is currently using 32bit
fields.

lpaa5:~# ss -i 
..
	 skmem:(r0,rb357120,t0,tb2097152,f1584,w1980880,o0,bl0) ts sack cubic
wscale:6,6 rto:400 rtt:0.875/0.75 mss:1448 cwnd:1 ssthresh:12 send
13.2Mbps pacing_rate 3336.2Mbps unacked:15 retrans:1/5448 lost:15
rcv_space:29200

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/uapi/linux/tcp.h |    3 +++
 net/ipv4/tcp.c           |    5 +++++
 2 files changed, 8 insertions(+)

diff --git a/include/uapi/linux/tcp.h b/include/uapi/linux/tcp.h
index 377f1e59411d..3b9718328d8b 100644
--- a/include/uapi/linux/tcp.h
+++ b/include/uapi/linux/tcp.h
@@ -186,6 +186,9 @@ struct tcp_info {
 	__u32	tcpi_rcv_space;
 
 	__u32	tcpi_total_retrans;
+
+	__u64	tcpi_pacing_rate;
+	__u64	tcpi_max_pacing_rate;
 };
 
 /* for TCP_MD5SIG socket option */
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 9f3a2db9109e..bed379c7abcd 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2794,6 +2794,11 @@ void tcp_get_info(const struct sock *sk, struct tcp_info *info)
 	info->tcpi_rcv_space = tp->rcvq_space.space;
 
 	info->tcpi_total_retrans = tp->total_retrans;
+
+	info->tcpi_pacing_rate = sk->sk_pacing_rate != ~0U ?
+					sk->sk_pacing_rate : ~0ULL;
+	info->tcpi_max_pacing_rate = sk->sk_max_pacing_rate != ~0U ?
+					sk->sk_max_pacing_rate : ~0ULL;
 }
 EXPORT_SYMBOL_GPL(tcp_get_info);
 

^ permalink raw reply related

* [PATCH net-next v2 00/14] tipc: clean up media and bearer layer
From: Jon Maloy @ 2014-02-13 22:27 UTC (permalink / raw)
  To: davem
  Cc: netdev, Paul Gortmaker, erik.hugne, ying.xue, maloy,
	tipc-discussion, Jon Maloy

This commit series aims at facilitating future changes to the
locking policy around nodes, links and bearers.

Currently, we have a big read/write lock (net_lock) that is used for
serializing all changes to the node, link and bearer lists, as well
as to their mutual pointers and references.

But, in order to allow for concurrent access to the contents of these
structures, net_lock is only used in read mode by the data path code,
and hence a finer granular locking policy must be applied inside the
scope of net_lock: a spinlock (node_lock) for each node structure,
and another one (bearer_lock) for protection of bearer structures.

This locking policy has proved hard to maintain. We have several 
times encountered contention problems between node_lock and 
bearer_lock, and with the advent of the RCU locking mechanism we
feel it is anyway obsolete and ripe for improvements.

We now plan to replace net_lock with an RCU lock, as well as
getting rid of bearer_lock altogether. This will both reduce data
path overhead and make the code more manageable, while reducing the
risk of future lock contention problems.

Prior to these changes, we need to do some necessary cleanup and
code consolidation. This is what we do with this commit series,
before we finally remove bearer_lock. In a later series we will
replace net_lock with an RCU lock.

v2:
 - Re-inserted a removed kerneldoc entry in commit#5, based on 
   feedback from D. Miller.


Jon Maloy (9):
  tipc: stricter behavior of message reassembly function
  tipc: change reception of tunnelled duplicate packets
  tipc: change reception of tunnelled failover packets
  tipc: change signature of tunnelling reception function
  tipc: more cleanup of tunnelling reception function
  tipc: rename stack variables in function tipc_link_tunnel_rcv
  tipc: changes to general packet reception algorithm
  tipc: delay delete of link when failover is needed
  tipc: add node_lock protection to link lookup function

Ying Xue (5):
  tipc: move code for resetting links from bearer.c to link.c
  tipc: move code for deleting links from bearer.c to link.c
  tipc: redefine 'started' flag in struct link to bitmap
  tipc: remove 'links' list from tipc_bearer struct
  tipc: remove bearer_lock from tipc_bearer struct

 net/tipc/bcast.c  |    7 +-
 net/tipc/bearer.c |   42 ++----
 net/tipc/bearer.h |    7 +-
 net/tipc/core.c   |    2 +-
 net/tipc/link.c   |  432 ++++++++++++++++++++++++++++++-----------------------
 net/tipc/link.h   |   34 ++---
 net/tipc/node.c   |    8 +-
 7 files changed, 283 insertions(+), 249 deletions(-)

-- 
1.7.9.5

^ permalink raw reply

* Re: [PATCH 2/3] net: UDP gro_receive accept csum=0
From: Tom Herbert @ 2014-02-13 22:27 UTC (permalink / raw)
  To: Or Gerlitz
  Cc: David Miller, Linux Netdev List, Joseph Gasparakis, Jerry Chu,
	Eric Dumazet
In-Reply-To: <52FC7D0C.1020601@mellanox.com>

> Hi Tom,
>
> Considering the patch just "as is" vs. the current code, its OK.
>
> However, as skbs have only one indicator for the status of the checksum
> checks done by the receiving hardware, the basic assertion I thought we
> needed here is to reject skb if either it has the udp mark set or the
> encapsulation field is false, this is according to the conventions set by
> these two commits
>
> 0afb166 vxlan: Add capability of Rx checksum offload for inner packet
> 6a674e9 net: Add support for hardware-offloaded encapsulation
>
> B/c after finalizing the GRO work and decapsulating, skb injected up into
> the TCP stack with ip_summed equals to CHECKSUM_NONE are rejected. If this
> assumption is wrong, maybe we can remove testing the ip_summed field here
> altogether?
>
If I'm interpreting the semantics correctly, when skb->encapsulation
is set the ip_summed is valid for both the inner and outer header
(e.g. CHECKSUM_COMPLETE is always assumed okay for both layers). If
skb->encapsulation is not set then ip_summed is only valid for outer
header. So then the patch is broken in the case that encap is not set,
ip_summed is CHECKSUM_UNNECESSARY, csum == 0, and we need to validate
the inner checksum.

But even worse, is there a fundamental issue where udp4_csum_init is
able to change ip_summed to be CHECKSUM_UNNECESSARY (either check == 0
or ip_summed == CHECKSUM_UNNECESSARY) regardless of
skb->encapsulation, sending the packet into encap_rcv which could
ultimately incorrectly apply ip_summed on the inner TCP/UDP packet?

Tom

> Or.
>

^ permalink raw reply

* Re: [net-next 00/15] Intel Wired LAN Driver Updates
From: David Miller @ 2014-02-13 22:27 UTC (permalink / raw)
  To: aaron.f.brown; +Cc: netdev, gospo, sassmann
In-Reply-To: <1392292133-32120-1-git-send-email-aaron.f.brown@intel.com>

From: Aaron Brown <aaron.f.brown@intel.com>
Date: Thu, 13 Feb 2014 03:48:38 -0800

> This series contains updates to i40e and i40evf, primarily reset
> handling / refactoring along with a fair amount of minor cleanup.
> 
> Jesse fixes some spelling, bumps the version and other trivial fixes.
> Akeem sets a bit that is needed before shutdown in the case of 
> tx_timeout recovery failure.  Mitch refactors reset handling along 
> with a whole bunch of clean up.

Series applied, thanks.

^ permalink raw reply

* Re: [PATCH net-next] net: remove unnecessary return's
From: Julia Lawall @ 2014-02-13 22:28 UTC (permalink / raw)
  To: Dave Jones
  Cc: Julia Lawall, Joe Perches, Stephen Hemminger, David Miller,
	netdev
In-Reply-To: <20140213220021.GA1174@redhat.com>



On Thu, 13 Feb 2014, Dave Jones wrote:

> On Thu, Feb 13, 2014 at 10:55:23PM +0100, Julia Lawall wrote:
> 
>  > The patch below converts label: return; } to label: ; }.  I have only 
>  > scanned through the patches, not patched the code and looked at the 
>  > results, so I am not sure that it is completely correct.  On the other 
>  > hand, I'm also not sure that label: ; } is better than label: return; }, 
>  > either.  If people think it is, then I can cheack the results in more 
>  > detail.
> 
> Why not delete the label, and just replace the goto with a return if
> the label is at the end of the function ?

Here is an example.  Perhaps the uniformity of the if ... goto pattern is 
valuable, though?

julia

diff -u -p a/drivers/mfd/ab3100-core.c b/drivers/mfd/ab3100-core.c
--- a/drivers/mfd/ab3100-core.c
+++ b/drivers/mfd/ab3100-core.c
@@ -586,7 +586,7 @@ static void ab3100_setup_debugfs(struct
 
 	ab3100_dir = debugfs_create_dir("ab3100", NULL);
 	if (!ab3100_dir)
-		goto exit_no_debugfs;
+		return;
 
 	ab3100_reg_file = debugfs_create_file("registers",
 				S_IRUGO, ab3100_dir, ab3100,
@@ -623,7 +623,6 @@ static void ab3100_setup_debugfs(struct
 	debugfs_remove(ab3100_reg_file);
  exit_destroy_dir:
 	debugfs_remove(ab3100_dir);
- exit_no_debugfs:
 	return;
 }
 static inline void ab3100_remove_debugfs(void)

^ permalink raw reply

* [PATCH net-next v2 03/14] tipc: move code for deleting links from bearer.c to link.c
From: Jon Maloy @ 2014-02-13 22:29 UTC (permalink / raw)
  To: davem; +Cc: Jon Maloy, netdev, tipc-discussion
In-Reply-To: <1392330558-19048-1-git-send-email-jon.maloy@ericsson.com>

From: Ying Xue <ying.xue@windriver.com>

We break out the code for deleting attached links in the
function bearer_disable(), and define a new function named
tipc_link_delete_list() to do this job.

This commit incurs no functional changes, but makes the code of
function bearer_disable() cleaner. It is also a preparation
for a more important change to the bearer code, in a subsequent
commit in this series.

Signed-off-by: Ying Xue <ying.xue@windriver.com>
Reviewed-by: Paul Gortmaker <paul.gortmaker@windriver.com>
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
 net/tipc/bearer.c |    6 +-----
 net/tipc/link.c   |    9 +++++++++
 net/tipc/link.h   |    1 +
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
index a3bdf5c..a5be053 100644
--- a/net/tipc/bearer.c
+++ b/net/tipc/bearer.c
@@ -366,16 +366,12 @@ static int tipc_reset_bearer(struct tipc_bearer *b_ptr)
  */
 static void bearer_disable(struct tipc_bearer *b_ptr)
 {
-	struct tipc_link *l_ptr;
-	struct tipc_link *temp_l_ptr;
 	struct tipc_link_req *temp_req;
 
 	pr_info("Disabling bearer <%s>\n", b_ptr->name);
 	spin_lock_bh(&b_ptr->lock);
 	b_ptr->media->disable_media(b_ptr);
-	list_for_each_entry_safe(l_ptr, temp_l_ptr, &b_ptr->links, link_list) {
-		tipc_link_delete(l_ptr);
-	}
+	tipc_link_delete_list(b_ptr);
 	temp_req = b_ptr->link_req;
 	b_ptr->link_req = NULL;
 	spin_unlock_bh(&b_ptr->lock);
diff --git a/net/tipc/link.c b/net/tipc/link.c
index 3ff34e8..424e9f3 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -313,6 +313,15 @@ void tipc_link_delete(struct tipc_link *l_ptr)
 	kfree(l_ptr);
 }
 
+void tipc_link_delete_list(struct tipc_bearer *b_ptr)
+{
+	struct tipc_link *l_ptr;
+	struct tipc_link *temp_l_ptr;
+
+	list_for_each_entry_safe(l_ptr, temp_l_ptr, &b_ptr->links, link_list) {
+		tipc_link_delete(l_ptr);
+	}
+}
 
 /**
  * link_schedule_port - schedule port for deferred sending
diff --git a/net/tipc/link.h b/net/tipc/link.h
index 73beecb..994ebd1 100644
--- a/net/tipc/link.h
+++ b/net/tipc/link.h
@@ -219,6 +219,7 @@ void tipc_link_delete(struct tipc_link *l_ptr);
 void tipc_link_failover_send_queue(struct tipc_link *l_ptr);
 void tipc_link_dup_send_queue(struct tipc_link *l_ptr,
 			      struct tipc_link *dest);
+void tipc_link_delete_list(struct tipc_bearer *b_ptr);
 void tipc_link_reset_fragments(struct tipc_link *l_ptr);
 int tipc_link_is_up(struct tipc_link *l_ptr);
 int tipc_link_is_active(struct tipc_link *l_ptr);
-- 
1.7.9.5


------------------------------------------------------------------------------
Android apps run on BlackBerry 10
Introducing the new BlackBerry 10.2.1 Runtime for Android apps.
Now with support for Jelly Bean, Bluetooth, Mapview and more.
Get your Android app in front of a whole new audience.  Start now.
http://pubads.g.doubleclick.net/gampad/clk?id=124407151&iu=/4140/ostg.clktrk

^ permalink raw reply related

* [PATCH net-next v2 04/14] tipc: redefine 'started' flag in struct link to bitmap
From: Jon Maloy @ 2014-02-13 22:29 UTC (permalink / raw)
  To: davem; +Cc: Jon Maloy, netdev, tipc-discussion
In-Reply-To: <1392330558-19048-1-git-send-email-jon.maloy@ericsson.com>

From: Ying Xue <ying.xue@windriver.com>

Currently, the 'started' field in struct tipc_link represents only a
binary state, 'started' or 'not started'. We need it to represent
more link execution states in the coming commits in this series.
Hence, we rename the field to 'flags', and define the current
started/non-started state to be represented by the LSB bit of
that field.

Signed-off-by: Ying Xue <ying.xue@windriver.com>
Reviewed-by: Paul Gortmaker <paul.gortmaker@windriver.com>
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
 net/tipc/link.c |    4 ++--
 net/tipc/link.h |   22 +++++++++++-----------
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/net/tipc/link.c b/net/tipc/link.c
index 424e9f3..2070d03 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -500,7 +500,7 @@ static void link_state_event(struct tipc_link *l_ptr, unsigned int event)
 	struct tipc_link *other;
 	u32 cont_intv = l_ptr->continuity_interval;
 
-	if (!l_ptr->started && (event != STARTING_EVT))
+	if (!(l_ptr->flags & LINK_STARTED) && (event != STARTING_EVT))
 		return;		/* Not yet. */
 
 	/* Check whether changeover is going on */
@@ -626,7 +626,7 @@ static void link_state_event(struct tipc_link *l_ptr, unsigned int event)
 			link_set_timer(l_ptr, cont_intv);
 			break;
 		case STARTING_EVT:
-			l_ptr->started = 1;
+			l_ptr->flags |= LINK_STARTED;
 			/* fall through */
 		case TIMEOUT_EVT:
 			tipc_link_send_proto_msg(l_ptr, RESET_MSG, 0, 0, 0, 0, 0);
diff --git a/net/tipc/link.h b/net/tipc/link.h
index 994ebd1..a900e74 100644
--- a/net/tipc/link.h
+++ b/net/tipc/link.h
@@ -1,7 +1,7 @@
 /*
  * net/tipc/link.h: Include file for TIPC link code
  *
- * Copyright (c) 1995-2006, Ericsson AB
+ * Copyright (c) 1995-2006, 2013, Ericsson AB
  * Copyright (c) 2004-2005, 2010-2011, Wind River Systems
  * All rights reserved.
  *
@@ -40,27 +40,27 @@
 #include "msg.h"
 #include "node.h"
 
-/*
- * Link reassembly status codes
+/* Link reassembly status codes
  */
 #define LINK_REASM_ERROR	-1
 #define LINK_REASM_COMPLETE	1
 
-/*
- * Out-of-range value for link sequence numbers
+/* Out-of-range value for link sequence numbers
  */
 #define INVALID_LINK_SEQ 0x10000
 
-/*
- * Link states
+/* Link working states
  */
 #define WORKING_WORKING 560810u
 #define WORKING_UNKNOWN 560811u
 #define RESET_UNKNOWN   560812u
 #define RESET_RESET     560813u
 
-/*
- * Starting value for maximum packet size negotiation on unicast links
+/* Link endpoint execution states
+ */
+#define LINK_STARTED    0x0001
+
+/* Starting value for maximum packet size negotiation on unicast links
  * (unless bearer MTU is less)
  */
 #define MAX_PKT_DEFAULT 1500
@@ -103,7 +103,7 @@ struct tipc_stats {
  * @timer: link timer
  * @owner: pointer to peer node
  * @link_list: adjacent links in bearer's list of links
- * @started: indicates if link has been started
+ * @flags: execution state flags for link endpoint instance
  * @checkpoint: reference point for triggering link continuity checking
  * @peer_session: link session # being used by peer end of link
  * @peer_bearer_id: bearer id used by link's peer endpoint
@@ -152,7 +152,7 @@ struct tipc_link {
 	struct list_head link_list;
 
 	/* Management and link supervision data */
-	int started;
+	unsigned int flags;
 	u32 checkpoint;
 	u32 peer_session;
 	u32 peer_bearer_id;
-- 
1.7.9.5


------------------------------------------------------------------------------
Android apps run on BlackBerry 10
Introducing the new BlackBerry 10.2.1 Runtime for Android apps.
Now with support for Jelly Bean, Bluetooth, Mapview and more.
Get your Android app in front of a whole new audience.  Start now.
http://pubads.g.doubleclick.net/gampad/clk?id=124407151&iu=/4140/ostg.clktrk

^ permalink raw reply related

* [PATCH net-next v2 06/14] tipc: change reception of tunnelled duplicate packets
From: Jon Maloy @ 2014-02-13 22:29 UTC (permalink / raw)
  To: davem; +Cc: Jon Maloy, netdev, tipc-discussion
In-Reply-To: <1392330558-19048-1-git-send-email-jon.maloy@ericsson.com>

When a second link to a destination comes up, some sender sockets will
steer their subsequent traffic through the new link. In order to
guarantee preserved packet order and cardinality for those sockets, we
tunnel a duplicate of the old link's send queue through the new link
before we open it for regular traffic. The last arriving packet copy,
on whichever link, will be dropped at the receiving end based on the
original sequence number, to ensure that only one copy is delivered to
the end receiver.

In this commit, we change the algorithm for receiving DUPLICATE_MSG
packets, at the same time delegating it to a new subfunction,
tipc_link_dup_rcv(). Instead of returning an extracted inner packet to
the packet reception loop in tipc_rcv(), we just add it to the receiving
(new) link's deferred packet queue. The packet will then be processed by
that link when it receives its first non-tunneled packet, i.e., at
latest when the changeover procedure is finished.

Because tipc_link_tunnel_rcv()/tipc_link_dup_rcv() now is consuming all
packets of type DUPLICATE_MSG, the calling tipc_rcv() function can omit
testing for this. This in turn means that the current conditional jump
to the label 'protocol_check' becomes redundant, and we can remove that
label.

Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
Signed-off-by: Ying Xue <ying.xue@windriver.com>
---
 net/tipc/link.c |   53 ++++++++++++++++++++++++++++++++---------------------
 1 file changed, 32 insertions(+), 21 deletions(-)

diff --git a/net/tipc/link.c b/net/tipc/link.c
index e7e44ab..f227a38 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -1437,7 +1437,6 @@ void tipc_rcv(struct sk_buff *head, struct tipc_bearer *b_ptr)
 		u32 seq_no;
 		u32 ackd;
 		u32 released = 0;
-		int type;
 
 		head = head->next;
 		buf->next = NULL;
@@ -1525,7 +1524,6 @@ void tipc_rcv(struct sk_buff *head, struct tipc_bearer *b_ptr)
 		}
 
 		/* Now (finally!) process the incoming message */
-protocol_check:
 		if (unlikely(!link_working_working(l_ptr))) {
 			if (msg_user(msg) == LINK_PROTOCOL) {
 				link_recv_proto_msg(l_ptr, buf);
@@ -1599,15 +1597,11 @@ deliver:
 			tipc_node_unlock(n_ptr);
 			continue;
 		case CHANGEOVER_PROTOCOL:
-			type = msg_type(msg);
-			if (tipc_link_tunnel_rcv(&l_ptr, &buf)) {
-				msg = buf_msg(buf);
-				seq_no = msg_seqno(msg);
-				if (type == ORIGINAL_MSG)
-					goto deliver;
-				goto protocol_check;
-			}
-			break;
+			if (!tipc_link_tunnel_rcv(&l_ptr, &buf))
+				break;
+			msg = buf_msg(buf);
+			seq_no = msg_seqno(msg);
+			goto deliver;
 		default:
 			kfree_skb(buf);
 			buf = NULL;
@@ -2107,7 +2101,30 @@ static struct sk_buff *buf_extract(struct sk_buff *skb, u32 from_pos)
 	return eb;
 }
 
-/*  tipc_link_tunnel_rcv(): Receive a tunneled packet, sent
+
+
+/* tipc_link_dup_rcv(): Receive a tunnelled DUPLICATE_MSG packet.
+ * Owner node is locked.
+ */
+static void tipc_link_dup_rcv(struct tipc_link *l_ptr,
+			      struct sk_buff *t_buf)
+{
+	struct sk_buff *buf;
+
+	if (!tipc_link_is_up(l_ptr))
+		return;
+
+	buf = buf_extract(t_buf, INT_H_SIZE);
+	if (buf == NULL) {
+		pr_warn("%sfailed to extract inner dup pkt\n", link_co_err);
+		return;
+	}
+
+	/* Add buffer to deferred queue, if applicable: */
+	link_handle_out_of_seq_msg(l_ptr, buf);
+}
+
+/*  tipc_link_tunnel_rcv(): Receive a tunnelled packet, sent
  *  via other link as result of a failover (ORIGINAL_MSG) or
  *  a new active link (DUPLICATE_MSG). Failover packets are
  *  returned to the active link for delivery upwards.
@@ -2126,6 +2143,7 @@ static int tipc_link_tunnel_rcv(struct tipc_link **l_ptr,
 
 	if (bearer_id >= MAX_BEARERS)
 		goto exit;
+
 	dest_link = (*l_ptr)->owner->links[bearer_id];
 	if (!dest_link)
 		goto exit;
@@ -2138,15 +2156,8 @@ static int tipc_link_tunnel_rcv(struct tipc_link **l_ptr,
 	msg = msg_get_wrapped(tunnel_msg);
 
 	if (msg_typ == DUPLICATE_MSG) {
-		if (less(msg_seqno(msg), mod(dest_link->next_in_no)))
-			goto exit;
-		*buf = buf_extract(tunnel_buf, INT_H_SIZE);
-		if (*buf == NULL) {
-			pr_warn("%sduplicate msg dropped\n", link_co_err);
-			goto exit;
-		}
-		kfree_skb(tunnel_buf);
-		return 1;
+		tipc_link_dup_rcv(dest_link, tunnel_buf);
+		goto exit;
 	}
 
 	/* First original message ?: */
-- 
1.7.9.5


------------------------------------------------------------------------------
Android apps run on BlackBerry 10
Introducing the new BlackBerry 10.2.1 Runtime for Android apps.
Now with support for Jelly Bean, Bluetooth, Mapview and more.
Get your Android app in front of a whole new audience.  Start now.
http://pubads.g.doubleclick.net/gampad/clk?id=124407151&iu=/4140/ostg.clktrk

^ permalink raw reply related

* [PATCH net-next v2 08/14] tipc: change signature of tunnelling reception function
From: Jon Maloy @ 2014-02-13 22:29 UTC (permalink / raw)
  To: davem; +Cc: Jon Maloy, netdev, tipc-discussion
In-Reply-To: <1392330558-19048-1-git-send-email-jon.maloy@ericsson.com>

After the earlier commits in this series related to the function
tipc_link_tunnel_rcv(), we can now go further and simplify its
signature.

The function now consumes all DUPLICATE packets, and only returns such
ORIGINAL packets that are ready for immediate delivery, i.e., no
more link level protocol processing needs to be done by the caller.
As a consequence, the the caller, tipc_rcv(), does not access the link
pointer after call return, and it becomes unnecessary to pass a link
pointer reference in the call. Instead, we now only pass it the tunnel
link's owner node, which is sufficient to find the destination link for
the tunnelled packet.

Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
Reviewed-by: Ying Xue <ying.xue@windriver.com>
---
 net/tipc/link.c |   14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/net/tipc/link.c b/net/tipc/link.c
index 26a54f4..f9f9068 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -78,7 +78,7 @@ static const char *link_unk_evt = "Unknown link event ";
 static void link_handle_out_of_seq_msg(struct tipc_link *l_ptr,
 				       struct sk_buff *buf);
 static void link_recv_proto_msg(struct tipc_link *l_ptr, struct sk_buff *buf);
-static int  tipc_link_tunnel_rcv(struct tipc_link **l_ptr,
+static int  tipc_link_tunnel_rcv(struct tipc_node *n_ptr,
 				 struct sk_buff **buf);
 static void link_set_supervision_props(struct tipc_link *l_ptr, u32 tolerance);
 static int  link_send_sections_long(struct tipc_port *sender,
@@ -1597,7 +1597,7 @@ deliver:
 			tipc_node_unlock(n_ptr);
 			continue;
 		case CHANGEOVER_PROTOCOL:
-			if (!tipc_link_tunnel_rcv(&l_ptr, &buf))
+			if (!tipc_link_tunnel_rcv(n_ptr, &buf))
 				break;
 			msg = buf_msg(buf);
 			seq_no = msg_seqno(msg);
@@ -2174,7 +2174,7 @@ exit:
  *  returned to the active link for delivery upwards.
  *  Owner node is locked.
  */
-static int tipc_link_tunnel_rcv(struct tipc_link **l_ptr,
+static int tipc_link_tunnel_rcv(struct tipc_node *n_ptr,
 				struct sk_buff **buf)
 {
 	struct sk_buff *tunnel_buf = *buf;
@@ -2186,15 +2186,9 @@ static int tipc_link_tunnel_rcv(struct tipc_link **l_ptr,
 	if (bearer_id >= MAX_BEARERS)
 		goto exit;
 
-	dest_link = (*l_ptr)->owner->links[bearer_id];
+	dest_link = n_ptr->links[bearer_id];
 	if (!dest_link)
 		goto exit;
-	if (dest_link == *l_ptr) {
-		pr_err("Unexpected changeover message on link <%s>\n",
-		       (*l_ptr)->name);
-		goto exit;
-	}
-	*l_ptr = dest_link;
 
 	if (msg_typ == DUPLICATE_MSG) {
 		tipc_link_dup_rcv(dest_link, tunnel_buf);
-- 
1.7.9.5


------------------------------------------------------------------------------
Android apps run on BlackBerry 10
Introducing the new BlackBerry 10.2.1 Runtime for Android apps.
Now with support for Jelly Bean, Bluetooth, Mapview and more.
Get your Android app in front of a whole new audience.  Start now.
http://pubads.g.doubleclick.net/gampad/clk?id=124407151&iu=/4140/ostg.clktrk

^ permalink raw reply related

* [PATCH net-next v2 13/14] tipc: remove bearer_lock from tipc_bearer struct
From: Jon Maloy @ 2014-02-13 22:29 UTC (permalink / raw)
  To: davem; +Cc: Jon Maloy, netdev, tipc-discussion
In-Reply-To: <1392330558-19048-1-git-send-email-jon.maloy@ericsson.com>

From: Ying Xue <ying.xue@windriver.com>

After the earlier commits ("tipc: remove 'links' list from
tipc_bearer struct") and ("tipc: introduce new spinlock to protect
struct link_req"), there is no longer any need to protect struct
link_req or or any link list by use of bearer_lock. Furthermore,
we have eliminated the need for using bearer_lock during downcalls
(send) from the link to the bearer, since we have ensured that
bearers always have a longer life cycle that their associated links,
and always contain valid data.

So, the only need now for a lock protecting bearers is for guaranteeing
consistency of the bearer list itself. For this, it is sufficient, at
least for the time being, to continue applying 'net_lock´ in write mode.

By removing bearer_lock we also pre-empt introduction of issue b) descibed
in the previous commit "tipc: remove 'links' list from tipc_bearer struct":

"b) When the outer protection from net_lock is gone, taking
    bearer_lock and node_lock in opposite order of method 1) and 2)
    will become an obvious deadlock hazard".

Therefore, we now eliminate the bearer_lock spinlock.

Signed-off-by: Ying Xue <ying.xue@windriver.com>
Reviewed-by: Paul Gortmaker <paul.gortmaker@windriver.com>
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
 net/tipc/bcast.c  |    1 -
 net/tipc/bearer.c |   16 +++-------------
 net/tipc/bearer.h |    5 +----
 3 files changed, 4 insertions(+), 18 deletions(-)

diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c
index af35f76..06a639c 100644
--- a/net/tipc/bcast.c
+++ b/net/tipc/bcast.c
@@ -785,7 +785,6 @@ void tipc_bclink_init(void)
 	bcl->owner = &bclink->node;
 	bcl->max_pkt = MAX_PKT_DEFAULT_MCAST;
 	tipc_link_set_queue_limits(bcl, BCLINK_WIN_DEFAULT);
-	spin_lock_init(&bcbearer->bearer.lock);
 	bcl->b_ptr = &bcbearer->bearer;
 	bcl->state = WORKING_WORKING;
 	strlcpy(bcl->name, tipc_bclink_name, TIPC_MAX_LINK_NAME);
diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
index 60caa45..242cddd 100644
--- a/net/tipc/bearer.c
+++ b/net/tipc/bearer.c
@@ -327,7 +327,6 @@ restart:
 	b_ptr->net_plane = bearer_id + 'A';
 	b_ptr->active = 1;
 	b_ptr->priority = priority;
-	spin_lock_init(&b_ptr->lock);
 
 	res = tipc_disc_create(b_ptr, &b_ptr->bcast_addr, disc_domain);
 	if (res) {
@@ -351,9 +350,7 @@ static int tipc_reset_bearer(struct tipc_bearer *b_ptr)
 {
 	read_lock_bh(&tipc_net_lock);
 	pr_info("Resetting bearer <%s>\n", b_ptr->name);
-	spin_lock_bh(&b_ptr->lock);
 	tipc_link_reset_list(b_ptr->identity);
-	spin_unlock_bh(&b_ptr->lock);
 	read_unlock_bh(&tipc_net_lock);
 	return 0;
 }
@@ -365,19 +362,12 @@ static int tipc_reset_bearer(struct tipc_bearer *b_ptr)
  */
 static void bearer_disable(struct tipc_bearer *b_ptr, bool shutting_down)
 {
-	struct tipc_link_req *temp_req;
-
 	pr_info("Disabling bearer <%s>\n", b_ptr->name);
-	spin_lock_bh(&b_ptr->lock);
 	b_ptr->media->disable_media(b_ptr);
-	tipc_link_delete_list(b_ptr->identity, shutting_down);
-	temp_req = b_ptr->link_req;
-	b_ptr->link_req = NULL;
-	spin_unlock_bh(&b_ptr->lock);
-
-	if (temp_req)
-		tipc_disc_delete(temp_req);
 
+	tipc_link_delete_list(b_ptr->identity, shutting_down);
+	if (b_ptr->link_req)
+		tipc_disc_delete(b_ptr->link_req);
 	memset(b_ptr, 0, sizeof(struct tipc_bearer));
 }
 
diff --git a/net/tipc/bearer.h b/net/tipc/bearer.h
index 647cb1d..425dd81 100644
--- a/net/tipc/bearer.h
+++ b/net/tipc/bearer.h
@@ -107,10 +107,8 @@ struct tipc_media {
 
 /**
  * struct tipc_bearer - Generic TIPC bearer structure
- * @dev: ptr to associated network device
- * @usr_handle: pointer to additional media-specific information about bearer
+ * @media_ptr: pointer to additional media-specific information about bearer
  * @mtu: max packet size bearer can support
- * @lock: spinlock for controlling access to bearer
  * @addr: media-specific address associated with bearer
  * @name: bearer name (format = media:interface)
  * @media: ptr to media structure associated with bearer
@@ -133,7 +131,6 @@ struct tipc_bearer {
 	u32 mtu;				/* initalized by media */
 	struct tipc_media_addr addr;		/* initalized by media */
 	char name[TIPC_MAX_BEARER_NAME];
-	spinlock_t lock;
 	struct tipc_media *media;
 	struct tipc_media_addr bcast_addr;
 	u32 priority;
-- 
1.7.9.5


------------------------------------------------------------------------------
Android apps run on BlackBerry 10
Introducing the new BlackBerry 10.2.1 Runtime for Android apps.
Now with support for Jelly Bean, Bluetooth, Mapview and more.
Get your Android app in front of a whole new audience.  Start now.
http://pubads.g.doubleclick.net/gampad/clk?id=124407151&iu=/4140/ostg.clktrk
_______________________________________________
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox