[PATCH V2 1/6] drivers/staging/ramster: cluster/messaging foundation

[PATCH V2 1/6] drivers/staging/ramster: cluster/messaging foundation

[Dan Magenheimer]

Re: [PATCH V2 1/6] drivers/staging/ramster: cluster/messaging foundation

[Greg KH]

On Thu, Dec 22, 2011 at 07:50:50AM -0800, Dan Magenheimer wrote:
> >From 93c00028709a5d423de77a2fc24d32ec10eca443 Mon Sep 17 00:00:00 2001
> From: Dan Magenheimer <dan.magenheimer@oracle.com>
> Date: Wed, 21 Dec 2011 14:01:54 -0700
> Subject: [PATCH V2 1/6] drivers/staging/ramster: cluster/messaging foundation

Why duplicate this in the email body?  That forces me to edit the
patches and remove them, please use git send-email...

> Copy cluster subdirectory from ocfs2.  These files implement
> the basic cluster discovery, mapping, heartbeat / keepalive, and
> messaging ("o2net") that ramster requires for internode communication.
> Note: there are NO ramster-specific changes yet; this commit
> does NOT pass checkpatch since the copied source files do not.

Why are you copying the files, and not just exporting the symbols you
need/want to use here?  Are you going to be able to properly track
keeping this code in sync?

Same goes for your other patch in this series that copies code, why do
that?  Are there goals to eventually not have duplicated code?  If so,
what are they, and why not mention them?

thanks,

greg k-h

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

RE: [PATCH V2 1/6] drivers/staging/ramster: cluster/messaging foundation

[Dan Magenheimer]

> From: Greg KH [mailto:greg@kroah.com]

Thanks for the quick response!

> Sent: Thursday, December 22, 2011 10:31 AM
> To: Dan Magenheimer
> Cc: devel@driverdev.osuosl.org; linux-kernel@vger.kernel.org; linux-mm@kvack.org; ngupta@vflare.org;
> Konrad Wilk; Kurt Hackel; sjenning@linux.vnet.ibm.com; Chris Mason
> Subject: Re: [PATCH V2 1/6] drivers/staging/ramster: cluster/messaging foundation
> 
> On Thu, Dec 22, 2011 at 07:50:50AM -0800, Dan Magenheimer wrote:
> > >From 93c00028709a5d423de77a2fc24d32ec10eca443 Mon Sep 17 00:00:00 2001
> > From: Dan Magenheimer <dan.magenheimer@oracle.com>
> > Date: Wed, 21 Dec 2011 14:01:54 -0700
> > Subject: [PATCH V2 1/6] drivers/staging/ramster: cluster/messaging foundation
> 
> Why duplicate this in the email body?  That forces me to edit the
> patches and remove them, please use git send-email...

OK, sorry, will do.  Still learning the git tools and my mutt
scripts use that line to create the subject line.  I'll try
git-send-email next time.
 
> > Copy cluster subdirectory from ocfs2.  These files implement
> > the basic cluster discovery, mapping, heartbeat / keepalive, and
> > messaging ("o2net") that ramster requires for internode communication.
> > Note: there are NO ramster-specific changes yet; this commit
> > does NOT pass checkpatch since the copied source files do not.
> 
> Why are you copying the files, and not just exporting the symbols you
> need/want to use here?  Are you going to be able to properly track
> keeping this code in sync?

This particular part of ocfs2 has never been broken out for non-ocfs2
use before, some changes to the ocfs2 core cluster code is necessary
for ramster to use that code (see patch 3), and ramster is currently
incompatible with real ocfs2 anyway (requires !CONFIG_OCFS2_FS).  I will
definitely work with Joel Becker to see if these code interdependencies
can be merged before ramster could possibly be promoted out of staging but,
for now for staging, this seemed to be an expedient way to make
use of the ocfs2 core cluster code but still incorporate some required
ramster changes.  This way, also, I think it is not necessary to keep the
code in sync every release, but still allow for easy merging later.

> Same goes for your other patch in this series that copies code, why do
> that?

The other code copies are from drivers/staging/zcache.  The tmem.c/h
changes can definitely be shared between zcache and ramster (and
I've said before that the eventual destination tmem.c/h is the linux
lib directory).  The zcache.c changes are most likely mergeable but
I know that Seth Jennings and Nitin Gupta are working on some
other significant changes (including a new allocator which would
replace the lengthy zbud code and breaking zcache.c into many smaller
files), so thought it best to branch temporarily and merge later.

> Are there goals to eventually not have duplicated code?  If so,
> what are they, and why not mention them?

Sorry, you're right, I should have included the above paragraphs
in the commit comments.

If you disagree with any of this, please let me know.

Thanks and happy holidays!
Dan

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH V2 1/6] drivers/staging/ramster: cluster/messaging foundation

[Greg KH]

On Thu, Dec 22, 2011 at 02:06:26PM -0800, Dan Magenheimer wrote:
> > From: Greg KH [mailto:greg@kroah.com]
> 
> Thanks for the quick response!
> 
> > Sent: Thursday, December 22, 2011 10:31 AM
> > To: Dan Magenheimer
> > Cc: devel@driverdev.osuosl.org; linux-kernel@vger.kernel.org; linux-mm@kvack.org; ngupta@vflare.org;
> > Konrad Wilk; Kurt Hackel; sjenning@linux.vnet.ibm.com; Chris Mason
> > Subject: Re: [PATCH V2 1/6] drivers/staging/ramster: cluster/messaging foundation
> > 
> > On Thu, Dec 22, 2011 at 07:50:50AM -0800, Dan Magenheimer wrote:
> > > >From 93c00028709a5d423de77a2fc24d32ec10eca443 Mon Sep 17 00:00:00 2001
> > > From: Dan Magenheimer <dan.magenheimer@oracle.com>
> > > Date: Wed, 21 Dec 2011 14:01:54 -0700
> > > Subject: [PATCH V2 1/6] drivers/staging/ramster: cluster/messaging foundation
> > 
> > Why duplicate this in the email body?  That forces me to edit the
> > patches and remove them, please use git send-email...
> 
> OK, sorry, will do.  Still learning the git tools and my mutt
> scripts use that line to create the subject line.  I'll try
> git-send-email next time.
>  
> > > Copy cluster subdirectory from ocfs2.  These files implement
> > > the basic cluster discovery, mapping, heartbeat / keepalive, and
> > > messaging ("o2net") that ramster requires for internode communication.
> > > Note: there are NO ramster-specific changes yet; this commit
> > > does NOT pass checkpatch since the copied source files do not.
> > 
> > Why are you copying the files, and not just exporting the symbols you
> > need/want to use here?  Are you going to be able to properly track
> > keeping this code in sync?
> 
> This particular part of ocfs2 has never been broken out for non-ocfs2
> use before, some changes to the ocfs2 core cluster code is necessary
> for ramster to use that code (see patch 3), and ramster is currently
> incompatible with real ocfs2 anyway (requires !CONFIG_OCFS2_FS).  I will
> definitely work with Joel Becker to see if these code interdependencies
> can be merged before ramster could possibly be promoted out of staging but,
> for now for staging, this seemed to be an expedient way to make
> use of the ocfs2 core cluster code but still incorporate some required
> ramster changes.  This way, also, I think it is not necessary to keep the
> code in sync every release, but still allow for easy merging later.
> 
> > Same goes for your other patch in this series that copies code, why do
> > that?
> 
> The other code copies are from drivers/staging/zcache.  The tmem.c/h
> changes can definitely be shared between zcache and ramster (and
> I've said before that the eventual destination tmem.c/h is the linux
> lib directory).  The zcache.c changes are most likely mergeable but
> I know that Seth Jennings and Nitin Gupta are working on some
> other significant changes (including a new allocator which would
> replace the lengthy zbud code and breaking zcache.c into many smaller
> files), so thought it best to branch temporarily and merge later.
> 
> > Are there goals to eventually not have duplicated code?  If so,
> > what are they, and why not mention them?
> 
> Sorry, you're right, I should have included the above paragraphs
> in the commit comments.

Ok, that makes sense.

Can you ensure that the TODO file in this driver's directory says that
you will remove the duplicated code from it before it can be merged into
the main part of the kernel tree?

That, and fix up the other things mentioned and resend it and I'll be
glad to queue it up.

thanks,

greg k-h

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

RE: [PATCH V2 1/6] drivers/staging/ramster: cluster/messaging foundation

[Dan Magenheimer]

> From: Greg KH [mailto:greg@kroah.com]
> 
> Ok, that makes sense.
> 
> Can you ensure that the TODO file in this driver's directory says that
> you will remove the duplicated code from it before it can be merged into
> the main part of the kernel tree?
> 
> That, and fix up the other things mentioned and resend it and I'll be
> glad to queue it up.

Done and reposted, but....

ARGH! Greg, somehow I left your direct email address off of the To list
for the repost!  Will you be able to get them from the drivers (or mm
or lkml) list, or will you need me to re-post them (just to you or
also again to the same To list?)

Sorry,
Dan

P.S. V3 starts with: https://lkml.org/lkml/2011/12/27/158 or
http://driverdev.linuxdriverproject.org/pipermail/devel/2011-December/023736.html 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH V2 1/6] drivers/staging/ramster: cluster/messaging foundation

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 422 bytes --]

On Thu, 22 Dec 2011 07:50:50 PST, Dan Magenheimer said:

> Copy cluster subdirectory from ocfs2.  These files implement
> the basic cluster discovery, mapping, heartbeat / keepalive, and
> messaging ("o2net") that ramster requires for internode communication.

Instead of doing this, can we have the shared files copied to a common
subdirectory so that ramster and ocfs2 can share them, and we only
have to fix bugs once?

[-- Attachment #2: Type: application/pgp-signature, Size: 227 bytes --]

RE: [PATCH V2 1/6] drivers/staging/ramster: cluster/messaging foundation

[Dan Magenheimer]

> From: Valdis.Kletnieks@vt.edu [mailto:Valdis.Kletnieks@vt.edu]
> Sent: Tuesday, December 27, 2011 1:06 PM
> To: Dan Magenheimer
> Cc: greg@kroah.com; devel@driverdev.osuosl.org; linux-kernel@vger.kernel.org; linux-mm@kvack.org;
> ngupta@vflare.org; Konrad Wilk; Kurt Hackel; sjenning@linux.vnet.ibm.com; Chris Mason
> Subject: Re: [PATCH V2 1/6] drivers/staging/ramster: cluster/messaging foundation
> 
> On Thu, 22 Dec 2011 07:50:50 PST, Dan Magenheimer said:
> 
> > Copy cluster subdirectory from ocfs2.  These files implement
> > the basic cluster discovery, mapping, heartbeat / keepalive, and
> > messaging ("o2net") that ramster requires for internode communication.
> 
> Instead of doing this, can we have the shared files copied to a common
> subdirectory so that ramster and ocfs2 can share them, and we only
> have to fix bugs once?

Hi Valdis --

Thanks for your reply!

Per the discussion at:
https://lkml.org/lkml/2011/12/22/369 
your suggestion of the common subdirectory will definitely need to happen
before ramster can be promoted out of staging and, at GregKH's request,
I have added a TODO file in V3 to state that.  Before that can happen,
we'll need to work with the ocfs2 maintainers to merge the
necessary ramster-specific changes and implement a separately CONFIG-able
subdirectory for the ocfs2 cluster code.

Dan

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>