Staging: hv: Driver cleanup
Subsystems (3)
cover Staging: hv: Driver cleanup
Further cleanup of the hv drivers.
1) Cleanup reference counting.
2) Handle all block devices using the storvsc driver. I have modified the implementation here based on Christoph's feedback on my earlier implementation.
3) Fix bugs.
4) Accomodate some host side scsi emulation bugs.
5) In case of scsi errors off-line the device.
This patch-set further reduces the size of Hyper-V drivers - the code is about 10% smaller. This reduction is mainly because we have eliminated the blkvsc driver.
Regards,
K. Y
PATCH 1/46 Staging: hv: storvsc: Inline free_stor_device()
Inline the code for free_stor_device() and get rid of the function.
--- drivers/staging/hv/storvsc.c | 8 ++------ 1 files changed, 2 insertions(+), 6 deletions(-)
drivers/staging/hv/storvsc.c +2 -6
@@ -51,10 +51,6 @@ static inline struct storvsc_device *alloc_stor_device(struct hv_device *device)return stor_device; } -static inline void free_stor_device (struct storvsc_device *device) -{ -kfree (device); -}/* Get the stordevice object iff exists and its refcount > 0 */ static inline struct storvsc_device *must_get_stor_device(
@@ -394,7 +390,7 @@ int storvsc_dev_add(struct hv_device *device,/* Send it back up */ ret =storvsc_connect_to_vsp (device, device_info->ring_buffer_size);if (ret) { -free_stor_device (stor_device); +kfree (stor_device);return ret; } device_info->path_id = stor_device->path_id;
@@ -422,7 +418,7 @@ int storvsc_dev_remove(struct hv_device *device)/* Close the channel */ vmbus_close (device->channel); -free_stor_device (stor_device); +kfree (stor_device);return 0 ; }
Signature · 1 line
1.7.4.1
PATCH 2/46 Staging: hv: storvsc: Do not aquire an unnecessary reference on stor_device
On entry into storvsc_on_io_completion() we have already acquired a reference on the stor_device; there is no need to acquire an additional reference here.
--- drivers/staging/hv/storvsc.c | 5 +---- 1 files changed, 1 insertions(+), 4 deletions(-)
drivers/staging/hv/storvsc.c +1 -4
@@ -232,9 +232,7 @@ static void storvsc_on_io_completion(struct hv_device *device,struct storvsc_device *stor_device;struct vstor_packet *stor_pkt; - stor_device =must_get_stor_device (device); -if (!stor_device) -return ; + stor_device = (struct storvsc_device *)device->ext; stor_pkt = &request->vstor_packet;
@@ -279,7 +277,6 @@ static void storvsc_on_io_completion(struct hv_device *device,wake_up (&stor_device->waiting_to_drain); -put_stor_device (device); } static void storvsc_on_receive(struct hv_device *device,
Signature · 1 line
1.7.4.1
PATCH 3/46 Staging: hv: storvsc: Rename must_get_stor_device()
In reply to K. Y. SrinivasanIn preparation for cleaning up how we manage reference counts on the stor device, clearly distinguish why we are attempting to acquire a reference.
--- drivers/staging/hv/storvsc.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
drivers/staging/hv/storvsc.c +3 -3
@@ -41,7 +41,7 @@ static inline struct storvsc_device *alloc_stor_device(struct hv_device *device)return NULL ;/* Set to 2 to allow both inbound and outbound traffics */ -/* (ie get_stor_device() and must_get_stor_device()) to proceed. */ +/* (ie get_stor_device() and get_in_stor_device()) to proceed. */ atomic_cmpxchg (&stor_device->ref_count,0 ,2 );init_waitqueue_head (&stor_device->waiting_to_drain);
@@ -53,7 +53,7 @@ static inline struct storvsc_device *alloc_stor_device(struct hv_device *device)/* Get the stordevice object iff exists and its refcount > 0 */ -static inline struct storvsc_device *must_get_stor_device ( +static inline struct storvsc_device *get_in_stor_device (struct hv_device *device) {struct storvsc_device *stor_device;
@@ -305,7 +305,7 @@ static void storvsc_on_channel_callback(void *context)int ret; - stor_device =must_get_stor_device (device); + stor_device =get_in_stor_device (device);if (!stor_device)return ;
Signature · 1 line
1.7.4.1
PATCH 4/46 Staging: hv: storvsc: Rename get_stor_device()
In reply to K. Y. SrinivasanIn preparation for cleaning up how we manage reference counts on the stor device, clearly distinguish why we are attempting to acquire a reference.
--- drivers/staging/hv/hyperv_storage.h | 3 ++- drivers/staging/hv/storvsc.c | 8 ++++---- drivers/staging/hv/storvsc_drv.c | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-)
drivers/staging/hv/hyperv_storage.h +2 -1
@@ -288,7 +288,8 @@ struct storvsc_device {/* Get the stordevice object iff exists and its refcount > 1 */ -static inline struct storvsc_device *get_stor_device (struct hv_device *device) +static inline struct storvsc_device *get_out_stor_device ( +struct hv_device *device) {struct storvsc_device *stor_device;
drivers/staging/hv/storvsc.c +4 -4
@@ -41,7 +41,7 @@ static inline struct storvsc_device *alloc_stor_device(struct hv_device *device)return NULL ;/* Set to 2 to allow both inbound and outbound traffics */ -/* (ie get_stor_device() and get_in_stor_device()) to proceed. */ +/* (ie get_out_stor_device() and get_in_stor_device()) to proceed. */ atomic_cmpxchg (&stor_device->ref_count,0 ,2 );init_waitqueue_head (&stor_device->waiting_to_drain);
@@ -67,7 +67,7 @@ static inline struct storvsc_device *get_in_stor_device(return stor_device; } -/* Drop ref count to 1 to effectively disable get_stor_device() */ +/* Drop ref count to 1 to effectively disable get_out_stor_device() */ static inline struct storvsc_device *release_stor_device (struct hv_device *device) {
@@ -105,7 +105,7 @@ static int storvsc_channel_init(struct hv_device *device)struct vstor_packet *vstor_packet;int ret, t; - stor_device =get_stor_device (device); + stor_device =get_out_stor_device (device);if (!stor_device)return -ENODEV ;
@@ -427,7 +427,7 @@ int storvsc_do_io(struct hv_device *device,int ret =0 ; vstor_packet = &request->vstor_packet; - stor_device =get_stor_device (device); + stor_device =get_out_stor_device (device);if (!stor_device)return -ENODEV ;
drivers/staging/hv/storvsc_drv.c +1 -1
@@ -344,7 +344,7 @@ static int storvsc_host_reset(struct hv_device *device)int ret, t; - stor_device =get_stor_device (device); + stor_device =get_out_stor_device (device);if (!stor_device)return -ENODEV ;
Signature · 1 line
1.7.4.1
PATCH 5/46 Staging: hv: storvsc: Cleanup alloc_stor_device()
In reply to K. Y. SrinivasanCleanup alloc_stor_device(), we can set the ref_count directly.
--- drivers/staging/hv/storvsc.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/staging/hv/storvsc.c +1 -1
@@ -42,7 +42,7 @@ static inline struct storvsc_device *alloc_stor_device(struct hv_device *device)/* Set to 2 to allow both inbound and outbound traffics */ /* (ie get_out_stor_device() and get_in_stor_device()) to proceed. */ -atomic_cmpxchg (&stor_device->ref_count,0 ,2 ); +atomic_set (&stor_device->ref_count,2 );init_waitqueue_head (&stor_device->waiting_to_drain); stor_device->device = device;
Signature · 1 line
1.7.4.1
PATCH 6/46 Staging: hv: storvsc: Introduce state to manage the lifecycle of stor device
In reply to K. Y. SrinivasanIntroduce state to manage the lifecycle of stor device. This would be the basis for managing the references on the stor object.
--- drivers/staging/hv/hyperv_storage.h | 2 +- drivers/staging/hv/storvsc.c | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-)
drivers/staging/hv/hyperv_storage.h +1 -1
@@ -266,7 +266,7 @@ struct storvsc_device {/* 0 indicates the device is being destroyed */ atomic_t ref_count; - +bool destroy;bool drain_notify;atomic_t num_outstanding_req;
drivers/staging/hv/storvsc.c +7 -1
@@ -43,7 +43,7 @@ static inline struct storvsc_device *alloc_stor_device(struct hv_device *device)/* Set to 2 to allow both inbound and outbound traffics */ /* (ie get_out_stor_device() and get_in_stor_device()) to proceed. */ atomic_set (&stor_device->ref_count,2 ); - + stor_device->destroy = false;init_waitqueue_head (&stor_device->waiting_to_drain); stor_device->device = device; device->ext = stor_device;
@@ -399,9 +399,15 @@ int storvsc_dev_add(struct hv_device *device,int storvsc_dev_remove (struct hv_device *device) {struct storvsc_device *stor_device; +unsigned long flags; + stor_device =release_stor_device (device); +spin_lock_irqsave (&device->channel->inbound_lock, flags); + stor_device->destroy = true; +spin_unlock_irqrestore (&device->channel->inbound_lock, flags); +/* * At this point, all outbound traffic should be disable. We * only allow inbound traffic (responses) to proceed so that
Signature · 1 line
1.7.4.1
PATCH 7/46 Staging: hv: storvsc: Prevent outgoing traffic when stor dev is being destroyed
In reply to K. Y. SrinivasanPrevent outgoing traffic when stor dev is destroyed.
--- drivers/staging/hv/hyperv_storage.h | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
drivers/staging/hv/hyperv_storage.h +2 -1
@@ -294,7 +294,8 @@ static inline struct storvsc_device *get_out_stor_device(struct storvsc_device *stor_device; stor_device = (struct storvsc_device *)device->ext; -if (stor_device &&atomic_read (&stor_device->ref_count) >1 ) +if (stor_device && (atomic_read (&stor_device->ref_count) >1 ) && + !stor_device->destroy)atomic_inc (&stor_device->ref_count);else stor_device =NULL ;
Signature · 1 line
1.7.4.1
PATCH 8/46 Staging: hv: storvsc: Get rid of release_stor_device() by inlining the code
In reply to K. Y. SrinivasanGet rid of release_stor_device() by inlining the code.
--- drivers/staging/hv/storvsc.c | 19 ++----------------- 1 files changed, 2 insertions(+), 17 deletions(-)
drivers/staging/hv/storvsc.c +2 -17
@@ -67,21 +67,6 @@ static inline struct storvsc_device *get_in_stor_device(return stor_device; } -/* Drop ref count to 1 to effectively disable get_out_stor_device() */ -static inline struct storvsc_device *release_stor_device ( -struct hv_device *device) -{ -struct storvsc_device *stor_device; - - stor_device = (struct storvsc_device *)device->ext; - -/* Busy wait until the ref drop to 2, then set it to 1 */ -while (atomic_cmpxchg (&stor_device->ref_count,2 ,1 ) !=2 ) -udelay (100 ); - -return stor_device; -} -/* Drop ref count to 0. No one can use stor_device object. */ static inline struct storvsc_device *final_release_stor_device (struct hv_device *device)
@@ -401,8 +386,8 @@ int storvsc_dev_remove(struct hv_device *device)struct storvsc_device *stor_device;unsigned long flags; - - stor_device =release_stor_device (device); + stor_device = (struct storvsc_device *)device->ext; +atomic_dec (&stor_device->ref_count);spin_lock_irqsave (&device->channel->inbound_lock, flags); stor_device->destroy = true;
Signature · 1 line
1.7.4.1
PATCH 9/46 Staging: hv: storvsc: Get rid of final_release_stor_device() by inlining code
In reply to K. Y. SrinivasanGet rid of final_release_stor_device() by inlining code.
--- drivers/staging/hv/storvsc.c | 23 ++++++----------------- 1 files changed, 6 insertions(+), 17 deletions(-)
drivers/staging/hv/storvsc.c +6 -17
@@ -67,22 +67,6 @@ static inline struct storvsc_device *get_in_stor_device(return stor_device; } -/* Drop ref count to 0. No one can use stor_device object. */ -static inline struct storvsc_device *final_release_stor_device ( -struct hv_device *device) -{ -struct storvsc_device *stor_device; - - stor_device = (struct storvsc_device *)device->ext; - -/* Busy wait until the ref drop to 1, then set it to 0 */ -while (atomic_cmpxchg (&stor_device->ref_count,1 ,0 ) !=1 ) -udelay (100 ); - - device->ext =NULL ; -return stor_device; -} -static int storvsc_channel_init (struct hv_device *device) {struct storvsc_device *stor_device;
@@ -401,7 +385,12 @@ int storvsc_dev_remove(struct hv_device *device)storvsc_wait_to_drain (stor_device); - stor_device =final_release_stor_device (device); +/* +* Since we have already drained, we don't need to busy wait +* as was done in final_release_stor_device() +*/ +atomic_set (&stor_device->ref_count,0 ); + device->ext =NULL ;/* Close the channel */ vmbus_close (device->channel);
Signature · 1 line
1.7.4.1
PATCH 10/46 Staging: hv: storvsc: Get rid of the reference counting in struct storvsc_device
In reply to K. Y. SrinivasanGet rid of the reference counting in struct storvsc_device. We manage the lifecycle with the following logic: If the device is marked for destruction, we dot allow any outgoing traffic on the device. Incoming traffic is allowed only to drain pending outgoing traffic. Note that while the upper level code in Linux deals with outstanding I/Os, we may have situations on Hyper-V where some book keeping messages are sent out that the upper level Linux code may not be aware of.
--- drivers/staging/hv/hyperv_storage.h | 18 ++---------------- drivers/staging/hv/storvsc.c | 33 +++++++++++++++++++++------------ drivers/staging/hv/storvsc_drv.c | 1 - 3 files changed, 23 insertions(+), 29 deletions(-)
drivers/staging/hv/hyperv_storage.h +2 -16
@@ -264,8 +264,6 @@ struct storvsc_major_info {struct storvsc_device {struct hv_device *device; -/* 0 indicates the device is being destroyed */ -atomic_t ref_count;bool destroy;bool drain_notify;atomic_t num_outstanding_req;
@@ -287,32 +285,20 @@ struct storvsc_device { }; -/* Get the stordevice object iff exists and its refcount > 1 */ static inline struct storvsc_device *get_out_stor_device (struct hv_device *device) {struct storvsc_device *stor_device; stor_device = (struct storvsc_device *)device->ext; -if (stor_device && (atomic_read (&stor_device->ref_count) >1 ) && - !stor_device->destroy) -atomic_inc (&stor_device->ref_count); -else + +if (stor_device && stor_device->destroy) stor_device =NULL ;return stor_device; } -static inline void put_stor_device (struct hv_device *device) -{ -struct storvsc_device *stor_device; - - stor_device = (struct storvsc_device *)device->ext; - -atomic_dec (&stor_device->ref_count); -} -static inline void storvsc_wait_to_drain (struct storvsc_device *dev) { dev->drain_notify = true;
drivers/staging/hv/storvsc.c +21 -12
@@ -40,9 +40,6 @@ static inline struct storvsc_device *alloc_stor_device(struct hv_device *device)if (!stor_device)return NULL ; -/* Set to 2 to allow both inbound and outbound traffics */ -/* (ie get_out_stor_device() and get_in_stor_device()) to proceed. */ -atomic_set (&stor_device->ref_count,2 ); stor_device->destroy = false;init_waitqueue_head (&stor_device->waiting_to_drain); stor_device->device = device;
@@ -52,19 +49,31 @@ static inline struct storvsc_device *alloc_stor_device(struct hv_device *device) } -/* Get the stordevice object iff exists and its refcount > 0 */ static inline struct storvsc_device *get_in_stor_device (struct hv_device *device) {struct storvsc_device *stor_device; +unsigned long flags; +spin_lock_irqsave (&device->channel->inbound_lock, flags); stor_device = (struct storvsc_device *)device->ext; -if (stor_device &&atomic_read (&stor_device->ref_count)) -atomic_inc (&stor_device->ref_count); -else + +if (!stor_device) + goto get_in_err; + +/* +* If the device is being destroyed; allow incoming +* traffic only to cleanup outstanding requests. +*/ + +if (stor_device->destroy && + (atomic_read (&stor_device->num_outstanding_req) ==0 )) stor_device =NULL ; +get_in_err: +spin_unlock_irqrestore (&device->channel->inbound_lock, flags);return stor_device; + }static int storvsc_channel_init (struct hv_device *device)
@@ -190,7 +199,6 @@ static int storvsc_channel_init(struct hv_device *device) cleanup: -put_stor_device (device);return ret; }
@@ -303,7 +311,6 @@ static void storvsc_on_channel_callback(void *context) } }while (1 ); -put_stor_device (device);return ; }
@@ -371,7 +378,6 @@ int storvsc_dev_remove(struct hv_device *device)unsigned long flags; stor_device = (struct storvsc_device *)device->ext; -atomic_dec (&stor_device->ref_count);spin_lock_irqsave (&device->channel->inbound_lock, flags); stor_device->destroy = true;
@@ -388,9 +394,13 @@ int storvsc_dev_remove(struct hv_device *device)/* * Since we have already drained, we don't need to busy wait * as was done in final_release_stor_device() +* Note that we cannot set the ext pointer to NULL until +* we have drained - to drain the outgoing packets, we need to +* allow incoming packets. */ -atomic_set (&stor_device->ref_count,0 ); +spin_lock_irqsave (&device->channel->inbound_lock, flags); device->ext =NULL ; +spin_unlock_irqrestore (&device->channel->inbound_lock, flags);/* Close the channel */ vmbus_close (device->channel);
@@ -448,7 +458,6 @@ int storvsc_do_io(struct hv_device *device,atomic_inc (&stor_device->num_outstanding_req); -put_stor_device (device);return ret; }
drivers/staging/hv/storvsc_drv.c +0 -1
@@ -378,7 +378,6 @@ static int storvsc_host_reset(struct hv_device *device)*/ cleanup: -put_stor_device (device);return ret; }
Signature · 1 line
1.7.4.1
PATCH 11/46 Staging: hv: netvsc: Inline the code for free_net_device()
In reply to K. Y. SrinivasanInline the code for free_net_device().
--- drivers/staging/hv/netvsc.c | 12 ++---------- 1 files changed, 2 insertions(+), 10 deletions(-)
drivers/staging/hv/netvsc.c +2 -10
@@ -49,14 +49,6 @@ static struct netvsc_device *alloc_net_device(struct hv_device *device)return net_device; } -static void free_net_device (struct netvsc_device *device) -{ -WARN_ON (atomic_read (&device->refcnt) !=0 ); - device->dev->ext =NULL ; -kfree (device); -} - -/* Get the net device object iff exists and its refcount > 1 */ static struct netvsc_device *get_outbound_net_device (struct hv_device *device) {
@@ -438,7 +430,7 @@ int netvsc_device_remove(struct hv_device *device)kfree (netvsc_packet); } -free_net_device (net_device); +kfree (net_device);return 0 ; }
@@ -980,7 +972,7 @@ cleanup:release_outbound_net_device (device);release_inbound_net_device (device); -free_net_device (net_device); +kfree (net_device); }return ret;
Signature · 1 line
1.7.4.1
PATCH 12/46 Staging: hv: netvsc: Cleanup alloc_net_device()
In reply to K. Y. SrinivasanCleanup alloc_net_device(); we can directly set the refcnt.
--- drivers/staging/hv/netvsc.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/staging/hv/netvsc.c +1 -1
@@ -41,7 +41,7 @@ static struct netvsc_device *alloc_net_device(struct hv_device *device)return NULL ;/* Set to 2 to allow both inbound and outbound traffic */ -atomic_cmpxchg (&net_device->refcnt,0 ,2 ); +atomic_set (&net_device->refcnt,2 ); net_device->dev = device; device->ext = net_device;
Signature · 1 line
1.7.4.1
PATCH 13/46 Staging: hv: netvsc: Introduce state to manage the lifecycle of net device
In reply to K. Y. SrinivasanIntroduce state to manage the lifecycle of net device.
--- drivers/staging/hv/hyperv_net.h | 1 + drivers/staging/hv/netvsc.c | 6 ++++++ 2 files changed, 7 insertions(+), 0 deletions(-)
drivers/staging/hv/hyperv_net.h +1 -0
@@ -371,6 +371,7 @@ struct netvsc_device {atomic_t refcnt;atomic_t num_outstanding_sends; +bool destroy;/* * List of free preallocated hv_netvsc_packet to represent receive * packet
drivers/staging/hv/netvsc.c +6 -0
@@ -43,6 +43,7 @@ static struct netvsc_device *alloc_net_device(struct hv_device *device)/* Set to 2 to allow both inbound and outbound traffic */ atomic_set (&net_device->refcnt,2 ); + net_device->destroy = false; net_device->dev = device; device->ext = net_device;
@@ -396,6 +397,7 @@ int netvsc_device_remove(struct hv_device *device) {struct netvsc_device *net_device;struct hv_netvsc_packet *netvsc_packet, *pos; +unsigned long flags;/* Stop outbound traffic ie sends and receives completions */ net_device =release_outbound_net_device (device);
@@ -404,6 +406,10 @@ int netvsc_device_remove(struct hv_device *device)return -ENODEV ; } +spin_lock_irqsave (&device->channel->inbound_lock, flags); + net_device->destroy = true; +spin_unlock_irqrestore (&device->channel->inbound_lock, flags); +/* Wait for all send completions */ while (atomic_read (&net_device->num_outstanding_sends)) { dev_err(&device->device,
Signature · 1 line
1.7.4.1
PATCH 14/46 Staging: hv: netvsc: Prevent outgoing traffic when netvsc dev is destroyed
In reply to K. Y. SrinivasanPrevent outgoing traffic when netvsc dev is destroyed.
--- drivers/staging/hv/netvsc.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
drivers/staging/hv/netvsc.c +2 -1
@@ -56,7 +56,8 @@ static struct netvsc_device *get_outbound_net_device(struct hv_device *device)struct netvsc_device *net_device; net_device = device->ext; -if (net_device &&atomic_read (&net_device->refcnt) >1 ) +if (net_device && (atomic_read (&net_device->refcnt) >1 ) && + !net_device->destroy)atomic_inc (&net_device->refcnt);else net_device =NULL ;
Signature · 1 line
1.7.4.1
PATCH 15/46 Staging: hv: netvsc: Get rid of release_outbound_net_device() by inlining the code
In reply to K. Y. SrinivasanGet rid of release_outbound_net_device() by inlining the code.
--- drivers/staging/hv/netvsc.c | 38 ++++++++++++++------------------------ 1 files changed, 14 insertions(+), 24 deletions(-)
drivers/staging/hv/netvsc.c +14 -24
@@ -88,22 +88,6 @@ static void put_net_device(struct hv_device *device)atomic_dec (&net_device->refcnt); } -static struct netvsc_device *release_outbound_net_device ( -struct hv_device *device) -{ -struct netvsc_device *net_device; - - net_device = device->ext; -if (net_device ==NULL ) -return NULL ; - -/* Busy wait until the ref drop to 2, then set it to 1 */ -while (atomic_cmpxchg (&net_device->refcnt,2 ,1 ) !=2 ) -udelay (100 ); - -return net_device; -} -static struct netvsc_device *release_inbound_net_device (struct hv_device *device) {
@@ -400,13 +384,8 @@ int netvsc_device_remove(struct hv_device *device)struct hv_netvsc_packet *netvsc_packet, *pos;unsigned long flags; -/* Stop outbound traffic ie sends and receives completions */ - net_device =release_outbound_net_device (device); -if (!net_device) { -dev_err (&device->device,"No net device present!!" ); -return -ENODEV ; - } - + net_device = (struct netvsc_device *)device->ext; +atomic_dec (&net_device->refcnt);spin_lock_irqsave (&device->channel->inbound_lock, flags); net_device->destroy = true;spin_unlock_irqrestore (&device->channel->inbound_lock, flags);
@@ -424,6 +403,18 @@ int netvsc_device_remove(struct hv_device *device)/* Stop inbound traffic ie receives and sends completions */ net_device =release_inbound_net_device (device); +/* +* Wait until the ref cnt falls to 0. +* We have already stopped any new references +* for outgoing traffic. Also, at this point we don't have any +* incoming traffic as well. So this must be outgoing refrences +* established prior to marking the device as being destroyed. +* Since the send path is non-blocking, it is reasonable to busy +* wait here. +*/ +while (atomic_read (&net_device->refcnt)) +udelay (100 ); +/* At this point, no one should be accessing netDevice except in here */ dev_notice (&device->device,"net device safe to remove" );
@@ -976,7 +967,6 @@ cleanup:kfree (packet); } -release_outbound_net_device (device);release_inbound_net_device (device);kfree (net_device);
Signature · 1 line
1.7.4.1
PATCH 16/46 Staging: hv: netvsc: Get rid of release_inbound_net_device() by inlining the code
In reply to K. Y. SrinivasanGet rid of release_inbound_net_device() by inlining the code.
--- drivers/staging/hv/netvsc.c | 24 ++---------------------- 1 files changed, 2 insertions(+), 22 deletions(-)
drivers/staging/hv/netvsc.c +2 -22
@@ -88,23 +88,6 @@ static void put_net_device(struct hv_device *device)atomic_dec (&net_device->refcnt); } -static struct netvsc_device *release_inbound_net_device ( -struct hv_device *device) -{ -struct netvsc_device *net_device; - - net_device = device->ext; -if (net_device ==NULL ) -return NULL ; - -/* Busy wait until the ref drop to 1, then set it to 0 */ -while (atomic_cmpxchg (&net_device->refcnt,1 ,0 ) !=1 ) -udelay (100 ); - - device->ext =NULL ; -return net_device; -} -static int netvsc_destroy_recv_buf (struct netvsc_device *net_device) {struct nvsp_message *revoke_packet;
@@ -400,9 +383,8 @@ int netvsc_device_remove(struct hv_device *device)netvsc_disconnect_vsp (net_device); -/* Stop inbound traffic ie receives and sends completions */ - net_device =release_inbound_net_device (device); - +atomic_dec (&net_device->refcnt); + device->ext =NULL ;/* * Wait until the ref cnt falls to 0. * We have already stopped any new references
@@ -967,8 +949,6 @@ cleanup:kfree (packet); } -release_inbound_net_device (device); -kfree (net_device); }
Signature · 1 line
1.7.4.1
PATCH 17/46 Staging: hv: netvsc: Get rid of the refcnt field in struct netvsc_device
In reply to K. Y. SrinivasanGet rid of the refcnt field in struct netvsc_device. We implement the following logic to manage the life cycle of the device: If the device is being destroyed, we do not allow any outgoing traffic. Furthermore, if the device is being destroyed, we allow incoming traffic only to drain outgoing traffic. Note that the driver may send some book keeping messages to the host not known to upper level Linux code.
--- drivers/staging/hv/hyperv_net.h | 1 - drivers/staging/hv/netvsc.c | 62 ++++++++++++-------------------------- 2 files changed, 20 insertions(+), 43 deletions(-)
drivers/staging/hv/hyperv_net.h +0 -1
@@ -369,7 +369,6 @@ struct nvsp_message {struct netvsc_device {struct hv_device *dev; -atomic_t refcnt;atomic_t num_outstanding_sends;bool destroy;/*
drivers/staging/hv/netvsc.c +20 -42
174 lines not on this page · raw message
Signature · 1 line
1.7.4.1
PATCH 18/46 Staging: hv: storvsc: Add code to handle IDE devices using the storvsc driver
In reply to K. Y. SrinivasanAdd code to handle IDE devices using the storvsc driver. The storvsc_probe() is modified so that the storvsc driver can surface all disks presented to the guest as scsi devices using generic upper level Linux scsi drivers.
--- drivers/staging/hv/storvsc_drv.c | 60 ++++++++++++++++++++++++++++++++----- 1 files changed, 52 insertions(+), 8 deletions(-)
drivers/staging/hv/storvsc_drv.c +52 -8
@@ -59,6 +59,17 @@ struct storvsc_cmd_request {struct hv_storvsc_request request; }; +static void storvsc_get_ide_info (struct hv_device *dev,int *target,int *path) +{ + *target = + dev->dev_instance.b[5 ] <<8 | dev->dev_instance.b[4 ]; + + *path = + dev->dev_instance.b[3 ] <<24 | + dev->dev_instance.b[2 ] <<16 | + dev->dev_instance.b[1 ] <<8 | dev->dev_instance.b[0 ]; +} +static int storvsc_device_alloc (struct scsi_device *sdevice) {
@@ -642,6 +653,20 @@ static const struct hv_vmbus_device_id id_table[] = { };MODULE_DEVICE_TABLE (vmbus, id_table); + +/* +* This declaration is temporary; once we get the +* infrastructure in place, we will integrate with +* id_table. +*/ + +static const uuid_le ide_blk_guid = { + .b = { +0x32 ,0x26 ,0x41 ,0x32 ,0xcb ,0x86 ,0xa2 ,0x44 , +0x9b ,0x5c ,0x50 ,0xd1 ,0x41 ,0x73 ,0x54 ,0xf5 + } +}; +/* * storvsc_probe - Add a new device for this driver */
@@ -652,6 +677,14 @@ static int storvsc_probe(struct hv_device *device)struct Scsi_Host *host;struct hv_host_device *host_dev;struct storvsc_device_info device_info; +bool dev_is_ide; +int path =0 ; +int target =0 ; + +if (!uuid_le_cmp (device->dev_type, ide_blk_guid)) + dev_is_ide = true; +else + dev_is_ide = false; host =scsi_host_alloc (&scsi_driver,sizeof (struct hv_host_device ));
@@ -687,6 +720,9 @@ static int storvsc_probe(struct hv_device *device)return -ENODEV ; } +if (dev_is_ide) +storvsc_get_ide_info (device, &target, &path); + host_dev->path = device_info.path_id; host_dev->target = device_info.target_id;
@@ -699,17 +735,25 @@ static int storvsc_probe(struct hv_device *device)/* Register the HBA and start the scsi bus scan */ ret =scsi_add_host (host, &device->device); -if (ret !=0 ) { - -storvsc_dev_remove (device); +if (ret !=0 ) + goto err_out; -kmem_cache_destroy (host_dev->request_pool); -scsi_host_put (host); -return -ENODEV ; +if (!dev_is_ide) { +scsi_scan_host (host); +return 0 ; + } + ret =scsi_add_device (host,0 , target,0 ); +if (ret) { +scsi_remove_host (host); + goto err_out; } +return 0 ; -scsi_scan_host (host); -return ret; +err_out: +storvsc_dev_remove (device); +kmem_cache_destroy (host_dev->request_pool); +scsi_host_put (host); +return -ENODEV ; }/* The one and only one */
Signature · 1 line
1.7.4.1
Re: [PATCH 18/46] Staging: hv: storvsc: Add code to handle IDE devices using the storvsc driver
Quoted 5 lines · K. Y. Srinivasan
drivers/staging/hv/storvsc_drv.c +1
-0@@ -59,6 +59,17 @@ struct storvsc_cmd_request {struct hv_storvsc_request request; }; +static void storvsc_get_ide_info (struct hv_device *dev,int *target,int *path)
drivers/staging/hv/storvsc_drv.c struct storvsc_cmd_request { · line 72
Quoted 10 lines
drivers/staging/hv/storvsc_drv.c +10
-0+{ + *target = + dev->dev_instance.b[5 ] <<8 | dev->dev_instance.b[4 ]; + + *path = + dev->dev_instance.b[3 ] <<24 | + dev->dev_instance.b[2 ] <<16 | + dev->dev_instance.b[1 ] <<8 | dev->dev_instance.b[0 ]; +} +
Does endianness matter here? It seems like *path isn't actually used anywhere. Probably target and path should be u32 types?
regards,
dan carpenter
PATCH 19/46 Staging: hv: storvsc: Handle IDE devices using the storvsc driver
In reply to K. Y. SrinivasanNow, enable handling of all IDE devices by extending the storvsc device id table to handle IDE guid. As part of this cleanup Kconfig and Hyper-V Makefile to not build the IDE driver (blkvsc).
--- drivers/staging/hv/Kconfig | 7 ------- drivers/staging/hv/Makefile | 2 -- drivers/staging/hv/storvsc_drv.c | 21 ++++++++------------- 3 files changed, 8 insertions(+), 22 deletions(-)
drivers/staging/hv/Kconfig +0 -7
@@ -15,13 +15,6 @@ config HYPERV_STORAGE help Select this option to enable the Hyper-V virtual storage driver. -config HYPERV_BLOCK - tristate "Microsoft Hyper-V virtual block driver" - depends on BLOCK && SCSI && (LBDAF || 64BIT) - default HYPERV - help - Select this option to enable the Hyper-V virtual block driver. - config HYPERV_NET tristate "Microsoft Hyper-V virtual network driver" depends on NET
drivers/staging/hv/Makefile +0 -2
@@ -1,6 +1,5 @@ obj-$(CONFIG_HYPERV) += hv_vmbus.o hv_timesource.o obj-$(CONFIG_HYPERV_STORAGE) += hv_storvsc.o -obj-$(CONFIG_HYPERV_BLOCK) += hv_blkvsc.o obj-$(CONFIG_HYPERV_NET) += hv_netvsc.o obj-$(CONFIG_HYPERV_UTILS) += hv_utils.o obj-$(CONFIG_HYPERV_MOUSE) += hv_mouse.o
@@ -9,6 +8,5 @@ hv_vmbus-y := vmbus_drv.o \ hv.o connection.o channel.o \ channel_mgmt.o ring_buffer.o hv_storvsc-y := storvsc_drv.o storvsc.o -hv_blkvsc-y := blkvsc_drv.o storvsc.o hv_netvsc-y := netvsc_drv.o netvsc.o rndis_filter.o hv_utils-y := hv_util.o hv_kvp.o
drivers/staging/hv/storvsc_drv.c +8 -13
@@ -645,27 +645,22 @@ static struct scsi_host_template scsi_driver = { .dma_boundary = PAGE_SIZE-1, }; +/* +* The storvsc_probe function assumes that the IDE guid +* is the second entry. +*/ static const struct hv_vmbus_device_id id_table[] = {/* SCSI guid */ {VMBUS_DEVICE (0xd9 ,0x63 ,0x61 ,0xba ,0xa1 ,0x04 ,0x29 ,0x4d ,0xb6 ,0x05 ,0x72 ,0xe2 ,0xff ,0xb1 ,0xdc ,0x7f ) }, +/* IDE guid */ + {VMBUS_DEVICE (0x32 ,0x26 ,0x41 ,0x32 ,0xcb ,0x86 ,0xa2 ,0x44 , +0x9b ,0x5c ,0x50 ,0xd1 ,0x41 ,0x73 ,0x54 ,0xf5 ) }, { }, };MODULE_DEVICE_TABLE (vmbus, id_table); -/* -* This declaration is temporary; once we get the -* infrastructure in place, we will integrate with -* id_table. -*/ - -static const uuid_le ide_blk_guid = { - .b = { -0x32 ,0x26 ,0x41 ,0x32 ,0xcb ,0x86 ,0xa2 ,0x44 , -0x9b ,0x5c ,0x50 ,0xd1 ,0x41 ,0x73 ,0x54 ,0xf5 - } -};/* * storvsc_probe - Add a new device for this driver
@@ -681,7 +676,7 @@ static int storvsc_probe(struct hv_device *device)int path =0 ;int target =0 ; -if (!uuid_le_cmp (device->dev_type, ide_blk_guid)) +if (!memcmp (&device->dev_type.b, id_table[1 ].guid,sizeof (uuid_le))) dev_is_ide = true;else dev_is_ide = false;
Signature · 1 line
1.7.4.1
PATCH 20/46 Staging: hv: blkvsc: Get rid of blkvsc_drv.c as this code is not used
In reply to K. Y. SrinivasanNow that blkvsc driver is no longer needed, remove blkvsc_drv.c
--- drivers/staging/hv/blkvsc_drv.c | 1014 --------------------------------------- 1 files changed, 0 insertions(+), 1014 deletions(-) delete mode 100644 drivers/staging/hv/blkvsc_drv.c
drivers/staging/hv/blkvsc_drv.c deleted +0 -1014
@@ -1,1014 +0,0 @@ -/* -* Copyright (c) 2009, Microsoft Corporation. -* -* This program is free software; you can redistribute it and/or modify it -* under the terms and conditions of the GNU General Public License, -* version 2, as published by the Free Software Foundation. -* -* This program is distributed in the hope it will be useful, but WITHOUT -* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or -* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for -* more details. -* -* You should have received a copy of the GNU General Public License along with -* this program; if not, write to the Free Software Foundation, Inc., 59 Temple -* Place - Suite 330, Boston, MA 02111-1307 USA. -* -* Authors: -* Haiyang Zhang <[email protected]> -* Hank Janssen <[email protected]> -* K. Y. Srinivasan <[email protected]> -*/ -#include <linux/init.h> -#include <linux/module.h> -#include <linux/device.h> -#include <linux/blkdev.h> -#include <linux/major.h> -#include <linux/delay.h> -#include <linux/hdreg.h> -#include <linux/slab.h> -#include <scsi/scsi.h> -#include <scsi/scsi_cmnd.h> -#include <scsi/scsi_eh.h> -#include <scsi/scsi_dbg.h> - -#include "hyperv.h" -#include "hyperv_storage.h" - - -#define BLKVSC_MINORS 64 - -enum blkvsc_device_type { -UNKNOWN_DEV_TYPE , -HARDDISK_TYPE , -DVD_TYPE , -}; - -enum blkvsc_op_type { -DO_INQUIRY , -DO_CAPACITY , -DO_FLUSH , -}; - -/* -* This request ties the struct request and struct -* blkvsc_request/hv_storvsc_request together A struct request may be -* represented by 1 or more struct blkvsc_request -*/ -struct blkvsc_request_group { -int outstanding; -int status; -struct list_head blkvsc_req_list;/* list of blkvsc_requests */ -}; - -struct blkvsc_request { -/* blkvsc_request_group.blkvsc_req_list */ -struct list_head req_entry; - -/* block_device_context.pending_list */ -struct list_head pend_entry; - -/* This may be null if we generate a request internally */ -struct request *req; - -struct block_device_context *dev; - -/* The group this request is part of. Maybe null */ -struct blkvsc_request_group *group; - -int write; -sector_t sector_start; -unsigned long sector_count; - -unsigned char sense_buffer[SCSI_SENSE_BUFFERSIZE ]; -unsigned char cmd_len; -unsigned char cmnd[MAX_COMMAND_SIZE ]; - -struct hv_storvsc_request request; -}; - -/* Per device structure */ -struct block_device_context { -/* point back to our device context */ -struct hv_device *device_ctx; -struct kmem_cache *request_pool; -spinlock_t lock; -struct gendisk *gd; -enum blkvsc_device_type device_type; -struct list_head pending_list; - -unsigned char device_id[64 ]; -unsigned int device_id_len; -int num_outstanding_reqs; -int shutting_down; -unsigned int sector_size; -sector_t capacity; -unsigned int port; -unsigned char path; -unsigned char target; -int users; -}; - - -/* -* There is a circular dependency involving blkvsc_request_completion() -* and blkvsc_do_request(). -*/ -static void blkvsc_request_completion (struct hv_storvsc_request *request); - -static int blkvsc_ringbuffer_size =BLKVSC_RING_BUFFER_SIZE ; - -module_param (blkvsc_ringbuffer_size, int,S_IRUGO ); -MODULE_PARM_DESC (ring_size,"Ring buffer size (in bytes)" ); - -/* -* There is a circular dependency involving blkvsc_probe() -* and block_ops. -*/ -static int blkvsc_probe (struct hv_device *dev); - -static int blkvsc_device_add (struct hv_device *device, -void *additional_info) -{ -struct storvsc_device_info *device_info; -int ret =0 ; - - device_info = (struct storvsc_device_info *)additional_info; - - device_info->ring_buffer_size = blkvsc_ringbuffer_size; - - ret =storvsc_dev_add (device, additional_info); -if (ret !=0 ) -return ret; - -/* -* We need to use the device instance guid to set the path and target -* id. For IDE devices, the device instance id is formatted as -* <bus id> * - <device id> - 8899 - 000000000000. -*/ - device_info->path_id = device->dev_instance.b[3 ] <<24 | - device->dev_instance.b[2 ] <<16 | - device->dev_instance.b[1 ] <<8 | - device->dev_instance.b[0 ]; - - device_info->target_id = device->dev_instance.b[5 ] <<8 | - device->dev_instance.b[4 ]; - -return ret; -} - -static int blkvsc_submit_request (struct blkvsc_request *blkvsc_req, -void (*request_completion)(struct hv_storvsc_request *)) -{ -struct block_device_context *blkdev = blkvsc_req->dev; -struct hv_storvsc_request *storvsc_req; -struct vmscsi_request *vm_srb; -int ret; - - - storvsc_req = &blkvsc_req->request; - vm_srb = &storvsc_req->vstor_packet.vm_srb; - - vm_srb->data_in = blkvsc_req->write ?WRITE_TYPE :READ_TYPE ; - - storvsc_req->on_io_completion = request_completion; - storvsc_req->context = blkvsc_req; - - vm_srb->port_number = blkdev->port; - vm_srb->path_id = blkdev->path; - vm_srb->target_id = blkdev->target; - vm_srb->lun =0 ;/* this is not really used at all */ - - vm_srb->cdb_length = blkvsc_req->cmd_len; - -memcpy (vm_srb->cdb, blkvsc_req->cmnd, vm_srb->cdb_length); - - storvsc_req->sense_buffer = blkvsc_req->sense_buffer; - - ret =storvsc_do_io (blkdev->device_ctx, - &blkvsc_req->request); -if (ret ==0 ) - blkdev->num_outstanding_reqs++; - -return ret; -} - - -static int blkvsc_open (struct block_device *bdev,fmode_t mode) -{ -struct block_device_context *blkdev = bdev->bd_disk->private_data; -unsigned long flags; - -spin_lock_irqsave (&blkdev->lock, flags); - - blkdev->users++; - -spin_unlock_irqrestore (&blkdev->lock, flags); - -return 0 ; -} - - -static int blkvsc_getgeo (struct block_device *bd,struct hd_geometry *hg) -{ -sector_t nsect =get_capacity (bd->bd_disk); -sector_t cylinders = nsect; - -/* -* We are making up these values; let us keep it simple. -*/ - hg->heads =0xff ; - hg->sectors =0x3f ; -sector_div (cylinders, hg->heads * hg->sectors); - hg->cylinders = cylinders; -if ((sector_t)(hg->cylinders +1 ) * hg->heads * hg->sectors < nsect) - hg->cylinders =0xffff ; -return 0 ; - -} - - -static void blkvsc_init_rw (struct blkvsc_request *blkvsc_req) -{ - - blkvsc_req->cmd_len =16 ; - -if (rq_data_dir (blkvsc_req->req)) { - blkvsc_req->write =1 ; - blkvsc_req->cmnd[0 ] =WRITE_16 ; - }else { - blkvsc_req->write =0 ; - blkvsc_req->cmnd[0 ] =READ_16 ; - } - - blkvsc_req->cmnd[1 ] |= - (blkvsc_req->req->cmd_flags &REQ_FUA ) ?0x8 :0 ; - - *(unsigned long long *)&blkvsc_req->cmnd[2 ] = -cpu_to_be64 (blkvsc_req->sector_start); - *(unsigned int *)&blkvsc_req->cmnd[10 ] = -cpu_to_be32 (blkvsc_req->sector_count); -} - - -static int blkvsc_ioctl (struct block_device *bd,fmode_t mode, -unsigned cmd,unsigned long arg) -{ -struct block_device_context *blkdev = bd->bd_disk->private_data; -int ret =0 ; - -switch (cmd) { -case HDIO_GET_IDENTITY : -if (copy_to_user ((void __user *)arg, blkdev->device_id, - blkdev->device_id_len)) - ret = -EFAULT ; -break ; -default : - ret = -EINVAL ; -break ; - } - -return ret; -} - -static void blkvsc_cmd_completion (struct hv_storvsc_request *request) -{ -struct blkvsc_request *blkvsc_req = - (struct blkvsc_request *)request->context; -struct block_device_context *blkdev = - (struct block_device_context *)blkvsc_req->dev; -struct scsi_sense_hdr sense_hdr; -struct vmscsi_request *vm_srb; -unsigned long flags; - - - vm_srb = &blkvsc_req->request.vstor_packet.vm_srb; - -spin_lock_irqsave (&blkdev->lock, flags); - blkdev->num_outstanding_reqs--; -spin_unlock_irqrestore (&blkdev->lock, flags); - -if (vm_srb->scsi_status) -if (scsi_normalize_sense (blkvsc_req->sense_buffer, -SCSI_SENSE_BUFFERSIZE , &sense_hdr)) -scsi_print_sense_hdr ("blkvsc" , &sense_hdr); - -complete (&blkvsc_req->request.wait_event); -} - - -static int blkvsc_do_operation (struct block_device_context *blkdev, -enum blkvsc_op_type op) -{ -struct blkvsc_request *blkvsc_req; -struct page *page_buf; -unsigned char *buf; -unsigned char device_type; -struct scsi_sense_hdr sense_hdr; -struct vmscsi_request *vm_srb; -unsigned long flags; - -int ret =0 ; - - blkvsc_req =kmem_cache_zalloc (blkdev->request_pool,GFP_KERNEL ); -if (!blkvsc_req) -return -ENOMEM ; - - page_buf =alloc_page (GFP_KERNEL ); -if (!page_buf) { -kmem_cache_free (blkdev->request_pool, blkvsc_req); -return -ENOMEM ; - } - - vm_srb = &blkvsc_req->request.vstor_packet.vm_srb; -init_completion (&blkvsc_req->request.wait_event); - blkvsc_req->dev = blkdev; - blkvsc_req->req =NULL ; - blkvsc_req->write =0 ; - - blkvsc_req->request.data_buffer.pfn_array[0 ] = -page_to_pfn (page_buf); - blkvsc_req->request.data_buffer.offset =0 ; - -switch (op) { -case DO_INQUIRY : - blkvsc_req->cmnd[0 ] =INQUIRY ; - blkvsc_req->cmnd[1 ] =0x1 ;/* Get product data */ - blkvsc_req->cmnd[2 ] =0x83 ;/* mode page 83 */ - blkvsc_req->cmnd[4 ] =64 ; - blkvsc_req->cmd_len =6 ; - blkvsc_req->request.data_buffer.len =64 ; -break ; - -case DO_CAPACITY : - blkdev->sector_size =0 ; - blkdev->capacity =0 ; - - blkvsc_req->cmnd[0 ] =READ_CAPACITY ; - blkvsc_req->cmd_len =16 ; - blkvsc_req->request.data_buffer.len =8 ; -break ; - -case DO_FLUSH : - blkvsc_req->cmnd[0 ] =SYNCHRONIZE_CACHE ; - blkvsc_req->cmd_len =10 ; - blkvsc_req->request.data_buffer.pfn_array[0 ] =0 ; - blkvsc_req->request.data_buffer.len =0 ; -break ; -default : - ret = -EINVAL ; - goto cleanup; - } - -spin_lock_irqsave (&blkdev->lock, flags); -blkvsc_submit_request (blkvsc_req, blkvsc_cmd_completion); -spin_unlock_irqrestore (&blkdev->lock, flags); - -wait_for_completion_interruptible (&blkvsc_req->request.wait_event); - -/* check error */ -if (vm_srb->scsi_status) { -scsi_normalize_sense (blkvsc_req->sense_buffer, -SCSI_SENSE_BUFFERSIZE , &sense_hdr); - -return 0 ; - } - - buf =kmap (page_buf); - -switch (op) { -case DO_INQUIRY : - device_type = buf[0 ] &0x1F ; - -if (device_type ==0x0 ) - blkdev->device_type =HARDDISK_TYPE ; -else - blkdev->device_type =UNKNOWN_DEV_TYPE ; - - blkdev->device_id_len = buf[7 ]; -if (blkdev->device_id_len >64 ) - blkdev->device_id_len =64 ; - -memcpy (blkdev->device_id, &buf[8 ], blkdev->device_id_len); -break ; - -case DO_CAPACITY : -/* be to le */ - blkdev->capacity = - ((buf[0 ] <<24 ) | (buf[1 ] <<16 ) | - (buf[2 ] <<8 ) | buf[3 ]) +1 ; - - blkdev->sector_size = - (buf[4 ] <<24 ) | (buf[5 ] <<16 ) | - (buf[6 ] <<8 ) | buf[7 ]; -break ; -default : -break ; - - } - -cleanup: - -kunmap (page_buf); - -__free_page (page_buf); - -kmem_cache_free (blkdev->request_pool, blkvsc_req); - -return ret; -} - - -static int blkvsc_cancel_pending_reqs (struct block_device_context *blkdev) -{ -struct blkvsc_request *pend_req, *tmp; -struct blkvsc_request *comp_req, *tmp2; -struct vmscsi_request *vm_srb; - -int ret =0 ; - - -/* Flush the pending list first */ -list_for_each_entry_safe (pend_req, tmp, &blkdev->pending_list, - pend_entry) { -/* -* The pend_req could be part of a partially completed -* request. If so, complete those req first until we -* hit the pend_req -*/ -list_for_each_entry_safe (comp_req, tmp2, - &pend_req->group->blkvsc_req_list, - req_entry) { - -if (comp_req == pend_req) -break ; - -list_del (&comp_req->req_entry); - -if (comp_req->req) { - vm_srb = - &comp_req->request.vstor_packet. - vm_srb; - ret =__blk_end_request (comp_req->req, - (!vm_srb->scsi_status ?0 : -EIO ), - comp_req->sector_count * - blkdev->sector_size); - -/* FIXME: shouldn't this do more than return? */ -if (ret) - goto out; - } - -kmem_cache_free (blkdev->request_pool, comp_req); - } - -list_del (&pend_req->pend_entry); - -list_del (&pend_req->req_entry); - -if (comp_req->req) { -if (!__blk_end_request (pend_req->req, -EIO , - pend_req->sector_count * - blkdev->sector_size)) { -/* -* All the sectors have been xferred ie the -* request is done -*/ -kmem_cache_free (blkdev->request_pool, - pend_req->group); - } - } - -kmem_cache_free (blkdev->request_pool, pend_req); - } - -out: -return ret; -} - - -/* -* blkvsc_remove() - Callback when our device is removed -*/ -static int blkvsc_remove (struct hv_device *dev) -{ -struct block_device_context *blkdev =dev_get_drvdata (&dev->device); -unsigned long flags; - - -/* Get to a known state */ -spin_lock_irqsave (&blkdev->lock, flags); - - blkdev->shutting_down =1 ; - -blk_stop_queue (blkdev->gd->queue); - -blkvsc_cancel_pending_reqs (blkdev); - -spin_unlock_irqrestore (&blkdev->lock, flags); - -blkvsc_do_operation (blkdev,DO_FLUSH ); - -if (blkdev->users ==0 ) { -del_gendisk (blkdev->gd); -put_disk (blkdev->gd); -blk_cleanup_queue (blkdev->gd->queue); - -storvsc_dev_remove (blkdev->device_ctx); - -kmem_cache_destroy (blkdev->request_pool); -kfree (blkdev); - } - -return 0 ; -} - -static void blkvsc_shutdown (struct hv_device *dev) -{ -struct block_device_context *blkdev =dev_get_drvdata (&dev->device); -unsigned long flags; - -if (!blkdev) -return ; - -spin_lock_irqsave (&blkdev->lock, flags); - - blkdev->shutting_down =1 ; - -blk_stop_queue (blkdev->gd->queue); - -blkvsc_cancel_pending_reqs (blkdev); - -spin_unlock_irqrestore (&blkdev->lock, flags); - -blkvsc_do_operation (blkdev,DO_FLUSH ); - -/* -* Now wait for all outgoing I/O to be drained. -*/ -storvsc_wait_to_drain ((struct storvsc_device *)dev->ext); - -} - -static int blkvsc_release (struct gendisk *disk,fmode_t mode) -{ -struct block_device_context *blkdev = disk->private_data; -unsigned long flags; - -spin_lock_irqsave (&blkdev->lock, flags); - -if ((--blkdev->users ==0 ) && (blkdev->shutting_down)) { -blk_stop_queue (blkdev->gd->queue); -spin_unlock_irqrestore (&blkdev->lock, flags); - -blkvsc_do_operation (blkdev,DO_FLUSH ); -del_gendisk (blkdev->gd); -put_disk (blkdev->gd); -blk_cleanup_queue (blkdev->gd->queue); - -storvsc_dev_remove (blkdev->device_ctx); - -kmem_cache_destroy (blkdev->request_pool); -kfree (blkdev); - }else -spin_unlock_irqrestore (&blkdev->lock, flags); - -return 0 ; -} - - -/* -* We break the request into 1 or more blkvsc_requests and submit -* them. If we cant submit them all, we put them on the -* pending_list. The blkvsc_request() will work on the pending_list. -*/ -static int blkvsc_do_request (struct block_device_context *blkdev, -struct request *req) -{ -struct bio *bio =NULL ; -struct bio_vec *bvec =NULL ; -struct bio_vec *prev_bvec =NULL ; -struct blkvsc_request *blkvsc_req =NULL ; -struct blkvsc_request *tmp; -int databuf_idx =0 ; -int seg_idx =0 ; -sector_t start_sector; -unsigned long num_sectors =0 ; -int ret =0 ; -int pending =0 ; -struct blkvsc_request_group *group =NULL ; - -/* Create a group to tie req to list of blkvsc_reqs */ - group =kmem_cache_zalloc (blkdev->request_pool,GFP_ATOMIC ); -if (!group) -return -ENOMEM ; - -INIT_LIST_HEAD (&group->blkvsc_req_list); - group->outstanding = group->status =0 ; - - start_sector =blk_rq_pos (req); - -/* foreach bio in the request */ -if (req->bio) { -for (bio = req->bio; bio; bio = bio->bi_next) { -/* -* Map this bio into an existing or new storvsc request -*/ -bio_for_each_segment (bvec, bio, seg_idx) { -/* Get a new storvsc request */ -/* 1st-time */ -if ((!blkvsc_req) || - (databuf_idx >=MAX_MULTIPAGE_BUFFER_COUNT ) -/* hole at the begin of page */ - || (bvec->bv_offset !=0 ) || -/* hold at the end of page */ - (prev_bvec && - (prev_bvec->bv_len !=PAGE_SIZE ))) { -/* submit the prev one */ -if (blkvsc_req) { - blkvsc_req->sector_start = - start_sector; -sector_div ( - blkvsc_req->sector_start, - (blkdev->sector_size >>9 )); - - blkvsc_req->sector_count = - num_sectors / - (blkdev->sector_size >>9 ); -blkvsc_init_rw (blkvsc_req); - } - -/* -* Create new blkvsc_req to represent -* the current bvec -*/ - blkvsc_req = -kmem_cache_zalloc ( - blkdev->request_pool,GFP_ATOMIC ); -if (!blkvsc_req) { -/* free up everything */ -list_for_each_entry_safe ( - blkvsc_req, tmp, - &group->blkvsc_req_list, - req_entry) { -list_del ( - &blkvsc_req->req_entry); -kmem_cache_free ( - blkdev->request_pool, - blkvsc_req); - } - -kmem_cache_free ( - blkdev->request_pool, group); -return -ENOMEM ; - } - -memset (blkvsc_req,0 , -sizeof (struct blkvsc_request )); - - blkvsc_req->dev = blkdev; - blkvsc_req->req = req; - blkvsc_req->request. - data_buffer.offset - = bvec->bv_offset; - blkvsc_req->request. - data_buffer.len =0 ; - -/* Add to the group */ - blkvsc_req->group = group; - blkvsc_req->group->outstanding++; -list_add_tail (&blkvsc_req->req_entry, - &blkvsc_req->group->blkvsc_req_list); - - start_sector += num_sectors; - num_sectors =0 ; - databuf_idx =0 ; - } - -/* -* Add the curr bvec/segment to the curr -* blkvsc_req -*/ - blkvsc_req->request.data_buffer. - pfn_array[databuf_idx] - =page_to_pfn (bvec->bv_page); - blkvsc_req->request.data_buffer.len - += bvec->bv_len; - - prev_bvec = bvec; - - databuf_idx++; - num_sectors += bvec->bv_len >>9 ; - - }/* bio_for_each_segment */ - - }/* rq_for_each_bio */ - } - -/* Handle the last one */ -if (blkvsc_req) { - blkvsc_req->sector_start = start_sector; -sector_div (blkvsc_req->sector_start, - (blkdev->sector_size >>9 )); - - blkvsc_req->sector_count = num_sectors / - (blkdev->sector_size >>9 ); - -blkvsc_init_rw (blkvsc_req); - } - -list_for_each_entry (blkvsc_req, &group->blkvsc_req_list, req_entry) { -if (pending) { - -list_add_tail (&blkvsc_req->pend_entry, - &blkdev->pending_list); - }else { - ret =blkvsc_submit_request (blkvsc_req, - blkvsc_request_completion); -if (ret == -EAGAIN ) { - pending =1 ; -list_add_tail (&blkvsc_req->pend_entry, - &blkdev->pending_list); - } - - } - } - -return pending; -} - -static int blkvsc_do_pending_reqs (struct block_device_context *blkdev) -{ -struct blkvsc_request *pend_req, *tmp; -int ret =0 ; - -/* Flush the pending list first */ -list_for_each_entry_safe (pend_req, tmp, &blkdev->pending_list, - pend_entry) { - - ret =blkvsc_submit_request (pend_req, - blkvsc_request_completion); -if (ret !=0 ) -break ; -else -list_del (&pend_req->pend_entry); - } - -return ret; -} - - -static void blkvsc_request (struct request_queue *queue) -{ -struct block_device_context *blkdev =NULL ; -struct request *req; -int ret =0 ; - -while ((req =blk_peek_request (queue)) !=NULL ) { - - blkdev = req->rq_disk->private_data; -if (blkdev->shutting_down || req->cmd_type !=REQ_TYPE_FS ) { -__blk_end_request_cur (req,0 ); -continue ; - } - - ret =blkvsc_do_pending_reqs (blkdev); - -if (ret !=0 ) { -blk_stop_queue (queue); -break ; - } - -blk_start_request (req); - - ret =blkvsc_do_request (blkdev, req); -if (ret >0 ) { -blk_stop_queue (queue); -break ; - }else if (ret <0 ) { -blk_requeue_request (queue, req); -blk_stop_queue (queue); -break ; - } - } -} - -static const struct hv_vmbus_device_id id_table[] = { -/* IDE guid */ - {VMBUS_DEVICE (0x32 ,0x26 ,0x41 ,0x32 ,0xcb ,0x86 ,0xa2 ,0x44 , -0x9b ,0x5c ,0x50 ,0xd1 ,0x41 ,0x73 ,0x54 ,0xf5 ) }, - { }, -}; - -MODULE_DEVICE_TABLE (vmbus, id_table); - -/* The one and only one */ -static struct hv_driver blkvsc_drv = { - .name ="blkvsc" , - .id_table = id_table, - .probe = blkvsc_probe, - .remove = blkvsc_remove, - .shutdown = blkvsc_shutdown, -}; - -static const struct block_device_operations block_ops = { - .owner =THIS_MODULE , - .open = blkvsc_open, - .release = blkvsc_release, - .getgeo = blkvsc_getgeo, - .ioctl = blkvsc_ioctl, -}; - -/* -* blkvsc_drv_init - BlkVsc driver initialization. -*/ -static int blkvsc_drv_init (void ) -{ -BUILD_BUG_ON (sizeof (sector_t) !=8 ); -return vmbus_driver_register (&blkvsc_drv); -} - -static void blkvsc_drv_exit (void ) -{ -vmbus_driver_unregister (&blkvsc_drv); -} - -/* -* blkvsc_probe - Add a new device for this driver -*/ -static int blkvsc_probe (struct hv_device *dev) -{ -struct block_device_context *blkdev =NULL ; -struct storvsc_device_info device_info; -struct storvsc_major_info major_info; -int ret =0 ; - - blkdev =kzalloc (sizeof (struct block_device_context ),GFP_KERNEL ); -if (!blkdev) { - ret = -ENOMEM ; - goto cleanup; - } - -INIT_LIST_HEAD (&blkdev->pending_list); - -/* Initialize what we can here */ -spin_lock_init (&blkdev->lock); - - - blkdev->request_pool =kmem_cache_create (dev_name (&dev->device), -sizeof (struct blkvsc_request ),0 , -SLAB_HWCACHE_ALIGN ,NULL ); -if (!blkdev->request_pool) { - ret = -ENOMEM ; - goto cleanup; - } - - - ret =blkvsc_device_add (dev, &device_info); -if (ret !=0 ) - goto cleanup; - - blkdev->device_ctx = dev; -/* this identified the device 0 or 1 */ - blkdev->target = device_info.target_id; -/* this identified the ide ctrl 0 or 1 */ - blkdev->path = device_info.path_id; - -dev_set_drvdata (&dev->device, blkdev); - - ret =storvsc_get_major_info (&device_info, &major_info); - -if (ret) - goto cleanup; - -if (major_info.do_register) { - ret =register_blkdev (major_info.major, major_info.devname); - -if (ret !=0 ) { -DPRINT_ERR (BLKVSC_DRV , -"register_blkdev() failed! ret %d" , ret); - goto remove; - } - } - -DPRINT_INFO (BLKVSC_DRV ,"blkvsc registered for major %d!!" , - major_info.major); - - blkdev->gd =alloc_disk (BLKVSC_MINORS ); -if (!blkdev->gd) { - ret =-1 ; - goto cleanup; - } - - blkdev->gd->queue =blk_init_queue (blkvsc_request, &blkdev->lock); - -blk_queue_max_segment_size (blkdev->gd->queue,PAGE_SIZE ); -blk_queue_max_segments (blkdev->gd->queue,MAX_MULTIPAGE_BUFFER_COUNT ); -blk_queue_segment_boundary (blkdev->gd->queue,PAGE_SIZE -1 ); -blk_queue_bounce_limit (blkdev->gd->queue,BLK_BOUNCE_ANY ); -blk_queue_dma_alignment (blkdev->gd->queue,511 ); - - blkdev->gd->major = major_info.major; -if (major_info.index ==1 || major_info.index ==3 ) - blkdev->gd->first_minor =BLKVSC_MINORS ; -else - blkdev->gd->first_minor =0 ; - blkdev->gd->fops = &block_ops; - blkdev->gd->private_data = blkdev; - blkdev->gd->driverfs_dev = &(blkdev->device_ctx->device); -sprintf (blkdev->gd->disk_name,"hd%c" ,'a' + major_info.index); - -blkvsc_do_operation (blkdev,DO_INQUIRY ); -blkvsc_do_operation (blkdev,DO_CAPACITY ); - -set_capacity (blkdev->gd, blkdev->capacity * (blkdev->sector_size/512 )); -blk_queue_logical_block_size (blkdev->gd->queue, blkdev->sector_size); -/* go! */ -add_disk (blkdev->gd); - -DPRINT_INFO (BLKVSC_DRV ,"%s added!! capacity %lu sector_size %d" , - blkdev->gd->disk_name, (unsigned long )blkdev->capacity, - blkdev->sector_size); - -return ret; - -remove: -storvsc_dev_remove (dev); - -cleanup: -if (blkdev) { -if (blkdev->request_pool) { -kmem_cache_destroy (blkdev->request_pool); - blkdev->request_pool =NULL ; - } -kfree (blkdev); - blkdev =NULL ; - } - -return ret; -} - -static void blkvsc_request_completion (struct hv_storvsc_request *request) -{ -struct blkvsc_request *blkvsc_req = - (struct blkvsc_request *)request->context; -struct block_device_context *blkdev = - (struct block_device_context *)blkvsc_req->dev; -unsigned long flags; -struct blkvsc_request *comp_req, *tmp; -struct vmscsi_request *vm_srb; - - -spin_lock_irqsave (&blkdev->lock, flags); - - blkdev->num_outstanding_reqs--; - blkvsc_req->group->outstanding--; - -/* -* Only start processing when all the blkvsc_reqs are -* completed. This guarantees no out-of-order blkvsc_req -* completion when calling end_that_request_first() -*/ -if (blkvsc_req->group->outstanding ==0 ) { -list_for_each_entry_safe (comp_req, tmp, - &blkvsc_req->group->blkvsc_req_list, - req_entry) { - -list_del (&comp_req->req_entry); - - vm_srb = - &comp_req->request.vstor_packet.vm_srb; -if (!__blk_end_request (comp_req->req, - (!vm_srb->scsi_status ?0 : -EIO ), - comp_req->sector_count * blkdev->sector_size)) { -/* -* All the sectors have been xferred ie the -* request is done -*/ -kmem_cache_free (blkdev->request_pool, - comp_req->group); - } - -kmem_cache_free (blkdev->request_pool, comp_req); - } - -if (!blkdev->shutting_down) { -blkvsc_do_pending_reqs (blkdev); -blk_start_queue (blkdev->gd->queue); -blkvsc_request (blkdev->gd->queue); - } - } - -spin_unlock_irqrestore (&blkdev->lock, flags); -} - -static void __exit blkvsc_exit(void) -{ -blkvsc_drv_exit (); -} - -MODULE_LICENSE ("GPL" ); -MODULE_VERSION (HV_DRV_VERSION ); -MODULE_DESCRIPTION ("Microsoft Hyper-V virtual block driver" ); -module_init (blkvsc_drv_init); -module_exit (blkvsc_exit);
Signature · 1 line
1.7.4.1
PATCH 21/46 Staging: hv: storvsc: Optimize bounce buffer handling for the "write" case
In reply to K. Y. SrinivasanOptimize bounce buffer handling for the "write" case.
--- drivers/staging/hv/storvsc_drv.c | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-)
drivers/staging/hv/storvsc_drv.c +6 -6
@@ -425,17 +425,17 @@ static void storvsc_commmand_completion(struct hv_storvsc_request *request)struct scsi_sense_hdr sense_hdr;struct vmscsi_request *vm_srb; + vm_srb = &request->vstor_packet.vm_srb;if (cmd_request->bounce_sgl_count) { - -/* FIXME: We can optimize on writes by just skipping this */ -copy_from_bounce_buffer (scsi_sglist (scmnd), +if (vm_srb->data_in ==READ_TYPE ) { +copy_from_bounce_buffer (scsi_sglist (scmnd), cmd_request->bounce_sgl,scsi_sg_count (scmnd)); -destroy_bounce_buffer (cmd_request->bounce_sgl, - cmd_request->bounce_sgl_count); +destroy_bounce_buffer (cmd_request->bounce_sgl, + cmd_request->bounce_sgl_count); + } } - vm_srb = &request->vstor_packet.vm_srb; scmnd->result = vm_srb->scsi_status;if (scmnd->result) {
Signature · 1 line
1.7.4.1
PATCH 22/46 Staging: hv: storvsc: Optimize the bounce buffer handling in the "read" case
In reply to K. Y. SrinivasanOptimize the bounce buffer handling in the "read" case.
--- drivers/staging/hv/storvsc_drv.c | 10 ++++------ 1 files changed, 4 insertions(+), 6 deletions(-)
drivers/staging/hv/storvsc_drv.c +4 -6
@@ -560,12 +560,10 @@ static int storvsc_queuecommand_lck(struct scsi_cmnd *scmnd,ALIGN (scsi_bufflen (scmnd),PAGE_SIZE ) >>PAGE_SHIFT ; -/* -* FIXME: We can optimize on reads by just skipping -* this -*/ -copy_to_bounce_buffer (sgl, cmd_request->bounce_sgl, -scsi_sg_count (scmnd)); +if (vm_srb->data_in ==WRITE_TYPE ) +copy_to_bounce_buffer (sgl, + cmd_request->bounce_sgl, +scsi_sg_count (scmnd)); sgl = cmd_request->bounce_sgl; sg_count = cmd_request->bounce_sgl_count;
Signature · 1 line
1.7.4.1
PATCH 23/46 Staging: hv: storvsc: Include storvsc.c in storvsc_drv.c
In reply to K. Y. SrinivasanAs part of further cleanup of our storage drivers, include the content of storvsc.c into storvsc_drv.c and delete storvsc.c and do the necessary adjustments.
--- drivers/staging/hv/Makefile | 2 +- drivers/staging/hv/storvsc.c | 528 ------------------------------------- drivers/staging/hv/storvsc_drv.c | 529 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 530 insertions(+), 529 deletions(-) delete mode 100644 drivers/staging/hv/storvsc.c
drivers/staging/hv/Makefile +1 -1
@@ -7,6 +7,6 @@ obj-$(CONFIG_HYPERV_MOUSE) += hv_mouse.o hv_vmbus-y := vmbus_drv.o \ hv.o connection.o channel.o \ channel_mgmt.o ring_buffer.o -hv_storvsc-y := storvsc_drv.o storvsc.o +hv_storvsc-y := storvsc_drv.o hv_netvsc-y := netvsc_drv.o netvsc.o rndis_filter.o hv_utils-y := hv_util.o hv_kvp.o
drivers/staging/hv/storvsc.c deleted +0 -528
528 lines not on this page · raw message
drivers/staging/hv/storvsc_drv.c +529 -0
535 lines not on this page · raw message
Signature · 1 line
1.7.4.1
PATCH 24/46 Staging: hv: storvsc: Cleanup storvsc_drv.c after adding the contents of storvsc.c
In reply to K. Y. SrinivasanCleanup storvsc_drv.c after adding the contents of storvsc.c.
--- drivers/staging/hv/storvsc_drv.c | 147 +++++++------------------------------- 1 files changed, 27 insertions(+), 120 deletions(-)
drivers/staging/hv/storvsc_drv.c +27 -120
@@ -19,11 +19,17 @@* Hank Janssen <[email protected]> * K. Y. Srinivasan <[email protected]> */ + +#include <linux/kernel.h> +#include <linux/sched.h> +#include <linux/completion.h> +#include <linux/string.h> +#include <linux/mm.h> +#include <linux/delay.h> #include <linux/init.h> #include <linux/slab.h> #include <linux/module.h> #include <linux/device.h> -#include <linux/blkdev.h> #include <scsi/scsi.h> #include <scsi/scsi_cmnd.h> #include <scsi/scsi_host.h>
@@ -37,39 +43,28 @@#include "hyperv_storage.h" -/* -* Copyright (c) 2009, Microsoft Corporation. -* -* This program is free software; you can redistribute it and/or modify it -* under the terms and conditions of the GNU General Public License, -* version 2, as published by the Free Software Foundation. -* -* This program is distributed in the hope it will be useful, but WITHOUT -* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or -* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for -* more details. -* -* You should have received a copy of the GNU General Public License along with -* this program; if not, write to the Free Software Foundation, Inc., 59 Temple -* Place - Suite 330, Boston, MA 02111-1307 USA. -* -* Authors: -* Haiyang Zhang <[email protected]> -* Hank Janssen <[email protected]> -* K. Y. Srinivasan <[email protected]> -* -*/ -#include <linux/kernel.h> -#include <linux/sched.h> -#include <linux/completion.h> -#include <linux/string.h> -#include <linux/slab.h> -#include <linux/mm.h> -#include <linux/delay.h> +static int storvsc_ringbuffer_size =STORVSC_RING_BUFFER_SIZE ; -#include "hyperv.h" -#include "hyperv_storage.h" +module_param (storvsc_ringbuffer_size, int,S_IRUGO ); +MODULE_PARM_DESC (storvsc_ringbuffer_size,"Ring buffer size (bytes)" ); + +struct hv_host_device { +struct hv_device *dev; +struct kmem_cache *request_pool; +unsigned int port; +unsigned char path; +unsigned char target; +}; + +struct storvsc_cmd_request { +struct list_head entry; +struct scsi_cmnd *cmd; +unsigned int bounce_sgl_count; +struct scatterlist *bounce_sgl; + +struct hv_storvsc_request request; +};static inline struct storvsc_device *alloc_stor_device (struct hv_device *device) {
@@ -500,94 +495,6 @@ int storvsc_do_io(struct hv_device *device,return ret; } -/* -* The channel properties uniquely specify how the device is to be -* presented to the guest. Map this information for use by the block -* driver. For Linux guests on Hyper-V, we emulate a scsi HBA in the guest -* (storvsc_drv) and so scsi devices in the guest are handled by -* native upper level Linux drivers. Consequently, Hyper-V -* block driver, while being a generic block driver, presently does not -* deal with anything other than devices that would need to be presented -* to the guest as an IDE disk. -* -* This function maps the channel properties as embedded in the input -* parameter device_info onto information necessary to register the -* corresponding block device. -* -* Currently, there is no way to stop the emulation of the block device -* on the host side. And so, to prevent the native IDE drivers in Linux -* from taking over these devices (to be managedby Hyper-V block -* driver), we will take over if need be the major of the IDE controllers. -* -*/ - -int storvsc_get_major_info (struct storvsc_device_info *device_info, -struct storvsc_major_info *major_info) -{ -static bool ide0_registered; -static bool ide1_registered; - -/* -* For now we only support IDE disks. -*/ - major_info->devname ="ide" ; - major_info->diskname ="hd" ; - -if (device_info->path_id) { - major_info->major =22 ; -if (!ide1_registered) { - major_info->do_register = true; - ide1_registered = true; - }else - major_info->do_register = false; - -if (device_info->target_id) - major_info->index =3 ; -else - major_info->index =2 ; - -return 0 ; - }else { - major_info->major =3 ; -if (!ide0_registered) { - major_info->do_register = true; - ide0_registered = true; - }else - major_info->do_register = false; - -if (device_info->target_id) - major_info->index =1 ; -else - major_info->index =0 ; - -return 0 ; - } - -return -ENODEV ; -} -static int storvsc_ringbuffer_size =STORVSC_RING_BUFFER_SIZE ; - -module_param (storvsc_ringbuffer_size, int,S_IRUGO ); -MODULE_PARM_DESC (storvsc_ringbuffer_size,"Ring buffer size (bytes)" ); - -struct hv_host_device { -struct hv_device *dev; -struct kmem_cache *request_pool; -unsigned int port; -unsigned char path; -unsigned char target; -}; - -struct storvsc_cmd_request { -struct list_head entry; -struct scsi_cmnd *cmd; - -unsigned int bounce_sgl_count; -struct scatterlist *bounce_sgl; - -struct hv_storvsc_request request; -}; -static void storvsc_get_ide_info (struct hv_device *dev,int *target,int *path) { *target =
Signature · 1 line
1.7.4.1
PATCH 25/46 Staging: hv: storvsc: Add the contents of hyperv_storage.h to storvsc_drv.c
In reply to K. Y. SrinivasanAs part of further cleanup of storvsc, add the contents of hyperv_storage.h to storvsc_drv.c and do the necessary adjustments.
--- drivers/staging/hv/hyperv_storage.h | 322 ----------------------------------- drivers/staging/hv/storvsc_drv.c | 277 ++++++++++++++++++++++++++++++- 2 files changed, 276 insertions(+), 323 deletions(-) delete mode 100644 drivers/staging/hv/hyperv_storage.h
drivers/staging/hv/hyperv_storage.h deleted +0 -322
@@ -1,322 +0,0 @@ -/* -* -* Copyright (c) 2011, Microsoft Corporation. -* -* This program is free software; you can redistribute it and/or modify it -* under the terms and conditions of the GNU General Public License, -* version 2, as published by the Free Software Foundation. -* -* This program is distributed in the hope it will be useful, but WITHOUT -* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or -* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for -* more details. -* -* You should have received a copy of the GNU General Public License along with -* this program; if not, write to the Free Software Foundation, Inc., 59 Temple -* Place - Suite 330, Boston, MA 02111-1307 USA. -* -* Authors: -* Haiyang Zhang <[email protected]> -* Hank Janssen <[email protected]> -* K. Y. Srinivasan <[email protected]> -* -*/ - -#ifndef _HYPERV_STORAGE_H -#define _HYPERV_STORAGE_H - - -/* vstorage.w revision number. This is used in the case of a version match, */ -/* to alert the user that structure sizes may be mismatched even though the */ -/* protocol versions match. */ - - -#define REVISION_STRING (REVISION_ ) #REVISION_ -#define FILL_VMSTOR_REVISION (RESULT_LVALUE_ ) \ - do { \ - char *revision_string \ - = REVISION_STRING($Rev : 6 $) + 6; \ - RESULT_LVALUE_ = 0; \ - while (*revision_string >= '0' \ - && *revision_string <= '9') { \ - RESULT_LVALUE_ *= 10; \ - RESULT_LVALUE_ += *revision_string - '0'; \ - revision_string++; \ - } \ - } while (0) - -/* Major/minor macros. Minor version is in LSB, meaning that earlier flat */ -/* version numbers will be interpreted as "0.x" (i.e., 1 becomes 0.1). */ -#define VMSTOR_PROTOCOL_MAJOR (VERSION_ ) (((VERSION_) >> 8) & 0xff) -#define VMSTOR_PROTOCOL_MINOR (VERSION_ ) (((VERSION_)) & 0xff) -#define VMSTOR_PROTOCOL_VERSION (MAJOR_ ,MINOR_ ) ((((MAJOR_) & 0xff) << 8) | \ - (((MINOR_) & 0xff))) -#define VMSTOR_INVALID_PROTOCOL_VERSION (-1) - -/* Version history: */ -/* V1 Beta 0.1 */ -/* V1 RC < 2008/1/31 1.0 */ -/* V1 RC > 2008/1/31 2.0 */ -#define VMSTOR_PROTOCOL_VERSION_CURRENT VMSTOR_PROTOCOL_VERSION(2, 0) - - - - -/* This will get replaced with the max transfer length that is possible on */ -/* the host adapter. */ -/* The max transfer length will be published when we offer a vmbus channel. */ -#define MAX_TRANSFER_LENGTH 0x40000 -#define DEFAULT_PACKET_SIZE (sizeof(struct vmdata_gpa_direct) + \ - sizeof(struct vstor_packet) + \ - sizesizeof(u64) * (MAX_TRANSFER_LENGTH / PAGE_SIZE))) - - -/* Packet structure describing virtual storage requests. */ -enum vstor_packet_operation { -VSTOR_OPERATION_COMPLETE_IO =1 , -VSTOR_OPERATION_REMOVE_DEVICE =2 , -VSTOR_OPERATION_EXECUTE_SRB =3 , -VSTOR_OPERATION_RESET_LUN =4 , -VSTOR_OPERATION_RESET_ADAPTER =5 , -VSTOR_OPERATION_RESET_BUS =6 , -VSTOR_OPERATION_BEGIN_INITIALIZATION =7 , -VSTOR_OPERATION_END_INITIALIZATION =8 , -VSTOR_OPERATION_QUERY_PROTOCOL_VERSION =9 , -VSTOR_OPERATION_QUERY_PROPERTIES =10 , -VSTOR_OPERATION_MAXIMUM =10 -}; - -/* -* Platform neutral description of a scsi request - -* this remains the same across the write regardless of 32/64 bit -* note: it's patterned off the SCSI_PASS_THROUGH structure -*/ -#define CDB16GENERIC_LENGTH 0x10 - -#ifndef SENSE_BUFFER_SIZE -#define SENSE_BUFFER_SIZE 0x12 -#endif - -#define MAX_DATA_BUF_LEN_WITH_PADDING 0x14 - -struct vmscsi_request { -unsigned short length; -unsigned char srb_status; -unsigned char scsi_status; - -unsigned char port_number; -unsigned char path_id; -unsigned char target_id; -unsigned char lun; - -unsigned char cdb_length; -unsigned char sense_info_length; -unsigned char data_in; -unsigned char reserved; - -unsigned int data_transfer_length; - -union { -unsigned char cdb[CDB16GENERIC_LENGTH ]; -unsigned char sense_data[SENSE_BUFFER_SIZE ]; -unsigned char reserved_array[MAX_DATA_BUF_LEN_WITH_PADDING ]; - }; -} __attribute((packed)); - - -/* -* This structure is sent during the intialization phase to get the different -* properties of the channel. -*/ -struct vmstorage_channel_properties { -unsigned short protocol_version; -unsigned char path_id; -unsigned char target_id; - -/* Note: port number is only really known on the client side */ -unsigned int port_number; -unsigned int flags; -unsigned int max_transfer_bytes; - -/* This id is unique for each channel and will correspond with */ -/* vendor specific data in the inquirydata */ -unsigned long long unique_id; -} __packed; - -/* This structure is sent during the storage protocol negotiations. */ -struct vmstorage_protocol_version { -/* Major (MSW) and minor (LSW) version numbers. */ -unsigned short major_minor; - -/* -* Revision number is auto-incremented whenever this file is changed -* (See FILL_VMSTOR_REVISION macro above). Mismatch does not -* definitely indicate incompatibility--but it does indicate mismatched -* builds. -*/ -unsigned short revision; -} __packed; - -/* Channel Property Flags */ -#define STORAGE_CHANNEL_REMOVABLE_FLAG 0x1 -#define STORAGE_CHANNEL_EMULATED_IDE_FLAG 0x2 - -struct vstor_packet { -/* Requested operation type */ -enum vstor_packet_operation operation; - -/* Flags - see below for values */ -unsigned int flags; - -/* Status of the request returned from the server side. */ -unsigned int status; - -/* Data payload area */ -union { -/* -* Structure used to forward SCSI commands from the -* client to the server. -*/ -struct vmscsi_request vm_srb; - -/* Structure used to query channel properties. */ -struct vmstorage_channel_properties storage_channel_properties; - -/* Used during version negotiations. */ -struct vmstorage_protocol_version version; - }; -} __packed; - -/* Packet flags */ -/* -* This flag indicates that the server should send back a completion for this -* packet. -*/ -#define REQUEST_COMPLETION_FLAG 0x1 - -/* This is the set of flags that the vsc can set in any packets it sends */ -#define VSC_LEGAL_FLAGS (REQUEST_COMPLETION_FLAG) - - -#include <linux/kernel.h> -#include <linux/wait.h> -#include "hyperv_storage.h" -#include "hyperv.h" - -/* Defines */ -#define STORVSC_RING_BUFFER_SIZE (20*PAGE_SIZE) -#define BLKVSC_RING_BUFFER_SIZE (20*PAGE_SIZE) - -#define STORVSC_MAX_IO_REQUESTS 128 - -/* -* In Hyper-V, each port/path/target maps to 1 scsi host adapter. In -* reality, the path/target is not used (ie always set to 0) so our -* scsi host adapter essentially has 1 bus with 1 target that contains -* up to 256 luns. -*/ -#define STORVSC_MAX_LUNS_PER_TARGET 64 -#define STORVSC_MAX_TARGETS 1 -#define STORVSC_MAX_CHANNELS 1 - -struct hv_storvsc_request ; - -/* Matches Windows-end */ -enum storvsc_request_type { -WRITE_TYPE , -READ_TYPE , -UNKNOWN_TYPE , -}; - - -struct hv_storvsc_request { -struct hv_storvsc_request *request; -struct hv_device *device; - -/* Synchronize the request/response if needed */ -struct completion wait_event; - -unsigned char *sense_buffer; -void *context; -void (*on_io_completion)(struct hv_storvsc_request *request); -struct hv_multipage_buffer data_buffer; - -struct vstor_packet vstor_packet; -}; - - -struct storvsc_device_info { -u32 ring_buffer_size; -unsigned int port_number; -unsigned char path_id; -unsigned char target_id; -}; - -struct storvsc_major_info { -int major; -int index; -bool do_register; -char *devname; -char *diskname; -}; - -/* A storvsc device is a device object that contains a vmbus channel */ -struct storvsc_device { -struct hv_device *device; - -bool destroy; -bool drain_notify; -atomic_t num_outstanding_req; - -wait_queue_head_t waiting_to_drain; - -/* -* Each unique Port/Path/Target represents 1 channel ie scsi -* controller. In reality, the pathid, targetid is always 0 -* and the port is set by us -*/ -unsigned int port_number; -unsigned char path_id; -unsigned char target_id; - -/* Used for vsc/vsp channel reset process */ -struct hv_storvsc_request init_request; -struct hv_storvsc_request reset_request; -}; - - -static inline struct storvsc_device *get_out_stor_device ( -struct hv_device *device) -{ -struct storvsc_device *stor_device; - - stor_device = (struct storvsc_device *)device->ext; - -if (stor_device && stor_device->destroy) - stor_device =NULL ; - -return stor_device; -} - - -static inline void storvsc_wait_to_drain (struct storvsc_device *dev) -{ - dev->drain_notify = true; -wait_event (dev->waiting_to_drain, -atomic_read (&dev->num_outstanding_req) ==0 ); - dev->drain_notify = false; -} - -/* Interface */ - -int storvsc_dev_add (struct hv_device *device, -void *additional_info); -int storvsc_dev_remove (struct hv_device *device); - -int storvsc_do_io (struct hv_device *device, -struct hv_storvsc_request *request); - -int storvsc_get_major_info (struct storvsc_device_info *device_info, -struct storvsc_major_info *major_info); - -#endif /* _HYPERV_STORAGE_H */
drivers/staging/hv/storvsc_drv.c +276 -1
289 lines not on this page · raw message
Signature · 1 line
1.7.4.1
PATCH 26/46 Staging: hv: storvsc: Cleanup storvsc_drv.c after adding the contents of hyperv_storage.h
In reply to K. Y. SrinivasanCleanup storvsc_drv.c after adding the contents of hyperv_storage.h.
--- drivers/staging/hv/storvsc_drv.c | 62 ++++++++++++++++---------------------- 1 files changed, 26 insertions(+), 36 deletions(-)
drivers/staging/hv/storvsc_drv.c +26 -36
@@ -42,6 +42,12 @@#include "hyperv.h" +#define STORVSC_RING_BUFFER_SIZE (20*PAGE_SIZE) +static int storvsc_ringbuffer_size =STORVSC_RING_BUFFER_SIZE ; + +module_param (storvsc_ringbuffer_size, int,S_IRUGO ); +MODULE_PARM_DESC (storvsc_ringbuffer_size,"Ring buffer size (bytes)" ); +/* to alert the user that structure sizes may be mismatched even though the */ /* protocol versions match. */
@@ -214,8 +220,6 @@ struct vstor_packet {/* Defines */ -#define STORVSC_RING_BUFFER_SIZE (20*PAGE_SIZE) -#define BLKVSC_RING_BUFFER_SIZE (20*PAGE_SIZE)#define STORVSC_MAX_IO_REQUESTS 128
@@ -262,13 +266,6 @@ struct storvsc_device_info {unsigned char target_id; }; -struct storvsc_major_info { -int major; -int index; -bool do_register; -char *devname; -char *diskname; -};/* A storvsc device is a device object that contains a vmbus channel */ struct storvsc_device {
@@ -294,6 +291,23 @@ struct storvsc_device {struct hv_storvsc_request reset_request; }; +struct hv_host_device { +struct hv_device *dev; +struct kmem_cache *request_pool; +unsigned int port; +unsigned char path; +unsigned char target; +}; + +struct storvsc_cmd_request { +struct list_head entry; +struct scsi_cmnd *cmd; + +unsigned int bounce_sgl_count; +struct scatterlist *bounce_sgl; + +struct hv_storvsc_request request; +};static inline struct storvsc_device *get_out_stor_device (struct hv_device *device)
@@ -317,30 +331,6 @@ static inline void storvsc_wait_to_drain(struct storvsc_device *dev) dev->drain_notify = false; } - -static int storvsc_ringbuffer_size =STORVSC_RING_BUFFER_SIZE ; - -module_param (storvsc_ringbuffer_size, int,S_IRUGO ); -MODULE_PARM_DESC (storvsc_ringbuffer_size,"Ring buffer size (bytes)" ); - -struct hv_host_device { -struct hv_device *dev; -struct kmem_cache *request_pool; -unsigned int port; -unsigned char path; -unsigned char target; -}; - -struct storvsc_cmd_request { -struct list_head entry; -struct scsi_cmnd *cmd; - -unsigned int bounce_sgl_count; -struct scatterlist *bounce_sgl; - -struct hv_storvsc_request request; -}; -static inline struct storvsc_device *alloc_stor_device (struct hv_device *device) {struct storvsc_device *stor_device;
@@ -646,7 +636,7 @@ static int storvsc_connect_to_vsp(struct hv_device *device, u32 ring_size)return ret; } -int storvsc_dev_add (struct hv_device *device, +static int storvsc_dev_add (struct hv_device *device,void *additional_info) {struct storvsc_device *stor_device;
@@ -681,7 +671,7 @@ int storvsc_dev_add(struct hv_device *device,return ret; } -int storvsc_dev_remove (struct hv_device *device) +static int storvsc_dev_remove (struct hv_device *device) {struct storvsc_device *stor_device;unsigned long flags;
@@ -718,7 +708,7 @@ int storvsc_dev_remove(struct hv_device *device)return 0 ; } -int storvsc_do_io (struct hv_device *device, +static int storvsc_do_io (struct hv_device *device,struct hv_storvsc_request *request) {struct storvsc_device *stor_device;
Signature · 1 line
1.7.4.1
PATCH 27/46 Staging: hv: storvsc: Fixup srb and scsi status for INQUIRY and MODE_SENSE
In reply to K. Y. SrinivasanThe current VHD handler on the Windows Host does not correctly handle INQUIRY and MODE_SENSE commands with some options. Fixup srb_status in these cases since the failure is not fatal.
--- drivers/staging/hv/storvsc_drv.c | 17 +++++++++++++++++ 1 files changed, 17 insertions(+), 0 deletions(-)
drivers/staging/hv/storvsc_drv.c +17 -0
@@ -512,6 +512,23 @@ static void storvsc_on_io_completion(struct hv_device *device, stor_pkt = &request->vstor_packet; +/* +* The current SCSI handling on the host side does +* not correctly handle: +* INQUIRY command with page code parameter set to 0x80 +* MODE_SENSE command with cmd[2] == 0x1c +* +* Setup srb and scsi status so this won't be fatal. +* We do this so we can distinguish truly fatal failues +* (srb status == 0x4) and off-line the device in that case. +*/ + +if ((stor_pkt->vm_srb.cdb[0 ] ==INQUIRY ) || + (stor_pkt->vm_srb.cdb[0 ] ==MODE_SENSE )) { + vstor_packet->vm_srb.scsi_status =0 ; + vstor_packet->vm_srb.srb_status =0x1 ; + } +/* Copy over the status...etc */ stor_pkt->vm_srb.scsi_status = vstor_packet->vm_srb.scsi_status;
Signature · 1 line
1.7.4.1
PATCH 28/46 Staging: hv: storvsc: Fix a typo
In reply to K. Y. SrinivasanFix a typo in a function name.
--- drivers/staging/hv/storvsc_drv.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
drivers/staging/hv/storvsc_drv.c +3 -3
@@ -1130,9 +1130,9 @@ static int storvsc_host_reset_handler(struct scsi_cmnd *scmnd)/* -* storvsc_commmand_completion - Command completion processing +* storvsc_command_completion - Command completion processing */ -static void storvsc_commmand_completion (struct hv_storvsc_request *request) +static void storvsc_command_completion (struct hv_storvsc_request *request) {struct storvsc_cmd_request *cmd_request = (struct storvsc_cmd_request *)request->context;
@@ -1240,7 +1240,7 @@ static int storvsc_queuecommand_lck(struct scsi_cmnd *scmnd,break ; } - request->on_io_completion = storvsc_commmand_completion; + request->on_io_completion = storvsc_command_completion; request->context = cmd_request;/* scmnd; */ vm_srb->port_number = host_dev->port;
Signature · 1 line
1.7.4.1
PATCH 29/46 Staging: hv: storvsc: In case of scsi errors offline the device
In reply to K. Y. SrinivasanWhen we do get fatal errors from the host, offline the device since the host has already tried all possible recovery actions.
--- drivers/staging/hv/storvsc_drv.c | 10 +++++++++- 1 files changed, 9 insertions(+), 1 deletions(-)
drivers/staging/hv/storvsc_drv.c +9 -1
@@ -1154,7 +1154,15 @@ static void storvsc_command_completion(struct hv_storvsc_request *request) } } - scmnd->result = vm_srb->scsi_status; +/* +* If there is an error; offline the device since all +* error recovery strategies would have already been +* deployed on the host side. +*/ +if (vm_srb->srb_status ==0x4 ) + scmnd->result =DID_TARGET_FAILURE <<16 ; +else + scmnd->result = vm_srb->scsi_status;if (scmnd->result) { if (scsi_normalize_sense(scmnd->sense_buffer,
Signature · 1 line
1.7.4.1
PATCH 30/46 Staging: hv: storvsc: No need to copy from bounce buffer in case of a failure
In reply to K. Y. SrinivasanNo need to copy from bounce buffer in case of a failure; cleanup the code accordingly.
--- drivers/staging/hv/storvsc_drv.c | 12 ++---------- 1 files changed, 2 insertions(+), 10 deletions(-)
drivers/staging/hv/storvsc_drv.c +2 -10
@@ -1315,17 +1315,9 @@ retry_request:if (ret == -EAGAIN ) {/* no more space */ -if (cmd_request->bounce_sgl_count) { -/* -* FIXME: We can optimize on writes by just skipping -* this -*/ -copy_from_bounce_buffer (scsi_sglist (scmnd), - cmd_request->bounce_sgl, -scsi_sg_count (scmnd)); +if (cmd_request->bounce_sgl_count)destroy_bounce_buffer (cmd_request->bounce_sgl, - cmd_request->bounce_sgl_count); - } + cmd_request->bounce_sgl_count);kmem_cache_free (host_dev->request_pool, cmd_request);
Signature · 1 line
1.7.4.1
PATCH 31/46 Staging: hv: util: Forcefully shutdown when shutdown is requested
In reply to K. Y. SrinivasanWhen the host requests a "shutdown", make sure we shutdown!
--- drivers/staging/hv/hv_util.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/staging/hv/hv_util.c +1 -1
@@ -89,7 +89,7 @@ static void shutdown_onchannelcallback(void *context) }if (execute_shutdown == true) -orderly_poweroff (false); +orderly_poweroff (true); }/*
Signature · 1 line
1.7.4.1
PATCH 32/46 Staging: hv: util: Adjust guest time in a process context
In reply to K. Y. SrinivasanThe current code was adjusting guest time in interrupt context; do this in process context since we may have to initiate cross-processor interrupts as part of setting time.
--- drivers/staging/hv/hv_util.c | 33 ++++++++++++++++++++++++++++++--- 1 files changed, 30 insertions(+), 3 deletions(-)
drivers/staging/hv/hv_util.c +30 -3
@@ -107,6 +107,24 @@ static inline void do_adj_guesttime(u64 hosttime) }/* +* Set the host time in a process context. +*/ + +struct adj_time_work { +struct work_struct work; +u64 host_time; +}; + +static void hv_set_host_time (struct work_struct *work) +{ +struct adj_time_work *wrk; + + wrk = container_of(work, struct adj_time_work, work); +do_adj_guesttime (wrk->host_time); +kfree (wrk); +} + +/* * Synchronize time with host after reboot, restore, etc. * * ICTIMESYNCFLAG_SYNC flag bit indicates reboot, restore events of the VM.
@@ -119,17 +137,26 @@ static inline void do_adj_guesttime(u64 hosttime)*/ static inline void adj_guesttime (u64 hosttime,u8 flags) { +struct adj_time_work *wrk;static s32 scnt =50 ; + wrk =kmalloc (sizeof (struct adj_time_work ),GFP_ATOMIC ); +if (wrk ==NULL ) +return ; + + wrk->host_time = hosttime;if ((flags &ICTIMESYNCFLAG_SYNC ) !=0 ) { -do_adj_guesttime (hosttime); +INIT_WORK (&wrk->work, hv_set_host_time); +schedule_work (&wrk->work);return ; }if ((flags &ICTIMESYNCFLAG_SAMPLE ) !=0 && scnt >0 ) { scnt--; -do_adj_guesttime (hosttime); - } +INIT_WORK (&wrk->work, hv_set_host_time); +schedule_work (&wrk->work); + }else +kfree (wrk); }/*
Signature · 1 line
1.7.4.1
PATCH 33/46 Staging: hv: vmbus: Check before invoking the channel callback
In reply to K. Y. SrinivasanWhen we close a channel, we set the corresponding callback function to NULL. Check before invoking the channel callback.
--- drivers/staging/hv/connection.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/staging/hv/connection.c +1 -1
@@ -222,7 +222,7 @@ static void process_chn_event(u32 relid)*/ channel =relid2channel (relid); -if (channel) { +if (channel && (channel->onchannel_callback !=NULL )) { channel->onchannel_callback (channel->channel_callback_context); }else {pr_err ("channel not found for relid - %u\n" , relid);
Signature · 1 line
1.7.4.1
PATCH 34/46 Staging: hv: vmbus: Properly deal with de-registering channel callback
In reply to K. Y. SrinivasanEnsure that we correctly handle racing invocations of the channel callback when the channel is being closed. We do this using the channel's inbound_lock. A side-effect of this strategy is that we avoid repeatedly picking up this lock as we drain the inbound ring-buffer.
--- drivers/staging/hv/channel.c | 20 +++++--------------- drivers/staging/hv/connection.c | 3 +++ drivers/staging/hv/netvsc.c | 3 --- drivers/staging/hv/storvsc_drv.c | 3 --- 4 files changed, 8 insertions(+), 21 deletions(-)
drivers/staging/hv/channel.c +5 -15
@@ -513,9 +513,12 @@ void vmbus_close(struct vmbus_channel *channel) {struct vmbus_channel_close_channel *msg;int ret; +unsigned long flags;/* Stop callback and cancel the timer asap */ +spin_lock_irqsave (&channel->inbound_lock, flags); channel->onchannel_callback =NULL ; +spin_unlock_irqrestore (&channel->inbound_lock, flags);/* Send a closing message */
@@ -735,19 +738,15 @@ int vmbus_recvpacket(struct vmbus_channel *channel, void *buffer,u32 packetlen;u32 userlen;int ret; -unsigned long flags; *buffer_actual_len =0 ; *requestid =0 ; -spin_lock_irqsave (&channel->inbound_lock, flags); ret =hv_ringbuffer_peek (&channel->inbound, &desc,sizeof (struct vmpacket_descriptor )); -if (ret !=0 ) { -spin_unlock_irqrestore (&channel->inbound_lock, flags); +if (ret !=0 )return 0 ; - } packetlen = desc.len8 <<3 ; userlen = packetlen - (desc.offset8 <<3 );
@@ -755,7 +754,6 @@ int vmbus_recvpacket(struct vmbus_channel *channel, void *buffer, *buffer_actual_len = userlen;if (userlen > bufferlen) { -spin_unlock_irqrestore (&channel->inbound_lock, flags);pr_err ("Buffer too small - got %d needs %d\n" , bufferlen, userlen);
@@ -768,7 +766,6 @@ int vmbus_recvpacket(struct vmbus_channel *channel, void *buffer, ret =hv_ringbuffer_read (&channel->inbound, buffer, userlen, (desc.offset8 <<3 )); -spin_unlock_irqrestore (&channel->inbound_lock, flags);return 0 ; }
@@ -785,19 +782,15 @@ int vmbus_recvpacket_raw(struct vmbus_channel *channel, void *buffer,u32 packetlen;u32 userlen;int ret; -unsigned long flags; *buffer_actual_len =0 ; *requestid =0 ; -spin_lock_irqsave (&channel->inbound_lock, flags); ret =hv_ringbuffer_peek (&channel->inbound, &desc,sizeof (struct vmpacket_descriptor )); -if (ret !=0 ) { -spin_unlock_irqrestore (&channel->inbound_lock, flags); +if (ret !=0 )return 0 ; - } packetlen = desc.len8 <<3 ;
@@ -806,8 +799,6 @@ int vmbus_recvpacket_raw(struct vmbus_channel *channel, void *buffer, *buffer_actual_len = packetlen;if (packetlen > bufferlen) { -spin_unlock_irqrestore (&channel->inbound_lock, flags); -pr_err ("Buffer too small - needed %d bytes but " "got space for only %d bytes\n" , packetlen, bufferlen);
@@ -819,7 +810,6 @@ int vmbus_recvpacket_raw(struct vmbus_channel *channel, void *buffer,/* Copy over the entire packet to the user buffer */ ret =hv_ringbuffer_read (&channel->inbound, buffer, packetlen,0 ); -spin_unlock_irqrestore (&channel->inbound_lock, flags);return 0 ; }EXPORT_SYMBOL_GPL (vmbus_recvpacket_raw);
drivers/staging/hv/connection.c +3 -0
@@ -215,6 +215,7 @@ struct vmbus_channel *relid2channel(u32 relid)static void process_chn_event (u32 relid) {struct vmbus_channel *channel; +unsigned long flags;/* * Find the channel based on this relid and invokes the
@@ -222,11 +223,13 @@ static void process_chn_event(u32 relid)*/ channel =relid2channel (relid); +spin_lock_irqsave (&channel->inbound_lock, flags);if (channel && (channel->onchannel_callback !=NULL )) { channel->onchannel_callback (channel->channel_callback_context); }else {pr_err ("channel not found for relid - %u\n" , relid); } +spin_unlock_irqrestore (&channel->inbound_lock, flags); }/*
drivers/staging/hv/netvsc.c +0 -3
@@ -62,9 +62,7 @@ static struct netvsc_device *get_outbound_net_device(struct hv_device *device)static struct netvsc_device *get_inbound_net_device (struct hv_device *device) {struct netvsc_device *net_device; -unsigned long flags; -spin_lock_irqsave (&device->channel->inbound_lock, flags); net_device = device->ext;if (!net_device)
@@ -75,7 +73,6 @@ static struct netvsc_device *get_inbound_net_device(struct hv_device *device) net_device =NULL ; get_in_err: -spin_unlock_irqrestore (&device->channel->inbound_lock, flags);return net_device; }
drivers/staging/hv/storvsc_drv.c +0 -3
@@ -352,9 +352,7 @@ static inline struct storvsc_device *get_in_stor_device( struct hv_device *device) {struct storvsc_device *stor_device; -unsigned long flags; -spin_lock_irqsave (&device->channel->inbound_lock, flags); stor_device = (struct storvsc_device *)device->ext;if (!stor_device)
@@ -370,7 +368,6 @@ static inline struct storvsc_device *get_in_stor_device( stor_device =NULL ; get_in_err: -spin_unlock_irqrestore (&device->channel->inbound_lock, flags);return stor_device; }
Signature · 1 line
1.7.4.1
PATCH 35/46 Staging: hv: Fix a bug in vmbus_match()
In reply to K. Y. SrinivasanThe recent checkin that add a private pointer to hv_vmbus_device_id introduced this bug in vmbus_match; fix it.
--- drivers/staging/hv/vmbus_drv.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/staging/hv/vmbus_drv.c +1 -1
@@ -273,7 +273,7 @@ static int vmbus_match(struct device *device, struct device_driver *driver)for (; !is_null_guid (id_array->guid); id_array++)if (!memcmp (&id_array->guid, &hv_dev->dev_type.b, -sizeof (struct hv_vmbus_device_id ))) +sizeof (uuid_le)))return 1 ;return 0 ;
Signature · 1 line
1.7.4.1
Re: [PATCH 35/46] Staging: hv: Fix a bug in vmbus_match()
Quoted 14 lines · K. Y. Srinivasan
The recent checkin that add a private pointer to
hv_vmbus_device_idintroduced this bug invmbus_match; fix it.Signed-off-by K. Y. Srinivasan <[email protected]>Signed-off-by Haiyang Zhang <[email protected]>--- drivers/staging/hv/vmbus_drv.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)drivers/staging/hv/vmbus_drv.c +0
-0
drivers/staging/hv/vmbus_drv.c static int vmbus_match(struct device *device, struct device_driver *driver) · line 276
Quoted 5 lines
drivers/staging/hv/vmbus_drv.c +1
-1@@ -273,7 +273,7 @@ static int vmbus_match(struct device *device, struct device_driver *driver)for (; !is_null_guid (id_array->guid); id_array++)if (!memcmp (&id_array->guid, &hv_dev->dev_type.b, -sizeof (struct hv_vmbus_device_id ))) +sizeof (uuid_le)))
Ick, sorry about this, it was my fault.
greg k-h
PATCH 36/46 Staging: hv: vmbus: Get rid of vmbus_on_isr() by inlining the code
In reply to K. Y. SrinivasanGet rid of vmbus_on_isr() by inlining the code.
--- drivers/staging/hv/vmbus_drv.c | 41 +++++++++++---------------------------- 1 files changed, 12 insertions(+), 29 deletions(-)
drivers/staging/hv/vmbus_drv.c +12 -29
@@ -437,53 +437,36 @@ static void vmbus_on_msg_dpc(unsigned long data) } } -/* -* vmbus_on_isr - ISR routine -*/ -static int vmbus_on_isr (void ) +static irqreturn_t vmbus_isr (int irq,void *dev_id) { -int ret =0 ;int cpu =smp_processor_id ();void *page_addr;struct hv_message *msg;union hv_synic_event_flags *event; +bool handled = false; page_addr = hv_context.synic_message_page[cpu]; msg = (struct hv_message *)page_addr +VMBUS_MESSAGE_SINT ;/* Check if there are actual msgs to be process */ -if (msg->header.message_type !=HVMSG_NONE ) - ret |=0x1 ; +if (msg->header.message_type !=HVMSG_NONE ) { + handled = true; +tasklet_schedule (&msg_dpc); + } page_addr = hv_context.synic_event_page[cpu]; event = (union hv_synic_event_flags *)page_addr +VMBUS_MESSAGE_SINT ;/* Since we are a child, we only need to check bit 0 */ -if (sync_test_and_clear_bit (0 , (unsigned long *) &event->flags32[0 ])) - ret |=0x2 ; - -return ret; -} - - -static irqreturn_t vmbus_isr (int irq,void *dev_id) -{ -int ret; - - ret =vmbus_on_isr (); - -/* Schedules a dpc if necessary */ -if (ret >0 ) { -if (test_bit (0 , (unsigned long *)&ret)) -tasklet_schedule (&msg_dpc); - -if (test_bit (1 , (unsigned long *)&ret)) -tasklet_schedule (&event_dpc); +if (sync_test_and_clear_bit (0 , (unsigned long *) &event->flags32[0 ])) { + handled = true; +tasklet_schedule (&event_dpc); + } +if (handled)return IRQ_HANDLED ; - }else { +else return IRQ_NONE ; - } }/*
Signature · 1 line
1.7.4.1
PATCH 37/46 Staging: hv: vmbus: Check for events before messages
In reply to K. Y. SrinivasanConform to Windows specification by checking for events before messages.
--- drivers/staging/hv/vmbus_drv.c | 18 +++++++++--------- 1 files changed, 9 insertions(+), 9 deletions(-)
drivers/staging/hv/vmbus_drv.c +9 -9
@@ -445,15 +445,6 @@ static irqreturn_t vmbus_isr(int irq, void *dev_id)union hv_synic_event_flags *event;bool handled = false; - page_addr = hv_context.synic_message_page[cpu]; - msg = (struct hv_message *)page_addr +VMBUS_MESSAGE_SINT ; - -/* Check if there are actual msgs to be process */ -if (msg->header.message_type !=HVMSG_NONE ) { - handled = true; -tasklet_schedule (&msg_dpc); - } - page_addr = hv_context.synic_event_page[cpu]; event = (union hv_synic_event_flags *)page_addr +VMBUS_MESSAGE_SINT ;
@@ -463,6 +454,15 @@ static irqreturn_t vmbus_isr(int irq, void *dev_id)tasklet_schedule (&event_dpc); } + page_addr = hv_context.synic_message_page[cpu]; + msg = (struct hv_message *)page_addr +VMBUS_MESSAGE_SINT ; + +/* Check if there are actual msgs to be processed */ +if (msg->header.message_type !=HVMSG_NONE ) { + handled = true; +tasklet_schedule (&msg_dpc); + } +if (handled)return IRQ_HANDLED ;else
Signature · 1 line
1.7.4.1
Re: [PATCH 37/46] Staging: hv: vmbus: Check for events before messages
Quoted 1 line · K. Y. Srinivasan
Conform to Windows specification by checking for events before messages.
What specification?
Care to provide a comment in the code that you are doing this in this explicit order because of some rule that the hypervisor imposes on us?
thanks,
greg k-h
RE: [PATCH 37/46] Staging: hv: vmbus: Check for events before messages
Quoted 11 lines
-----Original Message-----
From: Greg KH [mailto:[email protected]]
Sent: Monday, August 29, 2011 2:05 PM
To: KY Srinivasan
Cc: [email protected]; [email protected]; [email protected]; [email protected]; Haiyang Zhang
Subject: Re: [PATCH 37/46] Staging: hv: vmbus: Check for events before messages
Quoted 7 lines
Quoted 1 line · K. Y. Srinivasan
Conform to Windows specification by checking for events before messages.
What specification?
Care to provide a comment in the code that you are doing this in this explicit order because of some rule that the hypervisor imposes on us?
I am not sure if this is documented anywhere (publicly). In talking to Windows guys, they suggested this is the order in which it is done on Windows guests and suggested I should do the same. I will see if there is some public documentation that specifies this.
Regards,
K. Y
Quoted 4 lines
thanks,
greg k-h
Re: [PATCH 37/46] Staging: hv: vmbus: Check for events before messages
Quoted 11 lines · KY Srinivasan
Quoted 7 lines · K. Y. Srinivasan
Quoted 1 line
Conform to Windows specification by checking for events before messages.
What specification?
Care to provide a comment in the code that you are doing this in this explicit order because of some rule that the hypervisor imposes on us?
I am not sure if this is documented anywhere (publicly). In talking to Windows guys, they suggested this is the order in which it is done on Windows guests and suggested I should do the same. I will see if there is some public documentation that specifies this.
All you need to do is just document it in the code and I'll be happy. We don't rely on external documentation for things to be changed, in fact, there usually isn't any such thing :)
thanks,
greg k-h
RE: [PATCH 37/46] Staging: hv: vmbus: Check for events before messages
Quoted 22 lines
-----Original Message-----
From: Greg KH [mailto:[email protected]]
Sent: Tuesday, August 30, 2011 1:38 PM
To: KY Srinivasan
Cc: [email protected]; [email protected]; [email protected]; [email protected]; Haiyang Zhang
Subject: Re: [PATCH 37/46] Staging: hv: vmbus: Check for events before messagesQuoted 10 lines · KY Srinivasan
Quoted 7 lines · K. Y. Srinivasan
Quoted 1 line
Conform to Windows specification by checking for events before messages.
What specification?
Care to provide a comment in the code that you are doing this in this explicit order because of some rule that the hypervisor imposes on us?
I am not sure if this is documented anywhere (publicly). In talking to Windows guys, they suggested this is the order in which it is done on Windows guests
Quoted 7 lines
and suggested
Quoted 1 line
I should do the same. I will see if there is some public documentation that
specifies this.
All you need to do is just document it in the code and I'll be happy. We don't rely on external documentation for things to be changed, in fact, there usually isn't any such thing :)
Ok; will do.
Regards,
K. Y
PATCH 38/46 Staging: hv: vmbus: Do not enable auto eoi
In reply to K. Y. SrinivasanLinux interrupt handling code generates the eoi; don't enable auto eoi.
--- drivers/staging/hv/hv.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/staging/hv/hv.c +1 -1
@@ -369,7 +369,7 @@ void hv_synic_init(void *irqarg) shared_sint.as_uint64 =0 ; shared_sint.vector = irq_vector;/* HV_SHARED_SINT_IDT_VECTOR + 0x20; */ shared_sint.masked = false; - shared_sint.auto_eoi = true; + shared_sint.auto_eoi = false;wrmsrl (HV_X64_MSR_SINT0 +VMBUS_MESSAGE_SINT , shared_sint.as_uint64);
Signature · 1 line
1.7.4.1
PATCH 39/46 Staging: hv: vmbus: Fixup indentation in vmbus_acpi_add()
In reply to K. Y. SrinivasanFixup indentation in vmbus_acpi_add().
--- drivers/staging/hv/vmbus_drv.c | 5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-)
drivers/staging/hv/vmbus_drv.c +2 -3
@@ -678,9 +678,8 @@ static int vmbus_acpi_add(struct acpi_device *device) hv_acpi_dev = device; - result = -acpi_walk_resources (device->handle,METHOD_NAME__CRS , - vmbus_walk_resources, &irq); + result =acpi_walk_resources (device->handle,METHOD_NAME__CRS , + vmbus_walk_resources, &irq);if (ACPI_FAILURE (result)) {complete (&probe_event);
Signature · 1 line
1.7.4.1
PATCH 40/46 Staging: hv: vmbus: Get rid of some dated/redundant comments
In reply to K. Y. SrinivasanGet rid of some dated/redundant comments in vmbus_drv.c
--- drivers/staging/hv/vmbus_drv.c | 11 +---------- 1 files changed, 1 insertions(+), 10 deletions(-)
drivers/staging/hv/vmbus_drv.c +1 -10
@@ -391,9 +391,6 @@ static void vmbus_onmessage_work(struct work_struct *work)kfree (ctx); } -/* -* vmbus_on_msg_dpc - DPC routine to handle messages from the hypervisior -*/ static void vmbus_on_msg_dpc (unsigned long data) {int cpu =smp_processor_id ();
@@ -490,16 +487,13 @@ static int vmbus_bus_init(int irq)return ret; } -/* Initialize the bus context */ tasklet_init (&msg_dpc, vmbus_on_msg_dpc,0 );tasklet_init (&event_dpc, vmbus_on_event,0 ); -/* Now, register the bus with LDM */ ret =bus_register (&hv_bus);if (ret)return ret; -/* Get the interrupt resource */ ret =request_irq (irq, vmbus_isr,IRQF_SAMPLE_RANDOM , driver_name, hv_acpi_dev);
@@ -588,7 +582,6 @@ struct hv_device *vmbus_child_device_create(uuid_le *type, {struct hv_device *child_device_obj; -/* Allocate the new child device */ child_device_obj =kzalloc (sizeof (struct hv_device ),GFP_KERNEL );if (!child_device_obj) {pr_err ("Unable to allocate device object for child device\n" );
@@ -613,12 +606,10 @@ int vmbus_child_device_register(struct hv_device *child_device_obj)static atomic_t device_num =ATOMIC_INIT (0 ); -/* Set the device name. Otherwise, device_register() will fail. */ dev_set_name (&child_device_obj->device,"vmbus_0_%d" ,atomic_inc_return (&device_num)); -/* The new device belongs to this bus */ - child_device_obj->device.bus = &hv_bus;/* device->dev.bus; */ + child_device_obj->device.bus = &hv_bus; child_device_obj->device.parent = &hv_acpi_dev->dev; child_device_obj->device.release = vmbus_device_release;
Signature · 1 line
1.7.4.1
PATCH 41/46 Staging: hv: vmbus: Fix a bug in error handling in vmbus_bus_init()
In reply to K. Y. SrinivasanFix a bug in error handling in vmbus_bus_init().
--- drivers/staging/hv/vmbus_drv.c | 21 ++++++++++----------- 1 files changed, 10 insertions(+), 11 deletions(-)
drivers/staging/hv/vmbus_drv.c +10 -11
@@ -492,7 +492,7 @@ static int vmbus_bus_init(int irq) ret =bus_register (&hv_bus);if (ret) -return ret; + goto err1; ret =request_irq (irq, vmbus_isr,IRQF_SAMPLE_RANDOM , driver_name, hv_acpi_dev);
@@ -500,10 +500,7 @@ static int vmbus_bus_init(int irq)if (ret !=0 ) {pr_err ("Unable to request IRQ %d\n" , irq); - -bus_unregister (&hv_bus); - -return ret; + goto err2; } vector =IRQ0_VECTOR + irq;
@@ -514,16 +511,18 @@ static int vmbus_bus_init(int irq)*/ on_each_cpu (hv_synic_init, (void *)&vector,1 ); ret =vmbus_connect (); -if (ret) { -free_irq (irq, hv_acpi_dev); -bus_unregister (&hv_bus); -return ret; - } - +if (ret) + goto err3;vmbus_request_offers ();return 0 ; + +err3:free_irq (irq, hv_acpi_dev); +err2:bus_unregister (&hv_bus); +err1:hv_cleanup (); + +return ret; }/**
Signature · 1 line
1.7.4.1
Re: [PATCH 41/46] Staging: hv: vmbus: Fix a bug in error handling in vmbus_bus_init()
Quoted 43 lines · K. Y. Srinivasan
Fix a bug in error handling in
vmbus_bus_init().Signed-off-by K. Y. Srinivasan <[email protected]>Signed-off-by Haiyang Zhang <[email protected]>--- drivers/staging/hv/vmbus_drv.c | 21 ++++++++++----------- 1 files changed, 10 insertions(+), 11 deletions(-)drivers/staging/hv/vmbus_drv.c +2
-11@@ -492,7 +492,7 @@ static int vmbus_bus_init(int irq) ret =bus_register (&hv_bus);if (ret) -return ret; + goto err1; ret =request_irq (irq, vmbus_isr,IRQF_SAMPLE_RANDOM , driver_name, hv_acpi_dev);@@ -500,10 +500,7 @@ static int vmbus_bus_init(int irq)if (ret !=0 ) {pr_err ("Unable to request IRQ %d\n" , irq); - -bus_unregister (&hv_bus); - -return ret; + goto err2; } vector =IRQ0_VECTOR + irq;@@ -514,16 +511,18 @@ static int vmbus_bus_init(int irq)*/ on_each_cpu (hv_synic_init, (void *)&vector,1 ); ret =vmbus_connect (); -if (ret) { -free_irq (irq, hv_acpi_dev); -bus_unregister (&hv_bus); -return ret; - } -
drivers/staging/hv/vmbus_drv.c static int vmbus_bus_init(int irq) · line 523
Quoted 10 lines
drivers/staging/hv/vmbus_drv.c +6
-0+if (ret) + goto err3;vmbus_request_offers ();return 0 ; + +err3:free_irq (irq, hv_acpi_dev); +err2:bus_unregister (&hv_bus); +err1:hv_cleanup ();
The traditional way to write this is:
err3:
free_irq(irq, hv_acpi_dev); err2: bus_unregister(&hv_bus); err1: hv_cleanup();
Care to fix this up and resend it?
thanks,
greg k-h
Re: [PATCH 41/46] Staging: hv: vmbus: Fix a bug in error handling in vmbus_bus_init()
Quoted 6 lines
err3:
free_irq(irq, hv_acpi_dev); err2: bus_unregister(&hv_bus); err1: hv_cleanup();
Also here is an oldbie trick. You could use multiples of ten like err30, err20, and err10. That way if you can add more error handling in the middle without changing the numbering. I knew my GW-BASIC experience would come in handy one day. :)
The better way to label things is based on what happens when you get
there: err_irq, err_unregister, err_cleanup.
Some people label things based on where the goto is, but that breaks down if there is more than one goto. It doesn't really make sense to name the destination after the starting point.
regards,
dan carpenter
Re: [PATCH 41/46] Staging: hv: vmbus: Fix a bug in error handling in vmbus_bus_init()
Quoted 7 lines · Dan Carpenter
Quoted 6 lines
err3:
free_irq(irq, hv_acpi_dev); err2: bus_unregister(&hv_bus); err1: hv_cleanup();
Quoted 7 lines
Also here is an oldbie trick. You could use multiples of ten like err30, err20, and err10. That way if you can add more error handling in the middle without changing the numbering. I knew my GW-BASIC experience would come in handy one day. :)
The better way to label things is based on what happens when you get
there:err_irq,err_unregister,err_cleanup.
Yes, that's the best way to do this, thanks for pointing it out.
greg k-h
RE: [PATCH 41/46] Staging: hv: vmbus: Fix a bug in error handling in vmbus_bus_init()
Quoted 24 lines
-----Original Message-----
From: Greg KH [mailto:[email protected]]
Sent: Tuesday, August 30, 2011 10:07 AM
To: Dan Carpenter
Cc: KY Srinivasan; [email protected]; Haiyang Zhang; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 41/46] Staging: hv: vmbus: Fix a bug in error handling invmbus_bus_init()Quoted 12 lines · Dan Carpenter
Quoted 6 lines
err3:
free_irq(irq, hv_acpi_dev); err2: bus_unregister(&hv_bus); err1: hv_cleanup();Also here is an oldbie trick. You could use multiples of ten like err30, err20, and err10. That way if you can add more error handling in the middle without changing the numbering. I knew my GW-BASIC experience would come in handy one day. :)
Quoted 4 lines
Quoted 2 lines
The better way to label things is based on what happens when you get
there:err_irq,err_unregister,err_cleanup.Yes, that's the best way to do this, thanks for pointing it out.
I will fix this up.
Regards,
K. Y
Quoted 2 lines
greg k-h
RE: [PATCH 41/46] Staging: hv: vmbus: Fix a bug in error handling in vmbus_bus_init()
In reply to Greg KHQuoted 68 lines
-----Original Message-----
From: Greg KH [mailto:[email protected]]
Sent: Monday, August 29, 2011 2:09 PM
To: KY Srinivasan
Cc: [email protected]; [email protected]; [email protected]; [email protected]; Haiyang Zhang
Subject: Re: [PATCH 41/46] Staging: hv: vmbus: Fix a bug in error handling invmbus_bus_init()Quoted 43 lines · K. Y. Srinivasan
Fix a bug in error handling in
vmbus_bus_init().Signed-off-by K. Y. Srinivasan <[email protected]>Signed-off-by Haiyang Zhang <[email protected]>--- drivers/staging/hv/vmbus_drv.c | 21 ++++++++++----------- 1 files changed, 10 insertions(+), 11 deletions(-)drivers/staging/hv/vmbus_drv.c +2
-11@@ -492,7 +492,7 @@ static int vmbus_bus_init(int irq) ret =bus_register (&hv_bus);if (ret) -return ret; + goto err1; ret =request_irq (irq, vmbus_isr,IRQF_SAMPLE_RANDOM , driver_name, hv_acpi_dev);@@ -500,10 +500,7 @@ static int vmbus_bus_init(int irq)if (ret !=0 ) {pr_err ("Unable to request IRQ %d\n" , irq); - -bus_unregister (&hv_bus); - -return ret; + goto err2; } vector =IRQ0_VECTOR + irq;@@ -514,16 +511,18 @@ static int vmbus_bus_init(int irq)*/ on_each_cpu (hv_synic_init, (void *)&vector,1 ); ret =vmbus_connect (); -if (ret) { -free_irq (irq, hv_acpi_dev); -bus_unregister (&hv_bus); -return ret; - } -drivers/staging/hv/vmbus_drv.c static int vmbus_bus_init(int irq) · line 523
Quoted 10 lines
drivers/staging/hv/vmbus_drv.c +6
-0+if (ret) + goto err3;vmbus_request_offers ();return 0 ; + +err3:free_irq (irq, hv_acpi_dev); +err2:bus_unregister (&hv_bus); +err1:hv_cleanup ();The traditional way to write this is:
Quoted 8 lines
err3:
free_irq(irq, hv_acpi_dev); err2: bus_unregister(&hv_bus); err1: hv_cleanup();Care to fix this up and resend it?
Will do.
Regards,
K. Y
PATCH 42/46 Staging: hv: vmbus: Get rid of an unnecessary check in vmbus_connect()
In reply to K. Y. SrinivasanGet rid of an unnecessary check in vmbus_connect().
--- drivers/staging/hv/connection.c | 4 ---- 1 files changed, 0 insertions(+), 4 deletions(-)
drivers/staging/hv/connection.c +0 -4
@@ -50,10 +50,6 @@ int vmbus_connect(void)struct vmbus_channel_initiate_contact *msg;unsigned long flags; -/* Make sure we are not connecting or connected */ -if (vmbus_connection.conn_state !=DISCONNECTED ) -return -EISCONN ; -/* Initialize the vmbus connection */ vmbus_connection.conn_state =CONNECTING ; vmbus_connection.work_queue =create_workqueue ("hv_vmbus_con" );
Signature · 1 line
1.7.4.1
PATCH 43/46 Staging: hv: vmbus: Fix a checkpatch warning in ring_buffer.c
In reply to K. Y. SrinivasanFix a checkpatch warning in ring_buffer.c
--- drivers/staging/hv/ring_buffer.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
drivers/staging/hv/ring_buffer.c +2 -1
@@ -34,7 +34,8 @@/* Amount of space to write to */ -#define BYTES_AVAIL_TO_WRITE (r, w, z) ((w) >= (r)) ? ((z) - ((w) - (r))) : ((r) - (w)) +#define BYTES_AVAIL_TO_WRITE (r, w, z) \ + ((w) >= (r)) ? ((z) - ((w) - (r))) : ((r) - (w))/*
Signature · 1 line
1.7.4.1
PATCH 44/46 Staging: hv: vmbus: Fix checkpatch warnings in connection.c
In reply to K. Y. SrinivasanFix checkpatch warnings in connection.c.
--- drivers/staging/hv/connection.c | 13 +++++++------ 1 files changed, 7 insertions(+), 6 deletions(-)
drivers/staging/hv/connection.c +7 -6
@@ -220,11 +220,11 @@ static void process_chn_event(u32 relid) channel =relid2channel (relid);spin_lock_irqsave (&channel->inbound_lock, flags); -if (channel && (channel->onchannel_callback !=NULL )) { +if (channel && (channel->onchannel_callback !=NULL )) channel->onchannel_callback (channel->channel_callback_context); - }else { +else pr_err ("channel not found for relid - %u\n" , relid); - } +spin_unlock_irqrestore (&channel->inbound_lock, flags); }
@@ -246,16 +246,17 @@ void vmbus_on_event(unsigned long data)if (!recv_int_page[dword])continue ;for (bit =0 ; bit <32 ; bit++) { -if (sync_test_and_clear_bit (bit, (unsigned long *)&recv_int_page[dword])) { +if (sync_test_and_clear_bit (bit, + (unsigned long *)&recv_int_page[dword])) { relid = (dword <<5 ) + bit; -if (relid ==0 ) { +if (relid ==0 )/* * Special case - vmbus * channel protocol msg */ continue ; - } +process_chn_event (relid); } }
Signature · 1 line
1.7.4.1
Re: [PATCH 44/46] Staging: hv: vmbus: Fix checkpatch warnings in connection.c
Quoted 1 line · K. Y. Srinivasan
Fix checkpatch warnings in
connection.c.
[]
drivers/staging/hv/connection.c
Quoted 1 line
drivers/staging/hv/connection.c +0
-0
[]
drivers/staging/hv/connection.c static void process_chn_event(u32 relid) · line 223
Quoted 6 lines
drivers/staging/hv/connection.c +1
-1@@ -220,11 +220,11 @@ static void process_chn_event(u32 relid) channel =relid2channel (relid);spin_lock_irqsave (&channel->inbound_lock, flags); -if (channel && (channel->onchannel_callback !=NULL )) { +if (channel && (channel->onchannel_callback !=NULL ))
Useless test for channel or bad placement for spin_lock. channel has already been dereferenced by the spin_lock.
RE: [PATCH 44/46] Staging: hv: vmbus: Fix checkpatch warnings in connection.c
Quoted 19 lines
-----Original Message-----
From: Joe Perches [mailto:[email protected]]
Sent: Sunday, August 28, 2011 2:57 AM
To: KY Srinivasan
Cc: [email protected]; [email protected]; [email protected]; [email protected]; Haiyang Zhang
Subject: Re: [PATCH 44/46] Staging: hv: vmbus: Fix checkpatch warnings inconnection.cQuoted 1 line · K. Y. Srinivasan
Fix checkpatch warnings in
connection.c.[]
drivers/staging/hv/connection.c
Quoted 1 line
drivers/staging/hv/connection.c +0
-0[]
Quoted 3 lines
drivers/staging/hv/connection.c +0
-0@@ -220,11 +220,11 @@ static void process_chn_event(u32 relid) channel =relid2channel (relid);
Quoted 8 lines
drivers/staging/hv/connection.c static void process_chn_event(u32 relid) · line 223
Quoted 3 lines
drivers/staging/hv/connection.c +1
-1spin_lock_irqsave (&channel->inbound_lock, flags); -if (channel && (channel->onchannel_callback !=NULL )) { +if (channel && (channel->onchannel_callback !=NULL ))Useless test for channel or bad placement for
spin_lock. channel has already been dereferenced by thespin_lock.
Thanks Joe. I will fix this up.
Regards,
K. Y
Re: [PATCH 44/46] Staging: hv: vmbus: Fix checkpatch warnings in connection.c
In reply to K. Y. SrinivasanQuoted 13 lines · K. Y. Srinivasan
Fix checkpatch warnings in
connection.c.Signed-off-by K. Y. Srinivasan <[email protected]>Signed-off-by Haiyang Zhang <[email protected]>--- drivers/staging/hv/connection.c | 13 +++++++------ 1 files changed, 7 insertions(+), 6 deletions(-)drivers/staging/hv/connection.c +0
-0
drivers/staging/hv/connection.c static void process_chn_event(u32 relid) · line 225
Quoted 7 lines
drivers/staging/hv/connection.c +1
-2@@ -220,11 +220,11 @@ static void process_chn_event(u32 relid) channel =relid2channel (relid);spin_lock_irqsave (&channel->inbound_lock, flags); -if (channel && (channel->onchannel_callback !=NULL )) { +if (channel && (channel->onchannel_callback !=NULL )) channel->onchannel_callback (channel->channel_callback_context); - }else {
I agree with Joe here, if channel really was NULL, you just oopsed.
I'll apply this one, but please send me a follow-on one fixing this bug.
greg k-h
RE: [PATCH 44/46] Staging: hv: vmbus: Fix checkpatch warnings in connection.c
Quoted 36 lines
-----Original Message-----
From: Greg KH [mailto:[email protected]]
Sent: Monday, August 29, 2011 2:10 PM
To: KY Srinivasan
Cc: [email protected]; [email protected]; [email protected]; [email protected]; Haiyang Zhang
Subject: Re: [PATCH 44/46] Staging: hv: vmbus: Fix checkpatch warnings inconnection.cQuoted 13 lines · K. Y. Srinivasan
Fix checkpatch warnings in
connection.c.Signed-off-by K. Y. Srinivasan <[email protected]>Signed-off-by Haiyang Zhang <[email protected]>--- drivers/staging/hv/connection.c | 13 +++++++------ 1 files changed, 7 insertions(+), 6 deletions(-)drivers/staging/hv/connection.c +0
-0drivers/staging/hv/connection.c static void process_chn_event(u32 relid) · line 226
Quoted 8 lines
drivers/staging/hv/connection.c +1
-2@@ -220,11 +220,11 @@ static void process_chn_event(u32 relid) channel =relid2channel (relid);spin_lock_irqsave (&channel->inbound_lock, flags); -if (channel && (channel->onchannel_callback !=NULL )) { +if (channel && (channel->onchannel_callback !=NULL )) channel->onchannel_callback (channel- channel_callback_context); - }else {I agree with Joe here, if channel really was NULL, you just oopsed.
Quoted 1 line
I'll apply this one, but please send me a follow-on one fixing this bug.
Thanks Greg. I will fix this. I got these patches out just before Hurricane Irene hit the east coast. While we were lucky that it was not as bad as was predicted, we lost power and we still don't have power. I have come to a public library in a nearby town to check my email. So, my responses will be sporadic over the next couple of days (until we get power). I will try to address the issues you have raised as quickly as possible.
Regards,
K. Y
Re: [PATCH 44/46] Staging: hv: vmbus: Fix checkpatch warnings in connection.c
Quoted 6 lines · KY Srinivasan
Thanks Greg. I will fix this. I got these patches out just before Hurricane Irene hit the east coast. While we were lucky that it was not as bad as was predicted, we lost power and we still don't have power. I have come to a public library in a nearby town to check my email. So, my responses will be sporadic over the next couple of days (until we get power). I will try to address the issues you have raised as quickly as possible.
Ouch, good luck with the power and other cleanup, hopefully you didn't have any water damage.
I'm in no rush for further patches, that's being driven by your end :)
greg k-h
RE: [PATCH 44/46] Staging: hv: vmbus: Fix checkpatch warnings in connection.c
Quoted 19 lines
-----Original Message-----
From: Greg KH [mailto:[email protected]]
Sent: Tuesday, August 30, 2011 1:42 PM
To: KY Srinivasan
Cc: [email protected]; [email protected]; [email protected]; [email protected]; Haiyang Zhang
Subject: Re: [PATCH 44/46] Staging: hv: vmbus: Fix checkpatch warnings inconnection.cQuoted 6 lines · KY Srinivasan
Thanks Greg. I will fix this. I got these patches out just before Hurricane Irene hit the east coast. While we were lucky that it was not as bad as was predicted, we lost power and we still don't have power. I have come to a public library in a nearby town to check my email. So, my responses will be sporadic over the next couple of days (until we get power). I will try to address the issues you have raised as quickly as possible.
Quoted 4 lines
Ouch, good luck with the power and other cleanup, hopefully you didn't have any water damage.
I'm in no rush for further patches, that's being driven by your end :)
Power was restored early this morning. Fortunately, except for lack power and a few broken branches, we did not really feel the brunt of this hurricane. I am starting work on addressing the issues you and others have raised.
Regards,
K. Y
PATCH 45/46 Staging: hv: mousevsc: Fix checkpatch errors and warnings
In reply to K. Y. SrinivasanFix checkpatch errors and warnings.
--- drivers/staging/hv/hv_mouse.c | 15 ++++++++------- 1 files changed, 8 insertions(+), 7 deletions(-)
drivers/staging/hv/hv_mouse.c +8 -7
@@ -335,7 +335,8 @@ static void mousevsc_on_receive_device_info(struct mousevsc_dev *input_device, input_device->hid_desc =kzalloc (desc->bLength,GFP_KERNEL );if (!input_device->hid_desc) { -pr_err ("unable to allocate hid descriptor - size %d" , desc->bLength); +pr_err ("unable to allocate hid descriptor - size %d" , + desc->bLength); goto cleanup; }
@@ -598,12 +599,12 @@ static int mousevsc_connect_to_vsp(struct hv_device *device)pr_info ("synthhid protocol request..." ); ret =vmbus_sendpacket (device->channel, request, -sizeof (struct pipe_prt_msg ) - -sizeof (unsigned char ) + -sizeof (struct synthhid_protocol_request ), - (unsigned long )request, -VM_PKT_DATA_INBAND , -VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED ); +sizeof (struct pipe_prt_msg ) - +sizeof (unsigned char ) + +sizeof (struct synthhid_protocol_request ), + (unsigned long )request, +VM_PKT_DATA_INBAND , +VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED );if (ret !=0 ) {pr_err ("unable to send synthhid protocol request." ); goto cleanup;
Signature · 1 line
1.7.4.1
PATCH 46/46 Staging: hv: Update the TODO file
In reply to K. Y. SrinivasanUpdate the TODO file.
--- drivers/staging/hv/TODO | 24 ++++++++++++++++++++---- 1 files changed, 20 insertions(+), 4 deletions(-)
drivers/staging/hv/TODO +20 -4
@@ -1,14 +1,30 @@ TODO: - - fix remaining checkpatch warnings and errors - audit the vmbus to verify it is working properly with the driver model - - see if the vmbus can be merged with the other virtual busses - in the kernel + + STATUS (July 19, 2011): + All outstanding issues (known to us) with regards + to conforming to Linux Driver Model have been addressed. + - audit the network driver - checking for carrier inside open is wrong, network device API confusion?? + + STATUS (July 19, 2011): + The network driver has been reviewed by the community. + We have addressed the various issues that were raised in this + review. + - audit the block driver - audit the scsi driver + STATUS (July 19, 2011): + One of the longstanding comment on this body of code + has been to merge the block and scsi driver. This has been done + and patches have been submitted. We have also addressed all the + review comments on this body of code. + + Please send patches for this code to Greg Kroah-Hartman <[email protected]>, -Hank Janssen <[email protected]>, and Haiyang Zhang <[email protected]>. +Hank Janssen <[email protected]>, and Haiyang Zhang <[email protected]>, +K. Y. Srinivasan <[email protected]>.
Signature · 1 line
1.7.4.1
Re: [PATCH 46/46] Staging: hv: Update the TODO file
Quoted 13 lines · K. Y. Srinivasan
Update the TODO file.
Signed-off-by K. Y. Srinivasan <[email protected]>Signed-off-by Haiyang Zhang <[email protected]>--- drivers/staging/hv/TODO | 24 ++++++++++++++++++++---- 1 files changed, 20 insertions(+), 4 deletions(-)drivers/staging/hv/TODO +0
-0
drivers/staging/hv/TODO line 7
Quoted 10 lines
drivers/staging/hv/TODO +4
-3@@ -1,14 +1,30 @@ TODO: - - fix remaining checkpatch warnings and errors - audit the vmbus to verify it is working properly with the driver model - - see if the vmbus can be merged with the other virtual busses - in the kernel + + STATUS (July 19, 2011): + All outstanding issues (known to us) with regards + to conforming to Linux Driver Model have been addressed.
The TODO file is not a place to have a "conversation" about what is and is not completed by having status reports.
Let's discuss this through email, and if we all agree, then the TODO entries can be removed.
greg k-h
RE: [PATCH 46/46] Staging: hv: Update the TODO file
Quoted 38 lines
-----Original Message-----
From: Greg KH [mailto:[email protected]]
Sent: Monday, August 29, 2011 2:12 PM
To: KY Srinivasan
Cc: [email protected]; [email protected]; [email protected]; [email protected]; Haiyang Zhang
Subject: Re: [PATCH 46/46] Staging: hv: Update the TODO fileQuoted 13 lines · K. Y. Srinivasan
Update the TODO file.
Signed-off-by K. Y. Srinivasan <[email protected]>Signed-off-by Haiyang Zhang <[email protected]>--- drivers/staging/hv/TODO | 24 ++++++++++++++++++++---- 1 files changed, 20 insertions(+), 4 deletions(-)drivers/staging/hv/TODO +0
-0drivers/staging/hv/TODO line 7
Quoted 10 lines
drivers/staging/hv/TODO +4
-3@@ -1,14 +1,30 @@ TODO: - - fix remaining checkpatch warnings and errors - audit the vmbus to verify it is working properly with the driver model - - see if the vmbus can be merged with the other virtual busses - in the kernel + + STATUS (July 19, 2011): + All outstanding issues (known to us) with regards + to conforming to Linux Driver Model have been addressed.The TODO file is not a place to have a "conversation" about what is and is not completed by having status reports.
Quoted 2 lines
Let's discuss this through email, and if we all agree, then the TODO entries can be removed.
Ok; will do.
K. Y
Quoted 2 lines
greg k-h
Quoted 10 lines · K. Y. Srinivasan
Further cleanup of the hv drivers.
1) Cleanup reference counting.
2) Handle all block devices using the storvsc driver. I have modified the implementation here based on Christoph's feedback on my earlier implementation.
3) Fix bugs.
Quoted 7 lines
4) Accomodate some host side scsi emulation bugs.
5) In case of scsi errors off-line the device.
This patch-set further reduces the size of Hyper-V drivers - the code is about 10% smaller. This reduction is mainly because we have eliminated the blkvsc driver.
If my tracking is correct, I applied everything in this series except
the following patches
[PATCH 37/46] Staging: hv: vmbus: Check for events before messages [PATCH 41/46] Staging: hv: vmbus: Fix a bug in error handling in vmbus_bus_init() [PATCH 46/46] Staging: hv: Update the TODO file
Please fix up patches 37 and 41 and resend, and for 46, that should be totally different as mentioned, but you might want to send a patch that at the least adds your name to the file at the bottom, as you wanted. Also note that the MAINTAINERS file should also be adjusted to match.
thanks,
greg k-h
Quoted 36 lines
-----Original Message-----
From: Greg KH [mailto:[email protected]]
Sent: Monday, August 29, 2011 2:15 PM
To: KY Srinivasan
Cc: [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 0000/0046] Staging: hv: Driver cleanupQuoted 10 lines · K. Y. Srinivasan
Further cleanup of the hv drivers.
1) Cleanup reference counting.
2) Handle all block devices using the storvsc driver. I have modified the implementation here based on Christoph's feedback on my earlier implementation.
3) Fix bugs.
Quoted 7 lines
4) Accomodate some host side scsi emulation bugs.
5) In case of scsi errors off-line the device.
This patch-set further reduces the size of Hyper-V drivers - the code is about 10% smaller. This reduction is mainly because we have eliminated the blkvsc driver.
If my tracking is correct, I applied everything in this series except the following patches
[PATCH 37/46] Staging: hv: vmbus: Check for events before messages [PATCH 41/46] Staging: hv: vmbus: Fix a bug in error handling invmbus_bus_init()
[PATCH 46/46] Staging: hv: Update the TODO file
Quoted 4 lines
Please fix up patches 37 and 41 and resend, and for 46, that should be totally different as mentioned, but you might want to send a patch that at the least adds your name to the file at the bottom, as you wanted. Also note that the
MAINTAINERSfile should also be adjusted to match.
Thanks Greg. I will try to get these patches back to you as soon as possible.
Regards,
K. Y
Quoted 3 lines · K. Y. Srinivasan
2) Handle all block devices using the storvsc driver. I have modified the implementation here based on Christoph's feedback on my earlier implementation.
The upgrade path from hv_blkvsc to hv_storvsc is difficult.
hv_storvsc does not provide the vmbus:hv_block modalias, so mkinitrd does not know what module to choose when mkinitrd is called in a previous kernel (like sles11sp1).
In a guest with both SCSI and IDE controllers the IDE disks are discovered first, so the SCSI drives will also change their names. Thats not a problem for data partitions because they could be mounted by UUID or LABEL in fstab.
But its difficult to adjust /boot/grub/device.map for example because the old IDE drives do not provide an identifier. What is the best way to make sure the rename from hd* to sd* in such config files is done correctly? Is it safe to assume that all drives with a class_id of 32412632-86cb-44a2-9b5c50d1417354f5 are connected to IDE ports?
localhost:~ # head /sys/bus/vmbus/devices/vmbus_0_{1,2,3,10,11}/class_id ==> /sys/bus/vmbus/devices/vmbus_0_1/class_id <== {32412632-86cb-44a2-9b5c50d1417354f5}
==> /sys/bus/vmbus/devices/vmbus_0_2/class_id <==
{32412632-86cb-44a2-9b5c50d1417354f5}
==> /sys/bus/vmbus/devices/vmbus_0_3/class_id <==
{32412632-86cb-44a2-9b5c50d1417354f5}
==> /sys/bus/vmbus/devices/vmbus_0_10/class_id <== {ba6163d9-04a1-4d29-b60572e2ffb1dc7f}
==> /sys/bus/vmbus/devices/vmbus_0_11/class_id <== {ba6163d9-04a1-4d29-b60572e2ffb1dc7f}
In my test system, the IDE drives are now discovered twice, once by hv_storvsc and once by libata:
localhost:~ # lsscsi
[0:0:0:0] disk Msft Virtual Disk 1.0 /dev/sda [1:0:1:0] disk Msft Virtual Disk 1.0 /dev/sdb [2:0:1:0] disk Msft Virtual Disk 1.0 /dev/sdc [3:0:0:0] disk Msft Virtual Disk 1.0 /dev/sdd [3:0:0:1] disk Msft Virtual Disk 1.0 /dev/sde [4:0:0:0] disk Msft Virtual Disk 1.0 /dev/sdf [4:0:0:1] disk Msft Virtual Disk 1.0 /dev/sdg [5:0:0:0] disk ATA Virtual HD 1.1. /dev/sdh [5:0:1:0] disk ATA Virtual HD 1.1. /dev/sdi [6:0:0:0] cd/dvd Msft Virtual CD/ROM 1.0 /dev/sr0 [6:0:1:0] disk ATA Virtual HD 1.1. /dev/sdj
localhost:~ # blkid/dev/sda1: UUID="47aebbfb-13ee-49cc-b1e7-bc0c243fe3c0" TYPE="swap" /dev/sda2: UUID="ed5a182b-da28-42f2-acff-5649a8fae247" TYPE="ext3" /dev/sdh1: UUID="47aebbfb-13ee-49cc-b1e7-bc0c243fe3c0" TYPE="swap" /dev/sdh2: UUID="ed5a182b-da28-42f2-acff-5649a8fae247" TYPE="ext3"
sda/sdh was hda.
Olaf
Quoted 23 lines
-----Original Message-----
From: Olaf Hering [mailto:[email protected]]
Sent: Tuesday, August 30, 2011 8:49 AM
To: KY Srinivasan
Cc: [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 0000/0046] Staging: hv: Driver cleanupQuoted 3 lines · K. Y. Srinivasan
2) Handle all block devices using the storvsc driver. I have modified the implementation here based on Christoph's feedback on my earlier implementation.
The upgrade path from
hv_blkvsctohv_storvscis difficult.
hv_storvscdoes not provide the vmbus:hv_block modalias, so mkinitrd does not know what module to choose when mkinitrd is called in a previous kernel (like sles11sp1).
Quoted 9 lines
In a guest with both SCSI and IDE controllers the IDE disks are discovered first, so the SCSI drives will also change their names. Thats not a problem for data partitions because they could be mounted by UUID or LABEL in fstab.
But its difficult to adjust/boot/grub/device.mapfor example because the old IDE drives do not provide an identifier. What is the best way to make sure the rename from hd* to sd* in such config files is done correctly? Is it safe to assume that all drives with aclass_idof 32412632-86cb-44a2-9b5c50d1417354f5 are connected to IDE ports?
The class_id is invariant as we are returning the guid of the device under class_id.
So, if you see a IDE guid under class_id, you can be sure (a) it is an ide device and (b) it is a device managed via the PV drivers.
Quoted 14 lines
localhost:~ # head
/sys/bus/vmbus/devices/vmbus_0_{1,2,3,10,11}/class_id==>/sys/bus/vmbus/devices/vmbus_0_1/class_id<== {32412632-86cb-44a2-9b5c50d1417354f5}==>
/sys/bus/vmbus/devices/vmbus_0_2/class_id<==
{32412632-86cb-44a2-9b5c50d1417354f5}==>
/sys/bus/vmbus/devices/vmbus_0_3/class_id<==
{32412632-86cb-44a2-9b5c50d1417354f5}==>
/sys/bus/vmbus/devices/vmbus_0_10/class_id<== {ba6163d9-04a1-4d29-b60572e2ffb1dc7f}
Quoted 6 lines
==>
/sys/bus/vmbus/devices/vmbus_0_11/class_id<== {ba6163d9-04a1-4d29-b60572e2ffb1dc7f}In my test system, the IDE drives are now discovered twice, once by
hv_storvscand once by libata:
This is a known (old problem). The way this was handled earlier was to have the modprobe rules in place to setup a dependency that would force the load of the hyper-v driver (blk / stor) ahead of the native driver and if the load of the PV driver succeeded, we would not load the native driver. In sles11 sp1, we had a rule for loading blkvsc. With the merge of blkvsc and storvsc, the only change we need to make is to have storvsc in the rule (instaed of blkvsc).
Regards,
K. Y
Quoted 59 lines · KY Srinivasan
Quoted 23 lines
-----Original Message-----
From: Olaf Hering [mailto:[email protected]]
Sent: Tuesday, August 30, 2011 8:49 AM
To: KY Srinivasan
Cc: [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 0000/0046] Staging: hv: Driver cleanupQuoted 3 lines · K. Y. Srinivasan
2) Handle all block devices using the storvsc driver. I have modified the implementation here based on Christoph's feedback on my earlier implementation.
The upgrade path from
hv_blkvsctohv_storvscis difficult.
hv_storvscdoes not provide the vmbus:hv_block modalias, so mkinitrd does not know what module to choose when mkinitrd is called in a previous kernel (like sles11sp1).Quoted 9 lines
In a guest with both SCSI and IDE controllers the IDE disks are discovered first, so the SCSI drives will also change their names. Thats not a problem for data partitions because they could be mounted by UUID or LABEL in fstab.
But its difficult to adjust/boot/grub/device.mapfor example because the old IDE drives do not provide an identifier. What is the best way to make sure the rename from hd* to sd* in such config files is done correctly? Is it safe to assume that all drives with aclass_idof 32412632-86cb-44a2-9b5c50d1417354f5 are connected to IDE ports?The
class_idis invariant as we are returning the guid of the device underclass_id.So, if you see a IDE guid under
class_id, you can be sure (a) it is an ide device and (b) it is a device managed via the PV drivers.Quoted 14 lines
localhost:~ # head
/sys/bus/vmbus/devices/vmbus_0_{1,2,3,10,11}/class_id==>/sys/bus/vmbus/devices/vmbus_0_1/class_id<== {32412632-86cb-44a2-9b5c50d1417354f5}==>
/sys/bus/vmbus/devices/vmbus_0_2/class_id<==
{32412632-86cb-44a2-9b5c50d1417354f5}==>
/sys/bus/vmbus/devices/vmbus_0_3/class_id<==
{32412632-86cb-44a2-9b5c50d1417354f5}==>
/sys/bus/vmbus/devices/vmbus_0_10/class_id<== {ba6163d9-04a1-4d29-b60572e2ffb1dc7f}Quoted 6 lines
==>
/sys/bus/vmbus/devices/vmbus_0_11/class_id<== {ba6163d9-04a1-4d29-b60572e2ffb1dc7f}In my test system, the IDE drives are now discovered twice, once by
hv_storvscand once by libata:
Quoted 6 lines
This is a known (old problem). The way this was handled earlier was to have the modprobe rules in place to setup a dependency that would force the load of the hyper-v driver (blk / stor) ahead of the native driver and if the load of the PV driver succeeded, we would not load the native driver. In sles11 sp1, we had a rule for loading blkvsc. With the merge of blkvsc and storvsc, the only change we need to make is to have storvsc in the rule (instaed of blkvsc).
Why do we need a rule at all? Shouldn't the module dependancy stuff handle the autoloading of the drivers properly from the initrd now that the hotplug logic is hooked up properly?
What am I missing here?
Or is the hotplug code not working correctly?
greg k-h
Quoted 10 lines · Greg KH
Quoted 9 lines
Quoted 2 lines
In my test system, the IDE drives are now discovered twice, once by
hv_storvscand once by libata:This is a known (old problem). The way this was handled earlier was to have the modprobe rules in place to setup a dependency that would force the load of the hyper-v driver (blk / stor) ahead of the native driver and if the load of the PV driver succeeded, we would not load the native driver. In sles11 sp1, we had a rule for loading blkvsc. With the merge of blkvsc and storvsc, the only change we need to make is to have storvsc in the rule (instaed of blkvsc).
Quoted 3 lines
Why do we need a rule at all? Shouldn't the module dependancy stuff handle the autoloading of the drivers properly from the initrd now that the hotplug logic is hooked up properly?
There is no plan to load hv_vmbus (or xen-platform-pci) earlier than native drivers. That was the purpose of the modprobe.conf files. Now that there is a vmbus, that fact could be checked before any other attempt to load drivers is made and hv_vmbus should be loaded and all of its devices have to be probed manually by modprobe cat modulealias.
Quoted 1 line
Or is the hotplug code not working correctly?
There is nothing to hotplug. hv_vmbus has to be loaded first so that it can take over the devices. But it seems that there is no shutdown of the emulated hardware, thats why the disk "sda" is shown twice.
I spot a flaw here.
KY, can hv_vmbus shutdown emulated hardware? At least the disks, because cdroms are appearently still be handled by native drivers?
Olaf
Quoted 12 lines · Olaf Hering
Quoted 10 lines · Greg KH
Quoted 9 lines
Quoted 2 lines
In my test system, the IDE drives are now discovered twice, once by
hv_storvscand once by libata:This is a known (old problem). The way this was handled earlier was to have the modprobe rules in place to setup a dependency that would force the load of the hyper-v driver (blk / stor) ahead of the native driver and if the load of the PV driver succeeded, we would not load the native driver. In sles11 sp1, we had a rule for loading blkvsc. With the merge of blkvsc and storvsc, the only change we need to make is to have storvsc in the rule (instaed of blkvsc).
Quoted 6 lines
Quoted 3 lines
Why do we need a rule at all? Shouldn't the module dependancy stuff handle the autoloading of the drivers properly from the initrd now that the hotplug logic is hooked up properly?
There is no plan to load
hv_vmbus(or xen-platform-pci) earlier than native drivers.
Wait, what do you mean by "native drivers"?
Isn't the hv_vmbus drivers the "native drivers" happening here?
Or are you referring to the "emulated-slow-as-hell drivers" that are used to boot the machine?
Quoted 4 lines
That was the purpose of the modprobe.conf files. Now
that there is a vmbus, that fact could be checked before any other attempt to load drivers is made andhv_vmbusshould be loaded and all of its devices have to be probed manually by modprobecat modulealias.
I agree with the first part, but no modprobe should ever need to be done, the hotplug boot process should properly load those modules when the vmbus devices are seen by the vmbus core and the hotplug events generated, which in turn calls modprobe, right?
So there should not need to be any special module.conf file changes for hv systems, with the exception that the "emulated" drivers should be added to the blacklist.
Quoted 7 lines
Quoted 1 line
Or is the hotplug code not working correctly?
There is nothing to hotplug.
hv_vmbushas to be loaded first so that it can take over the devices. But it seems that there is no shutdown of the emulated hardware, thats why the disk "sda" is shown twice.I spot a flaw here.
I agree :)
Quoted 2 lines
KY, can
hv_vmbusshutdown emulated hardware? At least the disks, because cdroms are appearently still be handled by native drivers?
They are? Ick, why can't the vmbus storage driver see a cdrom device? It's just a scsi device, right?
thanks
greg k-h
Quoted 13 lines · Greg KH
Quoted 12 lines · Olaf Hering
Quoted 10 lines · Greg KH
Quoted 9 lines
Quoted 2 lines
In my test system, the IDE drives are now discovered twice, once by
hv_storvscand once by libata:This is a known (old problem). The way this was handled earlier was to have the modprobe rules in place to setup a dependency that would force the load of the hyper-v driver (blk / stor) ahead of the native driver and if the load of the PV driver succeeded, we would not load the native driver. In sles11 sp1, we had a rule for loading blkvsc. With the merge of blkvsc and storvsc, the only change we need to make is to have storvsc in the rule (instaed of blkvsc).
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
Why do we need a rule at all? Shouldn't the module dependancy stuff handle the autoloading of the drivers properly from the initrd now that the hotplug logic is hooked up properly?
There is no plan to load
hv_vmbus(or xen-platform-pci) earlier than native drivers.Wait, what do you mean by "native drivers"?
native means drivers for emulated hardware: ata_piix, piix.
Quoted 3 lines
Isn't the
hv_vmbusdrivers the "native drivers" happening here? Or are you referring to the "emulated-slow-as-hell drivers" that are used to boot the machine?
Not really. The virtual IDE controllers are emulated hardware, while the virtual SCSI controllers are paravirtualized hardware. KY, correct my if I'm wrong here.
Quoted 9 lines
Quoted 4 lines
That was the purpose of the modprobe.conf files. Now
that there is a vmbus, that fact could be checked before any other attempt to load drivers is made andhv_vmbusshould be loaded and all of its devices have to be probed manually by modprobecat modulealias.I agree with the first part, but no modprobe should ever need to be done, the hotplug boot process should properly load those modules when the vmbus devices are seen by the vmbus core and the hotplug events generated, which in turn calls modprobe, right?
With the IDE disks, the module loading order matters because both ata_piix/piix and hv_storvsc will bind to them.
With the SCSI disks, only hv_storvsc will bind to them.
Quoted 3 lines
So there should not need to be any special module.conf file changes for hv systems, with the exception that the "emulated" drivers should be added to the blacklist.
I agree, the modprobe.conf rules for hv should be removed. With this patchset a rule to prevent ata_piix loading would hide all cdrom devices.
Quoted 5 lines
Quoted 2 lines
KY, can
hv_vmbusshutdown emulated hardware? At least the disks, because cdroms are appearently still be handled by native drivers?They are? Ick, why can't the vmbus storage driver see a cdrom device? It's just a scsi device, right?
The GUI does not seem to offer a way to attach an iso to a virtual SCSI controller. It looks like one can add up to 4 SCSI controllers, each one can have up to 64 disk images.
DVD devices can only be attached to the 2 virtual IDE controllers, one can either attach iso images or physical hardware.
After some more testing it looks like the hv_blkvsc driver claimed the ide major #3, so the ide_core could not bind to the emulated IDE hardware. The modprobe.conf rules prevented that ata_piix gets loaded. If I force a module load order of 'ata_piix ;hv_blkvsc' both ata_piix and hv_blkvsc will bind to the same drive, just as shown in my previous mail. So there is appearently no way to shutdown the IDE ports like it is done with xen_emul_unplug= option for Xen guests.
Maybe some hack is needed for a hyper-v specific unplug if Windows does indeed lack that feature. Now that ata_piix is compiled into the kernel, there is nothing to a modprobe.conf rule can "fix".
Olaf
RFC PATCH 1/1 ata_piix: ignore disks in a hyper-v guest
Quoted 11 lines · Olaf Hering
After some more testing it looks like the
hv_blkvscdriver claimed the ide major #3, so theide_corecould not bind to the emulated IDE hardware. The modprobe.conf rules prevented thatata_piixgets loaded. If I force a module load order of 'ata_piix;hv_blkvsc' bothata_piixandhv_blkvscwill bind to the same drive, just as shown in my previous mail. So there is appearently no way to shutdown the IDE ports like it is done with xen_emul_unplug= option for Xen guests.Maybe some hack is needed for a hyper-v specific unplug if Windows does indeed lack that feature. Now that
ata_piixis compiled into the kernel, there is nothing to a modprobe.conf rule can "fix".
Here is an attempt to ignore disks in a hyper-v disks, and let ata_piix handle only the configured cdrom devices (iso of physical). If the disks are not ignored, they will appear twice, once through ata_piix and once through hv_storvsc.
Once hv_storvsc can also handle the configured cdrom devices, the added piix_pata_read_id() function can be removed again. Only the check in piix_init() or something similar needs to be done.
Maybe there is a better way enable only ATA_DEV_ATAPI?
--- drivers/ata/ata_piix.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+)
drivers/ata/ata_piix.c +54 -0
@@ -98,6 +98,10 @@#define DRV_NAME "ata_piix"#define DRV_VERSION "2.13" +#if defined(CONFIG_HYPERV_STORAGE ) || defined(CONFIG_HYPERV_STORAGE_MODULE ) +#define PIIX_IGNORE_ATA_ON_HYPERV 1 +#endif +enum {PIIX_IOCFG =0x54 ,/* IDE I/O configuration register */ ICH5_PMR =0x90 ,/* port mapping register */
@@ -164,6 +168,7 @@ struct piix_host_priv {static int piix_init_one (struct pci_dev *pdev,const struct pci_device_id *ent);static void piix_remove_one (struct pci_dev *pdev); +static unsigned int piix_pata_read_id (struct ata_device *adev,struct ata_taskfile *tf,u16 *id);static int piix_pata_prereset (struct ata_link *link,unsigned long deadline);static void piix_set_piomode (struct ata_port *ap,struct ata_device *adev);static void piix_set_dmamode (struct ata_port *ap,struct ata_device *adev);
@@ -346,6 +351,7 @@ static struct ata_port_operations piix_p .set_piomode = piix_set_piomode, .set_dmamode = piix_set_dmamode, .prereset = piix_pata_prereset, + .read_id = piix_pata_read_id, };static struct ata_port_operations piix_vmw_ops = {
@@ -619,6 +625,24 @@ MODULE_LICENSE("GPL");MODULE_DEVICE_TABLE (pci, piix_pci_tbl);MODULE_VERSION (DRV_VERSION ); +#if defined(PIIX_IGNORE_ATA_ON_HYPERV ) +static int piix_msft_hyperv (void ) +{ +static const struct dmi_system_id hv_dmi_ident[] = { + { + .ident ="Hyper-V" , + .matches = { +DMI_MATCH (DMI_SYS_VENDOR ,"Microsoft Corporation" ), +DMI_MATCH (DMI_PRODUCT_NAME ,"Virtual Machine" ), +DMI_MATCH (DMI_BOARD_NAME ,"Virtual Machine" ), + }, + }, + { }/* terminate list */ + }; +return !!dmi_check_system (hv_dmi_ident); +} +#endif +struct ich_laptop {u16 device;u16 subvendor;
@@ -702,6 +726,29 @@ static int piix_pata_prereset(struct atareturn ata_sff_prereset (link, deadline); } +/** +* +*/ +static unsigned int piix_pata_read_id (struct ata_device *adev,struct ata_taskfile *tf,u16 *id) +{ +unsigned int err_mask =ata_do_dev_read_id (adev, tf, id); +#if defined(PIIX_IGNORE_ATA_ON_HYPERV ) +/* +* Ignore disks in a hyper-v guest. +* There is no unplug protocol like it is done with xen_emul_unplug= option. +* Emulate the unplug by ignoring disks when the hv_storvsc driver is enabled. +* If the disks are not ignored, they will appear twice: once through piix and once through hv_storvsc. +* Because hv_storvsc does not handle ATAPI devices, the piix driver is still required. +* Once hv_storvsc handles all devices, this function can be removed and the whole driver should be disabled in a hyper-v guest. +*/ +if (ata_id_is_ata (id) &&piix_msft_hyperv ()) { +ata_dev_printk (adev,KERN_WARNING ,"ATA device ignored in Hyper-V guest\n" ); + id[ATA_ID_CONFIG ] |= (1 <<15 ); + } +#endif +return err_mask; +} +static DEFINE_SPINLOCK (piix_lock);/**
@@ -1679,6 +1726,13 @@ static int __init piix_init(void) {int rc; +#if defined(PIIX_IGNORE_ATA_ON_HYPERV ) +/* disabled until hv_storvsc drives also cdrom devices */ +if (0 &&piix_msft_hyperv ()) { +printk (KERN_DEBUG "%s: hv_storvsc will bind to storage\n" , __func__); +return -ENODEV ; + } +#endif DPRINTK ("pci_register_driver\n" ); rc =pci_register_driver (&piix_pci_driver);if (rc)
Quoted 30 lines
-----Original Message-----
From: Olaf Hering [mailto:[email protected]]
Sent: Tuesday, August 30, 2011 2:05 PM
To: Greg KH
Cc: KY Srinivasan; [email protected]; [email protected]; linux- [email protected]; [email protected]
Subject: Re: [PATCH 0000/0046] Staging: hv: Driver cleanupQuoted 4 lines · Greg KH
Quoted 4 lines
Quoted 2 lines
In my test system, the IDE drives are now discovered twice, once by
hv_storvscand once by libata:This is a known (old problem). The way this was handled earlier was to have
the
Quoted 1 line
Quoted 1 line
modprobe rules in place to setup a dependency that would force the load of
the
Quoted 2 lines
Quoted 2 lines
hyper-v driver (blk / stor) ahead of the native driver and if the load of the PV driver succeeded, we would not load the native driver. In sles11 sp1, we had a
rule for
Quoted 1 line
Quoted 1 line
loading blkvsc. With the merge of blkvsc and storvsc, the only change we
need to make
Quoted 5 lines
Quoted 1 line
is to have storvsc in the rule (instaed of blkvsc).
Why do we need a rule at all? Shouldn't the module dependancy stuff handle the autoloading of the drivers properly from the initrd now that the hotplug logic is hooked up properly?
Quoted 5 lines
There is no plan to load
hv_vmbus(or xen-platform-pci) earlier than native drivers. That was the purpose of the modprobe.conf files. Now that there is a vmbus, that fact could be checked before any other attempt to load drivers is made andhv_vmbusshould be loaded and all of its devices have to be probed manually by modprobecat modulealias.
The strategy I was describing was what was used in sles11 sp1.
Quoted 10 lines
Quoted 2 lines
Or is the hotplug code not working correctly?
There is nothing to hotplug.
hv_vmbushas to be loaded first so that it can take over the devices. But it seems that there is no shutdown of the emulated hardware, thats why the disk "sda" is shown twice.I spot a flaw here.
KY, canhv_vmbusshutdown emulated hardware? At least the disks, because cdroms are appearently still be handled by native drivers?
Olaf, unfortunately the vmbus driver does not stop the emulation of the disk devices. The earlier modprobe rule was also instrumental in preventing the loading of the native drivers if the hyperv- driver load succeeded.
Regards,
K. Y
Quoted 79 lines
-----Original Message-----
From: Greg KH [mailto:[email protected]]
Sent: Tuesday, August 30, 2011 1:44 PM
To: KY Srinivasan
Cc: Olaf Hering; [email protected]; [email protected]; linux- [email protected]; [email protected]
Subject: Re: [PATCH 0000/0046] Staging: hv: Driver cleanupQuoted 33 lines · KY Srinivasan
Quoted 23 lines
-----Original Message-----
From: Olaf Hering [mailto:[email protected]]
Sent: Tuesday, August 30, 2011 8:49 AM
To: KY Srinivasan
Cc: [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 0000/0046] Staging: hv: Driver cleanupQuoted 3 lines · K. Y. Srinivasan
2) Handle all block devices using the storvsc driver. I have modified the implementation here based on Christoph's feedback on my earlier implementation.
The upgrade path from
hv_blkvsctohv_storvscis difficult.
hv_storvscdoes not provide the vmbus:hv_block modalias, so mkinitrd does not know what module to choose when mkinitrd is called in a previous kernel (like sles11sp1).Quoted 9 lines
In a guest with both SCSI and IDE controllers the IDE disks are discovered first, so the SCSI drives will also change their names. Thats not a problem for data partitions because they could be mounted by UUID or LABEL in fstab.
But its difficult to adjust/boot/grub/device.mapfor example because the old IDE drives do not provide an identifier. What is the best way to make sure the rename from hd* to sd* in such config files is done correctly? Is it safe to assume that all drives with aclass_idof 32412632-86cb-44a2-9b5c50d1417354f5 are connected to IDE ports?Quoted 1 line
The
class_idis invariant as we are returning the guid of the device under
class_id.Quoted 2 lines
So, if you see a IDE guid under
class_id, you can be sure (a) it is an ide deviceand (b)
Quoted 20 lines
it is a device managed via the PV drivers.
Quoted 18 lines
localhost:~ # head
/sys/bus/vmbus/devices/vmbus_0_{1,2,3,10,11}/class_id==>/sys/bus/vmbus/devices/vmbus_0_1/class_id<== {32412632-86cb-44a2-9b5c50d1417354f5}==>
/sys/bus/vmbus/devices/vmbus_0_2/class_id<==
{32412632-86cb-44a2-9b5c50d1417354f5}==>
/sys/bus/vmbus/devices/vmbus_0_3/class_id<==
{32412632-86cb-44a2-9b5c50d1417354f5}==>
/sys/bus/vmbus/devices/vmbus_0_10/class_id<== {ba6163d9-04a1-4d29-b60572e2ffb1dc7f}==>
/sys/bus/vmbus/devices/vmbus_0_11/class_id<== {ba6163d9-04a1-4d29-b60572e2ffb1dc7f}Quoted 5 lines
Quoted 2 lines
In my test system, the IDE drives are now discovered twice, once by
hv_storvscand once by libata:This is a known (old problem). The way this was handled earlier was to have the modprobe rules in place to setup a dependency that would force the load of
the
Quoted 2 lines
hyper-v driver (blk / stor) ahead of the native driver and if the load of the PV driver succeeded, we would not load the native driver. In sles11 sp1, we had a
rule for
Quoted 1 line
loading blkvsc. With the merge of blkvsc and storvsc, the only change we need
Quoted 8 lines
to make
Quoted 1 line
is to have storvsc in the rule (instaed of blkvsc).
Why do we need a rule at all? Shouldn't the module dependancy stuff handle the autoloading of the drivers properly from the initrd now that the hotplug logic is hooked up properly?
What am I missing here?
On sles11 the PV drivers are used to manage the root device. Autoloading is not the issue here; to manage the root device, we want to load the Hyper-V disk driver before the native Linux disk driver for the emulated IDE device.
Quoted 2 lines
Or is the hotplug code not working correctly?
Hotplug code is working as expected; the issue is taking control of the root device and preventing the same device being presented once through native drivers and once through Hyper-V drivers.
Regards,
K. Y
Quoted 2 lines
greg k-h