[PATCH net-next, v3, 1/1] hv_netvsc: introduce netif-msg into netvsc module
From: Simon Xiao
1. Introduce netif-msg to netvsc to control debug logging output
and keep msg_enable in netvsc_device_context so that it is
kept persistently.
2. Only call dump_rndis_message() when NETIF_MSG_RX_ERR or above
is specified in netvsc module debug param.
In non-debug mode, in current code, dump_rndis_message() will not
dump anything but it still initialize some local variables and
process the switch logic which is unnecessary, especially in
high network throughput situation.
Signed-off-by: Simon Xiao
Reviewed-by: K. Y. Srinivasan
Reviewed-by: Haiyang Zhang
---
drivers/net/hyperv/hyperv_net.h | 12
drivers/net/hyperv/netvsc.c | 3 +++
drivers/net/hyperv/netvsc_drv.c | 20 ++--
drivers/net/hyperv/rndis_filter.c | 3 ++-
4 files changed, 31 insertions(+), 7 deletions(-)
diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index a10b316..e55c8f4 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -612,6 +612,15 @@ struct multi_send_data {
u32 count; /* counter of batched packets */
};
+/* The context of the netvsc device */
+struct net_device_context {
+ /* point back to our device context */
+ struct hv_device *device_ctx;
+ struct delayed_work dwork;
+ struct work_struct work;
+ u32 msg_enable; /* debug level */
+};
+
/* Per netvsc device */
struct netvsc_device {
struct hv_device *dev;
@@ -667,6 +676,9 @@ struct netvsc_device {
struct multi_send_data msd[NR_CPUS];
u32 max_pkt; /* max number of pkt in one send, e.g. 8 */
u32 pkt_align; /* alignment bytes, e.g. 8 */
+
+ /* The net device context */
+ struct net_device_context *nd_ctx;
};
/* NdisInitialize message */
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 2e8ad06..c651d4d 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -1197,6 +1197,9 @@ int netvsc_device_add(struct hv_device *device, void
*additional_info)
*/
ndev = net_device->ndev;
+ /* Add netvsc_device context to netvsc_device */
+ net_device->nd_ctx = netdev_priv(ndev);
+
/* Initialize the NetVSC channel extension */
init_completion(&net_device->channel_init_wait);
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index a3a9d38..66c4b0c 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -40,18 +40,21 @@
#include "hyperv_net.h"
-struct net_device_context {
- /* point back to our device context */
- struct hv_device *device_ctx;
- struct delayed_work dwork;
- struct work_struct work;
-};
#define RING_SIZE_MIN 64
static int ring_size = 128;
module_param(ring_size, int, S_IRUGO);
MODULE_PARM_DESC(ring_size, "Ring buffer size (# of pages)");
+static const u32 default_msg = NETIF_MSG_DRV | NETIF_MSG_PROBE |
+ NETIF_MSG_LINK | NETIF_MSG_IFUP |
+ NETIF_MSG_IFDOWN | NETIF_MSG_RX_ERR |
+ NETIF_MSG_TX_ERR;
+
+static int debug = -1;
+module_param(debug, int, S_IRUGO);
+MODULE_PARM_DESC(debug, "Debug level (0=none,...,16=all)");
+
static void do_set_multicast(struct work_struct *w)
{
struct net_device_context *ndevctx =
@@ -888,6 +891,11 @@ static int netvsc_probe(struct hv_device *dev,
net_device_ctx = netdev_priv(net);
net_device_ctx->device_ctx = dev;
+ net_device_ctx->msg_enable = netif_msg_init(debug, default_msg);
+ if (netif_msg_probe(net_device_ctx))
+ netdev_dbg(net, "netvsc msg_enable: %d\n",
+ net_device_ctx->msg_enable);
+
hv_set_drvdata(dev, net);
INIT_DELAYED_WORK(&net_device_ctx->dwork, netvsc_link_change);
INIT_WORK(&net_device_ctx->work, do_set_multicast);
diff --git a/drivers/net/hyperv/rndis_filter.c
b/drivers/net/hyperv/rndis_filter.c
index 0d92efe..9118cea 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -429,7 +429,8 @@ int rndis_filter_receive(struct hv_device *dev,
rndis_msg = pkt->data;
- dump_rndis_message(dev, rndis_msg);
+ if (netif_msg_rx_err(net_dev->nd_ctx))
+ dump_rndis_message(dev, rndis_msg);
switch (rndis_msg->ndis_msg_type) {
case RNDIS_MSG_PACKET:
--
1.8.5.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v2 0/6] Drivers: hv: vmbus: fair round robin algorithm for vmbus_get_outgoing_channel()
> -Original Message- > From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com] > Sent: Tuesday, April 28, 2015 1:04 > To: KY Srinivasan > Cc: Haiyang Zhang; de...@linuxdriverproject.org; linux- > ker...@vger.kernel.org; Dexuan Cui > Subject: [PATCH v2 0/6] Drivers: hv: vmbus: fair round robin algorithm for > vmbus_get_outgoing_channel() > > Changes in v2: > - Address Dexuan's review comments: > PATCH 3/6: s,channel,primary_channel; > PATCH 4/6: add a forward declaration instead of moving code around; > PATCH 6/6: fix an off-by-one > - Change the algorithm in PATCH 6/6: > Instead of a simple round robin we first try to find a (sub)channel with > the current_cpu == target_cpu and we fallback to a round robin when we > fail > to find one. > > K. Y., Dexuan, can you please give it a spin in various testing environments > you have? Thanks! > > Original description: > > This series is a continuation of the "Drivers: hv: vmbus: Use a round-robin > algorithm for picking the outgoing channel" work. It is supposed to bring > two > significant changes: > 1) Subchannels for a channel are distributed evenly across all vcpus we have. >Currently we try to distribute all channels (including subchannels) across >all vcpus, this approach doesn't guarantee that the particular channel's >subchannels will be distributed in the same way as we process all offer >requests in some random order. (Patch 05) > 2) Channel picking based on the current vcpu is dropped from >vmbus_get_outgoing_channel() in favor of a fair round robin. (Patch 06) > (this >is not true anymore, see 'Changes'). > > Patches 01 - 04 are cleanup/refactoring. > > Vitaly Kuznetsov (6): > Drivers: hv: vmbus: unify calls to percpu_channel_enq() > Drivers: hv: vmbus: briefly comment num_sc and next_oc > Drivers: hv: vmbus: decrease num_sc on subchannel removal > Drivers: hv: vmbus: move init_vp_index() call to vmbus_process_offer() > Drivers: hv: vmbus: distribute subchannels among all vcpus > Drivers: hv: vmbus: improve selection of an outgoing channel > > drivers/hv/channel_mgmt.c | 127 ++- > --- > include/linux/hyperv.h| 12 +++-- > 2 files changed, 80 insertions(+), 59 deletions(-) > > -- Patch 1, 2 and 3 are good to me. We'll have to test 4~6 for performance change. Thanks, -- Dexuan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH net 1/1] hv_netvsc: Fix a bug in netvsc_start_xmit()
> -Original Message- > From: David Miller [mailto:da...@davemloft.net] > Sent: Monday, April 27, 2015 7:57 PM > To: KY Srinivasan > Cc: net...@vger.kernel.org; linux-ker...@vger.kernel.org; > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; > jasow...@redhat.com > Subject: Re: [PATCH net 1/1] hv_netvsc: Fix a bug in netvsc_start_xmit() > > From: "K. Y. Srinivasan" > Date: Mon, 27 Apr 2015 18:14:50 -0700 > > > Commit commit b08cc79155fc26d0d112b1470d1ece5034651a4b eliminated > memory > > allocation in the packet send path. This commit introduced a bug since it > > did not account for the case if the skb was cloned. Fix this bug by > > using the pre-reserved head room only if the skb is not cloned. > > > > Signed-off-by: K. Y. Srinivasan > > We have generic infrastructure to do this, please try instead: > > err = skb_cow_head(skb, pkt_sz); > > this will take care of everything for you and you can get rid > of all of this dynamic memory allocation etc. in this code > path. Thanks David; I will resubmit this patch. Regards, K. Y ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH net 1/1] hv_netvsc: Fix a bug in netvsc_start_xmit()
From: "K. Y. Srinivasan" Date: Mon, 27 Apr 2015 18:14:50 -0700 > Commit commit b08cc79155fc26d0d112b1470d1ece5034651a4b eliminated memory > allocation in the packet send path. This commit introduced a bug since it > did not account for the case if the skb was cloned. Fix this bug by > using the pre-reserved head room only if the skb is not cloned. > > Signed-off-by: K. Y. Srinivasan We have generic infrastructure to do this, please try instead: err = skb_cow_head(skb, pkt_sz); this will take care of everything for you and you can get rid of all of this dynamic memory allocation etc. in this code path. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH net 1/1] hv_netvsc: Fix a bug in netvsc_start_xmit()
> -Original Message-
> From: devel [mailto:driverdev-devel-boun...@linuxdriverproject.org] On
> Behalf Of K. Y. Srinivasan
> Sent: Tuesday, April 28, 2015 9:15
> To: da...@davemloft.net; net...@vger.kernel.org; linux-
> ker...@vger.kernel.org; de...@linuxdriverproject.org; o...@aepfle.de;
> a...@canonical.com; jasow...@redhat.com
> Subject: [PATCH net 1/1] hv_netvsc: Fix a bug in netvsc_start_xmit()
>
> Commit commit b08cc79155fc26d0d112b1470d1ece5034651a4b
> eliminated memory
> allocation in the packet send path. This commit introduced a bug since it
> did not account for the case if the skb was cloned. Fix this bug by
> using the pre-reserved head room only if the skb is not cloned.
>
> Signed-off-by: K. Y. Srinivasan
> ---
> drivers/net/hyperv/netvsc_drv.c |2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/hyperv/netvsc_drv.c
> b/drivers/net/hyperv/netvsc_drv.c
> index a3a9d38..7eb0251 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -421,7 +421,7 @@ check_size:
>
> pkt_sz = sizeof(struct hv_netvsc_packet) + RNDIS_AND_PPI_SIZE;
>
> - if (head_room < pkt_sz) {
> + if (skb->cloned || head_room < pkt_sz) {
> packet = kmalloc(pkt_sz, GFP_ATOMIC);
> if (!packet) {
> /* out of memory, drop packet */
> --
Without the patch, the guest can panic due to memory corruption.
I confirm the patch can fix the panic I saw.
Tested-by: Dexuan Cui
Thanks,
-- Dexuan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH net 1/1] hv_netvsc: Fix a bug in netvsc_start_xmit()
Commit commit b08cc79155fc26d0d112b1470d1ece5034651a4b eliminated memory
allocation in the packet send path. This commit introduced a bug since it
did not account for the case if the skb was cloned. Fix this bug by
using the pre-reserved head room only if the skb is not cloned.
Signed-off-by: K. Y. Srinivasan
---
drivers/net/hyperv/netvsc_drv.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index a3a9d38..7eb0251 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -421,7 +421,7 @@ check_size:
pkt_sz = sizeof(struct hv_netvsc_packet) + RNDIS_AND_PPI_SIZE;
- if (head_room < pkt_sz) {
+ if (skb->cloned || head_room < pkt_sz) {
packet = kmalloc(pkt_sz, GFP_ATOMIC);
if (!packet) {
/* out of memory, drop packet */
--
1.7.4.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 5/6] Drivers: hv: vmbus: distribute subchannels among all vcpus
> -Original Message- > From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com] > Sent: Monday, April 27, 2015 6:30 AM > To: KY Srinivasan > Cc: Dexuan Cui; Haiyang Zhang; de...@linuxdriverproject.org; linux- > ker...@vger.kernel.org > Subject: Re: [PATCH 5/6] Drivers: hv: vmbus: distribute subchannels among > all vcpus > > KY Srinivasan writes: > > >> -Original Message- > >> From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com] > >> Sent: Friday, April 24, 2015 2:05 AM > >> To: Dexuan Cui > >> Cc: KY Srinivasan; Haiyang Zhang; de...@linuxdriverproject.org; linux- > >> ker...@vger.kernel.org > >> Subject: Re: [PATCH 5/6] Drivers: hv: vmbus: distribute subchannels > among > >> all vcpus > >> > >> Dexuan Cui writes: > >> > >> >> -Original Message- > >> >> From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com] > >> >> Sent: Tuesday, April 21, 2015 22:28 > >> >> To: KY Srinivasan > >> >> Cc: Haiyang Zhang; de...@linuxdriverproject.org; linux- > >> >> ker...@vger.kernel.org; Dexuan Cui > >> >> Subject: [PATCH 5/6] Drivers: hv: vmbus: distribute subchannels > among all > >> >> vcpus > >> >> > >> >> Primary channels are distributed evenly across all vcpus we have. > When > >> the > >> >> host asks us to create subchannels it usually makes us num_cpus-1 > offers > >> > > >> > Hi Vitaly, > >> > AFAIK, in the VSP of storvsc, the number of subchannel is > >> > (the_number_of_vcpus - 1) / 4. > >> > > >> > This means for a 8-vCPU guest, there is only 1 subchannel. > >> > > >> > Your new algorithm tends to make the vCPUs with small-number busier: > >> > e.g., in the 8-vCPU case, assuming we have 4 SCSI controllers: > >> > vCPU0: scsi0's PrimaryChannel (P) > >> > vCPU1: scsi0's SubChannel (S) + scsi1's P > >> > vCPU2: scsi1's S + scsi2's P > >> > vCPU3: scsi2's S + scsi3's P > >> > vCPU4: scsi3's S > >> > vCPU5, 6 and 7 are idle. > >> > > >> > In this special case, the existing algorithm is better. :-) > >> > > >> > However, I do like this idea in your patch, that is, making sure a > >> > device's > >> > primary/sub channels are assigned to differents vCPUs. > >> > >> Under special circumstances with the current code we can end up with > >> having all subchannels on the same vCPU with the primary channel I guess > >> :-) This is not something common, but possible. > >> > >> > > >> > I'm just wondering if we should use an even better (and complex) > >> > algorithm :-) > >> > >> The question here is - does sticking to the current vCPU help? If it > >> does, I can suggest the following (I think I even mentioned that in my > >> PATCH 00): first we try to find a (sub)channel with target_cpu == > >> current_vcpu and only when we fail we do the round robin. I'd like to > >> hear K.Y.'s opinion here as he's the original author :-) > > > > Sorry for the delayed response. Initially I had implemented a scheme that > would > > pick an outgoing CPU that was closest to the CPU on which the request > came (to maintain > > cache locality especially on NUMA systems). I changed this algorithm to > spread the load > > more uniformly as we were trying to improve Linux IOPS on Azure XIO > > (premium storage). We are currently testing > > this code on our Converged Offering - CPS and I am finding that the perf as > measured by IOS has regressed. > > I have not narrowed the reason for this regression and it may very well be > the change in the > > algorithm for selecting the outgoing channel. In general, I don't think the > logic here needs to be > > exact and locality (being on the same CPU or within the same NUMA node) > is important. Any change > > to this algorithm will have to be validated on different MSFT > > environments (Azure XIO, CPS etc.). > > Thanks, can you please compare two algorythms here: > 1) Simple round robin (the one my patch series implement but with issues > fixed, I'll send v2). > 2) Try to find a (sub)channel with matching VCPU and round-robin when we > fail (I can actually include it in v2). > We can later decide something based on these testing results. We will do some testing. K. Y > > > > > Regards, > > > > K. Y > > -- > Vitaly ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 4/6] Drivers: hv: vmbus: move init_vp_index() call to vmbus_process_offer()
We need to call init_vp_index() after we added the channel to the appropriate
list (global or subchannel) to be able to use this information when assigning
the channel to the particular vcpu. To do so we need to move a couple of
functions around. The only real change is the init_vp_index() call. This is a
small refactoring without a functional change.
Signed-off-by: Vitaly Kuznetsov
---
drivers/hv/channel_mgmt.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index c53a171..655c0a0 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -32,6 +32,9 @@
#include "hyperv_vmbus.h"
+static void init_vp_index(struct vmbus_channel *channel,
+ const uuid_le *type_guid);
+
/**
* vmbus_prep_negotiate_resp() - Create default response for Hyper-V Negotiate
message
* @icmsghdrp: Pointer to msg header structure
@@ -272,6 +275,8 @@ static void vmbus_process_offer(struct vmbus_channel
*newchannel)
goto err_free_chan;
}
+ init_vp_index(newchannel, &newchannel->offermsg.offer.if_type);
+
if (newchannel->target_cpu != get_cpu()) {
put_cpu();
smp_call_function_single(newchannel->target_cpu,
@@ -476,8 +481,6 @@ static void vmbus_onoffer(struct
vmbus_channel_message_header *hdr)
offer->connection_id;
}
- init_vp_index(newchannel, &offer->offer.if_type);
-
memcpy(&newchannel->offermsg, offer,
sizeof(struct vmbus_channel_offer_channel));
newchannel->monitor_grp = (u8)offer->monitorid / 32;
--
1.9.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 1/6] Drivers: hv: vmbus: unify calls to percpu_channel_enq()
Remove some code duplication, no functional change intended.
Signed-off-by: Vitaly Kuznetsov
---
drivers/hv/channel_mgmt.c | 51 +--
1 file changed, 18 insertions(+), 33 deletions(-)
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 4b9d89a..b28cbdf 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -233,7 +233,6 @@ static void vmbus_process_offer(struct vmbus_channel
*newchannel)
{
struct vmbus_channel *channel;
bool fnew = true;
- bool enq = false;
unsigned long flags;
/* Make sure this is a new offer */
@@ -249,25 +248,12 @@ static void vmbus_process_offer(struct vmbus_channel
*newchannel)
}
}
- if (fnew) {
+ if (fnew)
list_add_tail(&newchannel->listentry,
&vmbus_connection.chn_list);
- enq = true;
- }
spin_unlock_irqrestore(&vmbus_connection.channel_lock, flags);
- if (enq) {
- if (newchannel->target_cpu != get_cpu()) {
- put_cpu();
- smp_call_function_single(newchannel->target_cpu,
-percpu_channel_enq,
-newchannel, true);
- } else {
- percpu_channel_enq(newchannel);
- put_cpu();
- }
- }
if (!fnew) {
/*
* Check to see if this is a sub-channel.
@@ -280,26 +266,19 @@ static void vmbus_process_offer(struct vmbus_channel
*newchannel)
spin_lock_irqsave(&channel->lock, flags);
list_add_tail(&newchannel->sc_list, &channel->sc_list);
spin_unlock_irqrestore(&channel->lock, flags);
-
- if (newchannel->target_cpu != get_cpu()) {
- put_cpu();
- smp_call_function_single(newchannel->target_cpu,
-percpu_channel_enq,
-newchannel, true);
- } else {
- percpu_channel_enq(newchannel);
- put_cpu();
- }
-
- newchannel->state = CHANNEL_OPEN_STATE;
channel->num_sc++;
- if (channel->sc_creation_callback != NULL)
- channel->sc_creation_callback(newchannel);
-
- return;
- }
+ } else
+ goto err_free_chan;
+ }
- goto err_free_chan;
+ if (newchannel->target_cpu != get_cpu()) {
+ put_cpu();
+ smp_call_function_single(newchannel->target_cpu,
+percpu_channel_enq,
+newchannel, true);
+ } else {
+ percpu_channel_enq(newchannel);
+ put_cpu();
}
/*
@@ -309,6 +288,12 @@ static void vmbus_process_offer(struct vmbus_channel
*newchannel)
*/
newchannel->state = CHANNEL_OPEN_STATE;
+ if (!fnew) {
+ if (channel->sc_creation_callback != NULL)
+ channel->sc_creation_callback(newchannel);
+ return;
+ }
+
/*
* Start the process of binding this offer to the driver
* We need to set the DeviceObject field before calling
--
1.9.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 0/6] Drivers: hv: vmbus: fair round robin algorithm for vmbus_get_outgoing_channel()
Changes in v2: - Address Dexuan's review comments: PATCH 3/6: s,channel,primary_channel; PATCH 4/6: add a forward declaration instead of moving code around; PATCH 6/6: fix an off-by-one - Change the algorithm in PATCH 6/6: Instead of a simple round robin we first try to find a (sub)channel with the current_cpu == target_cpu and we fallback to a round robin when we fail to find one. K. Y., Dexuan, can you please give it a spin in various testing environments you have? Thanks! Original description: This series is a continuation of the "Drivers: hv: vmbus: Use a round-robin algorithm for picking the outgoing channel" work. It is supposed to bring two significant changes: 1) Subchannels for a channel are distributed evenly across all vcpus we have. Currently we try to distribute all channels (including subchannels) across all vcpus, this approach doesn't guarantee that the particular channel's subchannels will be distributed in the same way as we process all offer requests in some random order. (Patch 05) 2) Channel picking based on the current vcpu is dropped from vmbus_get_outgoing_channel() in favor of a fair round robin. (Patch 06) (this is not true anymore, see 'Changes'). Patches 01 - 04 are cleanup/refactoring. Vitaly Kuznetsov (6): Drivers: hv: vmbus: unify calls to percpu_channel_enq() Drivers: hv: vmbus: briefly comment num_sc and next_oc Drivers: hv: vmbus: decrease num_sc on subchannel removal Drivers: hv: vmbus: move init_vp_index() call to vmbus_process_offer() Drivers: hv: vmbus: distribute subchannels among all vcpus Drivers: hv: vmbus: improve selection of an outgoing channel drivers/hv/channel_mgmt.c | 127 ++ include/linux/hyperv.h| 12 +++-- 2 files changed, 80 insertions(+), 59 deletions(-) -- 1.9.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 6/6] Drivers: hv: vmbus: improve selection of an outgoing channel
vmbus_get_outgoing_channel() implements the following algorithm for selecting
an outgoing channel (despite the comment before the function saying it
distributes the load equally):
1) If we have no subchannels return the primary channel;
2) If primary->next_oc is grater than primary->num_sc reset the primary->next_oc
to 0 and return the primary channel;
3) Aim for the primary->next_oc subchannel, increment primary->next_oc;
4) Loop through all opened subchannels. If we see a channel which has
target_cpu == current_cpu return it. If we reached the primary->next_oc'th
open subchannel return it;
5) Return the primary channel.
The implementation also skips the subchannel No. 0 unless it matches the current
cpu as we assign i to 1 in the initialization.
Improve the algorithm to do the full scan searching for a (sub)channel with the
target_cpu == current_cpu, use round-robin as a fallback.
Signed-off-by: Vitaly Kuznetsov
---
drivers/hv/channel_mgmt.c | 39 +++
1 file changed, 19 insertions(+), 20 deletions(-)
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 1f1417d..cdc3914 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -823,46 +823,45 @@ cleanup:
}
/*
- * Retrieve the (sub) channel on which to send an outgoing request.
- * When a primary channel has multiple sub-channels, we try to
- * distribute the load equally amongst all available channels.
+ * Retrieve the (sub) channel on which to send an outgoing request. When a
+ * primary channel has multiple sub-channels, we try to find a (sub)channel
+ * with target_cpu == current CPU and we try to distribute the load equally
+ * amongst all available channels when we fail.
*/
struct vmbus_channel *vmbus_get_outgoing_channel(struct vmbus_channel *primary)
{
struct list_head *cur, *tmp;
- int cur_cpu;
struct vmbus_channel *cur_channel;
- struct vmbus_channel *outgoing_channel = primary;
- int next_channel;
- int i = 1;
+ struct vmbus_channel *out_channel_rr = NULL;
+ int i = 0;
if (list_empty(&primary->sc_list))
- return outgoing_channel;
+ return primary;
- next_channel = primary->next_oc++;
-
- if (next_channel > (primary->num_sc)) {
+ if (primary->next_oc >= primary->num_sc) {
primary->next_oc = 0;
- return outgoing_channel;
+ return primary;
}
- cur_cpu = hv_context.vp_index[get_cpu()];
- put_cpu();
+ if (primary->target_cpu == smp_processor_id())
+ return primary;
+
list_for_each_safe(cur, tmp, &primary->sc_list) {
+ i++;
cur_channel = list_entry(cur, struct vmbus_channel, sc_list);
if (cur_channel->state != CHANNEL_OPENED_STATE)
continue;
- if (cur_channel->target_vp == cur_cpu)
+ if (cur_channel->target_cpu == smp_processor_id())
return cur_channel;
- if (i == next_channel)
- return cur_channel;
-
- i++;
+ if (!out_channel_rr && i > primary->next_oc) {
+ primary->next_oc = i;
+ out_channel_rr = cur_channel;
+ }
}
- return outgoing_channel;
+ return out_channel_rr ? out_channel_rr : primary;
}
EXPORT_SYMBOL_GPL(vmbus_get_outgoing_channel);
--
1.9.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 5/6] Drivers: hv: vmbus: distribute subchannels among all vcpus
Primary channels are distributed evenly across all vcpus we have. When the host
asks us to create subchannels it usually makes us num_cpus-1 offers and we are
supposed to distribute the work evenly among the channel itself and all its
subchannels. Make sure they are all assigned to different vcpus.
Signed-off-by: Vitaly Kuznetsov
---
drivers/hv/channel_mgmt.c | 29 -
1 file changed, 28 insertions(+), 1 deletion(-)
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 655c0a0..1f1417d 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -387,6 +387,8 @@ static void init_vp_index(struct vmbus_channel *channel,
const uuid_le *type_gui
int i;
bool perf_chn = false;
u32 max_cpus = num_online_cpus();
+ struct vmbus_channel *primary = channel->primary_channel, *prev;
+ unsigned long flags;
for (i = IDE; i < MAX_PERF_CHN; i++) {
if (!memcmp(type_guid->b, hp_devs[i].guid,
@@ -407,7 +409,32 @@ static void init_vp_index(struct vmbus_channel *channel,
const uuid_le *type_gui
channel->target_vp = 0;
return;
}
- cur_cpu = (++next_vp % max_cpus);
+
+ /*
+* Primary channels are distributed evenly across all vcpus we have.
+* When the host asks us to create subchannels it usually makes us
+* num_cpus-1 offers and we are supposed to distribute the work evenly
+* among the channel itself and all its subchannels. Make sure they are
+* all assigned to different vcpus.
+*/
+ if (!primary)
+ cur_cpu = (++next_vp % max_cpus);
+ else {
+ /*
+* Let's assign the first subchannel of a channel to the
+* primary->target_cpu+1 and all the subsequent channels to
+* the prev->target_cpu+1.
+*/
+ spin_lock_irqsave(&primary->lock, flags);
+ if (primary->num_sc == 1)
+ cur_cpu = (primary->target_cpu + 1) % max_cpus;
+ else {
+ prev = list_prev_entry(channel, sc_list);
+ cur_cpu = (prev->target_cpu + 1) % max_cpus;
+ }
+ spin_unlock_irqrestore(&primary->lock, flags);
+ }
+
channel->target_cpu = cur_cpu;
channel->target_vp = hv_context.vp_index[cur_cpu];
}
--
1.9.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 3/6] Drivers: hv: vmbus: decrease num_sc on subchannel removal
It is unlikely that that host will ask us to close only one subchannel for a device but let's be consistent. Do both num_sc++ and num_sc-- with channel->lock to be on the safe side. Signed-off-by: Vitaly Kuznetsov --- drivers/hv/channel_mgmt.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c index b28cbdf..c53a171 100644 --- a/drivers/hv/channel_mgmt.c +++ b/drivers/hv/channel_mgmt.c @@ -205,6 +205,7 @@ void hv_process_channel_removal(struct vmbus_channel *channel, u32 relid) primary_channel = channel->primary_channel; spin_lock_irqsave(&primary_channel->lock, flags); list_del(&channel->sc_list); + primary_channel->num_sc--; spin_unlock_irqrestore(&primary_channel->lock, flags); } free_channel(channel); @@ -265,8 +266,8 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel) newchannel->primary_channel = channel; spin_lock_irqsave(&channel->lock, flags); list_add_tail(&newchannel->sc_list, &channel->sc_list); - spin_unlock_irqrestore(&channel->lock, flags); channel->num_sc++; + spin_unlock_irqrestore(&channel->lock, flags); } else goto err_free_chan; } -- 1.9.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 2/6] Drivers: hv: vmbus: briefly comment num_sc and next_oc
next_oc and num_sc fields of struct vmbus_channel deserve a description. Move
them closer to sc_list as these fields are related to it.
Signed-off-by: Vitaly Kuznetsov
---
include/linux/hyperv.h | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index c739cba..cce7f66 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -727,6 +727,15 @@ struct vmbus_channel {
*/
struct list_head sc_list;
/*
+* Current number of sub-channels.
+*/
+ int num_sc;
+ /*
+* Number of a sub-channel (position within sc_list) which is supposed
+* to be used as the next outgoing channel.
+*/
+ int next_oc;
+ /*
* The primary channel this sub-channel belongs to.
* This will be NULL for the primary channel.
*/
@@ -740,9 +749,6 @@ struct vmbus_channel {
* link up channels based on their CPU affinity.
*/
struct list_head percpu_list;
-
- int num_sc;
- int next_oc;
};
static inline void set_channel_read_state(struct vmbus_channel *c, bool state)
--
1.9.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/3] staging: comedi: serial2002: fix Coverity "Explicit null dereference"
serial2002_setup_subdevices() initializes each subdevice based on the
config read from the attached serial device. Part of this initialization
is to setup the subdevice range_table_list for the non digital subdevices.
The range_table_list is allocated only when a 'range' is passed to the
functions. Each channel of the subdevice then has it's 'range' initialized
and that range is added to the range_table_list.
The logic of this function works but causes Coverity complain about an
Explicit null dereference of the allocated 'range_table_list'. Add a check
for the 'range_table_list' to quiet the Coverity issue.
Reported-by: coverity (CID 1011632)
Signed-off-by: H Hartley Sweeten
Reviewed-by: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/serial2002.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/comedi/drivers/serial2002.c
b/drivers/staging/comedi/drivers/serial2002.c
index 304ebff..83da162 100644
--- a/drivers/staging/comedi/drivers/serial2002.c
+++ b/drivers/staging/comedi/drivers/serial2002.c
@@ -373,7 +373,7 @@ static int serial2002_setup_subdevice(struct
comedi_subdevice *s,
if (cfg[j].kind == kind) {
if (mapping)
mapping[chan] = j;
- if (range) {
+ if (range && range_table_list) {
range[j].length = 1;
range[j].range.min = cfg[j].min;
range[j].range.max = cfg[j].max;
--
2.3.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/3] staging: comedi: ni_nio_common: don't write non-existing caldac's
ni_write_caldac() checks the boardinfo 'caldac' array to determine what
caldac is used for a given 'addr'. It then calculates the 'bitstring' and
number of 'bits' used to write a value to that caldac address.
After checking the caldac array, if the number of bits is 0 there is no
caldac associated with the address. If this happens we shouldn't try
writing to the non-existing caldac.
Reported-by: coverity (CID 1192116)
Signed-off-by: H Hartley Sweeten
Reviewed-by: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/ni_mio_common.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c
b/drivers/staging/comedi/drivers/ni_mio_common.c
index c66affd9..69d71f3 100644
--- a/drivers/staging/comedi/drivers/ni_mio_common.c
+++ b/drivers/staging/comedi/drivers/ni_mio_common.c
@@ -4355,6 +4355,10 @@ static void ni_write_caldac(struct comedi_device *dev,
int addr, int val)
addr -= caldacs[type].n_chans;
}
+ /* bits will be 0 if there is no caldac for the given addr */
+ if (bits == 0)
+ return;
+
for (bit = 1 << (bits - 1); bit; bit >>= 1) {
ni_writeb(dev, ((bit & bitstring) ? 0x02 : 0), Serial_Command);
udelay(1);
--
2.3.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 0/3] staging: comedi: Coverity fixes
Fix a couple issues reported by Coverity. v2: change (1 << b_chans) to (1U << b_chans) in PATCH 1/3, at the request of Dan Carpenter. Added Ian Abbott's Reviewed-by to all patches. H Hartley Sweeten (3): staging: comedi: comedi_bond: fix 'b_mask' calc in bonding_dio_insn_bits() staging: comedi: ni_nio_common: don't write non-existing caldac's staging: comedi: serial2002: fix Coverity "Explicit null dereference" drivers/staging/comedi/drivers/comedi_bond.c | 3 ++- drivers/staging/comedi/drivers/ni_mio_common.c | 4 drivers/staging/comedi/drivers/serial2002.c| 2 +- 3 files changed, 7 insertions(+), 2 deletions(-) -- 2.3.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/3] staging: comedi: comedi_bond: fix 'b_mask' calc in bonding_dio_insn_bits()
'b_chans' may be a valud up to 32. 'b_mask' is an unsigned int and a left shift of more than 31 bits has undefined behavior. Fix the calc so it works correctly with a 'b_chans' of 32.. Reported-by: coverity (CID 1192244) Signed-off-by: H Hartley Sweeten Reviewed-by: Ian Abbott Cc: Greg Kroah-Hartman --- drivers/staging/comedi/drivers/comedi_bond.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/staging/comedi/drivers/comedi_bond.c b/drivers/staging/comedi/drivers/comedi_bond.c index 96db0c2..566b927 100644 --- a/drivers/staging/comedi/drivers/comedi_bond.c +++ b/drivers/staging/comedi/drivers/comedi_bond.c @@ -101,7 +101,8 @@ static int bonding_dio_insn_bits(struct comedi_device *dev, b_chans = bdev->nchans - base_chan; if (b_chans > n_left) b_chans = n_left; - b_mask = (1U << b_chans) - 1; + b_mask = (b_chans < 32) ? ((1U << b_chans) - 1) + : 0x; b_write_mask = (write_mask >> n_done) & b_mask; b_data_bits = (data_bits >> n_done) & b_mask; /* Read/Write the new digital lines. */ -- 2.3.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
答复: [PATCH V2] staging: sm750: Fix lynxfb_ops_imageblit() if image->depth != 1
The image->depth != 1 case means the image is color. The current driver only does 2d in mono color image. I think we can let the driver fall back to cfb_imageblit() currently. Then we implement the color image 2d later. If we don't do anything for color image, the color image will not be displayed. Best Regards, Teddy Wang -邮件原件- 发件人: Sudip Mukherjee [mailto:sudipm.mukher...@gmail.com] 发送时间: 2015年4月27日 17:00 收件人: Huacai Chen 抄送: Greg Kroah-Hartman; de...@driverdev.osuosl.org; Binbin Zhou; Fuxin Zhang; Teddy Wang 王力强 主题: Re: [PATCH V2] staging: sm750: Fix lynxfb_ops_imageblit() if image->depth != 1 On Mon, Apr 27, 2015 at 04:10:53PM +0800, Huacai Chen wrote: > If image->depth != 1, lynxfb_ops_imageblit() should fallback to call > cfb_imageblit(), not return directly. Otherwise it can't display the > boot logo. I think it is wrong. lynxfb_ops_imageblit() is the imageblit callback if 2D acceleration is enabled. but cfb_imageblit() is the default imageblit function that the framebuffer layer is providing for drivers which do not have 2D acceleration. You will notice the default is mentioned as cfb_imageblit() when we were defining lynxfb_ops. So we should not fall back again to the default function even if 2D acceleration is on. Teddy: any comments about what should be the proper fix for this? regards sudip ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: linux-4.1-rc1/drivers/staging/rtl8188eu/hal/Hal8188ERateAdaptive.c:424: bad if test ?
On Mon, Apr 27, 2015 at 08:33:30AM +, David Binderman wrote: > Hello there Greg, > > Static analyser cppcheck says > > [linux-4.1-rc1/drivers/staging/rtl8188eu/hal/Hal8188ERateAdaptive.c:424]: > (style) Expression is always false because 'else if' condition matches > previous condition at line 422. > > Source code is > > else if (pRaInfo->HighestRate> 0x0b) > pRaInfo->PTModeSS = 2; > else if (pRaInfo->HighestRate> 0x0b) > pRaInfo->PTModeSS = 1; > > > Regards > > David Binderman Patches always gladly accepted :) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/6] Drivers: hv: vmbus: distribute subchannels among all vcpus
KY Srinivasan writes: >> -Original Message- >> From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com] >> Sent: Friday, April 24, 2015 2:05 AM >> To: Dexuan Cui >> Cc: KY Srinivasan; Haiyang Zhang; de...@linuxdriverproject.org; linux- >> ker...@vger.kernel.org >> Subject: Re: [PATCH 5/6] Drivers: hv: vmbus: distribute subchannels among >> all vcpus >> >> Dexuan Cui writes: >> >> >> -Original Message- >> >> From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com] >> >> Sent: Tuesday, April 21, 2015 22:28 >> >> To: KY Srinivasan >> >> Cc: Haiyang Zhang; de...@linuxdriverproject.org; linux- >> >> ker...@vger.kernel.org; Dexuan Cui >> >> Subject: [PATCH 5/6] Drivers: hv: vmbus: distribute subchannels among all >> >> vcpus >> >> >> >> Primary channels are distributed evenly across all vcpus we have. When >> the >> >> host asks us to create subchannels it usually makes us num_cpus-1 offers >> > >> > Hi Vitaly, >> > AFAIK, in the VSP of storvsc, the number of subchannel is >> > (the_number_of_vcpus - 1) / 4. >> > >> > This means for a 8-vCPU guest, there is only 1 subchannel. >> > >> > Your new algorithm tends to make the vCPUs with small-number busier: >> > e.g., in the 8-vCPU case, assuming we have 4 SCSI controllers: >> > vCPU0: scsi0's PrimaryChannel (P) >> > vCPU1: scsi0's SubChannel (S) + scsi1's P >> > vCPU2: scsi1's S + scsi2's P >> > vCPU3: scsi2's S + scsi3's P >> > vCPU4: scsi3's S >> > vCPU5, 6 and 7 are idle. >> > >> > In this special case, the existing algorithm is better. :-) >> > >> > However, I do like this idea in your patch, that is, making sure a device's >> > primary/sub channels are assigned to differents vCPUs. >> >> Under special circumstances with the current code we can end up with >> having all subchannels on the same vCPU with the primary channel I guess >> :-) This is not something common, but possible. >> >> > >> > I'm just wondering if we should use an even better (and complex) >> > algorithm :-) >> >> The question here is - does sticking to the current vCPU help? If it >> does, I can suggest the following (I think I even mentioned that in my >> PATCH 00): first we try to find a (sub)channel with target_cpu == >> current_vcpu and only when we fail we do the round robin. I'd like to >> hear K.Y.'s opinion here as he's the original author :-) > > Sorry for the delayed response. Initially I had implemented a scheme that > would > pick an outgoing CPU that was closest to the CPU on which the request came > (to maintain > cache locality especially on NUMA systems). I changed this algorithm to > spread the load > more uniformly as we were trying to improve Linux IOPS on Azure XIO > (premium storage). We are currently testing > this code on our Converged Offering - CPS and I am finding that the perf as > measured by IOS has regressed. > I have not narrowed the reason for this regression and it may very well be > the change in the > algorithm for selecting the outgoing channel. In general, I don't think the > logic here needs to be > exact and locality (being on the same CPU or within the same NUMA node) is > important. Any change > to this algorithm will have to be validated on different MSFT > environments (Azure XIO, CPS etc.). Thanks, can you please compare two algorythms here: 1) Simple round robin (the one my patch series implement but with issues fixed, I'll send v2). 2) Try to find a (sub)channel with matching VCPU and round-robin when we fail (I can actually include it in v2). We can later decide something based on these testing results. > > Regards, > > K. Y -- Vitaly ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: Return error if memory allocation fails
On Mon, Apr 27, 2015 at 12:44:51PM +, Gujulan Elango, Hari Prasath (H.) wrote: > Should return ENOMEM if memory allocation occurs.There is a wrong code > jump here and this patch addresses this issue. > What? No, the original code is fine and this patch introduces a memory leak. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: comedi: Return error if memory allocation fails
Should return ENOMEM if memory allocation occurs.There is a wrong code
jump here and this patch addresses this issue.
Signed-off-by: Hari Prasath Gujulan Elango
---
drivers/staging/comedi/comedi_fops.c | 12
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/comedi/comedi_fops.c
b/drivers/staging/comedi/comedi_fops.c
index e78ddbe..b3e8f3d 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -1412,16 +1412,12 @@ static int do_insnlist_ioctl(struct comedi_device *dev,
return -EFAULT;
data = kmalloc_array(MAX_SAMPLES, sizeof(unsigned int), GFP_KERNEL);
- if (!data) {
- ret = -ENOMEM;
- goto error;
- }
+ if (!data)
+ return -ENOMEM;
insns = kcalloc(insnlist.n_insns, sizeof(*insns), GFP_KERNEL);
- if (!insns) {
- ret = -ENOMEM;
- goto error;
- }
+ if (!insns)
+ return -ENOMEM;
if (copy_from_user(insns, insnlist.insns,
sizeof(*insns) * insnlist.n_insns)) {
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8192u: ieee80211: Silence sparse warning
Can't we just export the tkip.c function? regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: iio: Check return value and handle error
Check the return value of function and handle error condition appropriately. Signed-off-by: Hari Prasath Gujulan Elango --- drivers/staging/iio/accel/lis3l02dq_core.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/staging/iio/accel/lis3l02dq_core.c b/drivers/staging/iio/accel/lis3l02dq_core.c index ebcab56..13d8018 100644 --- a/drivers/staging/iio/accel/lis3l02dq_core.c +++ b/drivers/staging/iio/accel/lis3l02dq_core.c @@ -289,6 +289,8 @@ static int lis3l02dq_read_raw(struct iio_dev *indio_dev, case IIO_CHAN_INFO_CALIBBIAS: reg = lis3l02dq_axis_map[LIS3L02DQ_BIAS][chan->address]; ret = lis3l02dq_spi_read_reg_8(indio_dev, reg, (u8 *)&stemp); + if (ret) + goto error_ret; /* to match with what previous code does */ *val = stemp; return IIO_VAL_INT; @@ -584,6 +586,8 @@ int lis3l02dq_disable_all_events(struct iio_dev *indio_dev) ret = lis3l02dq_spi_read_reg_8(indio_dev, LIS3L02DQ_REG_CTRL_2_ADDR, &control); + if (ret) + goto error_ret; control &= ~LIS3L02DQ_REG_CTRL_2_ENABLE_INTERRUPT; ret = lis3l02dq_spi_write_reg_8(indio_dev, -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V2] staging: sm750: Fix lynxfb_ops_imageblit() if image->depth != 1
On Mon, Apr 27, 2015 at 04:10:53PM +0800, Huacai Chen wrote: > If image->depth != 1, lynxfb_ops_imageblit() should fallback to call > cfb_imageblit(), not return directly. Otherwise it can't display the > boot logo. I think it is wrong. lynxfb_ops_imageblit() is the imageblit callback if 2D acceleration is enabled. but cfb_imageblit() is the default imageblit function that the framebuffer layer is providing for drivers which do not have 2D acceleration. You will notice the default is mentioned as cfb_imageblit() when we were defining lynxfb_ops. So we should not fall back again to the default function even if 2D acceleration is on. Teddy: any comments about what should be the proper fix for this? regards sudip ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
linux-4.1-rc1/drivers/staging/rtl8188eu/hal/Hal8188ERateAdaptive.c:424: bad if test ?
Hello there Greg, Static analyser cppcheck says [linux-4.1-rc1/drivers/staging/rtl8188eu/hal/Hal8188ERateAdaptive.c:424]: (style) Expression is always false because 'else if' condition matches previous condition at line 422. Source code is else if (pRaInfo->HighestRate> 0x0b) pRaInfo->PTModeSS = 2; else if (pRaInfo->HighestRate> 0x0b) pRaInfo->PTModeSS = 1; Regards David Binderman ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH V2] staging: sm750: Fix lynxfb_ops_imageblit() if image->depth != 1
If image->depth != 1, lynxfb_ops_imageblit() should fallback to call cfb_imageblit(), not return directly. Otherwise it can't display the boot logo. V2: Coding style ajustment. Cc: Teddy Wang Cc: Sudip Mukherjee Signed-off-by: Huacai Chen --- drivers/staging/sm750fb/sm750.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c index 3c7ea95..fab8dce 100644 --- a/drivers/staging/sm750fb/sm750.c +++ b/drivers/staging/sm750fb/sm750.c @@ -279,7 +279,9 @@ static void lynxfb_ops_imageblit(struct fb_info *info, } goto _do_work; } + cfb_imageblit(info, image); return; + _do_work: /* * If not use spin_lock, system will die if user load driver -- 1.7.7.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: sm750: Fix lynxfb_ops_imageblit() if image->depth != 1
Please run scripts/checkpatch.pl over your patches. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
