Re: [PATCH v1] parisc: Use for_each_console() helper

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Fri, Jan 24, 2020 at 09:59:48AM -0800, James Bottomley wrote:
> On Fri, 2020-01-24 at 19:38 +0200, Andy Shevchenko wrote:
> > On Fri, Jan 24, 2020 at 08:39:02AM -0800, James Bottomley wrote:
> > > On Fri, 2020-01-24 at 18:07 +0200, Andy Shevchenko wrote:
> > > > Replace open coded single-linked list iteration loop with
> > > > for_each_console()
> > > > helper in use.
> > > > -	while ((console = console_drivers) != NULL)
> > > > -		unregister_console(console_drivers);
> > > > +	for_each_console(console)
> > > > +		unregister_console(console);
> > > 
> > > This is wrong.  The old formulation iterates correctly in the face
> > > of element removal.  for_each_console is defined:
> > > 
> > > #define for_each_console(con) \
> > > 	for (con = console_drivers; con != NULL; con = con->next)
> > > 
> > > So it's not safe for any iteration that alters the list elements.
> > 
> > Ah, I see. In this case we need to keep a pointer to the next
> > element. Though, the original code assumes that console_drivers after
> > unregistration will be promoted to the next element. Do we have this
> > assumption solid?
> 
> Yes, the original code simply removes the head until the list is empty.
>  That's a recognized way of emptying any list while letting the remove
> code take care of the locking ... it works because parisc doesn't have
> a braille console.

By the way, consider this code from register_console()

  for_each_console(bcon)
    if (bcon->flags & CON_BOOT)
      unregister_console(bcon);

It works based on assumption that next pointer of the just unregistered console
is not damaged. So, My initial patch will work in the same way.


-- 
With Best Regards,
Andy Shevchenko