2014-12-29 22:13:50

by Dave Kleikamp

[permalink] [raw]
Subject: Re: Remove TO DO in jfs_xtree.c

On 12/27/2014 06:58 PM, nick wrote:
> Greetings Dave,
> I am wondering why there is a TO DO above this code:
> * ToDo: tlocks should be on doubly-linked list, so we can
> * quickly remove it and add it to the end.

I'm sure the idea was to avoid the for loop needed to find the previous
entry in the linked list. A doubly-linked list makes it much simpler to
remove an item from an arbitrary position in the list.

> */
>
> /*
> * Move parent page's tlock to the end of the tid's tlock list
> */
> if (log && mp->lid && (tblk->last != mp->lid) &&
> lid_to_tlock(mp->lid)->tid) {
> lid_t lid = mp->lid;
> struct tlock *prev;
>
> tlck = lid_to_tlock(lid);
>
> if (tblk->next == lid)
> tblk->next = tlck->next;
> else {
> for (prev = lid_to_tlock(tblk->next);
> prev->next != lid;
> prev = lid_to_tlock(prev->next)) {
> assert(prev->next);
> }
> prev->next = tlck->next;
> }
> lid_to_tlock(tblk->last)->next = lid;
> tlck->next = 0;
> tblk->last = lid;
> }
> As this code clearly moves the locks onto a linked list. Therefore I am recommend we remove this
> TO DO as this is clearly misleading and no longer needed.

That comment has been in the code forever and I don't have any intention
of changing things, but I don't think it's wrong. I don't mind removing
it, though. jfs has been barely maintained and there is a ton of cleanup
that can be done if someone were willing to take the time to do it.

Thanks,
Shaggy


2014-12-30 03:48:13

by Theodore Ts'o

[permalink] [raw]
Subject: Re: Remove TO DO in jfs_xtree.c

On Mon, Dec 29, 2014 at 04:13:37PM -0600, Dave Kleikamp wrote:
> On 12/27/2014 06:58 PM, nick wrote:
> > Greetings Dave,
> > I am wondering why there is a TO DO above this code:
> > * ToDo: tlocks should be on doubly-linked list, so we can
> > * quickly remove it and add it to the end.
>
> I'm sure the idea was to avoid the for loop needed to find the previous
> entry in the linked list. A doubly-linked list makes it much simpler to
> remove an item from an arbitrary position in the list.

Hi Dave,

Just in case you weren't aware, Nick has been banned from the LKML
list for being a troll.

A common troll trick is to send e-mail to a number of individuals with
a mailing list (in this case, LKML) cc'ed, in the hopes that people
will reply, quoting the troll's words, so they can get around the
mailing list ban.

The reason why he has been banned is because he has apparently been
desperate to get _some_ patches into the Linux kernel. Enough so that
he has proposed patches which do not compile, and/or were not tested
and/or for which he had no hardware (so he couldn't possibly have
tested it).

As a maintainer, you should be aware of his past history, and chose
for yourself whether, in the future, you feel you should respond to
any e-mail that he might send you.

Regards,

- Ted

P.S. Personally, the best reason for banning him isn't that he has
been wasting maintainers' time, but that he was responding to users
who were reporting bugs with nonsensical responses that were actively
harmful to users who were looking for help.

2014-12-30 04:03:13

by Dave Kleikamp

[permalink] [raw]
Subject: Re: Remove TO DO in jfs_xtree.c

On 12/29/2014 09:48 PM, [email protected] wrote:
> On Mon, Dec 29, 2014 at 04:13:37PM -0600, Dave Kleikamp wrote:
>> On 12/27/2014 06:58 PM, nick wrote:
>>> Greetings Dave,
>>> I am wondering why there is a TO DO above this code:
>>> * ToDo: tlocks should be on doubly-linked list, so we can
>>> * quickly remove it and add it to the end.
>>
>> I'm sure the idea was to avoid the for loop needed to find the previous
>> entry in the linked list. A doubly-linked list makes it much simpler to
>> remove an item from an arbitrary position in the list.
>
> Hi Dave,
>
> Just in case you weren't aware, Nick has been banned from the LKML
> list for being a troll.

Thanks, Ted. Now I remember some earlier threads. Forgot it was the same
person. Just figured him for a novice looking for something to contribute.

Thanks,
Shaggy