Hi Lucas, Em Sat, 15 Dec 2018 14:46:31 -0200 Lucas A. M. Magalhães <lucmaga@xxxxxxxxx> escreveu: > The previous code pipeline used the stack to walk on the graph and > process a frame. Basically the vimc-sensor entity starts a thread that > generates the frames and calls the propagate_process function to send > this frame to each entity linked with a sink pad. The propagate_process > will call the process_frame of the entities which will call the > propagate_frame for each one of it's sink pad. This cycle will continue > until it reaches a vimc-capture entity that will finally return and > unstack. I didn't review the code yet, but I have a few comments about the way you're describing this patch. When you mention about a "previous code pipeline". Well, by adding it at the main body of the patch description, reviewers should expect that you're mentioning an implementation that already reached upstream. I suspect that this is not the case here, as I don't think we merged any recursive algorithm using the stack, as this is something that we shouldn't do at Kernelspace, as a 4K stack is usually not OK with recursive algorithms. So, it seems that this entire patch description (as-is) is bogus[1]. [1] if this is not the case and a recursive approach was indeed sneaked into the Kernel, this is a bug. So, you should really use the "Fixes:" meta-tag indicating what changeset this patch is fixing, and a "Cc: stable@xxxxxxxxxxxxxxx", in order to hint stable maintainers that this require backports. Please notice that the patch description will be stored forever at the git tree. Mentioning something that were never merged (and that, years from now people will hardly remember, and will have lots of trouble to seek as you didn't even mentioned any ML archive with the past solution) shouldn't be done. So, you should rewrite the entire patch description explaining what the current approach took by this patch does. Then, in order to make easier for reviewers to compare with a previous implementation, you can add a "---" line and then a description about why this approach is better than the first version, e. g. something like: [PATCH v2] media: vimc: Add vimc-streamer for stream control Add a logic that will create a linear pipeline walking backwards on the graph. When the stream starts it will simply loop through the pipeline calling the respective process_frame function for each entity on the pipeline. Signed-off-by: Your Name <your@email> --- v2: The previous approach were to use a recursive function that it was using the stack to walk on the graph and process a frame. Basically the vimc-sensor entity starts a thread that generates the frames and calls the propagate_process function to send this frame to each entity linked with a sink pad. The propagate_process will call the process_frame of the entities which will call the propagate_frame for each one of it's sink pad. This cycle will continue until it reaches a vimc-capture entity that will finally return and unstack. ... If the past approach was written by somebody else (or if you sent it a long time ago), please add an URL (if possible using https://lore.kernel.org/linux-media/ archive) pointing to the previous approach, in order to help us to check what you're referring to. Regards, Mauro Thanks, Mauro
