* [PATCH] nvmet-fc: use zero-sized array and struct_size() in kzalloc()
@ 2019-02-23 18:51 Gustavo A. R. Silva
2019-02-23 20:05 ` Joe Perches
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Gustavo A. R. Silva @ 2019-02-23 18:51 UTC (permalink / raw)
To: James Smart, Christoph Hellwig, Sagi Grimberg
Cc: linux-nvme, linux-kernel, Gustavo A. R. Silva
Update the code to use a zero-sized array instead of a pointer in
structure nvmet_fc_tgt_queue and use struct_size() in kzalloc().
Notice that one of the more common cases of allocation size calculations
is finding the size of a structure that has a zero-sized array at the end,
along with memory for some number of elements for that array. For example:
struct foo {
int stuff;
struct boo entry[];
};
instance = kzalloc(sizeof(struct foo) + sizeof(struct boo) * count, GFP_KERNEL);
Instead of leaving these open-coded and prone to type mistakes, we can now
use the new struct_size() helper:
instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL);
This code was detected with the help of Coccinelle.
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
drivers/nvme/target/fc.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
index 1e9654f04c60..23baec38f97e 100644
--- a/drivers/nvme/target/fc.c
+++ b/drivers/nvme/target/fc.c
@@ -128,12 +128,12 @@ struct nvmet_fc_tgt_queue {
struct nvmet_cq nvme_cq;
struct nvmet_sq nvme_sq;
struct nvmet_fc_tgt_assoc *assoc;
- struct nvmet_fc_fcp_iod *fod; /* array of fcp_iods */
struct list_head fod_list;
struct list_head pending_cmd_list;
struct list_head avail_defer_list;
struct workqueue_struct *work_q;
struct kref ref;
+ struct nvmet_fc_fcp_iod fod[]; /* array of fcp_iods */
} __aligned(sizeof(unsigned long long));
struct nvmet_fc_tgt_assoc {
@@ -588,9 +588,7 @@ nvmet_fc_alloc_target_queue(struct nvmet_fc_tgt_assoc *assoc,
if (qid > NVMET_NR_QUEUES)
return NULL;
- queue = kzalloc((sizeof(*queue) +
- (sizeof(struct nvmet_fc_fcp_iod) * sqsize)),
- GFP_KERNEL);
+ queue = kzalloc(struct_size(queue, fod, sqsize), GFP_KERNEL);
if (!queue)
return NULL;
@@ -603,7 +601,6 @@ nvmet_fc_alloc_target_queue(struct nvmet_fc_tgt_assoc *assoc,
if (!queue->work_q)
goto out_a_put;
- queue->fod = (struct nvmet_fc_fcp_iod *)&queue[1];
queue->qid = qid;
queue->sqsize = sqsize;
queue->assoc = assoc;
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] nvmet-fc: use zero-sized array and struct_size() in kzalloc()
2019-02-23 18:51 [PATCH] nvmet-fc: use zero-sized array and struct_size() in kzalloc() Gustavo A. R. Silva
@ 2019-02-23 20:05 ` Joe Perches
2019-02-23 20:28 ` Gustavo A. R. Silva
2019-03-08 13:27 ` Christoph Hellwig
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Joe Perches @ 2019-02-23 20:05 UTC (permalink / raw)
To: Gustavo A. R. Silva, James Smart, Christoph Hellwig, Sagi Grimberg
Cc: linux-nvme, linux-kernel
On Sat, 2019-02-23 at 12:51 -0600, Gustavo A. R. Silva wrote:
> Update the code to use a zero-sized array instead of a pointer in
> structure nvmet_fc_tgt_queue and use struct_size() in kzalloc().
[]
> This code was detected with the help of Coccinelle.
Really?
Impressive script that found this one.
> diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
[]
> @@ -128,12 +128,12 @@ struct nvmet_fc_tgt_queue {
> struct nvmet_cq nvme_cq;
> struct nvmet_sq nvme_sq;
> struct nvmet_fc_tgt_assoc *assoc;
> - struct nvmet_fc_fcp_iod *fod; /* array of fcp_iods */
> struct list_head fod_list;
> struct list_head pending_cmd_list;
> struct list_head avail_defer_list;
> struct workqueue_struct *work_q;
> struct kref ref;
> + struct nvmet_fc_fcp_iod fod[]; /* array of fcp_iods */
> } __aligned(sizeof(unsigned long long));
Moving a pointer from the middle of a struct to
the end seems unusual for coccinelle.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] nvmet-fc: use zero-sized array and struct_size() in kzalloc()
2019-02-23 20:05 ` Joe Perches
@ 2019-02-23 20:28 ` Gustavo A. R. Silva
2019-02-24 1:34 ` Gustavo A. R. Silva
0 siblings, 1 reply; 8+ messages in thread
From: Gustavo A. R. Silva @ 2019-02-23 20:28 UTC (permalink / raw)
To: Joe Perches, James Smart, Christoph Hellwig, Sagi Grimberg
Cc: linux-nvme, linux-kernel
Hey Joe,
On 2/23/19 2:05 PM, Joe Perches wrote:
> On Sat, 2019-02-23 at 12:51 -0600, Gustavo A. R. Silva wrote:
>> Update the code to use a zero-sized array instead of a pointer in
>> structure nvmet_fc_tgt_queue and use struct_size() in kzalloc().
> []
>> This code was detected with the help of Coccinelle.
>
> Really?
> Impressive script that found this one.
>
See my comments below.
>> diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
> []
>> @@ -128,12 +128,12 @@ struct nvmet_fc_tgt_queue {
>> struct nvmet_cq nvme_cq;
>> struct nvmet_sq nvme_sq;
>> struct nvmet_fc_tgt_assoc *assoc;
>> - struct nvmet_fc_fcp_iod *fod; /* array of fcp_iods */
>> struct list_head fod_list;
>> struct list_head pending_cmd_list;
>> struct list_head avail_defer_list;
>> struct workqueue_struct *work_q;
>> struct kref ref;
>> + struct nvmet_fc_fcp_iod fod[]; /* array of fcp_iods */
>> } __aligned(sizeof(unsigned long long));
>
> Moving a pointer from the middle of a struct to
> the end seems unusual for coccinelle.
>
>
Notice that the commit log says "detected", which does not imply
the script made the transformation by itself. :)
And all the script detected was this piece of code:
queue = kzalloc((sizeof(*queue) +
(sizeof(struct nvmet_fc_fcp_iod) * sqsize)),
GFP_KERNEL);
Thanks
--
Gustavo
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] nvmet-fc: use zero-sized array and struct_size() in kzalloc()
2019-02-23 20:28 ` Gustavo A. R. Silva
@ 2019-02-24 1:34 ` Gustavo A. R. Silva
0 siblings, 0 replies; 8+ messages in thread
From: Gustavo A. R. Silva @ 2019-02-24 1:34 UTC (permalink / raw)
To: Joe Perches, James Smart, Christoph Hellwig, Sagi Grimberg
Cc: linux-nvme, linux-kernel
On 2/23/19 2:28 PM, Gustavo A. R. Silva wrote:
> Hey Joe,
>
> On 2/23/19 2:05 PM, Joe Perches wrote:
>> On Sat, 2019-02-23 at 12:51 -0600, Gustavo A. R. Silva wrote:
>>> Update the code to use a zero-sized array instead of a pointer in
>>> structure nvmet_fc_tgt_queue and use struct_size() in kzalloc().
>> []
>>> This code was detected with the help of Coccinelle.
>>
>> Really?
>> Impressive script that found this one.
>>
>
> See my comments below.
>
>>> diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
>> []
>>> @@ -128,12 +128,12 @@ struct nvmet_fc_tgt_queue {
>>> struct nvmet_cq nvme_cq;
>>> struct nvmet_sq nvme_sq;
>>> struct nvmet_fc_tgt_assoc *assoc;
>>> - struct nvmet_fc_fcp_iod *fod; /* array of fcp_iods */
>>> struct list_head fod_list;
>>> struct list_head pending_cmd_list;
>>> struct list_head avail_defer_list;
>>> struct workqueue_struct *work_q;
>>> struct kref ref;
>>> + struct nvmet_fc_fcp_iod fod[]; /* array of fcp_iods */
>>> } __aligned(sizeof(unsigned long long));
>>
>> Moving a pointer from the middle of a struct to
>> the end seems unusual for coccinelle.
>>
>>
>
> Notice that the commit log says "detected", which does not imply
> the script made the transformation by itself. :)
>
> And all the script detected was this piece of code:
>
> queue = kzalloc((sizeof(*queue) +
> (sizeof(struct nvmet_fc_fcp_iod) * sqsize)),
> GFP_KERNEL);
>
>
Which is enough to mention the tool.
Thanks
--
Gustavo
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] nvmet-fc: use zero-sized array and struct_size() in kzalloc()
2019-02-23 18:51 [PATCH] nvmet-fc: use zero-sized array and struct_size() in kzalloc() Gustavo A. R. Silva
2019-02-23 20:05 ` Joe Perches
@ 2019-03-08 13:27 ` Christoph Hellwig
2019-03-08 19:15 ` James Smart
2019-03-12 19:33 ` Christoph Hellwig
3 siblings, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2019-03-08 13:27 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: James Smart, Christoph Hellwig, Sagi Grimberg, linux-nvme, linux-kernel
James, can you take a look at this one?
On Sat, Feb 23, 2019 at 12:51:08PM -0600, Gustavo A. R. Silva wrote:
> Update the code to use a zero-sized array instead of a pointer in
> structure nvmet_fc_tgt_queue and use struct_size() in kzalloc().
>
> Notice that one of the more common cases of allocation size calculations
> is finding the size of a structure that has a zero-sized array at the end,
> along with memory for some number of elements for that array. For example:
>
> struct foo {
> int stuff;
> struct boo entry[];
> };
>
> instance = kzalloc(sizeof(struct foo) + sizeof(struct boo) * count, GFP_KERNEL);
>
> Instead of leaving these open-coded and prone to type mistakes, we can now
> use the new struct_size() helper:
>
> instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL);
>
> This code was detected with the help of Coccinelle.
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
> drivers/nvme/target/fc.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
> index 1e9654f04c60..23baec38f97e 100644
> --- a/drivers/nvme/target/fc.c
> +++ b/drivers/nvme/target/fc.c
> @@ -128,12 +128,12 @@ struct nvmet_fc_tgt_queue {
> struct nvmet_cq nvme_cq;
> struct nvmet_sq nvme_sq;
> struct nvmet_fc_tgt_assoc *assoc;
> - struct nvmet_fc_fcp_iod *fod; /* array of fcp_iods */
> struct list_head fod_list;
> struct list_head pending_cmd_list;
> struct list_head avail_defer_list;
> struct workqueue_struct *work_q;
> struct kref ref;
> + struct nvmet_fc_fcp_iod fod[]; /* array of fcp_iods */
> } __aligned(sizeof(unsigned long long));
>
> struct nvmet_fc_tgt_assoc {
> @@ -588,9 +588,7 @@ nvmet_fc_alloc_target_queue(struct nvmet_fc_tgt_assoc *assoc,
> if (qid > NVMET_NR_QUEUES)
> return NULL;
>
> - queue = kzalloc((sizeof(*queue) +
> - (sizeof(struct nvmet_fc_fcp_iod) * sqsize)),
> - GFP_KERNEL);
> + queue = kzalloc(struct_size(queue, fod, sqsize), GFP_KERNEL);
> if (!queue)
> return NULL;
>
> @@ -603,7 +601,6 @@ nvmet_fc_alloc_target_queue(struct nvmet_fc_tgt_assoc *assoc,
> if (!queue->work_q)
> goto out_a_put;
>
> - queue->fod = (struct nvmet_fc_fcp_iod *)&queue[1];
> queue->qid = qid;
> queue->sqsize = sqsize;
> queue->assoc = assoc;
> --
> 2.20.1
---end quoted text---
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] nvmet-fc: use zero-sized array and struct_size() in kzalloc()
2019-02-23 18:51 [PATCH] nvmet-fc: use zero-sized array and struct_size() in kzalloc() Gustavo A. R. Silva
2019-02-23 20:05 ` Joe Perches
2019-03-08 13:27 ` Christoph Hellwig
@ 2019-03-08 19:15 ` James Smart
2019-03-12 19:33 ` Christoph Hellwig
3 siblings, 0 replies; 8+ messages in thread
From: James Smart @ 2019-03-08 19:15 UTC (permalink / raw)
To: Gustavo A. R. Silva, Christoph Hellwig, Sagi Grimberg
Cc: linux-nvme, linux-kernel
On 2/23/2019 10:51 AM, Gustavo A. R. Silva wrote:
> Update the code to use a zero-sized array instead of a pointer in
> structure nvmet_fc_tgt_queue and use struct_size() in kzalloc().
>
> Notice that one of the more common cases of allocation size calculations
> is finding the size of a structure that has a zero-sized array at the end,
> along with memory for some number of elements for that array. For example:
>
> struct foo {
> int stuff;
> struct boo entry[];
> };
>
> instance = kzalloc(sizeof(struct foo) + sizeof(struct boo) * count, GFP_KERNEL);
>
> Instead of leaving these open-coded and prone to type mistakes, we can now
> use the new struct_size() helper:
>
> instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL);
>
> This code was detected with the help of Coccinelle.
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
> drivers/nvme/target/fc.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
> index 1e9654f04c60..23baec38f97e 100644
> --- a/drivers/nvme/target/fc.c
> +++ b/drivers/nvme/target/fc.c
> @@ -128,12 +128,12 @@ struct nvmet_fc_tgt_queue {
> struct nvmet_cq nvme_cq;
> struct nvmet_sq nvme_sq;
> struct nvmet_fc_tgt_assoc *assoc;
> - struct nvmet_fc_fcp_iod *fod; /* array of fcp_iods */
> struct list_head fod_list;
> struct list_head pending_cmd_list;
> struct list_head avail_defer_list;
> struct workqueue_struct *work_q;
> struct kref ref;
> + struct nvmet_fc_fcp_iod fod[]; /* array of fcp_iods */
> } __aligned(sizeof(unsigned long long));
>
> struct nvmet_fc_tgt_assoc {
> @@ -588,9 +588,7 @@ nvmet_fc_alloc_target_queue(struct nvmet_fc_tgt_assoc *assoc,
> if (qid > NVMET_NR_QUEUES)
> return NULL;
>
> - queue = kzalloc((sizeof(*queue) +
> - (sizeof(struct nvmet_fc_fcp_iod) * sqsize)),
> - GFP_KERNEL);
> + queue = kzalloc(struct_size(queue, fod, sqsize), GFP_KERNEL);
> if (!queue)
> return NULL;
>
> @@ -603,7 +601,6 @@ nvmet_fc_alloc_target_queue(struct nvmet_fc_tgt_assoc *assoc,
> if (!queue->work_q)
> goto out_a_put;
>
> - queue->fod = (struct nvmet_fc_fcp_iod *)&queue[1];
> queue->qid = qid;
> queue->sqsize = sqsize;
> queue->assoc = assoc;
Reviewed-by: James Smart <james.smart@broadcom.com>
I guess this is a better style.
-- james
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] nvmet-fc: use zero-sized array and struct_size() in kzalloc()
2019-02-23 18:51 [PATCH] nvmet-fc: use zero-sized array and struct_size() in kzalloc() Gustavo A. R. Silva
` (2 preceding siblings ...)
2019-03-08 19:15 ` James Smart
@ 2019-03-12 19:33 ` Christoph Hellwig
2019-03-12 19:47 ` Gustavo A. R. Silva
3 siblings, 1 reply; 8+ messages in thread
From: Christoph Hellwig @ 2019-03-12 19:33 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: James Smart, Christoph Hellwig, Sagi Grimberg, linux-nvme, linux-kernel
Thanks,
applied to nvme-5.2.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] nvmet-fc: use zero-sized array and struct_size() in kzalloc()
2019-03-12 19:33 ` Christoph Hellwig
@ 2019-03-12 19:47 ` Gustavo A. R. Silva
0 siblings, 0 replies; 8+ messages in thread
From: Gustavo A. R. Silva @ 2019-03-12 19:47 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: James Smart, Sagi Grimberg, linux-nvme, linux-kernel
On 3/12/19 2:33 PM, Christoph Hellwig wrote:
> Thanks,
>
> applied to nvme-5.2.
>
Great.
Thanks, Christoph.
--
Gustavo
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-03-12 19:48 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-23 18:51 [PATCH] nvmet-fc: use zero-sized array and struct_size() in kzalloc() Gustavo A. R. Silva
2019-02-23 20:05 ` Joe Perches
2019-02-23 20:28 ` Gustavo A. R. Silva
2019-02-24 1:34 ` Gustavo A. R. Silva
2019-03-08 13:27 ` Christoph Hellwig
2019-03-08 19:15 ` James Smart
2019-03-12 19:33 ` Christoph Hellwig
2019-03-12 19:47 ` Gustavo A. R. Silva
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).