Re: [Xen-devel] [resend PATCH] xen: common: rbtree: ported updates from linux tree

[Dario Faggioli <dario.faggioli@xxxxxxxxxx>]

On Thu, 2017-05-18 at 07:59 -0600, Jan Beulich wrote:
> > > > On 11.05.17 at 19:21, <kpraveen.lkml@xxxxxxxxx> wrote:
> > 
> > The patch contains the updated version of rbtree implementation
> > from linux
> > kernel tree containing the fixes so far handled.
> 
> I suppose this isn't just fixes, but also enhancements. Furthermore
> I'd appreciate if you recorded the Linux version this was taken from,
> so that anyone wanting to do another upgrade would know what
> the baseline is. In any event, as long as this is just a general
> overhaul and upgrade, I'd like to either see individual bugs pointed
> out which get fixed _and_ which affect us, or I'd expect this to be
> part of a series which actually requires some of the new
> functionality.
>
I fully agree.

And in fact, this is actually quite a big patch, and does (although it
touches only a few files) a bunch of different things (new
functionalities, improved comments, etc).

So, Jan, would it be ok for this thing that Praveen is trying to do, to
be a series, with one patch for each original Linux commit? I think, if
it were me doing this, that would be how I'd do it.

Otherwise it is e.g. hard to understand why ...
> 
> > Signed-off-by: Praveen Kumar <kpraveen.lkml@xxxxxxxxx>
> > ---
> >  xen/common/rbtree.c                | 748
> > +++++++++++++++++++++++++------------
> >  xen/include/xen/compiler.h         |  60 +++
> >  xen/include/xen/rbtree.h           | 120 ++++--
> >  xen/include/xen/rbtree_augmented.h | 283 ++++++++++++++
> 
> ... namely this last (new) header (and what it provides) is needed
> at all.
> 
Indeed. And in fact, for our original purpose (which is to use rb-trees 
instead of linked lists for Credit2's runqueues), I don't think we
actually need the augmented variant.

Praveen, as we agreed on IRC, it is ok to send this patch (which I
think should have been a patch series) first, but stating why you are
actually doing this (i.e., a few words on the original purpose I'm
mentioning above), is really useful, to set the context, and should be
there (in the cover letter or a follow up email).

Also, do Cc me please (in addition to what get_maintainers.pl
says). :-)

Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

Attachment: signature.asc
Description: This is a digitally signed message part

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel