diff mbox series

[3/6] kernel: remove iterator use outside the loop

Message ID 20220301075839.4156-4-xiam0nd.tong@gmail.com (mailing list archive)
State New, archived
Headers show
Series list_for_each_entry*: make iterator invisiable outside the loop | expand

Commit Message

Xiaomeng Tong March 1, 2022, 7:58 a.m. UTC
Demonstrations for:
 - list_for_each_entry_inside
 - list_for_each_entry_continue_inside
 - list_for_each_entry_safe_continue_inside

Signed-off-by: Xiaomeng Tong <xiam0nd.tong@gmail.com>
---
 kernel/power/snapshot.c | 28 ++++++++++++++++------------
 kernel/signal.c         |  6 +++---
 2 files changed, 19 insertions(+), 15 deletions(-)

Comments

Greg Kroah-Hartman March 1, 2022, 10:41 a.m. UTC | #1
On Tue, Mar 01, 2022 at 03:58:36PM +0800, Xiaomeng Tong wrote:
> Demonstrations for:
>  - list_for_each_entry_inside
>  - list_for_each_entry_continue_inside
>  - list_for_each_entry_safe_continue_inside

This changelog does not make much sense at all.  Why are you making
these changes and how are we to review them?  Same for the others in
this series.

confused,

greg k-h
Xiaomeng Tong March 1, 2022, 11:34 a.m. UTC | #2
On Tue, 1 Mar 2022 11:41:34 +0100, greg k-h wrote:
> On Tue, Mar 01, 2022 at 03:58:36PM +0800, Xiaomeng Tong wrote:
> > Demonstrations for:
> >  - list_for_each_entry_inside
> >  - list_for_each_entry_continue_inside
> >  - list_for_each_entry_safe_continue_inside
>
> This changelog does not make much sense at all.  Why are you making
> these changes and how are we to review them?  Same for the others in
> this series.
> 
> confused,

I'm sorry for have created the confusion. I made this patch to remove
the use of iterator (in this patch is "ext", "cur", "aux", "q") outside
the list_for_each_entry* loop, using the new *_inside macros instead
which are introduced in PATCH v2, and to prove the effectiveness of
the new macros.

The detail for create_mem_extents patch:

1. remove the iterator use outside the loop:
-		struct mem_extent *ext, *cur, *aux;

2. declare a ptr outside the loop and set NULL:
+		struct mem_extent *me = NULL;

3. repace list_for_each_entry with list_for_each_entry_inside, and pass
   a new argument (struct mem_extent) as the struct type.
+		list_for_each_entry_inside(ext, struct mem_extent, list, hook)

4. when we hit the break in list_for_each_entry, record the found iterator
   for lately use:
+			if (zone_start <= ext->end) {
+				me = ext;
 				break;
+			}

5. when we miss the break, the "me" is NULL and can be used to determine if
   we already found the "ext". And replace every "ext" use after the loop
   with "me".
-		if (&ext->hook == list || zone_end < ext->start) {
+		if (!me || zone_end < me->start) {


6. repace list_for_each_entry_safe_continue with
   list_for_each_entry_safe_continue_inside, and pass a new argument (me)
   as the iterator to start/continue with.
-		list_for_each_entry_safe_continue(cur, aux, list, hook) {
+		list_for_each_entry_safe_continue_inside(cur, aux, me, list, hook) {

Best regards,
--
Xiaomeng Tong
Xiaomeng Tong March 1, 2022, 11:48 a.m. UTC | #3
typo:
 which are introduced in PATCH v2,
correct:
 which are introduced in PATCH 2/6,

Best regards,
--
Xiaomeng Tong
diff mbox series

Patch

diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
index 330d49937..75958b5fa 100644
--- a/kernel/power/snapshot.c
+++ b/kernel/power/snapshot.c
@@ -625,16 +625,18 @@  static int create_mem_extents(struct list_head *list, gfp_t gfp_mask)
 
 	for_each_populated_zone(zone) {
 		unsigned long zone_start, zone_end;
-		struct mem_extent *ext, *cur, *aux;
+		struct mem_extent *me = NULL;
 
 		zone_start = zone->zone_start_pfn;
 		zone_end = zone_end_pfn(zone);
 
-		list_for_each_entry(ext, list, hook)
-			if (zone_start <= ext->end)
+		list_for_each_entry_inside(ext, struct mem_extent, list, hook)
+			if (zone_start <= ext->end) {
+				me = ext;
 				break;
+			}
 
-		if (&ext->hook == list || zone_end < ext->start) {
+		if (!me || zone_end < me->start) {
 			/* New extent is necessary */
 			struct mem_extent *new_ext;
 
@@ -645,23 +647,25 @@  static int create_mem_extents(struct list_head *list, gfp_t gfp_mask)
 			}
 			new_ext->start = zone_start;
 			new_ext->end = zone_end;
-			list_add_tail(&new_ext->hook, &ext->hook);
+			if (!me)
+				list_add_tail(&new_ext->hook, list);
+			else
+				list_add_tail(&new_ext->hook, &me->hook);
 			continue;
 		}
 
 		/* Merge this zone's range of PFNs with the existing one */
-		if (zone_start < ext->start)
-			ext->start = zone_start;
-		if (zone_end > ext->end)
-			ext->end = zone_end;
+		if (zone_start < me->start)
+			me->start = zone_start;
+		if (zone_end > me->end)
+			me->end = zone_end;
 
 		/* More merging may be possible */
-		cur = ext;
-		list_for_each_entry_safe_continue(cur, aux, list, hook) {
+		list_for_each_entry_safe_continue_inside(cur, aux, me, list, hook) {
 			if (zone_end < cur->start)
 				break;
 			if (zone_end < cur->end)
-				ext->end = cur->end;
+				me->end = cur->end;
 			list_del(&cur->hook);
 			kfree(cur);
 		}
diff --git a/kernel/signal.c b/kernel/signal.c
index 9b04631ac..da2c20de1 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -711,7 +711,7 @@  static int dequeue_synchronous_signal(kernel_siginfo_t *info)
 {
 	struct task_struct *tsk = current;
 	struct sigpending *pending = &tsk->pending;
-	struct sigqueue *q, *sync = NULL;
+	struct sigqueue *sync = NULL;
 
 	/*
 	 * Might a synchronous signal be in the queue?
@@ -722,7 +722,7 @@  static int dequeue_synchronous_signal(kernel_siginfo_t *info)
 	/*
 	 * Return the first synchronous signal in the queue.
 	 */
-	list_for_each_entry(q, &pending->list, list) {
+	list_for_each_entry_inside(q, struct sigqueue, &pending->list, list) {
 		/* Synchronous signals have a positive si_code */
 		if ((q->info.si_code > SI_USER) &&
 		    (sigmask(q->info.si_signo) & SYNCHRONOUS_MASK)) {
@@ -735,7 +735,7 @@  static int dequeue_synchronous_signal(kernel_siginfo_t *info)
 	/*
 	 * Check if there is another siginfo for the same signal.
 	 */
-	list_for_each_entry_continue(q, &pending->list, list) {
+	list_for_each_entry_continue_inside(q, sync, &pending->list, list) {
 		if (q->info.si_signo == sync->info.si_signo)
 			goto still_pending;
 	}