Re: [for-next][PATCH 14/15] tracepoint: Do not fail unregistering a probe due to memory failure

On Wed, 3 Feb 2021 18:09:27 +0100
Peter Zijlstra [off-list ref] wrote:

On Wed, Feb 03, 2021 at 11:05:31AM -0500, Steven Rostedt wrote:

quoted

+		if (new) {
+			for (i = 0; old[i].func; i++)
+				if ((old[i].func != tp_func->func
+				     || old[i].data != tp_func->data)
+				    && old[i].func != tp_stub_func)  

logical operators go at the end..

Agreed. I just added that "if (new) {" around the original block, didn't
think about the formatting when doing so.

quoted

+					new[j++] = old[i];
+			new[nr_probes - nr_del].func = NULL;
+			*funcs = new;
+		} else {
+			/*
+			 * Failed to allocate, replace the old function
+			 * with calls to tp_stub_func.
+			 */
+			for (i = 0; old[i].func; i++)  

							{

quoted

+				if (old[i].func == tp_func->func &&
+				    old[i].data == tp_func->data) {  

like here.

quoted

+					old[i].func = tp_stub_func;
+					/* Set the prio to the next event. */
+					if (old[i + 1].func)
+						old[i].prio =
+							old[i + 1].prio;  

multi line demands { }, but in this case just don't line-break.

Sure.

quoted

+					else
+						old[i].prio = -1;
+				}  

			}

quoted

+			*funcs = old;
+		}
 	}
 	debug_print_probes(*funcs);
 	return old;

@@ -295,10 +341,12 @@ static int tracepoint_remove_func(struct tracepoint *tp,
 	tp_funcs = rcu_dereference_protected(tp->funcs,
 			lockdep_is_held(&tracepoints_mutex));
 	old = func_remove(&tp_funcs, func);
-	if (IS_ERR(old)) {
-		WARN_ON_ONCE(PTR_ERR(old) != -ENOMEM);
+	if (WARN_ON_ONCE(IS_ERR(old)))
 		return PTR_ERR(old);
-	}
+
+	if (tp_funcs == old)
+		/* Failed allocating new tp_funcs, replaced func with stub */
+		return 0;  

{ }

Even if it's just a comment that causes multiple lines? I could just move
the comment above the if.

This has already been through my test suite, and since the changes
requested are just formatting and non-functional, I'll just add a clean up
patch on top.

Thanks!

-- Steve