Skip to contentSkip to conversation

drivers/scsi: Use DIV_ROUND_UP

PATCH In review [email protected] 3 participants 4 messages Sep 22, 2008 – Sep 24, 2008 SCSI

lore
Share & tools
lore

[email protected]

author
#0 lore raw

PATCH 15/17 drivers/scsi: Use DIV_ROUND_UP

From: Julia Lawall <[email protected]>

The kernel.h macro DIV_ROUND_UP performs the computation (((n) + (d) - 1) / (d)) but is perhaps more readable.

An extract of the semantic patch that makes this change is as follows:
(http://www.emn.fr/x-info/coccinelle/)

// <smpl>
@haskernel@
@@

#include <linux/kernel.h>

@depends on haskernel@
expression n,d;
@@

(
- (n + d - 1) / d
+ DIV_ROUND_UP(n,d)
|
- (n + (d - 1)) / d
+ DIV_ROUND_UP(n,d)
)

@depends on haskernel@
expression n,d;
@@

- DIV_ROUND_UP((n),d)
+ DIV_ROUND_UP(n,d)

@depends on haskernel@
expression n,d;
@@

- DIV_ROUND_UP(n,(d))
+ DIV_ROUND_UP(n,d)
// </smpl>

Signed-off-by Julia Lawall <[email protected]>
Cc James Bottomley <[email protected]>
Signed-off-by Andrew Morton <[email protected]>

---

 drivers/scsi/esp_scsi.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/scsi/esp_scsi.c renamed from drivers/scsi/esp_scsi.c~drivers-scsi-use-div_round_up +1 -1
diff -puN drivers/scsi/esp_scsi.c~drivers-scsi-use-div_round_up drivers/scsi/esp_scsi.c
@@ -1464,7 +1464,7 @@ static void esp_msgin_sdtr(struct esp *e
 
 		one_clock = esp->ccycle / 1000;
 		rounded_up = (period << 2);
-		rounded_up = (rounded_up + one_clock - 1) / one_clock;
+		rounded_up = DIV_ROUND_UP(rounded_up, one_clock);
 		stp = rounded_up;
 		if (stp && esp->rev >= FAS236) {
 			if (stp >= 50)

_

#1 lore raw

drivers/scsi/esp_scsi.c static void esp_msgin_sdtr(struct esp *e · line 1470

Quoted 8 lines · On Mon, Sep 22, 2008 at 02:56:51PM -0700, [email protected] wrote:
drivers/scsi/esp_scsi.c renamed from drivers/scsi/esp_scsi.c~drivers-scsi-use-div_round_up +1 -1
 		one_clock = esp->ccycle / 1000;
 		rounded_up = (period << 2);
-		rounded_up = (rounded_up + one_clock - 1) / one_clock;
+		rounded_up = DIV_ROUND_UP(rounded_up, one_clock);
 		stp = rounded_up;
 		if (stp && esp->rev >= FAS236) {
 			if (stp >= 50)

Is this not exactly equivalent to:

		one_clock = esp->ccycle / 1000;
		stp = DIV_ROUND_UP(period << 2, one_clock);

(rounded_up is local to this if condition).

Signature · 4 lines
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."
#2 lore raw
Quoted 12 lines · Matthew Wilcox

Review

Quoted 8 lines · On Mon, Sep 22, 2008 at 02:56:51PM -0700, [email protected] wrote:
(quoted diff) +1 -1
 		one_clock = esp->ccycle / 1000;
 		rounded_up = (period << 2);
-		rounded_up = (rounded_up + one_clock - 1) / one_clock;
+		rounded_up = DIV_ROUND_UP(rounded_up, one_clock);
 		stp = rounded_up;
 		if (stp && esp->rev >= FAS236) {
 			if (stp >= 50)

Is this not exactly equivalent to:

Quoted 4 lines
		one_clock = esp->ccycle / 1000;
		stp = DIV_ROUND_UP(period << 2, one_clock);

(rounded_up is local to this if condition).

Agreed. An updated patch is below.

julia

--------------------

From: Julia Lawall <[email protected]>

Use the macro DIV_ROUND_UP and eliminate the variable rounded_up, as suggested by Matthew Wilcox.

Signed-off-by Julia Lawall <[email protected]>

---

 drivers/scsi/esp_scsi.c       |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)
drivers/scsi/esp_scsi.c +2 -4
index 62a4618..a680e18 100644
@@ -1453,7 +1453,7 @@ static void esp_msgin_sdtr(struct esp *esp, struct esp_target_data *tp)
 		offset = 0;
 
 	if (offset) {
-		int rounded_up, one_clock;
+		int one_clock;
 
 		if (period > esp->max_period) {
 			period = offset = 0;

@@ -1463,9 +1463,7 @@ static void esp_msgin_sdtr(struct esp *esp, struct esp_target_data *tp)
 			goto do_reject;
 
 		one_clock = esp->ccycle / 1000;
-		rounded_up = (period << 2);
-		rounded_up = (rounded_up + one_clock - 1) / one_clock;
-		stp = rounded_up;
+		stp = DIV_ROUND_UP(period << 2, one_clock);
 		if (stp && esp->rev >= FAS236) {
 			if (stp >= 50)
 				stp--;
Reviewed-by
#3 lore raw
Quoted 8 lines · Julia Lawall

--------------------

From: Julia Lawall <[email protected]>

Use the macro DIV_ROUND_UP and eliminate the variable rounded_up, as suggested by Matthew Wilcox.

Signed-off-by Julia Lawall <[email protected]>
Reviewed-by Matthew Wilcox <[email protected]>
Quoted 33 lines

---

 drivers/scsi/esp_scsi.c       |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)
drivers/scsi/esp_scsi.c +2 -5
index 62a4618..a680e18 100644
@@ -1453,7 +1453,7 @@ static void esp_msgin_sdtr(struct esp *esp, struct esp_target_data *tp)
 		offset = 0;
 
 	if (offset) {
-		int rounded_up, one_clock;
+		int one_clock;
 
 		if (period > esp->max_period) {
 			period = offset = 0;

@@ -1463,9 +1463,7 @@ static void esp_msgin_sdtr(struct esp *esp, struct esp_target_data *tp)
 			goto do_reject;
 
 		one_clock = esp->ccycle / 1000;
-		rounded_up = (period << 2);
-		rounded_up = (rounded_up + one_clock - 1) / one_clock;
-		stp = rounded_up;
+		stp = DIV_ROUND_UP(period << 2, one_clock);
 		if (stp && esp->rev >= FAS236) {
 			if (stp >= 50)
 				stp--;
--

To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 4 lines
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."