Skip to content

Commit cfad9ce

Browse files
George SpelvinSomasundaram Krishnasamy
authored andcommitted
lib/list_sort: simplify and remove MAX_LIST_LENGTH_BITS
[ Upstream commit 043b3f7 ] Rather than a fixed-size array of pending sorted runs, use the ->prev links to keep track of things. This reduces stack usage, eliminates some ugly overflow handling, and reduces the code size. Also: * merge() no longer needs to handle NULL inputs, so simplify. * The same applies to merge_and_restore_back_links(), which is renamed to the less ponderous merge_final(). (It's a static helper function, so we don't need a super-descriptive name; comments will do.) * Document the actual return value requirements on the (*cmp)() function; some callers are already using this feature. x86-64 code size 1086 -> 739 bytes (-347) (Yes, I see checkpatch complaining about no space after comma in "__attribute__((nonnull(2,3,4,5)))". Checkpatch is wrong.) Feedback from Rasmus Villemoes, Andy Shevchenko and Geert Uytterhoeven. [[email protected]: remove __pure usage due to mysterious warning] Link: http://lkml.kernel.org/r/f63c410e0ff76009c9b58e01027e751ff7fdb749.1552704200.git.lkml@sdf.org Signed-off-by: George Spelvin <[email protected]> Acked-by: Andrey Abramov <[email protected]> Acked-by: Rasmus Villemoes <[email protected]> Reviewed-by: Andy Shevchenko <[email protected]> Cc: Geert Uytterhoeven <[email protected]> Cc: Daniel Wagner <[email protected]> Cc: Dave Chinner <[email protected]> Cc: Don Mullis <[email protected]> Signed-off-by: Andrew Morton <[email protected]> Signed-off-by: Linus Torvalds <[email protected]> (cherry picked from commit 043b3f7) Orabug: 28894138 Signed-off-by: Thomas Tai <[email protected]> Reviewed-by: Tom Saeger <[email protected]> Signed-off-by: Somasundaram Krishnasamy <[email protected]>
1 parent 3e344ab commit cfad9ce

File tree

2 files changed

+104
-62
lines changed

2 files changed

+104
-62
lines changed

include/linux/list_sort.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
struct list_head;
88

9+
__attribute__((nonnull(2,3)))
910
void list_sort(void *priv, struct list_head *head,
1011
int (*cmp)(void *priv, struct list_head *a,
1112
struct list_head *b));

lib/list_sort.c

Lines changed: 103 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -7,33 +7,41 @@
77
#include <linux/list_sort.h>
88
#include <linux/list.h>
99

10-
#define MAX_LIST_LENGTH_BITS 20
10+
typedef int __attribute__((nonnull(2,3))) (*cmp_func)(void *,
11+
struct list_head const *, struct list_head const *);
1112

1213
/*
1314
* Returns a list organized in an intermediate format suited
1415
* to chaining of merge() calls: null-terminated, no reserved or
1516
* sentinel head node, "prev" links not maintained.
1617
*/
17-
static struct list_head *merge(void *priv,
18-
int (*cmp)(void *priv, struct list_head *a,
19-
struct list_head *b),
18+
__attribute__((nonnull(2,3,4)))
19+
static struct list_head *merge(void *priv, cmp_func cmp,
2020
struct list_head *a, struct list_head *b)
2121
{
22-
struct list_head head, *tail = &head;
22+
struct list_head *head, **tail = &head;
2323

24-
while (a && b) {
24+
for (;;) {
2525
/* if equal, take 'a' -- important for sort stability */
26-
if ((*cmp)(priv, a, b) <= 0) {
27-
tail->next = a;
26+
if (cmp(priv, a, b) <= 0) {
27+
*tail = a;
28+
tail = &a->next;
2829
a = a->next;
30+
if (!a) {
31+
*tail = b;
32+
break;
33+
}
2934
} else {
30-
tail->next = b;
35+
*tail = b;
36+
tail = &b->next;
3137
b = b->next;
38+
if (!b) {
39+
*tail = a;
40+
break;
41+
}
3242
}
33-
tail = tail->next;
3443
}
35-
tail->next = a?:b;
36-
return head.next;
44+
return head;
3745
}
3846

3947
/*
@@ -43,44 +51,52 @@ static struct list_head *merge(void *priv,
4351
* prev-link restoration pass, or maintaining the prev links
4452
* throughout.
4553
*/
46-
static void merge_and_restore_back_links(void *priv,
47-
int (*cmp)(void *priv, struct list_head *a,
48-
struct list_head *b),
49-
struct list_head *head,
50-
struct list_head *a, struct list_head *b)
54+
__attribute__((nonnull(2,3,4,5)))
55+
static void merge_final(void *priv, cmp_func cmp, struct list_head *head,
56+
struct list_head *a, struct list_head *b)
5157
{
5258
struct list_head *tail = head;
5359
u8 count = 0;
5460

55-
while (a && b) {
61+
for (;;) {
5662
/* if equal, take 'a' -- important for sort stability */
57-
if ((*cmp)(priv, a, b) <= 0) {
63+
if (cmp(priv, a, b) <= 0) {
5864
tail->next = a;
5965
a->prev = tail;
66+
tail = a;
6067
a = a->next;
68+
if (!a)
69+
break;
6170
} else {
6271
tail->next = b;
6372
b->prev = tail;
73+
tail = b;
6474
b = b->next;
75+
if (!b) {
76+
b = a;
77+
break;
78+
}
6579
}
66-
tail = tail->next;
6780
}
68-
tail->next = a ? : b;
6981

82+
/* Finish linking remainder of list b on to tail */
83+
tail->next = b;
7084
do {
7185
/*
72-
* In worst cases this loop may run many iterations.
86+
* If the merge is highly unbalanced (e.g. the input is
87+
* already sorted), this loop may run many iterations.
7388
* Continue callbacks to the client even though no
7489
* element comparison is needed, so the client's cmp()
7590
* routine can invoke cond_resched() periodically.
7691
*/
77-
if (unlikely(!(++count)))
78-
(*cmp)(priv, tail->next, tail->next);
79-
80-
tail->next->prev = tail;
81-
tail = tail->next;
82-
} while (tail->next);
83-
92+
if (unlikely(!++count))
93+
cmp(priv, b, b);
94+
b->prev = tail;
95+
tail = b;
96+
b = b->next;
97+
} while (b);
98+
99+
/* And the final links to make a circular doubly-linked list */
84100
tail->next = head;
85101
head->prev = tail;
86102
}
@@ -91,55 +107,80 @@ static void merge_and_restore_back_links(void *priv,
91107
* @head: the list to sort
92108
* @cmp: the elements comparison function
93109
*
94-
* This function implements "merge sort", which has O(nlog(n))
95-
* complexity.
110+
* This function implements a bottom-up merge sort, which has O(nlog(n))
111+
* complexity. We use depth-first order to take advantage of cacheing.
112+
* (E.g. when we get to the fourth element, we immediately merge the
113+
* first two 2-element lists.)
114+
*
115+
* The comparison funtion @cmp must return > 0 if @a should sort after
116+
* @b ("@a > @b" if you want an ascending sort), and <= 0 if @a should
117+
* sort before @b *or* their original order should be preserved. It is
118+
* always called with the element that came first in the input in @a,
119+
* and list_sort is a stable sort, so it is not necessary to distinguish
120+
* the @a < @b and @a == @b cases.
96121
*
97-
* The comparison function @cmp must return a negative value if @a
98-
* should sort before @b, and a positive value if @a should sort after
99-
* @b. If @a and @b are equivalent, and their original relative
100-
* ordering is to be preserved, @cmp must return 0.
122+
* This is compatible with two styles of @cmp function:
123+
* - The traditional style which returns <0 / =0 / >0, or
124+
* - Returning a boolean 0/1.
125+
* The latter offers a chance to save a few cycles in the comparison
126+
* (which is used by e.g. plug_ctx_cmp() in block/blk-mq.c).
127+
*
128+
* A good way to write a multi-word comparison is
129+
* if (a->high != b->high)
130+
* return a->high > b->high;
131+
* if (a->middle != b->middle)
132+
* return a->middle > b->middle;
133+
* return a->low > b->low;
101134
*/
135+
__attribute__((nonnull(2,3)))
102136
void list_sort(void *priv, struct list_head *head,
103137
int (*cmp)(void *priv, struct list_head *a,
104138
struct list_head *b))
105139
{
106-
struct list_head *part[MAX_LIST_LENGTH_BITS+1]; /* sorted partial lists
107-
-- last slot is a sentinel */
108-
int lev; /* index into part[] */
109-
int max_lev = 0;
110-
struct list_head *list;
140+
struct list_head *list = head->next, *pending = NULL;
141+
size_t count = 0; /* Count of pending */
111142

112-
if (list_empty(head))
143+
if (list == head->prev) /* Zero or one elements */
113144
return;
114145

115-
memset(part, 0, sizeof(part));
116-
146+
/* Convert to a null-terminated singly-linked list. */
117147
head->prev->next = NULL;
118-
list = head->next;
119148

120-
while (list) {
149+
/*
150+
* Data structure invariants:
151+
* - All lists are singly linked and null-terminated; prev
152+
* pointers are not maintained.
153+
* - pending is a prev-linked "list of lists" of sorted
154+
* sublists awaiting further merging.
155+
* - Each of the sorted sublists is power-of-two in size,
156+
* corresponding to bits set in "count".
157+
* - Sublists are sorted by size and age, smallest & newest at front.
158+
*/
159+
do {
160+
size_t bits;
121161
struct list_head *cur = list;
162+
163+
/* Extract the head of "list" as a single-element list "cur" */
122164
list = list->next;
123165
cur->next = NULL;
124166

125-
for (lev = 0; part[lev]; lev++) {
126-
cur = merge(priv, cmp, part[lev], cur);
127-
part[lev] = NULL;
167+
/* Do merges corresponding to set lsbits in count */
168+
for (bits = count; bits & 1; bits >>= 1) {
169+
cur = merge(priv, (cmp_func)cmp, pending, cur);
170+
pending = pending->prev; /* Untouched by merge() */
128171
}
129-
if (lev > max_lev) {
130-
if (unlikely(lev >= ARRAY_SIZE(part)-1)) {
131-
printk_once(KERN_DEBUG "list too long for efficiency\n");
132-
lev--;
133-
}
134-
max_lev = lev;
135-
}
136-
part[lev] = cur;
172+
/* And place the result at the head of "pending" */
173+
cur->prev = pending;
174+
pending = cur;
175+
count++;
176+
} while (list->next);
177+
178+
/* Now merge together last element with all pending lists */
179+
while (pending->prev) {
180+
list = merge(priv, (cmp_func)cmp, pending, list);
181+
pending = pending->prev;
137182
}
138-
139-
for (lev = 0; lev < max_lev; lev++)
140-
if (part[lev])
141-
list = merge(priv, cmp, part[lev], list);
142-
143-
merge_and_restore_back_links(priv, cmp, head, part[max_lev], list);
183+
/* The final merge, rebuilding prev links */
184+
merge_final(priv, (cmp_func)cmp, head, pending, list);
144185
}
145186
EXPORT_SYMBOL(list_sort);

0 commit comments

Comments
 (0)