Re: [PATCH 2/2] radix-tree: Fix optimisation problem

2016-09-26 Thread Cedric Blancher
You might also try to use valid, plain ISO C99 instead of perverted gcc extensions which only cause a lot of trouble in the long run. Ced On 26 September 2016 at 23:28, Matthew Wilcox wrote: > From: linus...@gmail.com [mailto:linus...@gmail.com] On Behalf Of Linus > Torvalds >> On Sun, Sep 25,

RE: [PATCH 2/2] radix-tree: Fix optimisation problem

2016-09-26 Thread Matthew Wilcox
From: linus...@gmail.com [mailto:linus...@gmail.com] On Behalf Of Linus Torvalds > On Sun, Sep 25, 2016 at 12:04 PM, Linus Torvalds > wrote: > >It gets rid of > > the ad-hoc arithmetic in radix_tree_descend(), and just makes all that > > be inside the is_sibling_entry() logic instead. Whic

Re: [PATCH 2/2] radix-tree: Fix optimisation problem

2016-09-25 Thread Linus Torvalds
On Sun, Sep 25, 2016 at 12:04 PM, Linus Torvalds wrote: >It gets rid of > the ad-hoc arithmetic in radix_tree_descend(), and just makes all that > be inside the is_sibling_entry() logic instead. Which got renamed and > made to actually return the main sibling. Sadly, it looks like gcc gen

Re: [PATCH 2/2] radix-tree: Fix optimisation problem

2016-09-25 Thread Cedric Blancher
LGTM, except that #define is_sibling_entry should be IS_SIBLING_ENTRY Ced On 25 September 2016 at 21:04, Linus Torvalds wrote: > On Sun, Sep 25, 2016 at 11:04 AM, Linus Torvalds > wrote: >> >> The more I look at that particular piece of code, the less I like it. It's >> buggy shit. It needs to

Re: [PATCH 2/2] radix-tree: Fix optimisation problem

2016-09-25 Thread Linus Torvalds
On Sun, Sep 25, 2016 at 11:04 AM, Linus Torvalds wrote: > > The more I look at that particular piece of code, the less I like it. It's > buggy shit. It needs to be rewritten entirely too actually check for sibling > entries, not that ad-hoc arithmetic crap. Here's my attempt at cleaning the mess

Re: [PATCH 2/2] radix-tree: Fix optimisation problem

2016-09-25 Thread Cedric Blancher
On 25 September 2016 at 02:18, Linus Torvalds wrote: > On Sat, Sep 24, 2016 at 4:35 PM, Cedric Blancher > wrote: >>> >>> void *entry = parent->slots[offset]; >>> int siboff = entry - parent->slots; >> >> If entry is a pointer to void, how can you do pointer arithmetic with it? > >

Re: [PATCH 2/2] radix-tree: Fix optimisation problem

2016-09-24 Thread Linus Torvalds
On Sat, Sep 24, 2016 at 4:35 PM, Cedric Blancher wrote: >> >> void *entry = parent->slots[offset]; >> int siboff = entry - parent->slots; > > If entry is a pointer to void, how can you do pointer arithmetic with it? It's actually void **. (That said, gcc has an extension that con

Re: [PATCH 2/2] radix-tree: Fix optimisation problem

2016-09-24 Thread Cedric Blancher
On 22 September 2016 at 20:53, Matthew Wilcox wrote: > From: Matthew Wilcox > > When compiling the radix tree with -O2, GCC thinks it can optimise: > > void *entry = parent->slots[offset]; > int siboff = entry - parent->slots; If entry is a pointer to void, how can you do pointer

Re: [PATCH 2/2] radix-tree: Fix optimisation problem

2016-09-24 Thread Linus Torvalds
On Sat, Sep 24, 2016 at 2:04 PM, Kirill A. Shutemov wrote: > > Well, my ext4-with-huge-pages patchset[1] uses multi-order entries. > It also converts shmem-with-huge-pages and hugetlb to them. Ok, so that code actually has a chance of being used. I guess we'll not remove it. But I *would* like th

Re: [PATCH 2/2] radix-tree: Fix optimisation problem

2016-09-24 Thread Kirill A. Shutemov
On Sat, Sep 24, 2016 at 01:21:36PM -0700, Linus Torvalds wrote: > On Fri, Sep 23, 2016 at 1:16 PM, Matthew Wilcox > wrote: > > > > #ifdef CONFIG_RADIX_TREE_MULTIORDER > > if (radix_tree_is_internal_node(entry)) { > > - unsigned long siboff = get_slot_offset(parent, entry);

Re: [PATCH 2/2] radix-tree: Fix optimisation problem

2016-09-24 Thread Linus Torvalds
On Sat, Sep 24, 2016 at 1:21 PM, Linus Torvalds wrote: > > That said, if this code isn't even used, as Konstantin says (THP > selects it - doesn't THP use it?), then the fix really should be to > just remove the odd code instead of adding to it. > > Looking around for uses that set "order" to anyt

Re: [PATCH 2/2] radix-tree: Fix optimisation problem

2016-09-24 Thread Linus Torvalds
On Fri, Sep 23, 2016 at 1:16 PM, Matthew Wilcox wrote: > > #ifdef CONFIG_RADIX_TREE_MULTIORDER > if (radix_tree_is_internal_node(entry)) { > - unsigned long siboff = get_slot_offset(parent, entry); > + unsigned long siboff = get_slot_offset(parent, > +

Re: [PATCH 2/2] radix-tree: Fix optimisation problem

2016-09-24 Thread Konstantin Khlebnikov
On Thu, Sep 22, 2016 at 9:53 PM, Matthew Wilcox wrote: > From: Matthew Wilcox > > When compiling the radix tree with -O2, GCC thinks it can optimise: > > void *entry = parent->slots[offset]; > int siboff = entry - parent->slots; > void *slot = parent->slots + siboff; > > i

RE: [PATCH 2/2] radix-tree: Fix optimisation problem

2016-09-23 Thread Matthew Wilcox
From: linus...@gmail.com [mailto:linus...@gmail.com] On Behalf Of Linus Torvalds > On Thu, Sep 22, 2016 at 11:53 AM, Matthew Wilcox > wrote: > > > > Change the test suite to compile with -O2, and > > fix the optimisation problem by passing 'entry' through entry_to_node() > > so gcc knows

Re: [PATCH 2/2] radix-tree: Fix optimisation problem

2016-09-22 Thread Linus Torvalds
On Thu, Sep 22, 2016 at 11:53 AM, Matthew Wilcox wrote: > > Change the test suite to compile with -O2, and > fix the optimisation problem by passing 'entry' through entry_to_node() > so gcc knows this isn't a plain pointer. Ugh. I really don't like this patch very much. Wouldn't it be