[PATCH 126/141] staging: unisys: Convert the epilog functions to visor_device

Greg KH gregkh at linuxfoundation.org
Fri May 8 16:44:18 UTC 2015


On Fri, May 08, 2015 at 12:06:37PM -0400, Don Zickus wrote:
> On Fri, May 08, 2015 at 04:02:06PM +0200, Greg KH wrote:
> > On Fri, May 08, 2015 at 09:56:41AM -0400, Don Zickus wrote:
> > > On Fri, May 08, 2015 at 11:00:18AM +0300, Dan Carpenter wrote:
> > > > On Tue, May 05, 2015 at 06:37:43PM -0400, Benjamin Romer wrote:
> > > > > From: Don Zickus <dzickus at redhat.com>
> > > > > @@ -1128,7 +1119,7 @@ device_epilog(u32 bus_no, u32 dev_no, struct spar_segment_state state, u32 cmd,
> > > > >  		switch (cmd) {
> > > > >  		case CONTROLVM_DEVICE_CREATE:
> > > > >  			if (notifiers->device_create) {
> > > > > -				(*notifiers->device_create) (bus_no, dev_no);
> > > > > +				(*notifiers->device_create) (dev_info);
> > > > >  				notified = true;
> > > > >  			}
> > > > >  			break;
> > > > 
> > > > This doesn't work.  We don't change the (*notifiers->device_create)
> > > > function pointer until the next patch.
> > > 
> > > Correct.  The changes were so intertwined that I could either post a giant
> > > monolithic patch and maintain bisectability or break things into smaller
> > > chunks for review purpose but lose the ability to bisect.
> > > 
> > > I chose the later.  The weak argument we used is that no one will enable
> > > this driver (except unisys) so it normally won't break bisecting by others.
> > 
> > Sorry, but no, you can't break the build at any point in any patch.
> 
> Alright, then I will ask Ben to fold the 5 patches together. :-(

How about reworking them in a different way?  There's lots of ways to
evolve code that doesn't require folding things all together.

Yes, it takes more work, but it makes reviewing stuff easier, which is
the most important thing here.

greg k-h


More information about the devel mailing list