From: Greg KH <greg@kroah.com>
To: KY Srinivasan <kys@microsoft.com>
Cc: "devel@linuxdriverproject.org" <devel@linuxdriverproject.org>,
"gregkh@suse.de" <gregkh@suse.de>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"virtualization@lists.osdl.org" <virtualization@lists.osdl.org>
Subject: Re: various vmbus review comments
Date: Sun, 8 May 2011 20:04:52 -0700 [thread overview]
Message-ID: <20110509030452.GA17182@kroah.com> (raw)
In-Reply-To: <6E21E5352C11B742B20C142EB499E0481E9404@TK5EX14MBXC128.redmond.corp.microsoft.com>
On Mon, May 09, 2011 at 01:46:56AM +0000, KY Srinivasan wrote:
> > - the instances of hv_driver structures need to be static and
> > not programatically defined, like all other USB and PCI
> > drivers are handled.
>
> Done. You had expressed some concern that this would expose some issue
> with the core vmbus driver (which is what I want to concentrate on this
> go around). I have done this for both the block driver and the mouse driver
> and I am pretty sure I can do the same with the network driver. I have not
> currently done this for the network driver, since the number of patches I have
> to submit is already very large.
Ok, but it shouldn't be a major change to the code, right?
> > - 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?
>
> I have already exchanged an email with you on this. To summarize, it
> does not look like there is a problem
>
> > - fix the sparse warnings.
> Mostly done; most of the errors are in the base kernel coming out of
> the macro page_to_pfn()
>
> > - 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.
>
> You are right; all accesses were already serialized with a spin lock and the
> Volatile attribute was unnecessary.
>
> > - fix the namespace on the ringbuffer code to show that it
> > really is only for the hyperv bus code internally.
>
> Done.
>
> >
> > That should be enough for at least one more set of patches for you all
> > to work on :)
>
> Greg,
>
> I have had so much fun cleaning up these drivers that I lost track of the patch count.
> I have addressed all the issues you have raised in addition to some other cleanup
> that I was doing since about a week. As I look at the patch-set, I have little over
> 200 patches. If it is ok with you, I would like to send them as a single set. Let me know
> what you prefer. I need to circulate these patches internally before I can send them out.
> I should be able to send these out early next week.
A single set is fine, if that's what you want to do, I can handle that
amount as long as they all build all along the way and don't break
anything.
thanks,
greg k-h
WARNING: multiple messages have this Message-ID (diff)
From: Greg KH <greg@kroah.com>
To: KY Srinivasan <kys@microsoft.com>
Cc: "gregkh@suse.de" <gregkh@suse.de>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"devel@linuxdriverproject.org" <devel@linuxdriverproject.org>,
"virtualization@lists.osdl.org" <virtualization@lists.osdl.org>
Subject: Re: various vmbus review comments
Date: Sun, 8 May 2011 20:04:52 -0700 [thread overview]
Message-ID: <20110509030452.GA17182@kroah.com> (raw)
In-Reply-To: <6E21E5352C11B742B20C142EB499E0481E9404@TK5EX14MBXC128.redmond.corp.microsoft.com>
On Mon, May 09, 2011 at 01:46:56AM +0000, KY Srinivasan wrote:
> > - the instances of hv_driver structures need to be static and
> > not programatically defined, like all other USB and PCI
> > drivers are handled.
>
> Done. You had expressed some concern that this would expose some issue
> with the core vmbus driver (which is what I want to concentrate on this
> go around). I have done this for both the block driver and the mouse driver
> and I am pretty sure I can do the same with the network driver. I have not
> currently done this for the network driver, since the number of patches I have
> to submit is already very large.
Ok, but it shouldn't be a major change to the code, right?
> > - 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?
>
> I have already exchanged an email with you on this. To summarize, it
> does not look like there is a problem
>
> > - fix the sparse warnings.
> Mostly done; most of the errors are in the base kernel coming out of
> the macro page_to_pfn()
>
> > - 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.
>
> You are right; all accesses were already serialized with a spin lock and the
> Volatile attribute was unnecessary.
>
> > - fix the namespace on the ringbuffer code to show that it
> > really is only for the hyperv bus code internally.
>
> Done.
>
> >
> > That should be enough for at least one more set of patches for you all
> > to work on :)
>
> Greg,
>
> I have had so much fun cleaning up these drivers that I lost track of the patch count.
> I have addressed all the issues you have raised in addition to some other cleanup
> that I was doing since about a week. As I look at the patch-set, I have little over
> 200 patches. If it is ok with you, I would like to send them as a single set. Let me know
> what you prefer. I need to circulate these patches internally before I can send them out.
> I should be able to send these out early next week.
A single set is fine, if that's what you want to do, I can handle that
amount as long as they all build all along the way and don't break
anything.
thanks,
greg k-h
next prev parent reply other threads:[~2011-05-09 3:04 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-05-03 20:46 various vmbus review comments Greg KH
2011-05-03 21:00 ` 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 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:33 ` Christoph Hellwig
2011-05-09 14:56 ` KY Srinivasan
2011-05-10 5:24 ` Christoph Hellwig
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 [this message]
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=20110509030452.GA17182@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.