Re: [PATCH] UBI WL-Subsystem: Improvement in prot tree

[Artem Bityutskiy <dedekind@infradead.org>]

On Mon, 2008-12-08 at 16:15 +0800, xiaochuan-xu wrote:
> Hi, all.
> 
> A new PEB protection method in UBI WL-Subsystem is implemented,
> It's simpler and higher efficiency than the older prot RB-tree, I think.
> 
> 1. without two prot RB-tree, there is only one prot array, But their
> functions are the same.
> 
> 2. no other structure needed except @ubi_wl_entry ubi_wl_prot_entry is
> discarded. and we need not malloc new struct every time in
> ubi_wl_get_peb() function.
> 
> 3. protarray add and del operation are O(1) operations, and check over
> opteration is O(n), which is better then the older prot RB-tree
> implement.

Hi,

yeah, I like the idea. Indeed there is no reason to have balanced trees
for this "protection" stuff, and a list should be enough. The list does
not have to be long, 8-16 entries are enough, so search should be quick
enough. Every time an eraseblock is erased, we take one element from the
head of the list, and we add new elements to the head of the list, so it
acts as a queue.

I'll accept such a change, but not with this patch, because there are
problems. Here are some of them.

1. It is line-wrapped. Please, learn how to send patches correctly.
Please, try send it to yourself, then try to apply what you have
received.

2. Let's call it "protection list", not array.

3. Please, split your patch on several pieces. One obvious piece would
be introducing the union to

 struct ubi_wl_entry {
-       struct rb_node rb;
+       union {
+               struct rb_node rb;
+       } u;

and at the nest patches you add a list to the union. This way you will
simplify further patches and make them easier to review because they
will not contain huge number of changes like

-               e1 = rb_entry(p, struct ubi_wl_entry, rb);
+               e1 = rb_entry(p, struct ubi_wl_entry, u.rb);

-- 
Best regards,
Artem Bityutskiy (Битюцкий Артём)