Re: [PATCH net] net: hyperv: initialize link status correctly
From: Jason Wang Date: Mon, 27 Jan 2014 15:30:54 +0800 > Call netif_carrier_on() after register_device(). Otherwise it won't work since > the device was still in NETREG_UNINITIALIZED state. > > Fixes a68f9614614749727286f675d15f1e09d13cb54a > (hyperv: Fix race between probe and open calls) > > Cc: Haiyang Zhang > Cc: K. Y. Srinivasan > Reported-by: Di Nie > Tested-by: Di Nie > Signed-off-by: Jason Wang A device up can occur at the moment you call register_netdevice(), therefore that up call can see the carrier as down and fail or similar. So you really cannot resolve the carrier to be on in this way. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH net] net: hyperv: initialize link status correctly
On 01/27/2014 04:35 PM, David Miller wrote: > From: Jason Wang > Date: Mon, 27 Jan 2014 15:30:54 +0800 > >> Call netif_carrier_on() after register_device(). Otherwise it won't work >> since >> the device was still in NETREG_UNINITIALIZED state. >> >> Fixes a68f9614614749727286f675d15f1e09d13cb54a >> (hyperv: Fix race between probe and open calls) >> >> Cc: Haiyang Zhang >> Cc: K. Y. Srinivasan >> Reported-by: Di Nie >> Tested-by: Di Nie >> Signed-off-by: Jason Wang > A device up can occur at the moment you call register_netdevice(), > therefore that up call can see the carrier as down and fail or > similar. So you really cannot resolve the carrier to be on in this > way. True, we need a workqueue to synchronize them. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH net] net: hyperv: initialize link status correctly
On Mon, 2014-01-27 at 17:40 +0800, Jason Wang wrote: > On 01/27/2014 04:35 PM, David Miller wrote: > > From: Jason Wang > > Date: Mon, 27 Jan 2014 15:30:54 +0800 > > > >> Call netif_carrier_on() after register_device(). Otherwise it won't work > >> since > >> the device was still in NETREG_UNINITIALIZED state. > >> > >> Fixes a68f9614614749727286f675d15f1e09d13cb54a > >> (hyperv: Fix race between probe and open calls) > >> > >> Cc: Haiyang Zhang > >> Cc: K. Y. Srinivasan > >> Reported-by: Di Nie > >> Tested-by: Di Nie > >> Signed-off-by: Jason Wang > > A device up can occur at the moment you call register_netdevice(), > > therefore that up call can see the carrier as down and fail or > > similar. So you really cannot resolve the carrier to be on in this > > way. > > True, we need a workqueue to synchronize them. Whatever for? All you need to do is: rtnl_lock(); register_netdevice(); netif_carrier_on(); rtnl_unlock(); It would be nice if we could make the current code work with a change in the core, though. Ben. -- Ben Hutchings If at first you don't succeed, you're doing about average. signature.asc Description: This is a digitally signed message part ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH net] net: hyperv: initialize link status correctly
On 01/27/2014 06:22 PM, Ben Hutchings wrote: > On Mon, 2014-01-27 at 17:40 +0800, Jason Wang wrote: >> On 01/27/2014 04:35 PM, David Miller wrote: >>> From: Jason Wang >>> Date: Mon, 27 Jan 2014 15:30:54 +0800 >>> Call netif_carrier_on() after register_device(). Otherwise it won't work since the device was still in NETREG_UNINITIALIZED state. Fixes a68f9614614749727286f675d15f1e09d13cb54a (hyperv: Fix race between probe and open calls) Cc: Haiyang Zhang Cc: K. Y. Srinivasan Reported-by: Di Nie Tested-by: Di Nie Signed-off-by: Jason Wang >>> A device up can occur at the moment you call register_netdevice(), >>> therefore that up call can see the carrier as down and fail or >>> similar. So you really cannot resolve the carrier to be on in this >>> way. >> True, we need a workqueue to synchronize them. > Whatever for? All you need to do is: > > rtnl_lock(); > register_netdevice(); > netif_carrier_on(); > rtnl_unlock(); > > It would be nice if we could make the current code work with a change in > the core, though. > > Ben. > Looks like the link status interrupt may happen during this (after netvsc_device_add() was called by rndis_filter_device_add()) without any synchronization. This may lead a wrong link status here. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH net] net: hyperv: initialize link status correctly
On Mon, 2014-01-27 at 18:28 +0800, Jason Wang wrote: > On 01/27/2014 06:22 PM, Ben Hutchings wrote: > > On Mon, 2014-01-27 at 17:40 +0800, Jason Wang wrote: > >> On 01/27/2014 04:35 PM, David Miller wrote: > >>> From: Jason Wang > >>> Date: Mon, 27 Jan 2014 15:30:54 +0800 > >>> > Call netif_carrier_on() after register_device(). Otherwise it won't work > since > the device was still in NETREG_UNINITIALIZED state. > > Fixes a68f9614614749727286f675d15f1e09d13cb54a > (hyperv: Fix race between probe and open calls) > > Cc: Haiyang Zhang > Cc: K. Y. Srinivasan > Reported-by: Di Nie > Tested-by: Di Nie > Signed-off-by: Jason Wang > >>> A device up can occur at the moment you call register_netdevice(), > >>> therefore that up call can see the carrier as down and fail or > >>> similar. So you really cannot resolve the carrier to be on in this > >>> way. > >> True, we need a workqueue to synchronize them. > > Whatever for? All you need to do is: > > > > rtnl_lock(); > > register_netdevice(); > > netif_carrier_on(); > > rtnl_unlock(); > > > > It would be nice if we could make the current code work with a change in > > the core, though. > > > > Ben. > > > > Looks like the link status interrupt may happen during this (after > netvsc_device_add() was called by rndis_filter_device_add()) without any > synchronization. This may lead a wrong link status here. Now I'm confused - if there's a link status interrupt, why are you setting the carrier on initially? Ben. -- Ben Hutchings If at first you don't succeed, you're doing about average. signature.asc Description: This is a digitally signed message part ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH net] net: hyperv: initialize link status correctly
On 01/27/2014 06:30 PM, Ben Hutchings wrote: > On Mon, 2014-01-27 at 18:28 +0800, Jason Wang wrote: >> On 01/27/2014 06:22 PM, Ben Hutchings wrote: >>> On Mon, 2014-01-27 at 17:40 +0800, Jason Wang wrote: On 01/27/2014 04:35 PM, David Miller wrote: > From: Jason Wang > Date: Mon, 27 Jan 2014 15:30:54 +0800 > >> Call netif_carrier_on() after register_device(). Otherwise it won't work >> since >> the device was still in NETREG_UNINITIALIZED state. >> >> Fixes a68f9614614749727286f675d15f1e09d13cb54a >> (hyperv: Fix race between probe and open calls) >> >> Cc: Haiyang Zhang >> Cc: K. Y. Srinivasan >> Reported-by: Di Nie >> Tested-by: Di Nie >> Signed-off-by: Jason Wang > A device up can occur at the moment you call register_netdevice(), > therefore that up call can see the carrier as down and fail or > similar. So you really cannot resolve the carrier to be on in this > way. True, we need a workqueue to synchronize them. >>> Whatever for? All you need to do is: >>> >>> rtnl_lock(); >>> register_netdevice(); >>> netif_carrier_on(); >>> rtnl_unlock(); >>> >>> It would be nice if we could make the current code work with a change in >>> the core, though. >>> >>> Ben. >>> >> Looks like the link status interrupt may happen during this (after >> netvsc_device_add() was called by rndis_filter_device_add()) without any >> synchronization. This may lead a wrong link status here. > Now I'm confused - if there's a link status interrupt, why are you > setting the carrier on initially? > > Ben. > I realize that setting carrier on initially was a bug after David's comment. So I think we need a workqueue. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH net] net: hyperv: initialize link status correctly
> -Original Message- > From: Jason Wang [mailto:jasow...@redhat.com] > Sent: Monday, January 27, 2014 2:31 AM > To: KY Srinivasan; Haiyang Zhang; de...@linuxdriverproject.org; > net...@vger.kernel.org; linux-ker...@vger.kernel.org > Cc: Jason Wang > Subject: [PATCH net] net: hyperv: initialize link status correctly > > Call netif_carrier_on() after register_device(). Otherwise it won't work since > the device was still in NETREG_UNINITIALIZED state. > > Fixes a68f9614614749727286f675d15f1e09d13cb54a > (hyperv: Fix race between probe and open calls) > > Cc: Haiyang Zhang > Cc: K. Y. Srinivasan > Reported-by: Di Nie > Tested-by: Di Nie > Signed-off-by: Jason Wang > --- I'm working on a fix for this, and will submit it soon. Thanks, - Haiyang ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH net-next] hyperv: Add support for physically discontinuous receive buffer
From: Haiyang Zhang Date: Mon, 27 Jan 2014 21:47:43 + > So, could this patch be taken first? You always need to resend patches that I've originally rejected and you've made arguments for. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH net-next] hyperv: Add support for physically discontinuous receive buffer
> -Original Message- > From: KY Srinivasan > Sent: Monday, January 20, 2014 5:11 PM > To: Haiyang Zhang; David Miller > Cc: net...@vger.kernel.org; o...@aepfle.de; jasow...@redhat.com; linux- > ker...@vger.kernel.org; driverdev-devel@linuxdriverproject.org > Subject: RE: [PATCH net-next] hyperv: Add support for physically discontinuous > receive buffer > > > > > -Original Message- > > From: Haiyang Zhang > > Sent: Monday, January 20, 2014 2:06 PM > > To: David Miller > > Cc: net...@vger.kernel.org; KY Srinivasan; o...@aepfle.de; > > jasow...@redhat.com; linux-ker...@vger.kernel.org; driverdev- > > de...@linuxdriverproject.org > > Subject: RE: [PATCH net-next] hyperv: Add support for physically > > discontinuous receive buffer > > > > > > > > > -Original Message- > > > From: David Miller [mailto:da...@davemloft.net] > > > Sent: Tuesday, January 14, 2014 5:32 PM > > > To: Haiyang Zhang > > > Cc: net...@vger.kernel.org; KY Srinivasan; o...@aepfle.de; > > > jasow...@redhat.com; linux-ker...@vger.kernel.org; driverdev- > > > de...@linuxdriverproject.org > > > Subject: Re: [PATCH net-next] hyperv: Add support for physically > > > discontinuous receive buffer > > > > > > From: Haiyang Zhang > > > Date: Thu, 9 Jan 2014 14:24:47 -0800 > > > > > > > This will allow us to use bigger receive buffer, and prevent > > > > allocation failure due to fragmented memory. > > > > > > > > Signed-off-by: Haiyang Zhang > > > > Reviewed-by: K. Y. Srinivasan > > > > > > Not until you start using paged SKBs in netvsc_recv_callback. > > > > > > Whatever fragmention you think you're avoiding in the hyperv layer, > > > you're still going to get from the: > > > > > > skb = netdev_alloc_skb_ip_align(net, packet->total_data_buflen); > > > > > > call there. > > > > > > This change makes no sense in isolation, therefore I'm not applying > > > it until you also include the appropriate changes to avoid the same > > > exact fragmentation issue in netvsc_drv.c as stated above. > > > > The receive buffer currently requires multiple MB of physically > > continuous memory, and may fail to be allocated when memory is > > fragmented. The patch is created for this issue. > > > > The SKB buffer is usually less than 1500 bytes or up to several KB > > with jumbo frame, so it's much less sensitive to fragmented memory. I > > will work on another patch to use SKB buffer with discontinuous pages. > > > > Could you accept this patch separately? > > Today, if we try to unload and load the network driver, the load may fail > because we may not be able to allocate the receive buffers if memory is > fragmented. This patch specifically addresses this problem. > > Regards, > > K. Y Dave, So, could this patch be taken first? Thanks, - Haiyang ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH net-next] hyperv: Add support for physically discontinuous receive buffer
> -Original Message- > From: David Miller [mailto:da...@davemloft.net] > Sent: Monday, January 27, 2014 4:51 PM > To: Haiyang Zhang > Cc: KY Srinivasan; net...@vger.kernel.org; o...@aepfle.de; > jasow...@redhat.com; linux-ker...@vger.kernel.org; driverdev- > de...@linuxdriverproject.org > Subject: Re: [PATCH net-next] hyperv: Add support for physically discontinuous > receive buffer > > From: Haiyang Zhang > Date: Mon, 27 Jan 2014 21:47:43 + > > > So, could this patch be taken first? > > You always need to resend patches that I've originally rejected and you've > made arguments for. Sure, I will re-send it now. Thanks, - Haiyang ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH net-next] hyperv: Add support for physically discontinuous receive buffer
This will allow us to use bigger receive buffer, and prevent allocation failure
due to fragmented memory.
Signed-off-by: Haiyang Zhang
Reviewed-by: K. Y. Srinivasan
---
drivers/hv/channel.c| 14 --
drivers/net/hyperv/hyperv_net.h |2 +-
drivers/net/hyperv/netvsc.c |7 ++-
3 files changed, 11 insertions(+), 12 deletions(-)
diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index cea623c..69ea36f 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -209,7 +209,6 @@ static int create_gpadl_header(void *kbuffer, u32 size,
{
int i;
int pagecount;
- unsigned long long pfn;
struct vmbus_channel_gpadl_header *gpadl_header;
struct vmbus_channel_gpadl_body *gpadl_body;
struct vmbus_channel_msginfo *msgheader;
@@ -219,7 +218,6 @@ static int create_gpadl_header(void *kbuffer, u32 size,
int pfnsum, pfncount, pfnleft, pfncurr, pfnsize;
pagecount = size >> PAGE_SHIFT;
- pfn = virt_to_phys(kbuffer) >> PAGE_SHIFT;
/* do we need a gpadl body msg */
pfnsize = MAX_SIZE_CHANNEL_MESSAGE -
@@ -248,7 +246,8 @@ static int create_gpadl_header(void *kbuffer, u32 size,
gpadl_header->range[0].byte_offset = 0;
gpadl_header->range[0].byte_count = size;
for (i = 0; i < pfncount; i++)
- gpadl_header->range[0].pfn_array[i] = pfn+i;
+ gpadl_header->range[0].pfn_array[i] = slow_virt_to_phys(
+ kbuffer + PAGE_SIZE * i) >> PAGE_SHIFT;
*msginfo = msgheader;
*messagecount = 1;
@@ -301,7 +300,9 @@ static int create_gpadl_header(void *kbuffer, u32 size,
* so the hypervisor gurantees that this is ok.
*/
for (i = 0; i < pfncurr; i++)
- gpadl_body->pfn[i] = pfn + pfnsum + i;
+ gpadl_body->pfn[i] = slow_virt_to_phys(
+ kbuffer + PAGE_SIZE * (pfnsum + i)) >>
+ PAGE_SHIFT;
/* add to msg header */
list_add_tail(&msgbody->msglistentry,
@@ -327,7 +328,8 @@ static int create_gpadl_header(void *kbuffer, u32 size,
gpadl_header->range[0].byte_offset = 0;
gpadl_header->range[0].byte_count = size;
for (i = 0; i < pagecount; i++)
- gpadl_header->range[0].pfn_array[i] = pfn+i;
+ gpadl_header->range[0].pfn_array[i] = slow_virt_to_phys(
+ kbuffer + PAGE_SIZE * i) >> PAGE_SHIFT;
*msginfo = msgheader;
*messagecount = 1;
@@ -344,7 +346,7 @@ nomem:
* vmbus_establish_gpadl - Estabish a GPADL for the specified buffer
*
* @channel: a channel
- * @kbuffer: from kmalloc
+ * @kbuffer: from kmalloc or vmalloc
* @size: page-size multiple
* @gpadl_handle: some funky thing
*/
diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index a26eecb..7b594ce 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -462,7 +462,7 @@ struct nvsp_message {
#define NETVSC_MTU 65536
-#define NETVSC_RECEIVE_BUFFER_SIZE (1024*1024*2) /* 2MB */
+#define NETVSC_RECEIVE_BUFFER_SIZE (1024*1024*16) /* 16MB */
#define NETVSC_RECEIVE_BUFFER_ID 0xcafe
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 93b485b..03a2c6e 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -136,8 +136,7 @@ static int netvsc_destroy_recv_buf(struct netvsc_device
*net_device)
if (net_device->recv_buf) {
/* Free up the receive buffer */
- free_pages((unsigned long)net_device->recv_buf,
- get_order(net_device->recv_buf_size));
+ vfree(net_device->recv_buf);
net_device->recv_buf = NULL;
}
@@ -163,9 +162,7 @@ static int netvsc_init_recv_buf(struct hv_device *device)
return -ENODEV;
ndev = net_device->ndev;
- net_device->recv_buf =
- (void *)__get_free_pages(GFP_KERNEL|__GFP_ZERO,
- get_order(net_device->recv_buf_size));
+ net_device->recv_buf = vzalloc(net_device->recv_buf_size);
if (!net_device->recv_buf) {
netdev_err(ndev, "unable to allocate receive "
"buffer of size %d\n", net_device->recv_buf_size);
--
1.7.4.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/1] usbip/userspace/libsrc/names.c: memory leak
From: Heinrich Schuchardt
p is freed if NULL.
p is leaked if second calloc fails.
Signed-off-by: Heinrich Schuchardt
---
drivers/staging/usbip/userspace/libsrc/names.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/usbip/userspace/libsrc/names.c
b/drivers/staging/usbip/userspace/libsrc/names.c
index 3c8d28b..b2904e8 100644
--- a/drivers/staging/usbip/userspace/libsrc/names.c
+++ b/drivers/staging/usbip/userspace/libsrc/names.c
@@ -170,12 +170,12 @@ static void *my_malloc(size_t size)
p = calloc(1, sizeof(struct pool));
if (!p) {
- free(p);
return NULL;
}
p->mem = calloc(1, size);
if (!p->mem)
+ free(p);
return NULL;
p->next = pool_head;
--
1.7.10.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] usbip/userspace/libsrc/names.c: memory leak
On Mon, Jan 27, 2014 at 11:29:48PM +0100, xypron.g...@gmx.de wrote: > From: Heinrich Schuchardt > > p is freed if NULL. Not a real problem, right? > p is leaked if second calloc fails. You just created a new bug in your "fix" :( Please run your patches through scripts/checkpatch.pl, odds are, it would have caught this error. greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] usbip/userspace/libsrc/names.c: memory leak
On Mon, Jan 27, 2014 at 11:29:48PM +0100, xypron.g...@gmx.de wrote:
> From: Heinrich Schuchardt
Fix your email account so we can get this automatically from your email.
Currently the From header on your email is just:
From: xypron.g...@gmx.de
without your name.
>
> p is freed if NULL.
> p is leaked if second calloc fails.
>
> Signed-off-by: Heinrich Schuchardt
> ---
> drivers/staging/usbip/userspace/libsrc/names.c |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/usbip/userspace/libsrc/names.c
> b/drivers/staging/usbip/userspace/libsrc/names.c
> index 3c8d28b..b2904e8 100644
> --- a/drivers/staging/usbip/userspace/libsrc/names.c
> +++ b/drivers/staging/usbip/userspace/libsrc/names.c
> @@ -170,12 +170,12 @@ static void *my_malloc(size_t size)
>
> p = calloc(1, sizeof(struct pool));
> if (!p) {
> - free(p);
> return NULL;
> }
Remove the curly braces from here since they are no longer needed.
>
> p->mem = calloc(1, size);
> if (!p->mem)
> + free(p);
> return NULL;
Add the curly braces here. They are required.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] usbip/userspace/libsrc/names.c: memory leak
Hello.
On 01/28/2014 01:29 AM, xypron.g...@gmx.de wrote:
From: Heinrich Schuchardt
p is freed if NULL.
This is not an issue.
p is leaked if second calloc fails.
Signed-off-by: Heinrich Schuchardt
---
drivers/staging/usbip/userspace/libsrc/names.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/usbip/userspace/libsrc/names.c
b/drivers/staging/usbip/userspace/libsrc/names.c
index 3c8d28b..b2904e8 100644
--- a/drivers/staging/usbip/userspace/libsrc/names.c
+++ b/drivers/staging/usbip/userspace/libsrc/names.c
@@ -170,12 +170,12 @@ static void *my_malloc(size_t size)
p = calloc(1, sizeof(struct pool));
if (!p) {
- free(p);
return NULL;
}
{} not needed anymore.
p->mem = calloc(1, size);
if (!p->mem)
+ free(p);
return NULL;
How about {} around the *if* arm?
WBR, Sergei
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] usbip/userspace/libsrc/names.c: memory leak
On Mon, Jan 27, 2014 at 02:50:04PM -0800, Greg KH wrote: > On Mon, Jan 27, 2014 at 11:29:48PM +0100, xypron.g...@gmx.de wrote: > > From: Heinrich Schuchardt > > > > p is freed if NULL. > > Not a real problem, right? > > > p is leaked if second calloc fails. > > You just created a new bug in your "fix" :( > > Please run your patches through scripts/checkpatch.pl, odds are, it > would have caught this error. > Checkpatch doesn't catch the problems here. I thought it would have caught the style issue but apparently it only looks for extra curly braces when you run it in --file mode. Fengguang would hopefully have caught the missing curly braces bug with Coccinelle. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] usbip/userspace/libsrc/names.c: memory leak
On Tue, Jan 28, 2014 at 02:02:12AM +0300, Dan Carpenter wrote: > On Mon, Jan 27, 2014 at 02:50:04PM -0800, Greg KH wrote: > > On Mon, Jan 27, 2014 at 11:29:48PM +0100, xypron.g...@gmx.de wrote: > > > From: Heinrich Schuchardt > > > > > > p is freed if NULL. > > > > Not a real problem, right? > > > > > p is leaked if second calloc fails. > > > > You just created a new bug in your "fix" :( > > > > Please run your patches through scripts/checkpatch.pl, odds are, it > > would have caught this error. > > > > Checkpatch doesn't catch the problems here. I thought it would have > caught the style issue but apparently it only looks for extra curly > braces when you run it in --file mode. Ah, that's good to know. > Fengguang would hopefully have caught the missing curly braces bug with > Coccinelle. Is Coccinelle run on the userspace .c code in the kernel? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] usbip/userspace/libsrc/names.c: memory leak
On Mon, Jan 27, 2014 at 03:27:36PM -0800, Greg KH wrote: > On Tue, Jan 28, 2014 at 02:02:12AM +0300, Dan Carpenter wrote: > > On Mon, Jan 27, 2014 at 02:50:04PM -0800, Greg KH wrote: > > > On Mon, Jan 27, 2014 at 11:29:48PM +0100, xypron.g...@gmx.de wrote: > > > > From: Heinrich Schuchardt > > > > > > > > p is freed if NULL. > > > > > > Not a real problem, right? > > > > > > > p is leaked if second calloc fails. > > > > > > You just created a new bug in your "fix" :( > > > > > > Please run your patches through scripts/checkpatch.pl, odds are, it > > > would have caught this error. > > > > > > > Checkpatch doesn't catch the problems here. I thought it would have > > caught the style issue but apparently it only looks for extra curly > > braces when you run it in --file mode. > > Ah, that's good to know. > > > Fengguang would hopefully have caught the missing curly braces bug with > > Coccinelle. > > Is Coccinelle run on the userspace .c code in the kernel? Hm... I'm not sure. Fengguang, do you know if we would have caught the missing curly braces in this patch? http://lkml.org/lkml/2014/1/27/460 regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH net-next] hyperv: Add support for physically discontinuous receive buffer
From: Haiyang Zhang Date: Mon, 27 Jan 2014 15:03:42 -0800 > This will allow us to use bigger receive buffer, and prevent allocation > failure > due to fragmented memory. > > Signed-off-by: Haiyang Zhang > Reviewed-by: K. Y. Srinivasan Applied, thanks. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/1] staging: imx-drm: Fix build error
Instead of redefining the enums, use the standard ones already
available to avoid the following build errors:
drivers/staging/imx-drm/imx-hdmi.c:56:13: error: nested redefinition of ‘enum
hdmi_colorimetry’
drivers/staging/imx-drm/imx-hdmi.c:56:13: error: redeclaration of ‘enum
hdmi_colorimetry’
In file included from include/drm/drm_crtc.h:33:0,
from include/drm/drmP.h:710,
from drivers/staging/imx-drm/imx-hdmi.c:24:
include/linux/hdmi.h:48:6: note: originally defined here
Signed-off-by: Sachin Kamat
Cc: Guennadi Liakhovetski
Cc: Fabio Estevam
---
Only compile tested.
---
drivers/staging/imx-drm/imx-hdmi.c | 22 +-
1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/imx-drm/imx-hdmi.c
b/drivers/staging/imx-drm/imx-hdmi.c
index f3a1f5e2e492..62ce0e86f14b 100644
--- a/drivers/staging/imx-drm/imx-hdmi.c
+++ b/drivers/staging/imx-drm/imx-hdmi.c
@@ -16,6 +16,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -52,11 +53,6 @@ enum hdmi_datamap {
YCbCr422_12B = 0x12,
};
-enum hdmi_colorimetry {
- ITU601,
- ITU709,
-};
-
enum imx_hdmi_devtype {
IMX6Q_HDMI,
IMX6DL_HDMI,
@@ -489,12 +485,12 @@ static void imx_hdmi_update_csc_coeffs(struct imx_hdmi
*hdmi)
if (is_color_space_conversion(hdmi)) {
if (hdmi->hdmi_data.enc_out_format == RGB) {
- if (hdmi->hdmi_data.colorimetry == ITU601)
+ if (hdmi->hdmi_data.colorimetry ==
HDMI_COLORIMETRY_ITU_601)
csc_coeff = &csc_coeff_rgb_out_eitu601;
else
csc_coeff = &csc_coeff_rgb_out_eitu709;
} else if (hdmi->hdmi_data.enc_in_format == RGB) {
- if (hdmi->hdmi_data.colorimetry == ITU601)
+ if (hdmi->hdmi_data.colorimetry ==
HDMI_COLORIMETRY_ITU_601)
csc_coeff = &csc_coeff_rgb_in_eitu601;
else
csc_coeff = &csc_coeff_rgb_in_eitu709;
@@ -1140,16 +1136,16 @@ static void hdmi_config_AVI(struct imx_hdmi *hdmi)
/* Set up colorimetry */
if (hdmi->hdmi_data.enc_out_format == XVYCC444) {
colorimetry = HDMI_FC_AVICONF1_COLORIMETRY_EXTENDED_INFO;
- if (hdmi->hdmi_data.colorimetry == ITU601)
+ if (hdmi->hdmi_data.colorimetry == HDMI_COLORIMETRY_ITU_601)
ext_colorimetry =
HDMI_FC_AVICONF2_EXT_COLORIMETRY_XVYCC601;
- else /* hdmi->hdmi_data.colorimetry == ITU709 */
+ else /*hdmi->hdmi_data.colorimetry == HDMI_COLORIMETRY_ITU_709*/
ext_colorimetry =
HDMI_FC_AVICONF2_EXT_COLORIMETRY_XVYCC709;
} else if (hdmi->hdmi_data.enc_out_format != RGB) {
- if (hdmi->hdmi_data.colorimetry == ITU601)
+ if (hdmi->hdmi_data.colorimetry == HDMI_COLORIMETRY_ITU_601)
colorimetry = HDMI_FC_AVICONF1_COLORIMETRY_SMPTE;
- else /* hdmi->hdmi_data.colorimetry == ITU709 */
+ else /*hdmi->hdmi_data.colorimetry == HDMI_COLORIMETRY_ITU_709*/
colorimetry = HDMI_FC_AVICONF1_COLORIMETRY_ITUR;
ext_colorimetry = HDMI_FC_AVICONF2_EXT_COLORIMETRY_XVYCC601;
} else { /* Carries no data */
@@ -1379,9 +1375,9 @@ static int imx_hdmi_setup(struct imx_hdmi *hdmi, struct
drm_display_mode *mode)
(hdmi->vic == 21) || (hdmi->vic == 22) ||
(hdmi->vic == 2) || (hdmi->vic == 3) ||
(hdmi->vic == 17) || (hdmi->vic == 18))
- hdmi->hdmi_data.colorimetry = ITU601;
+ hdmi->hdmi_data.colorimetry = HDMI_COLORIMETRY_ITU_601;
else
- hdmi->hdmi_data.colorimetry = ITU709;
+ hdmi->hdmi_data.colorimetry = HDMI_COLORIMETRY_ITU_709;
if ((hdmi->vic == 10) || (hdmi->vic == 11) ||
(hdmi->vic == 12) || (hdmi->vic == 13) ||
--
1.7.9.5
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
