* [PATCH 0/2] vhost/scsi: Adjustments for five function implementations
@ 2017-05-20 14:30 SF Markus Elfring
2017-05-20 14:31 ` [PATCH 1/2] vhost/scsi: Improve a size determination in four functions SF Markus Elfring
2017-05-20 14:32 ` [PATCH 2/2] vhost/scsi: Delete error messages for failed memory allocations in five functions SF Markus Elfring
0 siblings, 2 replies; 12+ messages in thread
From: SF Markus Elfring @ 2017-05-20 14:30 UTC (permalink / raw)
To: kvm, netdev, virtualization, Jason Wang, Michael S. Tsirkin
Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 20 May 2017 16:25:04 +0200
A few update suggestions were taken into account
from static source code analysis.
Markus Elfring (2):
Improve a size determination in four functions
Delete error messages for failed memory allocations in five functions
drivers/vhost/scsi.c | 33 +++++++++++----------------------
1 file changed, 11 insertions(+), 22 deletions(-)
--
2.13.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/2] vhost/scsi: Improve a size determination in four functions
2017-05-20 14:30 [PATCH 0/2] vhost/scsi: Adjustments for five function implementations SF Markus Elfring
@ 2017-05-20 14:31 ` SF Markus Elfring
2017-05-22 9:37 ` Stefan Hajnoczi
2017-05-20 14:32 ` [PATCH 2/2] vhost/scsi: Delete error messages for failed memory allocations in five functions SF Markus Elfring
1 sibling, 1 reply; 12+ messages in thread
From: SF Markus Elfring @ 2017-05-20 14:31 UTC (permalink / raw)
To: kvm, netdev, virtualization, Jason Wang, Michael S. Tsirkin
Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 20 May 2017 13:48:44 +0200
Replace the specification of four data structures by pointer dereferences
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/vhost/scsi.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index fd6c8b66f06f..650533916c19 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -597,8 +597,7 @@ vhost_scsi_get_tag(struct vhost_virtqueue *vq, struct vhost_scsi_tpg *tpg,
sg = cmd->tvc_sgl;
prot_sg = cmd->tvc_prot_sgl;
pages = cmd->tvc_upages;
- memset(cmd, 0, sizeof(struct vhost_scsi_cmd));
-
+ memset(cmd, 0, sizeof(*cmd));
cmd->tvc_sgl = sg;
cmd->tvc_prot_sgl = prot_sg;
cmd->tvc_upages = pages;
@@ -1757,5 +1756,5 @@ static int vhost_scsi_make_nexus(struct vhost_scsi_tpg *tpg,
return -EEXIST;
}
- tv_nexus = kzalloc(sizeof(struct vhost_scsi_nexus), GFP_KERNEL);
+ tv_nexus = kzalloc(sizeof(*tv_nexus), GFP_KERNEL);
if (!tv_nexus) {
@@ -1958,5 +1957,5 @@ vhost_scsi_make_tpg(struct se_wwn *wwn,
if (kstrtou16(name + 5, 10, &tpgt) || tpgt >= VHOST_SCSI_MAX_TARGET)
return ERR_PTR(-EINVAL);
- tpg = kzalloc(sizeof(struct vhost_scsi_tpg), GFP_KERNEL);
+ tpg = kzalloc(sizeof(*tpg), GFP_KERNEL);
if (!tpg) {
@@ -2012,5 +2011,5 @@ vhost_scsi_make_tport(struct target_fabric_configfs *tf,
/* if (vhost_scsi_parse_wwn(name, &wwpn, 1) < 0)
return ERR_PTR(-EINVAL); */
- tport = kzalloc(sizeof(struct vhost_scsi_tport), GFP_KERNEL);
+ tport = kzalloc(sizeof(*tport), GFP_KERNEL);
if (!tport) {
--
2.13.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/2] vhost/scsi: Delete error messages for failed memory allocations in five functions
2017-05-20 14:30 [PATCH 0/2] vhost/scsi: Adjustments for five function implementations SF Markus Elfring
2017-05-20 14:31 ` [PATCH 1/2] vhost/scsi: Improve a size determination in four functions SF Markus Elfring
@ 2017-05-20 14:32 ` SF Markus Elfring
2017-05-22 9:43 ` Stefan Hajnoczi
1 sibling, 1 reply; 12+ messages in thread
From: SF Markus Elfring @ 2017-05-20 14:32 UTC (permalink / raw)
To: kvm, netdev, virtualization, Jason Wang, Michael S. Tsirkin
Cc: LKML, kernel-janitors, Wolfram Sang
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 20 May 2017 15:50:30 +0200
Omit seven extra messages for memory allocation failures in these functions.
This issue was detected by using the Coccinelle software.
Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/vhost/scsi.c | 24 +++++++-----------------
1 file changed, 7 insertions(+), 17 deletions(-)
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 650533916c19..49d07950e2e5 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -417,5 +417,4 @@ vhost_scsi_allocate_evt(struct vhost_scsi *vs,
if (!evt) {
- vq_err(vq, "Failed to allocate vhost_scsi_evt\n");
vs->vs_events_missed = true;
return NULL;
}
@@ -1722,21 +1721,15 @@ static int vhost_scsi_nexus_cb(struct se_portal_group *se_tpg,
- if (!tv_cmd->tvc_sgl) {
- pr_err("Unable to allocate tv_cmd->tvc_sgl\n");
+ if (!tv_cmd->tvc_sgl)
goto out;
- }
tv_cmd->tvc_upages = kzalloc(sizeof(struct page *) *
VHOST_SCSI_PREALLOC_UPAGES, GFP_KERNEL);
- if (!tv_cmd->tvc_upages) {
- pr_err("Unable to allocate tv_cmd->tvc_upages\n");
+ if (!tv_cmd->tvc_upages)
goto out;
- }
tv_cmd->tvc_prot_sgl = kzalloc(sizeof(struct scatterlist) *
VHOST_SCSI_PREALLOC_PROT_SGLS, GFP_KERNEL);
- if (!tv_cmd->tvc_prot_sgl) {
- pr_err("Unable to allocate tv_cmd->tvc_prot_sgl\n");
+ if (!tv_cmd->tvc_prot_sgl)
goto out;
- }
}
return 0;
out:
@@ -1760,6 +1753,5 @@ static int vhost_scsi_make_nexus(struct vhost_scsi_tpg *tpg,
if (!tv_nexus) {
mutex_unlock(&tpg->tv_tpg_mutex);
- pr_err("Unable to allocate struct vhost_scsi_nexus\n");
return -ENOMEM;
}
/*
@@ -1961,7 +1953,6 @@ vhost_scsi_make_tpg(struct se_wwn *wwn,
- if (!tpg) {
- pr_err("Unable to allocate struct vhost_scsi_tpg");
+ if (!tpg)
return ERR_PTR(-ENOMEM);
- }
+
mutex_init(&tpg->tv_tpg_mutex);
INIT_LIST_HEAD(&tpg->tv_tpg_list);
tpg->tport = tport;
@@ -2015,7 +2006,6 @@ vhost_scsi_make_tport(struct target_fabric_configfs *tf,
- if (!tport) {
- pr_err("Unable to allocate struct vhost_scsi_tport");
+ if (!tport)
return ERR_PTR(-ENOMEM);
- }
+
tport->tport_wwpn = wwpn;
/*
* Determine the emulated Protocol Identifier and Target Port Name
--
2.13.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] vhost/scsi: Improve a size determination in four functions
2017-05-20 14:31 ` [PATCH 1/2] vhost/scsi: Improve a size determination in four functions SF Markus Elfring
@ 2017-05-22 9:37 ` Stefan Hajnoczi
0 siblings, 0 replies; 12+ messages in thread
From: Stefan Hajnoczi @ 2017-05-22 9:37 UTC (permalink / raw)
To: SF Markus Elfring
Cc: kvm, netdev, virtualization, Jason Wang, Michael S. Tsirkin,
kernel-janitors, LKML
[-- Attachment #1: Type: text/plain, Size: 621 bytes --]
On Sat, May 20, 2017 at 04:31:13PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 20 May 2017 13:48:44 +0200
>
> Replace the specification of four data structures by pointer dereferences
> as the parameter for the operator "sizeof" to make the corresponding size
> determination a bit safer according to the Linux coding style convention.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
> drivers/vhost/scsi.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] vhost/scsi: Delete error messages for failed memory allocations in five functions
2017-05-20 14:32 ` [PATCH 2/2] vhost/scsi: Delete error messages for failed memory allocations in five functions SF Markus Elfring
@ 2017-05-22 9:43 ` Stefan Hajnoczi
2017-05-22 10:50 ` SF Markus Elfring
0 siblings, 1 reply; 12+ messages in thread
From: Stefan Hajnoczi @ 2017-05-22 9:43 UTC (permalink / raw)
To: SF Markus Elfring
Cc: kvm, netdev, virtualization, Jason Wang, Michael S. Tsirkin,
kernel-janitors, LKML, Wolfram Sang
[-- Attachment #1: Type: text/plain, Size: 1437 bytes --]
On Sat, May 20, 2017 at 04:32:17PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 20 May 2017 15:50:30 +0200
>
> Omit seven extra messages for memory allocation failures in these functions.
>
> This issue was detected by using the Coccinelle software.
>
> Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
Please include an actual explanation for this change instead of linking
to slides. Why are you trying to get rid of memory allocation failure
messages?
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
> drivers/vhost/scsi.c | 24 +++++++-----------------
> 1 file changed, 7 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> index 650533916c19..49d07950e2e5 100644
> --- a/drivers/vhost/scsi.c
> +++ b/drivers/vhost/scsi.c
> @@ -417,5 +417,4 @@ vhost_scsi_allocate_evt(struct vhost_scsi *vs,
> if (!evt) {
> - vq_err(vq, "Failed to allocate vhost_scsi_evt\n");
#define vq_err(vq, fmt, ...) do { \
pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
if ((vq)->error_ctx) \
eventfd_signal((vq)->error_ctx, 1);\
} while (0)
You silently dropped the eventfd_signal() call. Please explain.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] vhost/scsi: Delete error messages for failed memory allocations in five functions
2017-05-22 9:43 ` Stefan Hajnoczi
@ 2017-05-22 10:50 ` SF Markus Elfring
2017-05-22 11:23 ` Stefan Hajnoczi
0 siblings, 1 reply; 12+ messages in thread
From: SF Markus Elfring @ 2017-05-22 10:50 UTC (permalink / raw)
To: Stefan Hajnoczi, kvm, netdev, virtualization, kernel-janitors
Cc: Jason Wang, Michael S. Tsirkin, LKML, Wolfram Sang
>> Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
>
> Please include an actual explanation for this change instead of linking
> to slides.
Do you care for a bit of code size reduction by removal of questionable
error messages?
> Why are you trying to get rid of memory allocation failure messages?
Do you find information from a Linux allocation failure report sufficient
for any function implementations here?
>> +++ b/drivers/vhost/scsi.c
>> @@ -417,5 +417,4 @@ vhost_scsi_allocate_evt(struct vhost_scsi *vs,
>> if (!evt) {
>> - vq_err(vq, "Failed to allocate vhost_scsi_evt\n");
>
> #define vq_err(vq, fmt, ...) do { \
> pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
> if ((vq)->error_ctx) \
> eventfd_signal((vq)->error_ctx, 1);\
> } while (0)
>
> You silently dropped the eventfd_signal() call.
Do you prefer to preserve this special error handling then?
Regards,
Markus
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] vhost/scsi: Delete error messages for failed memory allocations in five functions
2017-05-22 10:50 ` SF Markus Elfring
@ 2017-05-22 11:23 ` Stefan Hajnoczi
2017-05-22 11:34 ` SF Markus Elfring
2017-05-22 12:38 ` [PATCH 2/2] " Dan Carpenter
0 siblings, 2 replies; 12+ messages in thread
From: Stefan Hajnoczi @ 2017-05-22 11:23 UTC (permalink / raw)
To: SF Markus Elfring
Cc: kvm, netdev, virtualization, kernel-janitors, Jason Wang,
Michael S. Tsirkin, LKML, Wolfram Sang
[-- Attachment #1: Type: text/plain, Size: 1309 bytes --]
On Mon, May 22, 2017 at 12:50:39PM +0200, SF Markus Elfring wrote:
> > Why are you trying to get rid of memory allocation failure messages?
>
> Do you find information from a Linux allocation failure report sufficient
> for any function implementations here?
If kmalloc() and friends guarantee to print a warning and backtrace on
every allocation failure, then there's no need for error messages in
callers.
That seems like good justification that can go in the commit
description, but I'm not sure if kmalloc() and friends guarantee to show
a message (not just the first time, but for every failed allocation)?
> >> +++ b/drivers/vhost/scsi.c
> >> @@ -417,5 +417,4 @@ vhost_scsi_allocate_evt(struct vhost_scsi *vs,
> >> if (!evt) {
> >> - vq_err(vq, "Failed to allocate vhost_scsi_evt\n");
> >
> > #define vq_err(vq, fmt, ...) do { \
> > pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
> > if ((vq)->error_ctx) \
> > eventfd_signal((vq)->error_ctx, 1);\
> > } while (0)
> >
> > You silently dropped the eventfd_signal() call.
>
> Do you prefer to preserve this special error handling then?
Yes, please leave vq_err() calls.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: vhost/scsi: Delete error messages for failed memory allocations in five functions
2017-05-22 11:23 ` Stefan Hajnoczi
@ 2017-05-22 11:34 ` SF Markus Elfring
2017-05-22 14:09 ` Stefan Hajnoczi
2017-05-22 12:38 ` [PATCH 2/2] " Dan Carpenter
1 sibling, 1 reply; 12+ messages in thread
From: SF Markus Elfring @ 2017-05-22 11:34 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: kvm, netdev, virtualization, kernel-janitors, Jason Wang,
Michael S. Tsirkin, LKML, Wolfram Sang
>> Do you find information from a Linux allocation failure report sufficient
>> for any function implementations here?
>
> If kmalloc() and friends guarantee to print a warning and backtrace on
> every allocation failure, then there's no need for error messages in
> callers.
>
> That seems like good justification that can go in the commit
> description, but I'm not sure if kmalloc() and friends guarantee to show
> a message (not just the first time, but for every failed allocation)?
I am also looking for a more complete and easier accessible documentation
for this aspect of the desired exception handling.
How would we like to resolve any remaining open issues there?
Regards,
Markus
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] vhost/scsi: Delete error messages for failed memory allocations in five functions
2017-05-22 11:23 ` Stefan Hajnoczi
2017-05-22 11:34 ` SF Markus Elfring
@ 2017-05-22 12:38 ` Dan Carpenter
2017-05-22 14:08 ` Stefan Hajnoczi
1 sibling, 1 reply; 12+ messages in thread
From: Dan Carpenter @ 2017-05-22 12:38 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: SF Markus Elfring, kvm, netdev, virtualization, kernel-janitors,
Jason Wang, Michael S. Tsirkin, LKML, Wolfram Sang
On Mon, May 22, 2017 at 12:23:20PM +0100, Stefan Hajnoczi wrote:
> I'm not sure if kmalloc() and friends guarantee to show
> a message (not just the first time, but for every failed allocation)?
>
It prints multiple times, but it's ratelimited. It can also be disabled
using a config option.
See slab_out_of_memory().
regards,
dan carpenter
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] vhost/scsi: Delete error messages for failed memory allocations in five functions
2017-05-22 12:38 ` [PATCH 2/2] " Dan Carpenter
@ 2017-05-22 14:08 ` Stefan Hajnoczi
0 siblings, 0 replies; 12+ messages in thread
From: Stefan Hajnoczi @ 2017-05-22 14:08 UTC (permalink / raw)
To: Dan Carpenter
Cc: SF Markus Elfring, kvm, netdev, virtualization, kernel-janitors,
Jason Wang, Michael S. Tsirkin, LKML, Wolfram Sang
[-- Attachment #1: Type: text/plain, Size: 433 bytes --]
On Mon, May 22, 2017 at 03:38:33PM +0300, Dan Carpenter wrote:
> On Mon, May 22, 2017 at 12:23:20PM +0100, Stefan Hajnoczi wrote:
> > I'm not sure if kmalloc() and friends guarantee to show
> > a message (not just the first time, but for every failed allocation)?
> >
>
> It prints multiple times, but it's ratelimited. It can also be disabled
> using a config option.
>
> See slab_out_of_memory().
Thanks!
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: vhost/scsi: Delete error messages for failed memory allocations in five functions
2017-05-22 11:34 ` SF Markus Elfring
@ 2017-05-22 14:09 ` Stefan Hajnoczi
2017-05-22 14:21 ` SF Markus Elfring
0 siblings, 1 reply; 12+ messages in thread
From: Stefan Hajnoczi @ 2017-05-22 14:09 UTC (permalink / raw)
To: SF Markus Elfring
Cc: kvm, netdev, virtualization, kernel-janitors, Jason Wang,
Michael S. Tsirkin, LKML, Wolfram Sang
[-- Attachment #1: Type: text/plain, Size: 861 bytes --]
On Mon, May 22, 2017 at 01:34:34PM +0200, SF Markus Elfring wrote:
> >> Do you find information from a Linux allocation failure report sufficient
> >> for any function implementations here?
> >
> > If kmalloc() and friends guarantee to print a warning and backtrace on
> > every allocation failure, then there's no need for error messages in
> > callers.
> >
> > That seems like good justification that can go in the commit
> > description, but I'm not sure if kmalloc() and friends guarantee to show
> > a message (not just the first time, but for every failed allocation)?
>
> I am also looking for a more complete and easier accessible documentation
> for this aspect of the desired exception handling.
> How would we like to resolve any remaining open issues there?
No objection from me but please make sure to keep vq_err().
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: vhost/scsi: Delete error messages for failed memory allocations in five functions
2017-05-22 14:09 ` Stefan Hajnoczi
@ 2017-05-22 14:21 ` SF Markus Elfring
0 siblings, 0 replies; 12+ messages in thread
From: SF Markus Elfring @ 2017-05-22 14:21 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: kvm, netdev, virtualization, kernel-janitors, Jason Wang,
Michael S. Tsirkin, LKML
> No objection from me but please make sure to keep vq_err().
How long should I wait before I may dare to send another variant for the
discussed update suggestion?
Which commit message would be acceptable then for this update step?
Regards,
Markus
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-05-22 14:22 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-20 14:30 [PATCH 0/2] vhost/scsi: Adjustments for five function implementations SF Markus Elfring
2017-05-20 14:31 ` [PATCH 1/2] vhost/scsi: Improve a size determination in four functions SF Markus Elfring
2017-05-22 9:37 ` Stefan Hajnoczi
2017-05-20 14:32 ` [PATCH 2/2] vhost/scsi: Delete error messages for failed memory allocations in five functions SF Markus Elfring
2017-05-22 9:43 ` Stefan Hajnoczi
2017-05-22 10:50 ` SF Markus Elfring
2017-05-22 11:23 ` Stefan Hajnoczi
2017-05-22 11:34 ` SF Markus Elfring
2017-05-22 14:09 ` Stefan Hajnoczi
2017-05-22 14:21 ` SF Markus Elfring
2017-05-22 12:38 ` [PATCH 2/2] " Dan Carpenter
2017-05-22 14:08 ` Stefan Hajnoczi
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).