From: Greg KH <greg@kroah.com>
To: "K. Y. Srinivasan" <kys@microsoft.com>
Cc: gregkh@suse.de, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org
Subject: various vmbus review comments
Date: Tue, 3 May 2011 13:46:41 -0700 [thread overview]
Message-ID: <20110503204641.GA11132@kroah.com> (raw)
I just took a quick look at the vmbus code, and have the following
comments:
- why is count_hv_channel() even a function?
- your .h files need to be consolidated and renamed. You will
need a single hyperv.h file for include/linux/ that will
contain some of what the vmbus*.h files have in it, but not
all. Please merge things together to have:
- include/linux/hyperv.h
What is needed to build the drivers that attach to
the bus
- drivers/staging/hv/hyperv.h
The local .h file will have the vmbus*.h remaining
stuff that is only needed by the hv_vmbus.ko module
to be build.
- the instances of hv_driver structures need to be static and
not programatically defined, like all other USB and PCI
drivers are handled.
- module reference counting. Are you sure you got it all right
for the individual modules that attach to the bus? I don't
see any reference counting happening, is that correct?
- fix the sparse warnings.
- fix the use of volatile in the ring buffer code. It should
not be needed and if you are relying on it, then the code is
wrong.
- fix the namespace on the ringbuffer code to show that it
really is only for the hyperv bus code internally.
That should be enough for at least one more set of patches for you all
to work on :)
thanks,
greg k-h
next reply other threads:[~2011-05-03 20:46 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-05-03 20:46 Greg KH [this message]
2011-05-03 21:00 ` various vmbus review comments KY Srinivasan
2011-05-03 22:03 ` Greg KH
2011-05-03 22:49 ` KY Srinivasan
2011-05-04 16:20 ` KY Srinivasan
2011-05-04 16:32 ` Greg KH
2011-05-04 16:58 ` KY Srinivasan
2011-05-04 18:28 ` Greg KH
2011-05-06 13:10 ` KY Srinivasan
2011-05-06 14:59 ` Greg KH
2011-05-06 17:34 ` KY Srinivasan
2011-05-09 14:33 ` Christoph Hellwig
2011-05-09 14:56 ` KY Srinivasan
2011-05-10 5:24 ` Christoph Hellwig
2011-05-10 13:00 ` KY Srinivasan
2011-05-10 13:07 ` Christoph Hellwig
2011-05-10 13:16 ` Greg KH
2011-05-09 1:46 ` KY Srinivasan
2011-05-09 3:04 ` Greg KH
2011-05-09 12:35 ` KY Srinivasan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20110503204641.GA11132@kroah.com \
--to=greg@kroah.com \
--cc=devel@linuxdriverproject.org \
--cc=gregkh@suse.de \
--cc=kys@microsoft.com \
--cc=linux-kernel@vger.kernel.org \
--cc=virtualization@lists.osdl.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox