2015-11-27 21:26:47

by Fabian Frédérick

[permalink] [raw]
Subject: [PATCH 00/21] drbd: clean-up patchset

This small patchset does some code clean-up in drivers/block/drbd tree.

Fabian Frederick (21):
drbd: debugfs: constify drbd_version_fops
drbd: simplify drbd_md_set_sector_offsets()
drbd: use seq_put instead of seq_print where possible
drbd: include linux/uaccess.h instead of asm/uaccess.h
drbd: rework is_valid_state()
drbd: use const char * const for drbd strings
drbd: kerneldoc warning fix in w_e_end_data_req()
drbd: use unsigned for one bit fields
drbd: use bool for peer is_ states
drbd: fix typo
drbd: use | for bitmask combination
drbd: use true/false for bool
drbd: fix drbd_bm_init() comments
drbd: clean-up receive_SyncParam()
drbd: introduce peer state union
drbd: fix maybe_pull_ahead() locking comments
drbd: use bool for growing
drbd: remove redundant declarations
drbd: remove bm_vk_free()
drbd: remove unused definitions
drbd: replace if/BUG by BUG_ON

drivers/block/drbd/drbd_bitmap.c | 19 +++-----
drivers/block/drbd/drbd_debugfs.c | 2 +-
drivers/block/drbd/drbd_int.h | 7 ---
drivers/block/drbd/drbd_interval.h | 14 +++---
drivers/block/drbd/drbd_main.c | 2 +-
drivers/block/drbd/drbd_nl.c | 37 ++++++++--------
drivers/block/drbd/drbd_proc.c | 30 ++++++-------
drivers/block/drbd/drbd_receiver.c | 22 +++++-----
drivers/block/drbd/drbd_req.c | 2 +-
drivers/block/drbd/drbd_state.c | 90 +++++++++++++++++---------------------
drivers/block/drbd/drbd_state.h | 2 +-
drivers/block/drbd/drbd_strings.c | 8 ++--
drivers/block/drbd/drbd_worker.c | 9 ++--
include/linux/drbd.h | 8 ++++
14 files changed, 116 insertions(+), 136 deletions(-)

--
2.1.4


2015-11-27 21:27:26

by Fabian Frédérick

[permalink] [raw]
Subject: [PATCH 01/21] drbd: debugfs: constify drbd_version_fops

drbd_version_fops is used in debugfs_create_file() as const

Signed-off-by: Fabian Frederick <[email protected]>
---
drivers/block/drbd/drbd_debugfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/block/drbd/drbd_debugfs.c b/drivers/block/drbd/drbd_debugfs.c
index 96a0107..abccffa 100644
--- a/drivers/block/drbd/drbd_debugfs.c
+++ b/drivers/block/drbd/drbd_debugfs.c
@@ -908,7 +908,7 @@ static int drbd_version_open(struct inode *inode, struct file *file)
return single_open(file, drbd_version_show, NULL);
}

-static struct file_operations drbd_version_fops = {
+static const struct file_operations drbd_version_fops = {
.owner = THIS_MODULE,
.open = drbd_version_open,
.llseek = seq_lseek,
--
2.1.4

2015-11-27 21:27:29

by Fabian Frédérick

[permalink] [raw]
Subject: [PATCH 02/21] drbd: simplify drbd_md_set_sector_offsets()

Some metadata informations were duplicated in layout switch.

Signed-off-by: Fabian Frederick <[email protected]>
---
drivers/block/drbd/drbd_nl.c | 24 +++++++++++-------------
1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/block/drbd/drbd_nl.c b/drivers/block/drbd/drbd_nl.c
index c055c5e..ef136a9 100644
--- a/drivers/block/drbd/drbd_nl.c
+++ b/drivers/block/drbd/drbd_nl.c
@@ -794,20 +794,9 @@ static void drbd_md_set_sector_offsets(struct drbd_device *device,
unsigned int al_size_sect = bdev->md.al_size_4k * 8;

bdev->md.md_offset = drbd_md_ss(bdev);
+ md_size_sect = MD_128MB_SECT;

switch (bdev->md.meta_dev_idx) {
- default:
- /* v07 style fixed size indexed meta data */
- bdev->md.md_size_sect = MD_128MB_SECT;
- bdev->md.al_offset = MD_4kB_SECT;
- bdev->md.bm_offset = MD_4kB_SECT + al_size_sect;
- break;
- case DRBD_MD_INDEX_FLEX_EXT:
- /* just occupy the full device; unit: sectors */
- bdev->md.md_size_sect = drbd_get_capacity(bdev->md_bdev);
- bdev->md.al_offset = MD_4kB_SECT;
- bdev->md.bm_offset = MD_4kB_SECT + al_size_sect;
- break;
case DRBD_MD_INDEX_INTERNAL:
case DRBD_MD_INDEX_FLEX_INT:
/* al size is still fixed */
@@ -822,11 +811,20 @@ static void drbd_md_set_sector_offsets(struct drbd_device *device,
* and the activity log; */
md_size_sect += MD_4kB_SECT + al_size_sect;

- bdev->md.md_size_sect = md_size_sect;
/* bitmap offset is adjusted by 'super' block size */
bdev->md.bm_offset = -md_size_sect + MD_4kB_SECT;
break;
+ case DRBD_MD_INDEX_FLEX_EXT:
+ /* just occupy the full device; unit: sectors */
+ md_size_sect = drbd_get_capacity(bdev->md_bdev);
+ default:
+ /* v07 style fixed size indexed meta data */
+ bdev->md.al_offset = MD_4kB_SECT;
+ bdev->md.bm_offset = MD_4kB_SECT + al_size_sect;
+ break;
}
+
+ bdev->md.md_size_sect = md_size_sect;
}

/* input size is expected to be in KB */
--
2.1.4

2015-11-27 21:27:34

by Fabian Frédérick

[permalink] [raw]
Subject: [PATCH 03/21] drbd: use seq_put instead of seq_print where possible

seq_putc/seq_puts give the following improvements:

text data bss dec hex filename
4661 0 8 4669 123d drivers/block/drbd/drbd_proc.o
4603 0 8 4611 1203 drivers/block/drbd/drbd_proc-after.o

Signed-off-by: Fabian Frederick <[email protected]>
---
drivers/block/drbd/drbd_proc.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/block/drbd/drbd_proc.c b/drivers/block/drbd/drbd_proc.c
index 6537b25..233a710 100644
--- a/drivers/block/drbd/drbd_proc.c
+++ b/drivers/block/drbd/drbd_proc.c
@@ -122,18 +122,18 @@ static void drbd_syncer_progress(struct drbd_device *device, struct seq_file *se

x = res/50;
y = 20-x;
- seq_printf(seq, "\t[");
+ seq_puts(seq, "\t[");
for (i = 1; i < x; i++)
- seq_printf(seq, "=");
- seq_printf(seq, ">");
+ seq_putc(seq, '=');
+ seq_putc(seq, '>');
for (i = 0; i < y; i++)
seq_printf(seq, ".");
- seq_printf(seq, "] ");
+ seq_puts(seq, "] ");

if (state.conn == C_VERIFY_S || state.conn == C_VERIFY_T)
- seq_printf(seq, "verified:");
+ seq_puts(seq, "verified:");
else
- seq_printf(seq, "sync'ed:");
+ seq_puts(seq, "sync'ed:");
seq_printf(seq, "%3u.%u%% ", res / 10, res % 10);

/* if more than a few GB, display in MB */
@@ -146,7 +146,7 @@ static void drbd_syncer_progress(struct drbd_device *device, struct seq_file *se
(unsigned long) Bit2KB(rs_left),
(unsigned long) Bit2KB(rs_total));

- seq_printf(seq, "\n\t");
+ seq_puts(seq, "\n\t");

/* see drivers/md/md.c
* We do not want to overflow, so the order of operands and
@@ -175,9 +175,9 @@ static void drbd_syncer_progress(struct drbd_device *device, struct seq_file *se
rt / 3600, (rt % 3600) / 60, rt % 60);

dbdt = Bit2KB(db/dt);
- seq_printf(seq, " speed: ");
+ seq_puts(seq, " speed: ");
seq_printf_with_thousands_grouping(seq, dbdt);
- seq_printf(seq, " (");
+ seq_puts(seq, " (");
/* ------------------------- ~3s average ------------------------ */
if (proc_details >= 1) {
/* this is what drbd_rs_should_slow_down() uses */
@@ -188,7 +188,7 @@ static void drbd_syncer_progress(struct drbd_device *device, struct seq_file *se
db = device->rs_mark_left[i] - rs_left;
dbdt = Bit2KB(db/dt);
seq_printf_with_thousands_grouping(seq, dbdt);
- seq_printf(seq, " -- ");
+ seq_puts(seq, " -- ");
}

/* --------------------- long term average ---------------------- */
@@ -200,11 +200,11 @@ static void drbd_syncer_progress(struct drbd_device *device, struct seq_file *se
db = rs_total - rs_left;
dbdt = Bit2KB(db/dt);
seq_printf_with_thousands_grouping(seq, dbdt);
- seq_printf(seq, ")");
+ seq_putc(seq, ')');

if (state.conn == C_SYNC_TARGET ||
state.conn == C_VERIFY_S) {
- seq_printf(seq, " want: ");
+ seq_puts(seq, " want: ");
seq_printf_with_thousands_grouping(seq, device->c_sync_rate);
}
seq_printf(seq, " K/sec%s\n", stalled ? " (stalled)" : "");
@@ -231,7 +231,7 @@ static void drbd_syncer_progress(struct drbd_device *device, struct seq_file *se
(unsigned long long)bm_bits * BM_SECT_PER_BIT);
if (stop_sector != 0 && stop_sector != ULLONG_MAX)
seq_printf(seq, " stop sector: %llu", stop_sector);
- seq_printf(seq, "\n");
+ seq_putc(seq, '\n');
}
}

@@ -276,7 +276,7 @@ static int drbd_seq_show(struct seq_file *seq, void *v)
rcu_read_lock();
idr_for_each_entry(&drbd_devices, device, i) {
if (prev_i != i - 1)
- seq_printf(seq, "\n");
+ seq_putc(seq, '\n');
prev_i = i;

state = device->state;
--
2.1.4

2015-11-27 21:27:37

by Fabian Frédérick

[permalink] [raw]
Subject: [PATCH 04/21] drbd: include linux/uaccess.h instead of asm/uaccess.h

Signed-off-by: Fabian Frederick <[email protected]>
---
drivers/block/drbd/drbd_main.c | 2 +-
drivers/block/drbd/drbd_proc.c | 2 +-
drivers/block/drbd/drbd_receiver.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/block/drbd/drbd_main.c b/drivers/block/drbd/drbd_main.c
index 5b43dfb..be08885 100644
--- a/drivers/block/drbd/drbd_main.c
+++ b/drivers/block/drbd/drbd_main.c
@@ -31,7 +31,7 @@
#include <linux/module.h>
#include <linux/jiffies.h>
#include <linux/drbd.h>
-#include <asm/uaccess.h>
+#include <linux/uaccess.h>
#include <asm/types.h>
#include <net/sock.h>
#include <linux/ctype.h>
diff --git a/drivers/block/drbd/drbd_proc.c b/drivers/block/drbd/drbd_proc.c
index 233a710..be2b93f 100644
--- a/drivers/block/drbd/drbd_proc.c
+++ b/drivers/block/drbd/drbd_proc.c
@@ -25,7 +25,7 @@

#include <linux/module.h>

-#include <asm/uaccess.h>
+#include <linux/uaccess.h>
#include <linux/fs.h>
#include <linux/file.h>
#include <linux/proc_fs.h>
diff --git a/drivers/block/drbd/drbd_receiver.c b/drivers/block/drbd/drbd_receiver.c
index 1957fe8..1c72991 100644
--- a/drivers/block/drbd/drbd_receiver.c
+++ b/drivers/block/drbd/drbd_receiver.c
@@ -25,7 +25,7 @@

#include <linux/module.h>

-#include <asm/uaccess.h>
+#include <linux/uaccess.h>
#include <net/sock.h>

#include <linux/drbd.h>
--
2.1.4

2015-11-27 21:27:41

by Fabian Frédérick

[permalink] [raw]
Subject: [PATCH 05/21] drbd: rework is_valid_state()

Remove empty condition to avoid semantic warnings

Signed-off-by: Fabian Frederick <[email protected]>
---
drivers/block/drbd/drbd_state.c | 86 ++++++++++++++++++-----------------------
1 file changed, 38 insertions(+), 48 deletions(-)

diff --git a/drivers/block/drbd/drbd_state.c b/drivers/block/drbd/drbd_state.c
index 5a7ef78..a7631a3 100644
--- a/drivers/block/drbd/drbd_state.c
+++ b/drivers/block/drbd/drbd_state.c
@@ -813,54 +813,44 @@ is_valid_state(struct drbd_device *device, union drbd_state ns)
}
}

- if (rv <= 0)
- /* already found a reason to abort */;
- else if (ns.role == R_SECONDARY && device->open_cnt)
- rv = SS_DEVICE_IN_USE;
-
- else if (ns.role == R_PRIMARY && ns.conn < C_CONNECTED && ns.disk < D_UP_TO_DATE)
- rv = SS_NO_UP_TO_DATE_DISK;
-
- else if (fp >= FP_RESOURCE &&
- ns.role == R_PRIMARY && ns.conn < C_CONNECTED && ns.pdsk >= D_UNKNOWN)
- rv = SS_PRIMARY_NOP;
-
- else if (ns.role == R_PRIMARY && ns.disk <= D_INCONSISTENT && ns.pdsk <= D_INCONSISTENT)
- rv = SS_NO_UP_TO_DATE_DISK;
-
- else if (ns.conn > C_CONNECTED && ns.disk < D_INCONSISTENT)
- rv = SS_NO_LOCAL_DISK;
-
- else if (ns.conn > C_CONNECTED && ns.pdsk < D_INCONSISTENT)
- rv = SS_NO_REMOTE_DISK;
-
- else if (ns.conn > C_CONNECTED && ns.disk < D_UP_TO_DATE && ns.pdsk < D_UP_TO_DATE)
- rv = SS_NO_UP_TO_DATE_DISK;
-
- else if ((ns.conn == C_CONNECTED ||
- ns.conn == C_WF_BITMAP_S ||
- ns.conn == C_SYNC_SOURCE ||
- ns.conn == C_PAUSED_SYNC_S) &&
- ns.disk == D_OUTDATED)
- rv = SS_CONNECTED_OUTDATES;
-
- else if ((ns.conn == C_VERIFY_S || ns.conn == C_VERIFY_T) &&
- (nc->verify_alg[0] == 0))
- rv = SS_NO_VERIFY_ALG;
-
- else if ((ns.conn == C_VERIFY_S || ns.conn == C_VERIFY_T) &&
- first_peer_device(device)->connection->agreed_pro_version < 88)
- rv = SS_NOT_SUPPORTED;
-
- else if (ns.role == R_PRIMARY && ns.disk < D_UP_TO_DATE && ns.pdsk < D_UP_TO_DATE)
- rv = SS_NO_UP_TO_DATE_DISK;
-
- else if ((ns.conn == C_STARTING_SYNC_S || ns.conn == C_STARTING_SYNC_T) &&
- ns.pdsk == D_UNKNOWN)
- rv = SS_NEED_CONNECTION;
-
- else if (ns.conn >= C_CONNECTED && ns.pdsk == D_UNKNOWN)
- rv = SS_CONNECTED_OUTDATES;
+ if (rv > 0) {
+ if (ns.role == R_SECONDARY && device->open_cnt)
+ rv = SS_DEVICE_IN_USE;
+ else if (ns.role == R_PRIMARY && ns.conn < C_CONNECTED &&
+ ns.disk < D_UP_TO_DATE)
+ rv = SS_NO_UP_TO_DATE_DISK;
+ else if (fp >= FP_RESOURCE && ns.role == R_PRIMARY &&
+ ns.conn < C_CONNECTED && ns.pdsk >= D_UNKNOWN)
+ rv = SS_PRIMARY_NOP;
+ else if (ns.role == R_PRIMARY && ns.disk <= D_INCONSISTENT &&
+ ns.pdsk <= D_INCONSISTENT)
+ rv = SS_NO_UP_TO_DATE_DISK;
+ else if (ns.conn > C_CONNECTED && ns.disk < D_INCONSISTENT)
+ rv = SS_NO_LOCAL_DISK;
+ else if (ns.conn > C_CONNECTED && ns.pdsk < D_INCONSISTENT)
+ rv = SS_NO_REMOTE_DISK;
+ else if (ns.conn > C_CONNECTED && ns.disk < D_UP_TO_DATE &&
+ ns.pdsk < D_UP_TO_DATE)
+ rv = SS_NO_UP_TO_DATE_DISK;
+ else if ((ns.conn == C_CONNECTED || ns.conn == C_WF_BITMAP_S ||
+ ns.conn == C_SYNC_SOURCE ||
+ ns.conn == C_PAUSED_SYNC_S) && ns.disk == D_OUTDATED)
+ rv = SS_CONNECTED_OUTDATES;
+ else if ((ns.conn == C_VERIFY_S || ns.conn == C_VERIFY_T) &&
+ (nc->verify_alg[0] == 0))
+ rv = SS_NO_VERIFY_ALG;
+ else if ((ns.conn == C_VERIFY_S || ns.conn == C_VERIFY_T) &&
+ first_peer_device(device)->connection->agreed_pro_version < 88)
+ rv = SS_NOT_SUPPORTED;
+ else if (ns.role == R_PRIMARY && ns.disk < D_UP_TO_DATE &&
+ ns.pdsk < D_UP_TO_DATE)
+ rv = SS_NO_UP_TO_DATE_DISK;
+ else if ((ns.conn == C_STARTING_SYNC_S ||
+ ns.conn == C_STARTING_SYNC_T) && ns.pdsk == D_UNKNOWN)
+ rv = SS_NEED_CONNECTION;
+ else if (ns.conn >= C_CONNECTED && ns.pdsk == D_UNKNOWN)
+ rv = SS_CONNECTED_OUTDATES;
+ }

rcu_read_unlock();

--
2.1.4

2015-11-27 21:29:38

by Fabian Frédérick

[permalink] [raw]
Subject: [PATCH 06/21] drbd: use const char * const for drbd strings

Be more strict with constant arrays

Signed-off-by: Fabian Frederick <[email protected]>
---
drivers/block/drbd/drbd_strings.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/block/drbd/drbd_strings.c b/drivers/block/drbd/drbd_strings.c
index 80b0f63..0eeab14 100644
--- a/drivers/block/drbd/drbd_strings.c
+++ b/drivers/block/drbd/drbd_strings.c
@@ -26,7 +26,7 @@
#include <linux/drbd.h>
#include "drbd_strings.h"

-static const char *drbd_conn_s_names[] = {
+static const char * const drbd_conn_s_names[] = {
[C_STANDALONE] = "StandAlone",
[C_DISCONNECTING] = "Disconnecting",
[C_UNCONNECTED] = "Unconnected",
@@ -53,13 +53,13 @@ static const char *drbd_conn_s_names[] = {
[C_BEHIND] = "Behind",
};

-static const char *drbd_role_s_names[] = {
+static const char * const drbd_role_s_names[] = {
[R_PRIMARY] = "Primary",
[R_SECONDARY] = "Secondary",
[R_UNKNOWN] = "Unknown"
};

-static const char *drbd_disk_s_names[] = {
+static const char * const drbd_disk_s_names[] = {
[D_DISKLESS] = "Diskless",
[D_ATTACHING] = "Attaching",
[D_FAILED] = "Failed",
@@ -71,7 +71,7 @@ static const char *drbd_disk_s_names[] = {
[D_UP_TO_DATE] = "UpToDate",
};

-static const char *drbd_state_sw_errors[] = {
+static const char * const drbd_state_sw_errors[] = {
[-SS_TWO_PRIMARIES] = "Multiple primaries not allowed by config",
[-SS_NO_UP_TO_DATE_DISK] = "Need access to UpToDate data",
[-SS_NO_LOCAL_DISK] = "Can not resync without local disk",
--
2.1.4

2015-11-27 21:28:54

by Fabian Frédérick

[permalink] [raw]
Subject: [PATCH 07/21] drbd: kerneldoc warning fix in w_e_end_data_req()

device in not passed to that function.

Signed-off-by: Fabian Frederick <[email protected]>
---
drivers/block/drbd/drbd_worker.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/block/drbd/drbd_worker.c b/drivers/block/drbd/drbd_worker.c
index eff716c..e47bde9 100644
--- a/drivers/block/drbd/drbd_worker.c
+++ b/drivers/block/drbd/drbd_worker.c
@@ -994,7 +994,6 @@ static void move_to_net_ee_or_free(struct drbd_device *device, struct drbd_peer_

/**
* w_e_end_data_req() - Worker callback, to send a P_DATA_REPLY packet in response to a P_DATA_REQUEST
- * @device: DRBD device.
* @w: work object.
* @cancel: The connection will be closed anyways
*/
--
2.1.4

2015-11-27 21:28:58

by Fabian Frédérick

[permalink] [raw]
Subject: [PATCH 08/21] drbd: use unsigned for one bit fields

Fix sparse warnings and limit comments to fit in 80 columns

Signed-off-by: Fabian Frederick <[email protected]>
---
drivers/block/drbd/drbd_interval.h | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/block/drbd/drbd_interval.h b/drivers/block/drbd/drbd_interval.h
index f210543..23c5a94 100644
--- a/drivers/block/drbd/drbd_interval.h
+++ b/drivers/block/drbd/drbd_interval.h
@@ -6,13 +6,13 @@

struct drbd_interval {
struct rb_node rb;
- sector_t sector; /* start sector of the interval */
- unsigned int size; /* size in bytes */
- sector_t end; /* highest interval end in subtree */
- int local:1 /* local or remote request? */;
- int waiting:1; /* someone is waiting for this to complete */
- int completed:1; /* this has been completed already;
- * ignore for conflict detection */
+ sector_t sector; /* start sector of the interval */
+ unsigned int size; /* size in bytes */
+ sector_t end; /* highest interval end in subtree */
+ unsigned int local:1 /* local or remote request? */;
+ unsigned int waiting:1; /* someone is waiting for completion */
+ unsigned int completed:1; /* this has been completed already;
+ * ignore for conflict detection */
};

static inline void drbd_clear_interval(struct drbd_interval *i)
--
2.1.4

2015-11-27 21:29:18

by Fabian Frédérick

[permalink] [raw]
Subject: [PATCH 09/21] drbd: use bool for peer is_ states

is_write and is_discard are used as boolean.

Signed-off-by: Fabian Frederick <[email protected]>
---
drivers/block/drbd/drbd_worker.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/block/drbd/drbd_worker.c b/drivers/block/drbd/drbd_worker.c
index e47bde9..e427ba0 100644
--- a/drivers/block/drbd/drbd_worker.c
+++ b/drivers/block/drbd/drbd_worker.c
@@ -173,8 +173,8 @@ void drbd_peer_request_endio(struct bio *bio)
{
struct drbd_peer_request *peer_req = bio->bi_private;
struct drbd_device *device = peer_req->peer_device->device;
- int is_write = bio_data_dir(bio) == WRITE;
- int is_discard = !!(bio->bi_rw & REQ_DISCARD);
+ bool is_write = bio_data_dir(bio) == WRITE;
+ bool is_discard = !!(bio->bi_rw & REQ_DISCARD);

if (bio->bi_error && __ratelimit(&drbd_ratelimit_state))
drbd_warn(device, "%s: error=%d s=%llus\n",
--
2.1.4

2015-11-27 21:29:46

by Fabian Frédérick

[permalink] [raw]
Subject: [PATCH 10/21] drbd: fix typo

s/reqest/request

Signed-off-by: Fabian Frederick <[email protected]>
---
drivers/block/drbd/drbd_state.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/block/drbd/drbd_state.h b/drivers/block/drbd/drbd_state.h
index bd98953..6c9d5d4 100644
--- a/drivers/block/drbd/drbd_state.h
+++ b/drivers/block/drbd/drbd_state.h
@@ -140,7 +140,7 @@ extern void drbd_resume_al(struct drbd_device *device);
extern bool conn_all_vols_unconf(struct drbd_connection *connection);

/**
- * drbd_request_state() - Reqest a state change
+ * drbd_request_state() - Request a state change
* @device: DRBD device.
* @mask: mask of state bits to change.
* @val: value of new state bits.
--
2.1.4

2015-11-27 21:29:51

by Fabian Frédérick

[permalink] [raw]
Subject: [PATCH 11/21] drbd: use | for bitmask combination

Fix coccinelle warning: "sum of probable bitmasks, consider |"

Signed-off-by: Fabian Frederick <[email protected]>
---
drivers/block/drbd/drbd_receiver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/block/drbd/drbd_receiver.c b/drivers/block/drbd/drbd_receiver.c
index 1c72991..d4c807c 100644
--- a/drivers/block/drbd/drbd_receiver.c
+++ b/drivers/block/drbd/drbd_receiver.c
@@ -2449,7 +2449,7 @@ static int receive_Data(struct drbd_connection *connection, struct packet_info *
}

out_interrupted:
- drbd_may_finish_epoch(connection, peer_req->epoch, EV_PUT + EV_CLEANUP);
+ drbd_may_finish_epoch(connection, peer_req->epoch, EV_PUT | EV_CLEANUP);
put_ldev(device);
drbd_free_peer_req(device, peer_req);
return err;
--
2.1.4

2015-11-27 21:32:39

by Fabian Frédérick

[permalink] [raw]
Subject: [PATCH 12/21] drbd: use true/false for bool

Fix 4 coccinelle warnings

Signed-off-by: Fabian Frederick <[email protected]>
---
drivers/block/drbd/drbd_receiver.c | 4 ++--
drivers/block/drbd/drbd_worker.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/block/drbd/drbd_receiver.c b/drivers/block/drbd/drbd_receiver.c
index d4c807c..ad9b99b 100644
--- a/drivers/block/drbd/drbd_receiver.c
+++ b/drivers/block/drbd/drbd_receiver.c
@@ -2065,13 +2065,13 @@ static inline int overlaps(sector_t s1, int l1, sector_t s2, int l2)
static bool overlapping_resync_write(struct drbd_device *device, struct drbd_peer_request *peer_req)
{
struct drbd_peer_request *rs_req;
- bool rv = 0;
+ bool rv = false;

spin_lock_irq(&device->resource->req_lock);
list_for_each_entry(rs_req, &device->sync_ee, w.list) {
if (overlaps(peer_req->i.sector, peer_req->i.size,
rs_req->i.sector, rs_req->i.size)) {
- rv = 1;
+ rv = true;
break;
}
}
diff --git a/drivers/block/drbd/drbd_worker.c b/drivers/block/drbd/drbd_worker.c
index e427ba0..17874bc 100644
--- a/drivers/block/drbd/drbd_worker.c
+++ b/drivers/block/drbd/drbd_worker.c
@@ -1627,7 +1627,7 @@ static bool use_checksum_based_resync(struct drbd_connection *connection, struct
rcu_read_unlock();
return connection->agreed_pro_version >= 89 && /* supported? */
connection->csums_tfm && /* configured? */
- (csums_after_crash_only == 0 /* use for each resync? */
+ (csums_after_crash_only == false /* use for each resync? */
|| test_bit(CRASHED_PRIMARY, &device->flags)); /* or only after Primary crash? */
}

@@ -1762,7 +1762,7 @@ void drbd_start_resync(struct drbd_device *device, enum drbd_conns side)
device->bm_resync_fo = 0;
device->use_csums = use_checksum_based_resync(connection, device);
} else {
- device->use_csums = 0;
+ device->use_csums = false;
}

/* Since protocol 96, we must serialize drbd_gen_and_send_sync_uuid
--
2.1.4

2015-11-27 21:30:01

by Fabian Frédérick

[permalink] [raw]
Subject: [PATCH 13/21] drbd: fix drbd_bm_init() comments

Function is also called by drbd_create_device() so original comment
is now obsolete.

Signed-off-by: Fabian Frederick <[email protected]>
---
drivers/block/drbd/drbd_bitmap.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/block/drbd/drbd_bitmap.c b/drivers/block/drbd/drbd_bitmap.c
index 92d6fc0..aa2ced7 100644
--- a/drivers/block/drbd/drbd_bitmap.c
+++ b/drivers/block/drbd/drbd_bitmap.c
@@ -427,8 +427,7 @@ static struct page **bm_realloc_pages(struct drbd_bitmap *b, unsigned long want)
}

/*
- * called on driver init only. TODO call when a device is created.
- * allocates the drbd_bitmap, and stores it in device->bitmap.
+ * allocates the drbd_bitmap and stores it in device->bitmap.
*/
int drbd_bm_init(struct drbd_device *device)
{
--
2.1.4

2015-11-27 21:29:59

by Fabian Frédérick

[permalink] [raw]
Subject: [PATCH 14/21] drbd: clean-up receive_SyncParam()

Initialize header_size then data_size once.

Signed-off-by: Fabian Frederick <[email protected]>
---
drivers/block/drbd/drbd_receiver.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/block/drbd/drbd_receiver.c b/drivers/block/drbd/drbd_receiver.c
index ad9b99b..3ca6516 100644
--- a/drivers/block/drbd/drbd_receiver.c
+++ b/drivers/block/drbd/drbd_receiver.c
@@ -3556,18 +3556,16 @@ static int receive_SyncParam(struct drbd_connection *connection, struct packet_i
return -EIO;
}

- if (apv <= 88) {
+ if (apv <= 88)
header_size = sizeof(struct p_rs_param);
- data_size = pi->size - header_size;
- } else if (apv <= 94) {
+ else if (apv <= 94)
header_size = sizeof(struct p_rs_param_89);
- data_size = pi->size - header_size;
- D_ASSERT(device, data_size == 0);
- } else {
+ else
header_size = sizeof(struct p_rs_param_95);
- data_size = pi->size - header_size;
+
+ data_size = pi->size - header_size;
+ if (apv > 88)
D_ASSERT(device, data_size == 0);
- }

/* initialize verify_alg and csums_alg */
p = pi->data;
--
2.1.4

2015-11-27 21:32:00

by Fabian Frédérick

[permalink] [raw]
Subject: [PATCH 15/21] drbd: introduce peer state union

Add peer structure and propagate in conn_khelper() instead
of hardcoded values.

Signed-off-by: Fabian Frederick <[email protected]>
---
drivers/block/drbd/drbd_nl.c | 13 +++++++------
include/linux/drbd.h | 8 ++++++++
2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/block/drbd/drbd_nl.c b/drivers/block/drbd/drbd_nl.c
index ef136a9..7a5f148 100644
--- a/drivers/block/drbd/drbd_nl.c
+++ b/drivers/block/drbd/drbd_nl.c
@@ -387,7 +387,8 @@ int drbd_khelper(struct drbd_device *device, char *cmd)
return ret;
}

-static int conn_khelper(struct drbd_connection *connection, char *cmd)
+static enum drbd_peer_state conn_khelper(struct drbd_connection *connection,
+ char *cmd)
{
char *envp[] = { "HOME=/",
"TERM=linux",
@@ -485,17 +486,17 @@ bool conn_try_outdate_peer(struct drbd_connection *connection)
r = conn_khelper(connection, "fence-peer");

switch ((r>>8) & 0xff) {
- case 3: /* peer is inconsistent */
+ case P_INCONSISTENT: /* peer is inconsistent */
ex_to_string = "peer is inconsistent or worse";
mask.pdsk = D_MASK;
val.pdsk = D_INCONSISTENT;
break;
- case 4: /* peer got outdated, or was already outdated */
+ case P_OUTDATED: /* peer got outdated, or was already outdated */
ex_to_string = "peer was fenced";
mask.pdsk = D_MASK;
val.pdsk = D_OUTDATED;
break;
- case 5: /* peer was down */
+ case P_DOWN: /* peer was down */
if (conn_highest_disk(connection) == D_UP_TO_DATE) {
/* we will(have) create(d) a new UUID anyways... */
ex_to_string = "peer is unreachable, assumed to be dead";
@@ -505,7 +506,7 @@ bool conn_try_outdate_peer(struct drbd_connection *connection)
ex_to_string = "peer unreachable, doing nothing since disk != UpToDate";
}
break;
- case 6: /* Peer is primary, voluntarily outdate myself.
+ case P_PRIMARY: /* Peer is primary, voluntarily outdate myself.
* This is useful when an unconnected R_SECONDARY is asked to
* become R_PRIMARY, but finds the other peer being active. */
ex_to_string = "peer is active";
@@ -513,7 +514,7 @@ bool conn_try_outdate_peer(struct drbd_connection *connection)
mask.disk = D_MASK;
val.disk = D_OUTDATED;
break;
- case 7:
+ case P_FENCING:
if (fp != FP_STONITH)
drbd_err(connection, "fence-peer() = 7 && fencing != Stonith !!!\n");
ex_to_string = "peer was stonithed";
diff --git a/include/linux/drbd.h b/include/linux/drbd.h
index d6b3c99..1f20a62 100644
--- a/include/linux/drbd.h
+++ b/include/linux/drbd.h
@@ -370,6 +370,14 @@ enum drbd_notification_type {
NOTIFY_FLAGS = NOTIFY_CONTINUES,
};

+enum drbd_peer_state {
+ P_INCONSISTENT = 3,
+ P_OUTDATED = 4,
+ P_DOWN = 5,
+ P_PRIMARY = 6,
+ P_FENCING = 7
+};
+
#define UUID_JUST_CREATED ((__u64)4)

enum write_ordering_e {
--
2.1.4

2015-11-27 21:31:40

by Fabian Frédérick

[permalink] [raw]
Subject: [PATCH 16/21] drbd: fix maybe_pull_ahead() locking comments

maybe_pull_ahead() calls rcu_read_lock() itself and is only
called within req_lock (locked by drbd_send_and_submit()

Signed-off-by: Fabian Frederick <[email protected]>
---
drivers/block/drbd/drbd_req.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/block/drbd/drbd_req.c b/drivers/block/drbd/drbd_req.c
index 2255dcf..2228012 100644
--- a/drivers/block/drbd/drbd_req.c
+++ b/drivers/block/drbd/drbd_req.c
@@ -995,7 +995,7 @@ static void complete_conflicting_writes(struct drbd_request *req)
finish_wait(&device->misc_wait, &wait);
}

-/* called within req_lock and rcu_read_lock() */
+/* called within req_lock */
static void maybe_pull_ahead(struct drbd_device *device)
{
struct drbd_connection *connection = first_peer_device(device)->connection;
--
2.1.4

2015-11-27 21:30:07

by Fabian Frédérick

[permalink] [raw]
Subject: [PATCH 17/21] drbd: use bool for growing

growing is used for true/false in drbd_bm_resize()

Signed-off-by: Fabian Frederick <[email protected]>
---
drivers/block/drbd/drbd_bitmap.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/block/drbd/drbd_bitmap.c b/drivers/block/drbd/drbd_bitmap.c
index aa2ced7..17e5e60 100644
--- a/drivers/block/drbd/drbd_bitmap.c
+++ b/drivers/block/drbd/drbd_bitmap.c
@@ -632,7 +632,8 @@ int drbd_bm_resize(struct drbd_device *device, sector_t capacity, int set_new_bi
unsigned long bits, words, owords, obits;
unsigned long want, have, onpages; /* number of pages */
struct page **npages, **opages = NULL;
- int err = 0, growing;
+ int err = 0;
+ bool growing;

if (!expect(b))
return -ENOMEM;
--
2.1.4

2015-11-27 21:30:42

by Fabian Frédérick

[permalink] [raw]
Subject: [PATCH 18/21] drbd: remove redundant declarations

both drbd_conn_str() and drbd_role_str() were already declared
extern in drbd_strings.h included in drbd_int.h

Signed-off-by: Fabian Frederick <[email protected]>
---
drivers/block/drbd/drbd_int.h | 2 --
1 file changed, 2 deletions(-)

diff --git a/drivers/block/drbd/drbd_int.h b/drivers/block/drbd/drbd_int.h
index 34bc84e..c898947 100644
--- a/drivers/block/drbd/drbd_int.h
+++ b/drivers/block/drbd/drbd_int.h
@@ -1635,8 +1635,6 @@ void drbd_bump_write_ordering(struct drbd_resource *resource, struct drbd_backin
/* drbd_proc.c */
extern struct proc_dir_entry *drbd_proc;
extern const struct file_operations drbd_proc_fops;
-extern const char *drbd_conn_str(enum drbd_conns s);
-extern const char *drbd_role_str(enum drbd_role s);

/* drbd_actlog.c */
extern bool drbd_al_begin_io_prepare(struct drbd_device *device, struct drbd_interval *i);
--
2.1.4

2015-11-27 21:30:21

by Fabian Frédérick

[permalink] [raw]
Subject: [PATCH 19/21] drbd: remove bm_vk_free()

Commit 51fa48018a12
("tree wide: use kvfree() than conditional kfree()/vfree()")
simplified bm_vk_free() to one single call.

Signed-off-by: Fabian Frederick <[email protected]>
---
drivers/block/drbd/drbd_bitmap.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/block/drbd/drbd_bitmap.c b/drivers/block/drbd/drbd_bitmap.c
index 17e5e60..36c180f 100644
--- a/drivers/block/drbd/drbd_bitmap.c
+++ b/drivers/block/drbd/drbd_bitmap.c
@@ -364,11 +364,6 @@ static void bm_free_pages(struct page **pages, unsigned long number)
}
}

-static inline void bm_vk_free(void *ptr)
-{
- kvfree(ptr);
-}
-
/*
* "have" and "want" are NUMBER OF PAGES.
*/
@@ -407,7 +402,7 @@ static struct page **bm_realloc_pages(struct drbd_bitmap *b, unsigned long want)
page = alloc_page(GFP_NOIO | __GFP_HIGHMEM);
if (!page) {
bm_free_pages(new_pages + have, i - have);
- bm_vk_free(new_pages);
+ kvfree(new_pages);
return NULL;
}
/* we want to know which page it is
@@ -459,7 +454,7 @@ void drbd_bm_cleanup(struct drbd_device *device)
if (!expect(device->bitmap))
return;
bm_free_pages(device->bitmap->bm_pages, device->bitmap->bm_number_of_pages);
- bm_vk_free(device->bitmap->bm_pages);
+ kvfree(device->bitmap->bm_pages);
kfree(device->bitmap);
device->bitmap = NULL;
}
@@ -659,7 +654,7 @@ int drbd_bm_resize(struct drbd_device *device, sector_t capacity, int set_new_bi
b->bm_dev_capacity = 0;
spin_unlock_irq(&b->bm_lock);
bm_free_pages(opages, onpages);
- bm_vk_free(opages);
+ kvfree(opages);
goto out;
}
bits = BM_SECT_TO_BIT(ALIGN(capacity, BM_SECT_PER_BIT));
@@ -732,7 +727,7 @@ int drbd_bm_resize(struct drbd_device *device, sector_t capacity, int set_new_bi

spin_unlock_irq(&b->bm_lock);
if (opages != npages)
- bm_vk_free(opages);
+ kvfree(opages);
if (!growing)
b->bm_set = bm_count_bits(b);
drbd_info(device, "resync bitmap: bits=%lu words=%lu pages=%lu\n", bits, words, want);
--
2.1.4

2015-11-27 21:30:50

by Fabian Frédérick

[permalink] [raw]
Subject: [PATCH 20/21] drbd: remove unused definitions

div_ceil/div_floor are no longer used

Signed-off-by: Fabian Frederick <[email protected]>
---
drivers/block/drbd/drbd_int.h | 5 -----
1 file changed, 5 deletions(-)

diff --git a/drivers/block/drbd/drbd_int.h b/drivers/block/drbd/drbd_int.h
index c898947..365eb4c 100644
--- a/drivers/block/drbd/drbd_int.h
+++ b/drivers/block/drbd/drbd_int.h
@@ -188,11 +188,6 @@ drbd_insert_fault(struct drbd_device *device, unsigned int type) {
#endif
}

-/* integer division, round _UP_ to the next integer */
-#define div_ceil(A, B) ((A)/(B) + ((A)%(B) ? 1 : 0))
-/* usual integer division */
-#define div_floor(A, B) ((A)/(B))
-
extern struct ratelimit_state drbd_ratelimit_state;
extern struct idr drbd_devices; /* RCU, updates: genl_lock() */
extern struct list_head drbd_resources; /* RCU, updates: genl_lock() */
--
2.1.4

2015-11-27 21:31:02

by Fabian Frédérick

[permalink] [raw]
Subject: [PATCH 21/21] drbd: replace if/BUG by BUG_ON

Signed-off-by: Fabian Frederick <[email protected]>
---
drivers/block/drbd/drbd_state.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/block/drbd/drbd_state.c b/drivers/block/drbd/drbd_state.c
index a7631a3..5f16a11 100644
--- a/drivers/block/drbd/drbd_state.c
+++ b/drivers/block/drbd/drbd_state.c
@@ -2150,9 +2150,7 @@ conn_set_state(struct drbd_connection *connection, union drbd_state mask, union
ns.disk = os.disk;

rv = _drbd_set_state(device, ns, flags, NULL);
- if (rv < SS_SUCCESS)
- BUG();
-
+ BUG_ON(rv < SS_SUCCESS);
ns.i = device->state.i;
ns_max.role = max_role(ns.role, ns_max.role);
ns_max.peer = max_role(ns.peer, ns_max.peer);
--
2.1.4

2015-12-01 08:55:43

by Philipp Reisner

[permalink] [raw]
Subject: Re: [Drbd-dev] [PATCH 00/21] drbd: clean-up patchset

Hi Fabian,

I will take these patches and send them towards upstream in the
next round. (Probably squashing them into a smaller number of
patches while doing so)

Best regards,
Phil

Am Freitag, 27. November 2015, 22:26:05 schrieb Fabian Frederick:
> This small patchset does some code clean-up in drivers/block/drbd tree.
>
> Fabian Frederick (21):
> drbd: debugfs: constify drbd_version_fops
> drbd: simplify drbd_md_set_sector_offsets()
> drbd: use seq_put instead of seq_print where possible
> drbd: include linux/uaccess.h instead of asm/uaccess.h
> drbd: rework is_valid_state()
> drbd: use const char * const for drbd strings
> drbd: kerneldoc warning fix in w_e_end_data_req()
> drbd: use unsigned for one bit fields
> drbd: use bool for peer is_ states
> drbd: fix typo
> drbd: use | for bitmask combination
> drbd: use true/false for bool
> drbd: fix drbd_bm_init() comments
> drbd: clean-up receive_SyncParam()
> drbd: introduce peer state union
> drbd: fix maybe_pull_ahead() locking comments
> drbd: use bool for growing
> drbd: remove redundant declarations
> drbd: remove bm_vk_free()
> drbd: remove unused definitions
> drbd: replace if/BUG by BUG_ON
>
> drivers/block/drbd/drbd_bitmap.c | 19 +++-----
> drivers/block/drbd/drbd_debugfs.c | 2 +-
> drivers/block/drbd/drbd_int.h | 7 ---
> drivers/block/drbd/drbd_interval.h | 14 +++---
> drivers/block/drbd/drbd_main.c | 2 +-
> drivers/block/drbd/drbd_nl.c | 37 ++++++++--------
> drivers/block/drbd/drbd_proc.c | 30 ++++++-------
> drivers/block/drbd/drbd_receiver.c | 22 +++++-----
> drivers/block/drbd/drbd_req.c | 2 +-
> drivers/block/drbd/drbd_state.c | 90
> +++++++++++++++++--------------------- drivers/block/drbd/drbd_state.h |
> 2 +-
> drivers/block/drbd/drbd_strings.c | 8 ++--
> drivers/block/drbd/drbd_worker.c | 9 ++--
> include/linux/drbd.h | 8 ++++
> 14 files changed, 116 insertions(+), 136 deletions(-)