Re: [PATCH] staging: lustre: obd_support: Add obd_cpt_alloc function
On May 3, 2015, at 1:53 AM, Julia Lawall wrote:
> On Sat, 2 May 2015, Drokin, Oleg wrote:
>
>>
>> On May 2, 2015, at 5:16 PM, Julia Lawall wrote:
>>
>>> Summarize OBD_CPT_ALLOC_GFP, OBD_CPT_ALLOC, and OBD_CPT_ALLOC_PTR as a
>>> function, obd_cpt_alloc.
>>>
>>> Signed-off-by: Julia Lawall
>>>
>>> ---
>>>
>>> Some questions: Is the name OK? Is the NULL test needed? If not, should
>>> the call to kzalloc_node with the call to cfs_cpt_spread_node just be
>>> inlined into the call sites?
>>
>> I think we don't need this function at all, we can use kzalloc/kzalloc_node
>> directly with cfs_cpt_spread_node call in.
>
> So everywhere the CPT macro is called, it is known that the value is not
> NULL? I looked at some call sites, but it's not obvious to determine
> that.
It's not obvious, but I believe this is true now.
Basically in lustre/ptlrpc/service.c we use service->srv_cptable
and that comes from ptlrpc_register_service:
cptable = cconf->cc_cptable;
if (cptable == NULL)
cptable = cfs_cpt_table;
….
service->srv_cptable= cptable;
service->srv_cpts = cpts;
service->srv_ncpts = ncpts;
that on the client it's only called from:
lustre/ldlm/ldlm_lockd.c::ldlm_setup() where we unconditionally assign
.psc_cpt= {
.cc_pattern = ldlm_cpts,
},
But even if there was a different caller, we always use cfs_cpt_table as
fallback.
It's also directly used in ptlrpc_hr_init():
ptlrpc_hr.hr_cpt_table = cfs_cpt_table;
Two callers in lustre/ptlrpc/nrs.c use the same stuff as above.
One caller in lustre/ptlrpc/nrs_fifo.c uses nrs_pol2cptab which is defined as
nrs_pol2svc(policy)->srv_cptable which is again same as above.
There are no other callers.
Bye,
Oleg
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/3] staging: lustre: ptlrpc: Replace OBD_CPT_ALLOC etc by kzalloc_node
Replace OBD_CPT_ALLOC, OBD_CPT_ALLOC_PTR, and OBD_CPT_ALLOC_GFP by
corresponding calls to kzalloc_node. The semantic patch for the
OBD_CPT_ALLOC case is as follows: (http://coccinelle.lip6.fr/).
//
@@ expression ptr,cptab,cpt,size; @@
- OBD_CPT_ALLOC(ptr, cptab, cpt, size)
+ ptr = kzalloc_node(size, GFP_NOFS, cfs_cpt_spread_node(cptab, cpt))
//
Note that the original OBD macros would check if the cptab argument was
NULL and fall back on kzalloc in that case. Oleg Drokin argues that this
test is not needed because the code containing these calls is only invoked
after initialization has been completed, in which case the possible cptab
arguments are not NULL.
Signed-off-by: Julia Lawall
---
drivers/staging/lustre/lustre/ptlrpc/nrs.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/lustre/lustre/ptlrpc/nrs.c
b/drivers/staging/lustre/lustre/ptlrpc/nrs.c
index 68c754f..63a05f4 100644
--- a/drivers/staging/lustre/lustre/ptlrpc/nrs.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/nrs.c
@@ -746,8 +746,9 @@ static int nrs_policy_register(struct ptlrpc_nrs *nrs,
LASSERT(desc->pd_ops->op_req_dequeue != NULL);
LASSERT(desc->pd_compat != NULL);
- OBD_CPT_ALLOC_GFP(policy, svcpt->scp_service->srv_cptable,
- svcpt->scp_cpt, sizeof(*policy), GFP_NOFS);
+ policy = kzalloc_node(sizeof(*policy), GFP_NOFS,
+ cfs_cpt_spread_node(svcpt->scp_service->srv_cptable,
+ svcpt->scp_cpt));
if (policy == NULL)
return -ENOMEM;
@@ -961,9 +962,10 @@ static int nrs_svcpt_setup_locked(struct
ptlrpc_service_part *svcpt)
if (svcpt->scp_service->srv_ops.so_hpreq_handler == NULL)
goto out;
- OBD_CPT_ALLOC_PTR(svcpt->scp_nrs_hp,
- svcpt->scp_service->srv_cptable,
- svcpt->scp_cpt);
+ svcpt->scp_nrs_hp =
+ kzalloc_node(sizeof(*svcpt->scp_nrs_hp), GFP_NOFS,
+ cfs_cpt_spread_node(svcpt->scp_service->srv_cptable,
+ svcpt->scp_cpt));
if (svcpt->scp_nrs_hp == NULL) {
rc = -ENOMEM;
goto out;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/3] staging/lustre/ptlrpc: Replace OBD_CPT_ALLOC etc by kzalloc_node
Replace OBD_CPT_ALLOC, OBD_CPT_ALLOC_PTR, and OBD_CPT_ALLOC_GFP by
corresponding calls to kzalloc_node. The semantic patch for the
OBD_CPT_ALLOC case is as follows: (http://coccinelle.lip6.fr/).
//
@@ expression ptr,cptab,cpt,size; @@
- OBD_CPT_ALLOC(ptr, cptab, cpt, size)
+ ptr = kzalloc_node(size, GFP_NOFS, cfs_cpt_spread_node(cptab, cpt))
//
Note that the original OBD macros would check if the cptab argument was
NULL and fall back on kzalloc in that case. Oleg Drokin argues that this
test is not needed because the code containing these calls is only invoked
after initialization has been completed, in which case the possible cptab
arguments are not NULL.
Signed-off-by: Julia Lawall
---
drivers/staging/lustre/lustre/ptlrpc/service.c | 27 -
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/lustre/lustre/ptlrpc/service.c
b/drivers/staging/lustre/lustre/ptlrpc/service.c
index d85db06..3fa52f1 100644
--- a/drivers/staging/lustre/lustre/ptlrpc/service.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/service.c
@@ -75,7 +75,9 @@ ptlrpc_alloc_rqbd(struct ptlrpc_service_part *svcpt)
struct ptlrpc_service *svc = svcpt->scp_service;
struct ptlrpc_request_buffer_desc *rqbd;
- OBD_CPT_ALLOC_PTR(rqbd, svc->srv_cptable, svcpt->scp_cpt);
+ rqbd = kzalloc_node(sizeof(*rqbd), GFP_NOFS,
+ cfs_cpt_spread_node(svc->srv_cptable,
+ svcpt->scp_cpt));
if (rqbd == NULL)
return NULL;
@@ -630,16 +632,18 @@ ptlrpc_service_part_init(struct ptlrpc_service *svc,
array->paa_deadline = -1;
/* allocate memory for scp_at_array (ptlrpc_at_array) */
- OBD_CPT_ALLOC(array->paa_reqs_array,
- svc->srv_cptable, cpt, sizeof(struct list_head) * size);
+ array->paa_reqs_array =
+ kzalloc_node(sizeof(struct list_head) * size, GFP_NOFS,
+cfs_cpt_spread_node(svc->srv_cptable, cpt));
if (array->paa_reqs_array == NULL)
return -ENOMEM;
for (index = 0; index < size; index++)
INIT_LIST_HEAD(&array->paa_reqs_array[index]);
- OBD_CPT_ALLOC(array->paa_reqs_count,
- svc->srv_cptable, cpt, sizeof(__u32) * size);
+ array->paa_reqs_count =
+ kzalloc_node(sizeof(__u32) * size, GFP_NOFS,
+cfs_cpt_spread_node(svc->srv_cptable, cpt));
if (array->paa_reqs_count == NULL)
goto free_reqs_array;
@@ -772,7 +776,8 @@ ptlrpc_register_service(struct ptlrpc_service_conf *conf,
else
cpt = cpts != NULL ? cpts[i] : i;
- OBD_CPT_ALLOC(svcpt, cptable, cpt, sizeof(*svcpt));
+ svcpt = kzalloc_node(sizeof(*svcpt), GFP_NOFS,
+cfs_cpt_spread_node(cptable, cpt));
if (svcpt == NULL) {
rc = -ENOMEM;
goto failed;
@@ -2664,7 +2669,9 @@ int ptlrpc_start_thread(struct ptlrpc_service_part
*svcpt, int wait)
svcpt->scp_nthrs_running == svc->srv_nthrs_cpt_init - 1))
return -EMFILE;
- OBD_CPT_ALLOC_PTR(thread, svc->srv_cptable, svcpt->scp_cpt);
+ thread = kzalloc_node(sizeof(*thread), GFP_NOFS,
+ cfs_cpt_spread_node(svc->srv_cptable,
+ svcpt->scp_cpt));
if (thread == NULL)
return -ENOMEM;
init_waitqueue_head(&thread->t_ctl_waitq);
@@ -2774,8 +2781,10 @@ int ptlrpc_hr_init(void)
hrp->hrp_nthrs /= weight;
LASSERT(hrp->hrp_nthrs > 0);
- OBD_CPT_ALLOC(hrp->hrp_thrs, ptlrpc_hr.hr_cpt_table, i,
- hrp->hrp_nthrs * sizeof(*hrt));
+ hrp->hrp_thrs =
+ kzalloc_node(hrp->hrp_nthrs * sizeof(*hrt), GFP_NOFS,
+ cfs_cpt_spread_node(ptlrpc_hr.hr_cpt_table,
+ i));
if (hrp->hrp_thrs == NULL) {
rc = -ENOMEM;
goto out;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/3] Replace OBD_CPT_ALLOC etc by kzalloc_node
The complete semantic patch used to make this transformation is as follows: (http://coccinelle.lip6.fr/). // @@ expression ptr,cptab,cpt,size,gfp; @@ - OBD_CPT_ALLOC_GFP(ptr, cptab, cpt, size, gfp) + ptr = kzalloc_node(size, gfp, cfs_cpt_spread_node(cptab, cpt)) @@ expression ptr,cptab,cpt,size; @@ - OBD_CPT_ALLOC(ptr, cptab, cpt, size) + ptr = kzalloc_node(size, GFP_NOFS, cfs_cpt_spread_node(cptab, cpt)) @@ expression ptr,cptab,cpt; @@ - OBD_CPT_ALLOC_PTR(ptr, cptab, cpt) + ptr = kzalloc_node(sizeof(*ptr), GFP_NOFS, cfs_cpt_spread_node(cptab,cpt)) // Note that the previously proposed patch "Add obd_cpt_alloc function" is not needed, as the transformation is done without adding a new function. These patches should be applied after the patches with subjects "Use kzalloc and kfree" and "remove unneeded null test before free". ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/3] drivers: staging: lustre: lustre: Replace OBD_CPT_ALLOC etc by kzalloc_node
Replace OBD_CPT_ALLOC, OBD_CPT_ALLOC_PTR, and OBD_CPT_ALLOC_GFP by
corresponding calls to kzalloc_node. The semantic patch for the
OBD_CPT_ALLOC case is as follows: (http://coccinelle.lip6.fr/).
//
@@ expression ptr,cptab,cpt,size; @@
- OBD_CPT_ALLOC(ptr, cptab, cpt, size)
+ ptr = kzalloc_node(size, GFP_NOFS, cfs_cpt_spread_node(cptab, cpt))
//
Note that the original OBD macros would check if the cptab argument was
NULL and fall back on kzalloc in that case. Oleg Drokin argues that this
test is not needed because the code containing these calls is only invoked
after initialization has been completed, in which case the possible cptab
arguments are not NULL.
Signed-off-by: Julia Lawall
---
drivers/staging/lustre/lustre/ptlrpc/nrs_fifo.c |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/lustre/lustre/ptlrpc/nrs_fifo.c
b/drivers/staging/lustre/lustre/ptlrpc/nrs_fifo.c
index 2eefa25..6a61c85 100644
--- a/drivers/staging/lustre/lustre/ptlrpc/nrs_fifo.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/nrs_fifo.c
@@ -80,7 +80,9 @@ static int nrs_fifo_start(struct ptlrpc_nrs_policy *policy)
{
struct nrs_fifo_head *head;
- OBD_CPT_ALLOC_PTR(head, nrs_pol2cptab(policy), nrs_pol2cptid(policy));
+ head = kzalloc_node(sizeof(*head), GFP_NOFS,
+ cfs_cpt_spread_node(nrs_pol2cptab(policy),
+ nrs_pol2cptid(policy)));
if (head == NULL)
return -ENOMEM;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/3 v2] staging: lustre: ptlrpc: service: Replace OBD_CPT_ALLOC etc by kzalloc_node
Replace OBD_CPT_ALLOC, OBD_CPT_ALLOC_PTR, and OBD_CPT_ALLOC_GFP by
corresponding calls to kzalloc_node. The semantic patch for the
OBD_CPT_ALLOC case is as follows: (http://coccinelle.lip6.fr/).
//
@@ expression ptr,cptab,cpt,size; @@
- OBD_CPT_ALLOC(ptr, cptab, cpt, size)
+ ptr = kzalloc_node(size, GFP_NOFS, cfs_cpt_spread_node(cptab, cpt))
//
Note that the original OBD macros would check if the cptab argument was
NULL and fall back on kzalloc in that case. Oleg Drokin argues that this
test is not needed because the code containing these calls is only invoked
after initialization has been completed, in which case the possible cptab
arguments are not NULL.
Signed-off-by: Julia Lawall
---
v2: improve the subject line
drivers/staging/lustre/lustre/ptlrpc/service.c | 27 -
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/lustre/lustre/ptlrpc/service.c
b/drivers/staging/lustre/lustre/ptlrpc/service.c
index d85db06..3fa52f1 100644
--- a/drivers/staging/lustre/lustre/ptlrpc/service.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/service.c
@@ -75,7 +75,9 @@ ptlrpc_alloc_rqbd(struct ptlrpc_service_part *svcpt)
struct ptlrpc_service *svc = svcpt->scp_service;
struct ptlrpc_request_buffer_desc *rqbd;
- OBD_CPT_ALLOC_PTR(rqbd, svc->srv_cptable, svcpt->scp_cpt);
+ rqbd = kzalloc_node(sizeof(*rqbd), GFP_NOFS,
+ cfs_cpt_spread_node(svc->srv_cptable,
+ svcpt->scp_cpt));
if (rqbd == NULL)
return NULL;
@@ -630,16 +632,18 @@ ptlrpc_service_part_init(struct ptlrpc_service *svc,
array->paa_deadline = -1;
/* allocate memory for scp_at_array (ptlrpc_at_array) */
- OBD_CPT_ALLOC(array->paa_reqs_array,
- svc->srv_cptable, cpt, sizeof(struct list_head) * size);
+ array->paa_reqs_array =
+ kzalloc_node(sizeof(struct list_head) * size, GFP_NOFS,
+cfs_cpt_spread_node(svc->srv_cptable, cpt));
if (array->paa_reqs_array == NULL)
return -ENOMEM;
for (index = 0; index < size; index++)
INIT_LIST_HEAD(&array->paa_reqs_array[index]);
- OBD_CPT_ALLOC(array->paa_reqs_count,
- svc->srv_cptable, cpt, sizeof(__u32) * size);
+ array->paa_reqs_count =
+ kzalloc_node(sizeof(__u32) * size, GFP_NOFS,
+cfs_cpt_spread_node(svc->srv_cptable, cpt));
if (array->paa_reqs_count == NULL)
goto free_reqs_array;
@@ -772,7 +776,8 @@ ptlrpc_register_service(struct ptlrpc_service_conf *conf,
else
cpt = cpts != NULL ? cpts[i] : i;
- OBD_CPT_ALLOC(svcpt, cptable, cpt, sizeof(*svcpt));
+ svcpt = kzalloc_node(sizeof(*svcpt), GFP_NOFS,
+cfs_cpt_spread_node(cptable, cpt));
if (svcpt == NULL) {
rc = -ENOMEM;
goto failed;
@@ -2664,7 +2669,9 @@ int ptlrpc_start_thread(struct ptlrpc_service_part
*svcpt, int wait)
svcpt->scp_nthrs_running == svc->srv_nthrs_cpt_init - 1))
return -EMFILE;
- OBD_CPT_ALLOC_PTR(thread, svc->srv_cptable, svcpt->scp_cpt);
+ thread = kzalloc_node(sizeof(*thread), GFP_NOFS,
+ cfs_cpt_spread_node(svc->srv_cptable,
+ svcpt->scp_cpt));
if (thread == NULL)
return -ENOMEM;
init_waitqueue_head(&thread->t_ctl_waitq);
@@ -2774,8 +2781,10 @@ int ptlrpc_hr_init(void)
hrp->hrp_nthrs /= weight;
LASSERT(hrp->hrp_nthrs > 0);
- OBD_CPT_ALLOC(hrp->hrp_thrs, ptlrpc_hr.hr_cpt_table, i,
- hrp->hrp_nthrs * sizeof(*hrt));
+ hrp->hrp_thrs =
+ kzalloc_node(hrp->hrp_nthrs * sizeof(*hrt), GFP_NOFS,
+ cfs_cpt_spread_node(ptlrpc_hr.hr_cpt_table,
+ i));
if (hrp->hrp_thrs == NULL) {
rc = -ENOMEM;
goto out;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/3 v2] Replace OBD_CPT_ALLOC etc by kzalloc_node
The complete semantic patch used to make this transformation is as follows: (http://coccinelle.lip6.fr/). // @@ expression ptr,cptab,cpt,size,gfp; @@ - OBD_CPT_ALLOC_GFP(ptr, cptab, cpt, size, gfp) + ptr = kzalloc_node(size, gfp, cfs_cpt_spread_node(cptab, cpt)) @@ expression ptr,cptab,cpt,size; @@ - OBD_CPT_ALLOC(ptr, cptab, cpt, size) + ptr = kzalloc_node(size, GFP_NOFS, cfs_cpt_spread_node(cptab, cpt)) @@ expression ptr,cptab,cpt; @@ - OBD_CPT_ALLOC_PTR(ptr, cptab, cpt) + ptr = kzalloc_node(sizeof(*ptr), GFP_NOFS, cfs_cpt_spread_node(cptab,cpt)) // Note that the previously proposed patch "Add obd_cpt_alloc function" is not needed, as the transformation is done without adding a new function. These patches should be applied after the patches with subjects "Use kzalloc and kfree" and "remove unneeded null test before free". v2 makes the subject lines more uniform. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/3 v2] staging: lustre: ptlrpc: nrs: Replace OBD_CPT_ALLOC etc by kzalloc_node
Replace OBD_CPT_ALLOC, OBD_CPT_ALLOC_PTR, and OBD_CPT_ALLOC_GFP by
corresponding calls to kzalloc_node. The semantic patch for the
OBD_CPT_ALLOC case is as follows: (http://coccinelle.lip6.fr/).
//
@@ expression ptr,cptab,cpt,size; @@
- OBD_CPT_ALLOC(ptr, cptab, cpt, size)
+ ptr = kzalloc_node(size, GFP_NOFS, cfs_cpt_spread_node(cptab, cpt))
//
Note that the original OBD macros would check if the cptab argument was
NULL and fall back on kzalloc in that case. Oleg Drokin argues that this
test is not needed because the code containing these calls is only invoked
after initialization has been completed, in which case the possible cptab
arguments are not NULL.
Signed-off-by: Julia Lawall
---
v2: improve the subject line
drivers/staging/lustre/lustre/ptlrpc/nrs.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/lustre/lustre/ptlrpc/nrs.c
b/drivers/staging/lustre/lustre/ptlrpc/nrs.c
index 68c754f..63a05f4 100644
--- a/drivers/staging/lustre/lustre/ptlrpc/nrs.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/nrs.c
@@ -746,8 +746,9 @@ static int nrs_policy_register(struct ptlrpc_nrs *nrs,
LASSERT(desc->pd_ops->op_req_dequeue != NULL);
LASSERT(desc->pd_compat != NULL);
- OBD_CPT_ALLOC_GFP(policy, svcpt->scp_service->srv_cptable,
- svcpt->scp_cpt, sizeof(*policy), GFP_NOFS);
+ policy = kzalloc_node(sizeof(*policy), GFP_NOFS,
+ cfs_cpt_spread_node(svcpt->scp_service->srv_cptable,
+ svcpt->scp_cpt));
if (policy == NULL)
return -ENOMEM;
@@ -961,9 +962,10 @@ static int nrs_svcpt_setup_locked(struct
ptlrpc_service_part *svcpt)
if (svcpt->scp_service->srv_ops.so_hpreq_handler == NULL)
goto out;
- OBD_CPT_ALLOC_PTR(svcpt->scp_nrs_hp,
- svcpt->scp_service->srv_cptable,
- svcpt->scp_cpt);
+ svcpt->scp_nrs_hp =
+ kzalloc_node(sizeof(*svcpt->scp_nrs_hp), GFP_NOFS,
+ cfs_cpt_spread_node(svcpt->scp_service->srv_cptable,
+ svcpt->scp_cpt));
if (svcpt->scp_nrs_hp == NULL) {
rc = -ENOMEM;
goto out;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/3 v2] staging: lustre: ptlrpc: nrs_fifo: Replace OBD_CPT_ALLOC etc by kzalloc_node
Replace OBD_CPT_ALLOC, OBD_CPT_ALLOC_PTR, and OBD_CPT_ALLOC_GFP by
corresponding calls to kzalloc_node. The semantic patch for the
OBD_CPT_ALLOC case is as follows: (http://coccinelle.lip6.fr/).
//
@@ expression ptr,cptab,cpt,size; @@
- OBD_CPT_ALLOC(ptr, cptab, cpt, size)
+ ptr = kzalloc_node(size, GFP_NOFS, cfs_cpt_spread_node(cptab, cpt))
//
Note that the original OBD macros would check if the cptab argument was
NULL and fall back on kzalloc in that case. Oleg Drokin argues that this
test is not needed because the code containing these calls is only invoked
after initialization has been completed, in which case the possible cptab
arguments are not NULL.
Signed-off-by: Julia Lawall
---
v2: improve the subject line
drivers/staging/lustre/lustre/ptlrpc/nrs_fifo.c |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/lustre/lustre/ptlrpc/nrs_fifo.c
b/drivers/staging/lustre/lustre/ptlrpc/nrs_fifo.c
index 2eefa25..6a61c85 100644
--- a/drivers/staging/lustre/lustre/ptlrpc/nrs_fifo.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/nrs_fifo.c
@@ -80,7 +80,9 @@ static int nrs_fifo_start(struct ptlrpc_nrs_policy *policy)
{
struct nrs_fifo_head *head;
- OBD_CPT_ALLOC_PTR(head, nrs_pol2cptab(policy), nrs_pol2cptid(policy));
+ head = kzalloc_node(sizeof(*head), GFP_NOFS,
+ cfs_cpt_spread_node(nrs_pol2cptab(policy),
+ nrs_pol2cptid(policy)));
if (head == NULL)
return -ENOMEM;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/3 v2] Replace OBD_CPT_ALLOC etc by kzalloc_node
All three are good from my perspective. Thanks! On May 3, 2015, at 10:07 AM, Julia Lawall wrote: > The complete semantic patch used to make this transformation is as follows: > (http://coccinelle.lip6.fr/). > > // > @@ > expression ptr,cptab,cpt,size,gfp; > @@ > > - OBD_CPT_ALLOC_GFP(ptr, cptab, cpt, size, gfp) > + ptr = kzalloc_node(size, gfp, cfs_cpt_spread_node(cptab, cpt)) > > @@ > expression ptr,cptab,cpt,size; > @@ > > - OBD_CPT_ALLOC(ptr, cptab, cpt, size) > + ptr = kzalloc_node(size, GFP_NOFS, cfs_cpt_spread_node(cptab, cpt)) > > @@ > expression ptr,cptab,cpt; > @@ > > - OBD_CPT_ALLOC_PTR(ptr, cptab, cpt) > + ptr = kzalloc_node(sizeof(*ptr), GFP_NOFS, cfs_cpt_spread_node(cptab,cpt)) > // > > Note that the previously proposed patch "Add obd_cpt_alloc function" is not > needed, as the transformation is done without adding a new function. > > These patches should be applied after the patches with subjects "Use > kzalloc and kfree" and "remove unneeded null test before free". > > v2 makes the subject lines more uniform. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/3] staging/lustre: Generic helpers for sysfs
On Sat, May 02, 2015 at 11:10:06PM -0400, gr...@linuxhacker.ru wrote:
> From: Oleg Drokin
>
> Add generic helpers to allow displaying oof lustre-specific values
> in /sys/fs/lustre
>
> Signed-off-by: Oleg Drokin
> ---
> .../staging/lustre/lustre/include/lprocfs_status.h | 26
> ++
> .../lustre/lustre/obdclass/lprocfs_status.c| 24
> 2 files changed, 50 insertions(+)
>
> diff --git a/drivers/staging/lustre/lustre/include/lprocfs_status.h
> b/drivers/staging/lustre/lustre/include/lprocfs_status.h
> index d030847..03500a8 100644
> --- a/drivers/staging/lustre/lustre/include/lprocfs_status.h
> +++ b/drivers/staging/lustre/lustre/include/lprocfs_status.h
> @@ -350,6 +350,8 @@ enum {
> /* class_obd.c */
> extern struct proc_dir_entry *proc_lustre_root;
>
> +extern struct kobject lustre_kobj;
> +
> struct obd_device;
> struct obd_histogram;
>
> @@ -742,6 +744,30 @@ static struct file_operations name##_fops = {
> \
> .release = lprocfs_single_release, \
> }
>
> +struct lustre_attr {
> + struct attribute attr;
> + ssize_t (*show)(struct kobject *kobj, char *);
> + ssize_t (*store)(struct kobject *kobj, const char *, size_t);
> +};
> +
> +#define LUSTRE_ATTR(name, mode, show, store) \
> +static struct lustre_attr lustre_attr_##name = __ATTR(name, mode, show,
> store)
> +
> +#define LUSTRE_ATTR_VALUE(name, mode, show, store, value) \
> +static struct lustre_attr lustre_attr_##name = \
> + { __ATTR(name, mode, show, store), value }
> +
> +#define LUSTRE_RO_ATTR(name) LUSTRE_ATTR(name, 0444, name##_show, NULL)
> +#define LUSTRE_RW_ATTR(name) LUSTRE_ATTR(name, 0644, name##_show,
> name##_store)
> +#define LUSTRE_ATTR_LIST(name) &lustre_attr_##name.attr
> +
> +ssize_t lustre_attr_show(struct kobject *kobj,
> + struct attribute *attr, char *buf);
> +ssize_t lustre_attr_store(struct kobject *kobj, struct attribute *attr,
> + const char *buf, size_t len);
> +
> +extern const struct sysfs_ops lustre_sysfs_ops;
> +
> /* lproc_ptlrpc.c */
> struct ptlrpc_request;
> extern void target_print_req(void *seq_file, struct ptlrpc_request *req);
> diff --git a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
> b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
> index c171c6c..bdfd652 100644
> --- a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
> +++ b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
> @@ -2057,3 +2057,27 @@ int lprocfs_obd_rd_max_pages_per_rpc(struct seq_file
> *m, void *data)
> EXPORT_SYMBOL(lprocfs_obd_rd_max_pages_per_rpc);
>
> #endif
> +
> +ssize_t lustre_attr_show(struct kobject *kobj,
> + struct attribute *attr, char *buf)
> +{
> + struct lustre_attr *a = container_of(attr, struct lustre_attr, attr);
> +
> + return a->show ? a->show(kobj, buf) : 0;
> +}
> +EXPORT_SYMBOL(lustre_attr_show);
> +
> +ssize_t lustre_attr_store(struct kobject *kobj, struct attribute *attr,
> + const char *buf, size_t len)
> +{
> + struct lustre_attr *a = container_of(attr, struct lustre_attr, attr);
> +
> + return a->store ? a->store(kobj, buf, len) : 0;
> +}
> +EXPORT_SYMBOL(lustre_attr_store);
> +
> +const struct sysfs_ops lustre_sysfs_ops = {
> + .show = lustre_attr_show,
> + .store = lustre_attr_store,
> +};
> +EXPORT_SYMBOL(lustre_sysfs_ops);
EXPORT_SYMBOL_GPL for all of these please, as they just wrap sysfs
stuff directly.
Other than that, looks good to me.
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/3] staging/lustre: Move /proc/fs/lustre root level files to sysfs
On Sat, May 02, 2015 at 11:10:07PM -0400, gr...@linuxhacker.ru wrote:
> From: Oleg Drokin
>
> except devices, for now.
>
> Signed-off-by: Oleg Drokin
> ---
> .../lustre/lustre/obdclass/linux/linux-module.c| 116
> +
I need Documentation/ABI/ files for all of these sysfs files. Please
put them in the drivers/staging/lustre/ directory for now.
> 1 file changed, 72 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/staging/lustre/lustre/obdclass/linux/linux-module.c
> b/drivers/staging/lustre/lustre/obdclass/linux/linux-module.c
> index 06944b8..abfb671 100644
> --- a/drivers/staging/lustre/lustre/obdclass/linux/linux-module.c
> +++ b/drivers/staging/lustre/lustre/obdclass/linux/linux-module.c
> @@ -64,6 +64,7 @@
> #include
> #include
> #include
> +#include
>
> #include "../../../include/linux/libcfs/libcfs.h"
> #include "../../../include/linux/lnet/lnetctl.h"
> @@ -216,29 +217,26 @@ struct miscdevice obd_psdev = {
> };
>
>
> -#if defined (CONFIG_PROC_FS)
> -static int obd_proc_version_seq_show(struct seq_file *m, void *v)
> +static ssize_t version_show(struct kobject *kobj, char *buf)
> {
> - seq_printf(m, "lustre: %s\nkernel: %s\nbuild: %s\n",
> -LUSTRE_VERSION_STRING, "patchless_client", BUILD_VERSION);
> - return 0;
> + return snprintf(buf, PAGE_SIZE, "lustre: %s\nkernel: %s\nbuild: %s\n",
If you are using sysfs, you never need snprintf() as you had better only
be sending something smaller than a page, or your code is wrong.
As it is here, again, please sysfs is one-value-per-file.
Yes, in some places we mess up and don't do that, but as long as I'm
reviewing stuff, you better stick to that.
And you don't need the kernel version here, or the build version, as all
of that is obvious from other api calls (i.e. uname).
So this file isn't needed at all.
> + LUSTRE_VERSION_STRING, "patchless_client",
> + BUILD_VERSION);
> }
> -LPROC_SEQ_FOPS_RO(obd_proc_version);
>
> -int obd_proc_pinger_seq_show(struct seq_file *m, void *v)
> +static ssize_t pinger_show(struct kobject *kobj, char *buf)
> {
> - seq_printf(m, "%s\n", "on");
> - return 0;
> + return snprintf(buf, PAGE_SIZE, "%s\n", "on");
> }
If it can never be a different value, why have this file at all?
> -LPROC_SEQ_FOPS_RO(obd_proc_pinger);
>
> -static int obd_proc_health_seq_show(struct seq_file *m, void *v)
> +static ssize_t health_show(struct kobject *kobj, char *buf)
> {
> bool healthy = true;
> int i;
> + size_t offset = 0;
>
> if (libcfs_catastrophe)
> - seq_printf(m, "LBUG\n");
> + offset = snprintf(buf, PAGE_SIZE, "LBUG\n");
>
> read_lock(&obd_dev_lock);
> for (i = 0; i < class_devno_max(); i++) {
This is not a single value per file.
Are you sure you shouldn't just be using debugfs for some of these? You
can do whatever you want in debugfs. As long as you can still run if
debugfs isn't enabled.
> @@ -256,8 +254,9 @@ static int obd_proc_health_seq_show(struct seq_file *m,
> void *v)
> read_unlock(&obd_dev_lock);
>
> if (obd_health_check(NULL, obd)) {
> - seq_printf(m, "device %s reported unhealthy\n",
> -obd->obd_name);
> + offset += snprintf(buf + offset, PAGE_SIZE - offset,
> +"device %s reported unhealthy\n",
> +obd->obd_name);
> healthy = false;
> }
> class_decref(obd, __func__, current);
> @@ -266,32 +265,30 @@ static int obd_proc_health_seq_show(struct seq_file *m,
> void *v)
> read_unlock(&obd_dev_lock);
>
> if (healthy)
> - seq_puts(m, "healthy\n");
> + offset += snprintf(buf + offset, PAGE_SIZE - offset,
> +"healthy\n");
> else
> - seq_puts(m, "NOT HEALTHY\n");
> + offset += snprintf(buf + offset, PAGE_SIZE - offset,
> +"NOT HEALTHY\n");
>
> - return 0;
> + return offset;
> }
> -LPROC_SEQ_FOPS_RO(obd_proc_health);
>
> -static int obd_proc_jobid_var_seq_show(struct seq_file *m, void *v)
> +static ssize_t jobid_var_show(struct kobject *kobj, char *buf)
> {
> - seq_printf(m, "%s\n", obd_jobid_var);
> - return 0;
> + return snprintf(buf, PAGE_SIZE, "%s\n", obd_jobid_var);
> }
>
> -static ssize_t obd_proc_jobid_var_seq_write(struct file *file,
> - const char __user *buffer,
> - size_t count, loff_t *off)
> +static ssize_t jobid_var_store(struct kobject *kobj,
> +const char *buffer,
> +size_t count)
> {
> if (!count || count > JOBSTATS_JOBID_VAR_MAX_LEN)
> return -EINVAL;
>
> memset(obd_jobid_var, 0, JOB
Re: [PATCH 3/3] staging/lustre/llite: move some procfs files to sysfs
On Sat, May 02, 2015 at 11:10:08PM -0400, gr...@linuxhacker.ru wrote: > From: Oleg Drokin > > Move just first few files from /proc/fs/lustre/llite/*/ > to /sys/fs/lustre/llite/*/: > blocksizefilesfree fstype kbytesfree max_read_ahead_mb > client_type filestotal kbytesavail kbytestotal uuid > > More to follow. Document please, try just doing one file per patch to make it easier to understand and review. And again, one value per file. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC PATCH 0/3] Moving lustre procfs stuff to sysfs & questions.
On Sat, May 02, 2015 at 11:10:05PM -0400, gr...@linuxhacker.ru wrote:
> From: Oleg Drokin
>
> Greg, I wonder if you can take at least a brief look and tell me if this
> is the direction you were envisioning wrt this particular cleanup.
>
> It's not all-inclusive, but I have a long flight tomorrow so
> if the direction is right, I can convert more of this stuff.
>
> This does not touch some of the multi-value stuff that would probably
> need to move to debugfs or other places, that would be a next step.
>
> Additionally some questions:
> I know sysfs is supposedly 1 file 1 value, though I see some exceptions, e.g.:
> # cat /sys/kernel/vmcoreinfo
> 2b57500 1024
>
> So we have this /proc/fs/lustre/devices that prints status of all
> "lustre internal devices (or obd for short)" like this:
> # cat /proc/fs/lustre/devices
> 0 UP mgc MGC192.168.10.226@tcp d6815182-b36d-c4ba-6aa6-065aeb9e9769 5
> 1 UP lov lustre-clilov-8800bd617800
> 32be3f55-891e-ed82-cae9-6add0770d503 4
> ...
>
> Obviously I cannot retain it as a single file, but do I really need to
> create a tree that would look like:
> /sys/fs/lustre/obd/[1...]/{status,name,type,uuid,refcount}
> (or possibly even /sys/devices/virtual/lustre_obd/... )
> or can I get away with just
> /sys/fs/lustre/obd/[1...] files where there's one line per obd like:
> 0 UP mgc MGC192.168.10.226@tcp d6815182-b36d-c4ba-6aa6-065aeb9e9769 5
> (frankly, I have not figured out yet how to dynamically add files
> to sysfs dir).
Either make a tree of kobjects, or individual files, but not one big
file, that's not acceptable for sysfs, sorry.
> What about /proc/sys stuff - /proc/sys/lustre and /proc/sys/lnet?
> Hopefully it's ok to leave those where they are?
Nope, please remove. If you aren't a process value, you can't add new
proc files.
> Also I found that cgroup does calls into kernfs directly, gaining
> ability to register seq_file files and otherwise do all sorts of
> stuff that would be cool to do from lustre too ;)
If you want to make lustrefs, be my guest, you can do whatever you want
in that, using kernfs, but that's not ok for sysfs, sorry.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/11] Use kzalloc and kfree
On Fri, May 01, 2015 at 05:51:11PM +0200, Julia Lawall wrote: > Replace OBD_ALLOC, OBD_ALLOC_WAIT, OBD_ALLOC_PTR, and OBD_ALLOC_PTR_WAIT by > kalloc/kcalloc, and OBD_FREE and OBD_FREE_PTR by kfree. The complete > semantic patch that makes these changes is as follows: > (http://coccinelle.lip6.fr/) You lost the leading '0' on the early patches here, did you use git send-email for these? I'll just sort them by hand, but in the future, please fix it up to make it easier to apply things. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/11] Use kzalloc and kfree
On Sun, 3 May 2015, Greg Kroah-Hartman wrote: > On Fri, May 01, 2015 at 05:51:11PM +0200, Julia Lawall wrote: > > Replace OBD_ALLOC, OBD_ALLOC_WAIT, OBD_ALLOC_PTR, and OBD_ALLOC_PTR_WAIT by > > kalloc/kcalloc, and OBD_FREE and OBD_FREE_PTR by kfree. The complete > > semantic patch that makes these changes is as follows: > > (http://coccinelle.lip6.fr/) > > > > You lost the leading '0' on the early patches here, did you use git > send-email for these? I'll just sort them by hand, but in the future, > please fix it up to make it easier to apply things. I will fix it. julia ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging/lustre: Always try kmalloc first for OBD_ALLOC_LARGE
On Sat, May 02, 2015 at 11:58:21PM -0400, gr...@linuxhacker.ru wrote:
> From: Oleg Drokin
>
> Create libcfs_kvzalloc and libcfs_kvzalloc_cpt are
> are designed to replace OBD_ALLOC_LARGE and OBD_CPT_ALLOC_LARGE.
>
> Not a drop-in replacement as they also take gfp flags armument
> for more flexibility.
>
> Signed-off-by: Oleg Drokin
> ---
> This is how I envision the OBD_ALLOC_LARGE replacement.
>
> BTW, it appears we also have LIBCFS_ALLOC and friends that we probably dont'
> need either.
>
> Thanks your your great help!
>
> .../staging/lustre/include/linux/libcfs/libcfs.h | 4 ++
> .../staging/lustre/lustre/include/obd_support.h| 24 ++---
> drivers/staging/lustre/lustre/libcfs/Makefile | 1 +
> .../staging/lustre/lustre/libcfs/linux/linux-mem.c | 59
> ++
> 4 files changed, 67 insertions(+), 21 deletions(-)
> create mode 100644 drivers/staging/lustre/lustre/libcfs/linux/linux-mem.c
>
> diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs.h
> b/drivers/staging/lustre/include/linux/libcfs/libcfs.h
> index 4410d7f..947df7e 100644
> --- a/drivers/staging/lustre/include/linux/libcfs/libcfs.h
> +++ b/drivers/staging/lustre/include/linux/libcfs/libcfs.h
> @@ -184,4 +184,8 @@ static inline void *__container_of(void *ptr, unsigned
> long shift)
>
> #define _LIBCFS_H
>
> +void *libcfs_kvzalloc(size_t size, gfp_t flags);
> +void *libcfs_kvzalloc_cpt(struct cfs_cpt_table *cptab, int cpt, size_t size,
> + gfp_t flags);
> +
> #endif /* _LIBCFS_H */
> diff --git a/drivers/staging/lustre/lustre/include/obd_support.h
> b/drivers/staging/lustre/lustre/include/obd_support.h
> index 2991d2e..379266d 100644
> --- a/drivers/staging/lustre/lustre/include/obd_support.h
> +++ b/drivers/staging/lustre/lustre/include/obd_support.h
> @@ -676,37 +676,19 @@ do {
> \
>__OBD_VMALLOC_VEROBSE(ptr, cptab, cpt, size)
>
>
> -/* Allocations above this size are considered too big and could not be done
> - * atomically.
> - *
> - * Be very careful when changing this value, especially when decreasing it,
> - * since vmalloc in Linux doesn't perform well on multi-cores system, calling
> - * vmalloc in critical path would hurt performance badly. See LU-66.
> - */
> -#define OBD_ALLOC_BIG (4 * PAGE_CACHE_SIZE)
> -
> #define OBD_ALLOC_LARGE(ptr, size) \
> do { \
> - if (size > OBD_ALLOC_BIG)\
> - OBD_VMALLOC(ptr, size);\
> - else \
> - OBD_ALLOC(ptr, size);\
> + ptr = libcfs_kvzalloc(size, GFP_NOFS);\
> } while (0)
Just fix up all callers of these functions, if there are any anymore.
>
> #define OBD_CPT_ALLOC_LARGE(ptr, cptab, cpt, size) \
> do { \
> - if (size > OBD_ALLOC_BIG) \
> - OBD_CPT_VMALLOC(ptr, cptab, cpt, size); \
> - else \
> - OBD_CPT_ALLOC(ptr, cptab, cpt, size); \
> + ptr = libcfs_kvzalloc_cpt(cptab, cpt, size, GFP_NOFS);\
> } while (0)
Same here.
>
> #define OBD_FREE_LARGE(ptr, size) \
> do { \
> - if (size > OBD_ALLOC_BIG)\
> - OBD_VFREE(ptr, size);\
> - else \
> - OBD_FREE(ptr, size); \
> + kvfree(ptr); \
Same here.
> } while (0)
>
>
> diff --git a/drivers/staging/lustre/lustre/libcfs/Makefile
> b/drivers/staging/lustre/lustre/libcfs/Makefile
> index 2996a48..fabdd3e 100644
> --- a/drivers/staging/lustre/lustre/libcfs/Makefile
> +++ b/drivers/staging/lustre/lustre/libcfs/Makefile
> @@ -7,6 +7,7 @@ libcfs-linux-objs += linux-curproc.o
> libcfs-linux-objs += linux-module.o
> libcfs-linux-objs += linux-crypto.o
> libcfs-linux-objs += linux-crypto-adler.o
> +libcfs-linux-objs += linux-mem.o
>
> libcfs-linux-objs := $(addprefix linux/,$(libcfs-linux-objs))
>
> diff --git a/drivers/staging/lustre/lustre/libcfs/linux/linux-mem.c
> b/drivers/staging/lustre/lustre/libcfs/linux/linux-mem.c
> new file mode 100644
> index 000..1b068ae
> --- /dev/null
> +++ b/drivers/staging/lustre/lustre/libcfs/linux/linux-mem.c
> @@ -0,0 +1,59 @@
> +/*
Re: [PATCH V4] Fix pointer cast for 32 bits arch
On Thu, Apr 16, 2015 at 08:36:51PM +0200, Peter Senna Tschudin wrote:
> On Thu, Apr 16, 2015 at 7:05 PM, Alan Cox wrote:
> > On Thu, 2015-04-16 at 20:01 +0300, Dan Carpenter wrote:
> >> On Thu, Apr 16, 2015 at 06:14:55PM +0200, Geert Uytterhoeven wrote:
> >> > On Thu, Apr 16, 2015 at 3:39 PM, Peter Senna Tschudin
> >> > wrote:
> >> > > --- a/drivers/staging/goldfish/goldfish_audio.c
> >> > > +++ b/drivers/staging/goldfish/goldfish_audio.c
> >> > > @@ -63,7 +63,7 @@ struct goldfish_audio {
> >> > > #define AUDIO_READ(data, addr) (readl(data->reg_base + addr))
> >> > > #define AUDIO_WRITE(data, addr, x) (writel(x, data->reg_base +
> >> > > addr))
> >> > > #define AUDIO_WRITE64(data, addr, addr2, x)\
> >> > > - (gf_write64((u64)(x), data->reg_base + addr,
> >> > > data->reg_base+addr2))
> >> > > + (gf_write_ptr((void *)(x), data->reg_base + addr,
> >> > > data->reg_base+addr2))
> >> >
> >> > This one should not be converted, as all callers pass a dma_addr_t,
> >> > which may
> >> > be 64-bit on 32-bit systems, i.e. larger than void *.
> >>
> >> Ugh... You're right.
> >>
> >> I've been avoiding asking this but I can't any longer. What is
> >> gf_write64() actually doing? We are writing dma addresses, user space
> >> pointers and kernel space pointers to this hardware?
> >>
> >> This stuff doesn't seem to make any kind of sense and I can easily
> >> imagine a situation where it wrote a 64 bit pointer. Then we partially
> >> write over it with a 32 bit userspace pointer. Then it writes somewhere
> >> totally unintended.
> >>
> >> This thing doesn't make any sort of sense to me.
> >
> > Its a 64 on 64 or 32 on 32 virtual machine. Goldfish is used for Android
> > emulation for all the system level phone emulation tools. On the
> > emulation side it provides an interface for the emulated OS but makes no
> > effort to emulate it as if it was a real hardware. If you think of it as
> > a funky emulator interface all is good. If you think about it as
> > "hardware" you've got the wrong model and chunks of Goldfish make less
> > sense.
>
> Is is better to leave the code as is, and ignore the compiler / sparse
> warnings for i386? Or is the proposal welcome if done correctly? And
> if so what would be correctly?
What's the status with this mess?
Oh, and please, put a "staging: goldfish:" at the front of your subject
line in the future so I know to look at it, otherwise it gets lost at
times...
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: dgnc: Using a temporary value for repeated dereferences.
On Mon, Apr 20, 2015 at 09:33:04AM +0200, Yorick Rommers wrote: > Sorry, it has been changed in the patch below. > > --- > > A patch for a line being too long (>80) in dgnc_mgmt.c, > fixed by making a temporary value for dgnc_Board[brd], > replacing all instanced of dgnc_Board[brd] with temporary value, > and removing unnecessary typecasts. > > Signed-off-by: Yorick Rommers > --- > drivers/staging/dgnc/dgnc_mgmt.c | 19 +++ > 1 file changed, 11 insertions(+), 8 deletions(-) I can't apply this as-is, please fix up and resend in a format that I can apply it in. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/2 RESEND] staging: dgnc: remove dead code in dgnc_tty_write()
On Fri, Apr 10, 2015 at 05:48:54PM +0300, Giedrius Statkevičius wrote: > Remove the dead code protected by in_user in dgnc_tty_write() because it is > set > to 0 and never changed to 1 thus the code in ifs never gets executed. Please wrap your commit lines at 72 columns in the future. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: daqboard2000: Use preferred comment style
On Sat, May 02, 2015 at 05:00:34PM +0200, Arno Tiemersma wrote: > Use the preferred block comment style for the copyright and driver > description header comments. > > Signed-off-by: Arno Tiemersma > --- > drivers/staging/comedi/drivers/daqboard2000.c | 196 > +- > 1 file changed, 99 insertions(+), 97 deletions(-) Doesn't apply to my tree :( ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 10/21] staging: rtl8192e: replace memcpy() -> ether_addr_copy_unaligned()
On Mon, Apr 13, 2015 at 11:47:33PM +0200, Mateusz Kulikowski wrote: > rtl8192e driver uses memcpy() to copy hw addresses in several places. > checkpatch.pl suggests to use ether_addr_copy(), but most of > addresses in driver may be unaligned. > This patch replaces all memcpy occurences with single macro therby > silencing checkpatch.pl (single complaints in macro remains). > > Signed-off-by: Mateusz Kulikowski > --- > drivers/staging/rtl8192e/rtl819x_BAProc.c| 12 ++--- > drivers/staging/rtl8192e/rtllib.h| 3 ++ > drivers/staging/rtl8192e/rtllib_crypt_tkip.c | 18 +++ > drivers/staging/rtl8192e/rtllib_rx.c | 74 > +--- > drivers/staging/rtl8192e/rtllib_softmac.c| 67 + > drivers/staging/rtl8192e/rtllib_softmac_wx.c | 5 +- > drivers/staging/rtl8192e/rtllib_tx.c | 24 - > 7 files changed, 113 insertions(+), 90 deletions(-) I've stopped applying here, no need to resend the first 9 of these patches for your next round. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging/lustre: Always try kmalloc first for OBD_ALLOC_LARGE
Hello!
On May 3, 2015, at 2:31 PM, Greg KH wrote:
>> -/* Allocations above this size are considered too big and could not be done
>> - * atomically.
>> - *
>> - * Be very careful when changing this value, especially when decreasing it,
>> - * since vmalloc in Linux doesn't perform well on multi-cores system,
>> calling
>> - * vmalloc in critical path would hurt performance badly. See LU-66.
>> - */
>> -#define OBD_ALLOC_BIG (4 * PAGE_CACHE_SIZE)
>> -
>> #define OBD_ALLOC_LARGE(ptr, size) \
>> do { \
>> -if (size > OBD_ALLOC_BIG)\
>> -OBD_VMALLOC(ptr, size);\
>> -else \
>> -OBD_ALLOC(ptr, size);\
>> +ptr = libcfs_kvzalloc(size, GFP_NOFS);\
>> } while (0)
>
> Just fix up all callers of these functions, if there are any anymore.
This is what Julia is doing. I am providing the stub for her wonderful scripts
to unwrap per her request.
>> + */
>> +/*
>> + * Copyright (c) 2015, Oleg Drokin
>
> I think your employer would like a different line here...
Only on stuff that I do at work when I am getting paid.
Stuff that I do on my own uncompensated, I own all the rights to, I hope.
Bye,
Oleg
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 10/21] staging: rtl8192e: replace memcpy() -> ether_addr_copy_unaligned()
Hi, On 03.05.2015 21:10, Greg KH wrote: > On Mon, Apr 13, 2015 at 11:47:33PM +0200, Mateusz Kulikowski wrote: >> rtl8192e driver uses memcpy() to copy hw addresses in several places. >> checkpatch.pl suggests to use ether_addr_copy(), but most of >> addresses in driver may be unaligned. >> This patch replaces all memcpy occurences with single macro therby >> silencing checkpatch.pl (single complaints in macro remains). >> >> Signed-off-by: Mateusz Kulikowski >> --- >> drivers/staging/rtl8192e/rtl819x_BAProc.c| 12 ++--- >> drivers/staging/rtl8192e/rtllib.h| 3 ++ >> drivers/staging/rtl8192e/rtllib_crypt_tkip.c | 18 +++ >> drivers/staging/rtl8192e/rtllib_rx.c | 74 >> +--- >> drivers/staging/rtl8192e/rtllib_softmac.c| 67 + >> drivers/staging/rtl8192e/rtllib_softmac_wx.c | 5 +- >> drivers/staging/rtl8192e/rtllib_tx.c | 24 - >> 7 files changed, 113 insertions(+), 90 deletions(-) > > I've stopped applying here, no need to resend the first 9 of these > patches for your next round. Thanks, I was afraid you applied whole changeset by accident. I'm working on v3 (a bit slowly due to holidays/work) - hopefully will post it this/next week. Regards, Mateusz ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3] staging: sm750fb: use arch_phys_wc_add() and ioremap_wc()
On Tue, Apr 21, 2015 at 01:12:03PM -0700, Luis R. Rodriguez wrote: > From: "Luis R. Rodriguez" > > The same area used for ioremap() is used for the MTRR area. > Convert the driver from using the x86 specific MTRR code to > the architecture agnostic arch_phys_wc_add(). arch_phys_wc_add() > will avoid MTRR if write-combining is available, in order to > take advantage of that also ensure the ioremap'd area is requested > as write-combining. > > There are a few motivations for this: > > a) Take advantage of PAT when available > > b) Help bury MTRR code away, MTRR is architecture specific and on >x86 its replaced by PAT > > c) Help with the goal of eventually using _PAGE_CACHE_UC over >_PAGE_CACHE_UC_MINUS on x86 on ioremap_nocache() (see commit >de33c442e titled "x86 PAT: fix performance drop for glx, >use UC minus for ioremap(), ioremap_nocache() and >pci_mmap_page_range()") > > The conversion done is expressed by the following Coccinelle > SmPL patch, it additionally required manual intervention to > address all the #ifdery and removal of redundant things which > arch_phys_wc_add() already addresses such as verbose message > about when MTRR fails and doing nothing when we didn't get > an MTRR. > > @ mtrr_found @ > expression index, base, size; > @@ > > -index = mtrr_add(base, size, MTRR_TYPE_WRCOMB, 1); > +index = arch_phys_wc_add(base, size); > > @ mtrr_rm depends on mtrr_found @ > expression mtrr_found.index, mtrr_found.base, mtrr_found.size; > @@ > > -mtrr_del(index, base, size); > +arch_phys_wc_del(index); > > @ mtrr_rm_zero_arg depends on mtrr_found @ > expression mtrr_found.index; > @@ > > -mtrr_del(index, 0, 0); > +arch_phys_wc_del(index); > > @ mtrr_rm_fb_info depends on mtrr_found @ > struct fb_info *info; > expression mtrr_found.index; > @@ > > -mtrr_del(index, info->fix.smem_start, info->fix.smem_len); > +arch_phys_wc_del(index); > > @ ioremap_replace_nocache depends on mtrr_found @ > struct fb_info *info; > expression base, size; > @@ > > -info->screen_base = ioremap_nocache(base, size); > +info->screen_base = ioremap_wc(base, size); > > @ ioremap_replace_default depends on mtrr_found @ > struct fb_info *info; > expression base, size; > @@ > > -info->screen_base = ioremap(base, size); > +info->screen_base = ioremap_wc(base, size); > > Generated-by: Coccinelle SmPL > Cc: Sudip Mukherjee > Cc: Teddy Wang > Cc: Greg Kroah-Hartman > Cc: Suresh Siddha > Cc: Ingo Molnar > Cc: Thomas Gleixner > Cc: Juergen Gross > Cc: Daniel Vetter > Cc: Andy Lutomirski > Cc: Dave Airlie > Cc: Antonino Daplas > Cc: Jean-Christophe Plagniol-Villard > Cc: Tomi Valkeinen > Cc: de...@driverdev.osuosl.org > Cc: linux-fb...@vger.kernel.org > Cc: linux-ker...@vger.kernel.org > Signed-off-by: Luis R. Rodriguez > --- > drivers/staging/sm750fb/sm750.c| 36 > drivers/staging/sm750fb/sm750.h| 3 --- > drivers/staging/sm750fb/sm750_hw.c | 3 +-- > 3 files changed, 5 insertions(+), 37 deletions(-) This doesn't apply to my staging-next branch :( ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 2/2] staging: sm750fb: cleanup indentation
On Fri, Apr 24, 2015 at 11:10:56AM -0400, Charles Rose wrote: > This patch fixes indentation errors/warnings reported by checkpatch.pl. > > Signed-off-by: Charles Rose > --- > drivers/staging/sm750fb/ddk750_mode.c | 24 > 1 file changed, 16 insertions(+), 8 deletions(-) Does not apply to my tree :( ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC PATCH 0/3] Moving lustre procfs stuff to sysfs & questions.
On May 3, 2015, at 2:15 PM, Greg Kroah-Hartman wrote: >> Also I found that cgroup does calls into kernfs directly, gaining >> ability to register seq_file files and otherwise do all sorts of >> stuff that would be cool to do from lustre too ;) > If you want to make lustrefs, be my guest, you can do whatever you want > in that, using kernfs, but that's not ok for sysfs, sorry. If only __kernfs_create_file was exported… ;) Otherwise you cannot really do that from modules. Bye, Oleg ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging/lustre: Always try kmalloc first for OBD_ALLOC_LARGE
On Sun, May 03, 2015 at 03:14:58PM -0400, Oleg Drokin wrote:
> Hello!
>
> On May 3, 2015, at 2:31 PM, Greg KH wrote:
>
> >> -/* Allocations above this size are considered too big and could not be
> >> done
> >> - * atomically.
> >> - *
> >> - * Be very careful when changing this value, especially when decreasing
> >> it,
> >> - * since vmalloc in Linux doesn't perform well on multi-cores system,
> >> calling
> >> - * vmalloc in critical path would hurt performance badly. See LU-66.
> >> - */
> >> -#define OBD_ALLOC_BIG (4 * PAGE_CACHE_SIZE)
> >> -
> >> #define OBD_ALLOC_LARGE(ptr, size) \
> >> do {
> >> \
> >> - if (size > OBD_ALLOC_BIG)\
> >> - OBD_VMALLOC(ptr, size);\
> >> - else \
> >> - OBD_ALLOC(ptr, size);\
> >> + ptr = libcfs_kvzalloc(size, GFP_NOFS);\
> >> } while (0)
> >
> > Just fix up all callers of these functions, if there are any anymore.
>
> This is what Julia is doing. I am providing the stub for her wonderful
> scripts to unwrap per her request.
>
> >> + */
> >> +/*
> >> + * Copyright (c) 2015, Oleg Drokin
> >
> > I think your employer would like a different line here...
>
> Only on stuff that I do at work when I am getting paid.
>
> Stuff that I do on my own uncompensated, I own all the rights to, I hope.
I wouldn't be so sure about this, please read your employment contract,
almost no companies allow this, it is very rare.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/3] staging/lustre: Move /proc/fs/lustre root level files to sysfs
On May 3, 2015, at 2:13 PM, Greg Kroah-Hartman wrote:
>>
>> -#if defined (CONFIG_PROC_FS)
>> -static int obd_proc_version_seq_show(struct seq_file *m, void *v)
>> +static ssize_t version_show(struct kobject *kobj, char *buf)
>> {
>> -seq_printf(m, "lustre: %s\nkernel: %s\nbuild: %s\n",
>> - LUSTRE_VERSION_STRING, "patchless_client", BUILD_VERSION);
>> -return 0;
>> +return snprintf(buf, PAGE_SIZE, "lustre: %s\nkernel: %s\nbuild: %s\n",
>
> If you are using sysfs, you never need snprintf() as you had better only
> be sending something smaller than a page, or your code is wrong.
Well, I am copying straight from ext4 here (and in other places),
that I thought was a good model.
Is there a better model?
I guess I can convert to sprintfs in most of places, though.
> As it is here, again, please sysfs is one-value-per-file.
>
> Yes, in some places we mess up and don't do that, but as long as I'm
> reviewing stuff, you better stick to that.
>
> And you don't need the kernel version here, or the build version, as all
> of that is obvious from other api calls (i.e. uname).
That would be true if only people did not apply patches to their kernels.
I guess we don't care all that much about kernel version here as it could be
obtained from uname,
but lustre version is kind of important for the case that people patch it up
(and they do), so can I leave just that?
> So this file isn't needed at all.
>
>> +LUSTRE_VERSION_STRING, "patchless_client",
>> +BUILD_VERSION);
>> }
>> -LPROC_SEQ_FOPS_RO(obd_proc_version);
>>
>> -int obd_proc_pinger_seq_show(struct seq_file *m, void *v)
>> +static ssize_t pinger_show(struct kobject *kobj, char *buf)
>> {
>> -seq_printf(m, "%s\n", "on");
>> -return 0;
>> +return snprintf(buf, PAGE_SIZE, "%s\n", "on");
>> }
>
> If it can never be a different value, why have this file at all?
>
>> -LPROC_SEQ_FOPS_RO(obd_proc_pinger);
>>
>> -static int obd_proc_health_seq_show(struct seq_file *m, void *v)
>> +static ssize_t health_show(struct kobject *kobj, char *buf)
>> {
>> bool healthy = true;
>> int i;
>> +size_t offset = 0;
>>
>> if (libcfs_catastrophe)
>> -seq_printf(m, "LBUG\n");
>> +offset = snprintf(buf, PAGE_SIZE, "LBUG\n");
>>
>> read_lock(&obd_dev_lock);
>> for (i = 0; i < class_devno_max(); i++) {
>
> This is not a single value per file.
>
> Are you sure you shouldn't just be using debugfs for some of these? You
> can do whatever you want in debugfs. As long as you can still run if
> debugfs isn't enabled.
I am definitely going to use debugfs in some cases.
This particular one I guess we'll need to split into per-obd health status
and the overall "system health" that could be easily queried.
>> +#if defined(CONFIG_PROC_FS)
>> /* Root for /proc/fs/lustre */
>> struct proc_dir_entry *proc_lustre_root = NULL;
>> EXPORT_SYMBOL(proc_lustre_root);
>>
>> struct lprocfs_vars lprocfs_base[] = {
>> -{ "version", &obd_proc_version_fops },
>> -{ "pinger", &obd_proc_pinger_fops },
>> -{ "health_check", &obd_proc_health_fops },
>> -{ "jobid_var", &obd_proc_jobid_var_fops },
>> -{ .name = "jobid_name",
>> - .fops = &obd_proc_jobid_name_fops},
>> { NULL }
>> };
>>
>> +LUSTRE_RO_ATTR(version);
>> +LUSTRE_RO_ATTR(pinger);
>> +LUSTRE_RO_ATTR(health);
>> +LUSTRE_RW_ATTR(jobid_var);
>> +LUSTRE_RW_ATTR(jobid_name);
>
> Are these static? If not, why not?
Yes, they are static.
>> +static struct attribute *lustre_attrs[] = {
>> +LUSTRE_ATTR_LIST(version),
>> +LUSTRE_ATTR_LIST(pinger),
>> +LUSTRE_ATTR_LIST(health),
>> +LUSTRE_ATTR_LIST(jobid_name),
>> +LUSTRE_ATTR_LIST(jobid_var),
>
> Do you really need this type of macro? Just spell the structures out.
Again, I am going by the fine example of ext4 here.
But I am happy to spell it out if you think it's better that way.
>> +NULL,
>> +};
>> +
>> static void *obd_device_list_seq_start(struct seq_file *p, loff_t *pos)
>> {
>> if (*pos >= class_devno_max())
>> @@ -419,15 +422,36 @@ struct file_operations obd_device_list_fops = {
>> .release = seq_release,
>> };
>>
>> +struct kobject lustre_kobj;
>> +EXPORT_SYMBOL(lustre_kobj);
>
> EXPORT_SYMBOL_GPL?
>
>
>> +
>> +static DECLARE_COMPLETION(lustre_kobj_unregister);
>> +static void lustre_sysfs_release(struct kobject *kobj)
>> +{
>> +complete(&lustre_kobj_unregister);
>
> Not good, you should just clean up and go, this means something is
> really wrong, and you should NEVER have a static kobject. Which you do.
> Which is wrong.
This is how we tell the cleanup code that we are all done, though?
Otherwise if I do kobject_put there and move on (to module unload),
how all the other potential users of my kobject would be protected
from the code disappearign from under them?
I can dynamically allocate lustre_kobj, but that does not really help
this particular scenario.
>> +}
>> +
>>
Beloved !
From: Mrs.Sherry Page Address, Israel +97239150789 03/05/2015 Beloved I got your esteem contact out of desperate search for a reliable and trustworthy person. Your efficiency mercy is needed.The holy book emphasizes so much on goodness this offer must be kept secret, because whosoever that is sowing a seed must be honest and keep it within him/her self get back to me as soon as possible Regards, Yours, Mrs. Sherry Page ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] drivers: staging: fbtft: fbtft-bus.c: Fix different address space warning on I/O mem
On Fri, May 01, 2015 at 11:40:35PM -0700, Tolga Ceylan wrote: > To fix sparse warning of incorrect type in assignment > (different address space), added annotation __iomem to > vmem8 and modified direct reads with ioread8(). > > Signed-off-by: Tolga Ceylan > --- > drivers/staging/fbtft/fbtft-bus.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) Someone send this before you did, sorry :( ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: comedi: daqboard2000: Use preferred comment style
Use the preferred block comment style for the copyright and driver description header comments. Signed-off-by: Arno Tiemersma --- drivers/staging/comedi/drivers/daqboard2000.c | 196 +- 1 file changed, 98 insertions(+), 98 deletions(-) diff --git a/drivers/staging/comedi/drivers/daqboard2000.c b/drivers/staging/comedi/drivers/daqboard2000.c index 2ca8d3e..611b0a3 100644 --- a/drivers/staging/comedi/drivers/daqboard2000.c +++ b/drivers/staging/comedi/drivers/daqboard2000.c @@ -1,105 +1,105 @@ /* - comedi/drivers/daqboard2000.c - hardware driver for IOtech DAQboard/2000 - - COMEDI - Linux Control and Measurement Device Interface - Copyright (C) 1999 Anders Blomdell - - This program is free software; you can redistribute it and/or modify - it under the terms of the GNU General Public License as published by - the Free Software Foundation; either version 2 of the License, or - (at your option) any later version. - - This program is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - GNU General Public License for more details. + * comedi/drivers/daqboard2000.c + * hardware driver for IOtech DAQboard/2000 + * + * COMEDI - Linux Control and Measurement Device Interface + * Copyright (C) 1999 Anders Blomdell + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. */ /* -Driver: daqboard2000 -Description: IOTech DAQBoard/2000 -Author: Anders Blomdell -Status: works -Updated: Mon, 14 Apr 2008 15:28:52 +0100 -Devices: [IOTech] DAQBoard/2000 (daqboard2000) - -Much of the functionality of this driver was determined from reading -the source code for the Windows driver. - -The FPGA on the board requires fimware, which is available from -http://www.comedi.org in the comedi_nonfree_firmware tarball. - -Configuration options: not applicable, uses PCI auto config -*/ + * Driver: daqboard2000 + * Description: IOTech DAQBoard/2000 + * Author: Anders Blomdell + * Status: works + * Updated: Mon, 14 Apr 2008 15:28:52 +0100 + * Devices: [IOTech] DAQBoard/2000 (daqboard2000) + * + * Much of the functionality of this driver was determined from reading + * the source code for the Windows driver. + * + * The FPGA on the board requires fimware, which is available from + * http://www.comedi.org in the comedi_nonfree_firmware tarball. + * + * Configuration options: not applicable, uses PCI auto config + */ /* - This card was obviously never intended to leave the Windows world, - since it lacked all kind of hardware documentation (except for cable - pinouts, plug and pray has something to catch up with yet). - - With some help from our swedish distributor, we got the Windows sourcecode - for the card, and here are the findings so far. - - 1. A good document that describes the PCI interface chip is 9080db-106.pdf - available from http://www.plxtech.com/products/io/pci9080 - - 2. The initialization done so far is: - a. program the FPGA (windows code sans a lot of error messages) - b. - - 3. Analog out seems to work OK with DAC's disabled, if DAC's are enabled, - you have to output values to all enabled DAC's until result appears, I - guess that it has something to do with pacer clocks, but the source - gives me no clues. I'll keep it simple so far. - - 4. Analog in. - Each channel in the scanlist seems to be controlled by four - control words: - - Word0: - +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ - ! | | | ! | | | ! | | | ! | | | ! - +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ - - Word1: - +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ - ! | | | ! | | | ! | | | ! | | | ! - +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ - | | | | | | | - +--+--+ | | | | +-- Digital input (??) - | | | | + 10 us settling time - | | | +-- Suspend acquisition (last to scan) - | | + Simultaneous sample and hold - | +-- Signed data format - +- Correction offset low - - Word2: - +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ - ! | | | ! | | | ! | | | ! | | | ! - +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ - | | | | | | | | | | - +-+ +--+--+ +++ +++ +--+--+ - | | |
