2017-12-14 14:34:14

by Guilherme G. Piccoli

[permalink] [raw]
Subject: [PATCH 0/3] Some small clean-ups in genwqe

This patchset is composed by 3 small clean-ups to genwqe
driver. It aims to improve code clarity, by removing unused
stuff and make the code follow some conventions.

It was built and tested against v4.15-rc3, and aims merge
in v4.16 if possible.

Thanks in advance!

Guilherme G. Piccoli (3):
genwqe: Remove unused variable and rename function
genwqe: Make defines uppercase
genwqe: Remove unused parameter in some functions

drivers/misc/genwqe/card_base.c | 16 ++++++++--------
drivers/misc/genwqe/card_base.h | 20 +++++++++-----------
drivers/misc/genwqe/card_ddcb.c | 22 +++++++++++-----------
drivers/misc/genwqe/card_debugfs.c | 2 +-
drivers/misc/genwqe/card_dev.c | 19 ++++++++-----------
drivers/misc/genwqe/card_utils.c | 25 +++++++++----------------
6 files changed, 46 insertions(+), 58 deletions(-)

--
2.15.0


2017-12-14 14:34:19

by Guilherme G. Piccoli

[permalink] [raw]
Subject: [PATCH 1/3] genwqe: Remove unused variable and rename function

This is a clean-up patch, no functional changes intended.

It removes an unused variable from do_execute_ddcb() and
also renames the function free_user_pages(), prepending
"genwqe" prefix in order to clarify the code.

Signed-off-by: Guilherme G. Piccoli <[email protected]>
---
drivers/misc/genwqe/card_dev.c | 3 ---
drivers/misc/genwqe/card_utils.c | 20 +++++++-------------
2 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/drivers/misc/genwqe/card_dev.c b/drivers/misc/genwqe/card_dev.c
index 3ecfa35457e0..1f4986fcf363 100644
--- a/drivers/misc/genwqe/card_dev.c
+++ b/drivers/misc/genwqe/card_dev.c
@@ -1011,7 +1011,6 @@ static int do_execute_ddcb(struct genwqe_file *cfile,
{
int rc;
struct genwqe_ddcb_cmd *cmd;
- struct ddcb_requ *req;
struct genwqe_dev *cd = cfile->cd;
struct file *filp = cfile->filp;

@@ -1019,8 +1018,6 @@ static int do_execute_ddcb(struct genwqe_file *cfile,
if (cmd == NULL)
return -ENOMEM;

- req = container_of(cmd, struct ddcb_requ, cmd);
-
if (copy_from_user(cmd, (void __user *)arg, sizeof(*cmd))) {
ddcb_requ_free(cmd);
return -EFAULT;
diff --git a/drivers/misc/genwqe/card_utils.c b/drivers/misc/genwqe/card_utils.c
index 5c0d917636f7..0593105991da 100644
--- a/drivers/misc/genwqe/card_utils.c
+++ b/drivers/misc/genwqe/card_utils.c
@@ -524,22 +524,16 @@ int genwqe_free_sync_sgl(struct genwqe_dev *cd, struct genwqe_sgl *sgl)
}

/**
- * free_user_pages() - Give pinned pages back
+ * genwqe_free_user_pages() - Give pinned pages back
*
- * Documentation of get_user_pages is in mm/memory.c:
+ * Documentation of get_user_pages is in mm/gup.c:
*
* If the page is written to, set_page_dirty (or set_page_dirty_lock,
* as appropriate) must be called after the page is finished with, and
* before put_page is called.
- *
- * FIXME Could be of use to others and might belong in the generic
- * code, if others agree. E.g.
- * ll_free_user_pages in drivers/staging/lustre/lustre/llite/rw26.c
- * ceph_put_page_vector in net/ceph/pagevec.c
- * maybe more?
*/
-static int free_user_pages(struct page **page_list, unsigned int nr_pages,
- int dirty)
+static int genwqe_free_user_pages(struct page **page_list,
+ unsigned int nr_pages, int dirty)
{
unsigned int i;

@@ -617,7 +611,7 @@ int genwqe_user_vmap(struct genwqe_dev *cd, struct dma_mapping *m, void *uaddr,

/* assumption: get_user_pages can be killed by signals. */
if (rc < m->nr_pages) {
- free_user_pages(m->page_list, rc, m->write);
+ genwqe_free_user_pages(m->page_list, rc, m->write);
rc = -EFAULT;
goto fail_get_user_pages;
}
@@ -629,7 +623,7 @@ int genwqe_user_vmap(struct genwqe_dev *cd, struct dma_mapping *m, void *uaddr,
return 0;

fail_free_user_pages:
- free_user_pages(m->page_list, m->nr_pages, m->write);
+ genwqe_free_user_pages(m->page_list, m->nr_pages, m->write);

fail_get_user_pages:
kfree(m->page_list);
@@ -662,7 +656,7 @@ int genwqe_user_vunmap(struct genwqe_dev *cd, struct dma_mapping *m,
genwqe_unmap_pages(cd, m->dma_list, m->nr_pages);

if (m->page_list) {
- free_user_pages(m->page_list, m->nr_pages, m->write);
+ genwqe_free_user_pages(m->page_list, m->nr_pages, m->write);

kfree(m->page_list);
m->page_list = NULL;
--
2.15.0

2017-12-14 14:34:32

by Guilherme G. Piccoli

[permalink] [raw]
Subject: [PATCH 3/3] genwqe: Remove unused parameter in some functions

This is a clean-up patch, no functional changes intended.

It removes the unused parameter of type "struct ddcb_requ*" from
the functions genwqe_user_vmap() and genwqe_user_vunmap().

Signed-off-by: Guilherme G. Piccoli <[email protected]>
---
drivers/misc/genwqe/card_base.h | 6 ++----
drivers/misc/genwqe/card_dev.c | 12 ++++++------
drivers/misc/genwqe/card_utils.c | 5 ++---
3 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/misc/genwqe/card_base.h b/drivers/misc/genwqe/card_base.h
index a9314b06b97b..1c3967f10f55 100644
--- a/drivers/misc/genwqe/card_base.h
+++ b/drivers/misc/genwqe/card_base.h
@@ -490,11 +490,9 @@ int genwqe_read_app_id(struct genwqe_dev *cd, char *app_name, int len);

/* Memory allocation/deallocation; dma address handling */
int genwqe_user_vmap(struct genwqe_dev *cd, struct dma_mapping *m,
- void *uaddr, unsigned long size,
- struct ddcb_requ *req);
+ void *uaddr, unsigned long size);

-int genwqe_user_vunmap(struct genwqe_dev *cd, struct dma_mapping *m,
- struct ddcb_requ *req);
+int genwqe_user_vunmap(struct genwqe_dev *cd, struct dma_mapping *m);

static inline bool dma_mapping_used(struct dma_mapping *m)
{
diff --git a/drivers/misc/genwqe/card_dev.c b/drivers/misc/genwqe/card_dev.c
index 9ca6d18e52e7..0dd6b5ef314a 100644
--- a/drivers/misc/genwqe/card_dev.c
+++ b/drivers/misc/genwqe/card_dev.c
@@ -226,7 +226,7 @@ static void genwqe_remove_mappings(struct genwqe_file *cfile)
kfree(dma_map);
} else if (dma_map->type == GENWQE_MAPPING_SGL_TEMP) {
/* we use dma_map statically from the request */
- genwqe_user_vunmap(cd, dma_map, NULL);
+ genwqe_user_vunmap(cd, dma_map);
}
}
}
@@ -249,7 +249,7 @@ static void genwqe_remove_pinnings(struct genwqe_file *cfile)
* deleted.
*/
list_del_init(&dma_map->pin_list);
- genwqe_user_vunmap(cd, dma_map, NULL);
+ genwqe_user_vunmap(cd, dma_map);
kfree(dma_map);
}
}
@@ -790,7 +790,7 @@ static int genwqe_pin_mem(struct genwqe_file *cfile, struct genwqe_mem *m)
return -ENOMEM;

genwqe_mapping_init(dma_map, GENWQE_MAPPING_SGL_PINNED);
- rc = genwqe_user_vmap(cd, dma_map, (void *)map_addr, map_size, NULL);
+ rc = genwqe_user_vmap(cd, dma_map, (void *)map_addr, map_size);
if (rc != 0) {
dev_err(&pci_dev->dev,
"[%s] genwqe_user_vmap rc=%d\n", __func__, rc);
@@ -820,7 +820,7 @@ static int genwqe_unpin_mem(struct genwqe_file *cfile, struct genwqe_mem *m)
return -ENOENT;

genwqe_del_pin(cfile, dma_map);
- genwqe_user_vunmap(cd, dma_map, NULL);
+ genwqe_user_vunmap(cd, dma_map);
kfree(dma_map);
return 0;
}
@@ -841,7 +841,7 @@ static int ddcb_cmd_cleanup(struct genwqe_file *cfile, struct ddcb_requ *req)

if (dma_mapping_used(dma_map)) {
__genwqe_del_mapping(cfile, dma_map);
- genwqe_user_vunmap(cd, dma_map, req);
+ genwqe_user_vunmap(cd, dma_map);
}
if (req->sgls[i].sgl != NULL)
genwqe_free_sync_sgl(cd, &req->sgls[i]);
@@ -947,7 +947,7 @@ static int ddcb_cmd_fixups(struct genwqe_file *cfile, struct ddcb_requ *req)
m->write = 0;

rc = genwqe_user_vmap(cd, m, (void *)u_addr,
- u_size, req);
+ u_size);
if (rc != 0)
goto err_out;

diff --git a/drivers/misc/genwqe/card_utils.c b/drivers/misc/genwqe/card_utils.c
index 0593105991da..8f2e6442d88b 100644
--- a/drivers/misc/genwqe/card_utils.c
+++ b/drivers/misc/genwqe/card_utils.c
@@ -571,7 +571,7 @@ static int genwqe_free_user_pages(struct page **page_list,
* Return: 0 if success
*/
int genwqe_user_vmap(struct genwqe_dev *cd, struct dma_mapping *m, void *uaddr,
- unsigned long size, struct ddcb_requ *req)
+ unsigned long size)
{
int rc = -EINVAL;
unsigned long data, offs;
@@ -641,8 +641,7 @@ int genwqe_user_vmap(struct genwqe_dev *cd, struct dma_mapping *m, void *uaddr,
* @cd: pointer to genwqe device
* @m: mapping params
*/
-int genwqe_user_vunmap(struct genwqe_dev *cd, struct dma_mapping *m,
- struct ddcb_requ *req)
+int genwqe_user_vunmap(struct genwqe_dev *cd, struct dma_mapping *m)
{
struct pci_dev *pci_dev = cd->pci_dev;

--
2.15.0

2017-12-14 14:35:28

by Guilherme G. Piccoli

[permalink] [raw]
Subject: [PATCH 2/3] genwqe: Make defines uppercase

This is a clean-up patch, no functional changes intended.

It makes all defines uppercase, following a "tradition"
that helps to make code clearer.

Signed-off-by: Guilherme G. Piccoli <[email protected]>
---
drivers/misc/genwqe/card_base.c | 16 ++++++++--------
drivers/misc/genwqe/card_base.h | 14 +++++++-------
drivers/misc/genwqe/card_ddcb.c | 22 +++++++++++-----------
drivers/misc/genwqe/card_debugfs.c | 2 +-
drivers/misc/genwqe/card_dev.c | 4 ++--
5 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/drivers/misc/genwqe/card_base.c b/drivers/misc/genwqe/card_base.c
index 4fd21e86ad56..c7cd3675bcd1 100644
--- a/drivers/misc/genwqe/card_base.c
+++ b/drivers/misc/genwqe/card_base.c
@@ -153,11 +153,11 @@ static struct genwqe_dev *genwqe_dev_alloc(void)
cd->card_state = GENWQE_CARD_UNUSED;
spin_lock_init(&cd->print_lock);

- cd->ddcb_software_timeout = genwqe_ddcb_software_timeout;
- cd->kill_timeout = genwqe_kill_timeout;
+ cd->ddcb_software_timeout = GENWQE_DDCB_SOFTWARE_TIMEOUT;
+ cd->kill_timeout = GENWQE_KILL_TIMEOUT;

for (j = 0; j < GENWQE_MAX_VFS; j++)
- cd->vf_jobtimeout_msec[j] = genwqe_vf_jobtimeout_msec;
+ cd->vf_jobtimeout_msec[j] = GENWQE_VF_JOBTIMEOUT_MSEC;

genwqe_devices[i] = cd;
return cd;
@@ -324,11 +324,11 @@ static bool genwqe_setup_pf_jtimer(struct genwqe_dev *cd)
u32 T = genwqe_T_psec(cd);
u64 x;

- if (genwqe_pf_jobtimeout_msec == 0)
+ if (GENWQE_PF_JOBTIMEOUT_MSEC == 0)
return false;

/* PF: large value needed, flash update 2sec per block */
- x = ilog2(genwqe_pf_jobtimeout_msec *
+ x = ilog2(GENWQE_PF_JOBTIMEOUT_MSEC *
16000000000uL/(T * 15)) - 10;

genwqe_write_vreg(cd, IO_SLC_VF_APPJOB_TIMEOUT,
@@ -904,7 +904,7 @@ static int genwqe_reload_bistream(struct genwqe_dev *cd)
* b) a critical GFIR occured
*
* Informational GFIRs are checked and potentially printed in
- * health_check_interval seconds.
+ * GENWQE_HEALTH_CHECK_INTERVAL seconds.
*/
static int genwqe_health_thread(void *data)
{
@@ -918,7 +918,7 @@ static int genwqe_health_thread(void *data)
rc = wait_event_interruptible_timeout(cd->health_waitq,
(genwqe_health_check_cond(cd, &gfir) ||
(should_stop = kthread_should_stop())),
- genwqe_health_check_interval * HZ);
+ GENWQE_HEALTH_CHECK_INTERVAL * HZ);

if (should_stop)
break;
@@ -1028,7 +1028,7 @@ static int genwqe_health_check_start(struct genwqe_dev *cd)
{
int rc;

- if (genwqe_health_check_interval <= 0)
+ if (GENWQE_HEALTH_CHECK_INTERVAL <= 0)
return 0; /* valid for disabling the service */

/* moved before request_irq() */
diff --git a/drivers/misc/genwqe/card_base.h b/drivers/misc/genwqe/card_base.h
index 3743c87f8ab9..a9314b06b97b 100644
--- a/drivers/misc/genwqe/card_base.h
+++ b/drivers/misc/genwqe/card_base.h
@@ -47,13 +47,13 @@
#define GENWQE_CARD_NO_MAX (16 * GENWQE_MAX_FUNCS)

/* Compile parameters, some of them appear in debugfs for later adjustment */
-#define genwqe_ddcb_max 32 /* DDCBs on the work-queue */
-#define genwqe_polling_enabled 0 /* in case of irqs not working */
-#define genwqe_ddcb_software_timeout 10 /* timeout per DDCB in seconds */
-#define genwqe_kill_timeout 8 /* time until process gets killed */
-#define genwqe_vf_jobtimeout_msec 250 /* 250 msec */
-#define genwqe_pf_jobtimeout_msec 8000 /* 8 sec should be ok */
-#define genwqe_health_check_interval 4 /* <= 0: disabled */
+#define GENWQE_DDCB_MAX 32 /* DDCBs on the work-queue */
+#define GENWQE_POLLING_ENABLED 0 /* in case of irqs not working */
+#define GENWQE_DDCB_SOFTWARE_TIMEOUT 10 /* timeout per DDCB in seconds */
+#define GENWQE_KILL_TIMEOUT 8 /* time until process gets killed */
+#define GENWQE_VF_JOBTIMEOUT_MSEC 250 /* 250 msec */
+#define GENWQE_PF_JOBTIMEOUT_MSEC 8000 /* 8 sec should be ok */
+#define GENWQE_HEALTH_CHECK_INTERVAL 4 /* <= 0: disabled */

/* Sysfs attribute groups used when we create the genwqe device */
extern const struct attribute_group *genwqe_attribute_groups[];
diff --git a/drivers/misc/genwqe/card_ddcb.c b/drivers/misc/genwqe/card_ddcb.c
index ddfeefe39540..b7f8d35c17a9 100644
--- a/drivers/misc/genwqe/card_ddcb.c
+++ b/drivers/misc/genwqe/card_ddcb.c
@@ -500,7 +500,7 @@ int __genwqe_wait_ddcb(struct genwqe_dev *cd, struct ddcb_requ *req)

rc = wait_event_interruptible_timeout(queue->ddcb_waitqs[ddcb_no],
ddcb_requ_finished(cd, req),
- genwqe_ddcb_software_timeout * HZ);
+ GENWQE_DDCB_SOFTWARE_TIMEOUT * HZ);

/*
* We need to distinguish 3 cases here:
@@ -633,7 +633,7 @@ int __genwqe_purge_ddcb(struct genwqe_dev *cd, struct ddcb_requ *req)
__be32 old, new;

/* unsigned long flags; */
- if (genwqe_ddcb_software_timeout <= 0) {
+ if (GENWQE_DDCB_SOFTWARE_TIMEOUT <= 0) {
dev_err(&pci_dev->dev,
"[%s] err: software timeout is not set!\n", __func__);
return -EFAULT;
@@ -641,7 +641,7 @@ int __genwqe_purge_ddcb(struct genwqe_dev *cd, struct ddcb_requ *req)

pddcb = &queue->ddcb_vaddr[req->num];

- for (t = 0; t < genwqe_ddcb_software_timeout * 10; t++) {
+ for (t = 0; t < GENWQE_DDCB_SOFTWARE_TIMEOUT * 10; t++) {

spin_lock_irqsave(&queue->ddcb_lock, flags);

@@ -718,7 +718,7 @@ int __genwqe_purge_ddcb(struct genwqe_dev *cd, struct ddcb_requ *req)

dev_err(&pci_dev->dev,
"[%s] err: DDCB#%d not purged and not completed after %d seconds QSTAT=%016llx!!\n",
- __func__, req->num, genwqe_ddcb_software_timeout,
+ __func__, req->num, GENWQE_DDCB_SOFTWARE_TIMEOUT,
queue_status);

print_ddcb_info(cd, req->queue);
@@ -778,7 +778,7 @@ int __genwqe_enqueue_ddcb(struct genwqe_dev *cd, struct ddcb_requ *req,
/* FIXME circumvention to improve performance when no irq is
* there.
*/
- if (genwqe_polling_enabled)
+ if (GENWQE_POLLING_ENABLED)
genwqe_check_ddcb_queue(cd, queue);

/*
@@ -878,7 +878,7 @@ int __genwqe_enqueue_ddcb(struct genwqe_dev *cd, struct ddcb_requ *req,
pddcb->icrc_hsi_shi_32 = cpu_to_be32((u32)icrc << 16);

/* enable DDCB completion irq */
- if (!genwqe_polling_enabled)
+ if (!GENWQE_POLLING_ENABLED)
pddcb->icrc_hsi_shi_32 |= DDCB_INTR_BE32;

dev_dbg(&pci_dev->dev, "INPUT DDCB#%d\n", req->num);
@@ -1028,10 +1028,10 @@ static int setup_ddcb_queue(struct genwqe_dev *cd, struct ddcb_queue *queue)
unsigned int queue_size;
struct pci_dev *pci_dev = cd->pci_dev;

- if (genwqe_ddcb_max < 2)
+ if (GENWQE_DDCB_MAX < 2)
return -EINVAL;

- queue_size = roundup(genwqe_ddcb_max * sizeof(struct ddcb), PAGE_SIZE);
+ queue_size = roundup(GENWQE_DDCB_MAX * sizeof(struct ddcb), PAGE_SIZE);

queue->ddcbs_in_flight = 0; /* statistics */
queue->ddcbs_max_in_flight = 0;
@@ -1040,7 +1040,7 @@ static int setup_ddcb_queue(struct genwqe_dev *cd, struct ddcb_queue *queue)
queue->wait_on_busy = 0;

queue->ddcb_seq = 0x100; /* start sequence number */
- queue->ddcb_max = genwqe_ddcb_max; /* module parameter */
+ queue->ddcb_max = GENWQE_DDCB_MAX;
queue->ddcb_vaddr = __genwqe_alloc_consistent(cd, queue_size,
&queue->ddcb_daddr);
if (queue->ddcb_vaddr == NULL) {
@@ -1194,7 +1194,7 @@ static int genwqe_card_thread(void *data)

genwqe_check_ddcb_queue(cd, &cd->queue);

- if (genwqe_polling_enabled) {
+ if (GENWQE_POLLING_ENABLED) {
rc = wait_event_interruptible_timeout(
cd->queue_waitq,
genwqe_ddcbs_in_flight(cd) ||
@@ -1340,7 +1340,7 @@ static int queue_wake_up_all(struct genwqe_dev *cd)
int genwqe_finish_queue(struct genwqe_dev *cd)
{
int i, rc = 0, in_flight;
- int waitmax = genwqe_ddcb_software_timeout;
+ int waitmax = GENWQE_DDCB_SOFTWARE_TIMEOUT;
struct pci_dev *pci_dev = cd->pci_dev;
struct ddcb_queue *queue = &cd->queue;

diff --git a/drivers/misc/genwqe/card_debugfs.c b/drivers/misc/genwqe/card_debugfs.c
index c715534e7fe7..f921dd590271 100644
--- a/drivers/misc/genwqe/card_debugfs.c
+++ b/drivers/misc/genwqe/card_debugfs.c
@@ -198,7 +198,7 @@ static int genwqe_jtimer_show(struct seq_file *s, void *unused)

jtimer = genwqe_read_vreg(cd, IO_SLC_VF_APPJOB_TIMEOUT, 0);
seq_printf(s, " PF 0x%016llx %d msec\n", jtimer,
- genwqe_pf_jobtimeout_msec);
+ GENWQE_PF_JOBTIMEOUT_MSEC);

for (vf_num = 0; vf_num < cd->num_vfs; vf_num++) {
jtimer = genwqe_read_vreg(cd, IO_SLC_VF_APPJOB_TIMEOUT,
diff --git a/drivers/misc/genwqe/card_dev.c b/drivers/misc/genwqe/card_dev.c
index 1f4986fcf363..9ca6d18e52e7 100644
--- a/drivers/misc/genwqe/card_dev.c
+++ b/drivers/misc/genwqe/card_dev.c
@@ -1342,7 +1342,7 @@ static int genwqe_inform_and_stop_processes(struct genwqe_dev *cd)
rc = genwqe_kill_fasync(cd, SIGIO);
if (rc > 0) {
/* give kill_timeout seconds to close file descriptors ... */
- for (i = 0; (i < genwqe_kill_timeout) &&
+ for (i = 0; (i < GENWQE_KILL_TIMEOUT) &&
genwqe_open_files(cd); i++) {
dev_info(&pci_dev->dev, " %d sec ...", i);

@@ -1360,7 +1360,7 @@ static int genwqe_inform_and_stop_processes(struct genwqe_dev *cd)
rc = genwqe_force_sig(cd, SIGKILL); /* force terminate */
if (rc) {
/* Give kill_timout more seconds to end processes */
- for (i = 0; (i < genwqe_kill_timeout) &&
+ for (i = 0; (i < GENWQE_KILL_TIMEOUT) &&
genwqe_open_files(cd); i++) {
dev_warn(&pci_dev->dev, " %d sec ...", i);

--
2.15.0

2018-01-08 12:57:47

by haver

[permalink] [raw]
Subject: Re: [PATCH 0/3] Some small clean-ups in genwqe

Hi Guilherme,

On 2017-12-14 15:34, Guilherme G. Piccoli wrote:
> This patchset is composed by 3 small clean-ups to genwqe
> driver. It aims to improve code clarity, by removing unused
> stuff and make the code follow some conventions.
>
> It was built and tested against v4.15-rc3, and aims merge
> in v4.16 if possible.
>
> Thanks in advance!
>
> Guilherme G. Piccoli (3):
> genwqe: Remove unused variable and rename function
> genwqe: Make defines uppercase
> genwqe: Remove unused parameter in some functions
>
> drivers/misc/genwqe/card_base.c | 16 ++++++++--------
> drivers/misc/genwqe/card_base.h | 20 +++++++++-----------
> drivers/misc/genwqe/card_ddcb.c | 22 +++++++++++-----------
> drivers/misc/genwqe/card_debugfs.c | 2 +-
> drivers/misc/genwqe/card_dev.c | 19 ++++++++-----------
> drivers/misc/genwqe/card_utils.c | 25 +++++++++----------------
> 6 files changed, 46 insertions(+), 58 deletions(-)


Looks ok to me. Thanks for the contribution.
Regards

Frank

Acked-by: Frank Haverkamp <[email protected]>