Re: [CHECKER] repetitive/contradictory comparison bugs for 2.4.7

[Dawson Engler <engler@csl.Stanford.EDU>]

bunch of quoting for context:

> > other 10 are questionable.  Those 10 are all simple variations on the
> > following code:
> > 
> > Start --->
> > 	if (!tmp_buf) {
> > 		page = get_free_page(GFP_KERNEL);
> > 
> > Error --->
> > 		if (tmp_buf)
> > 			free_page(page);
> > 		else
> > 			tmp_buf = (unsigned char *) page;
> > 	}
> 
> That one is not a bug. The serial drivers do this to handle a race. Really
> it should be
> 
> 		page = get_free_page(GFP_KERNEL)
> 
> 		rmb();
> 		if (tmp_buf)
> 			..
> 
> but this will go away as and when someone switches the tty layer to new 
> style locking. The precise code flow (under lock_kernel in both cases) is
> 
> 	
> 	if (!tmp_buf)
> 	{
> 		/* tmp_buf was 0
> 		page = get_free_page (...)
> 		[SLEEPS, TASK SWITCH]
> 

Does this mean that the 'cli' in the following code is redundant?

	/* 2.4.7/drivers/char/generic_serial.c:953:gs_init_port: */
        if (!tmp_buf) {
                page = get_free_page(GFP_KERNEL);

                cli (); /* Don't expect this to make a difference. */
                if (tmp_buf)
                        free_page(page);
                else
                        tmp_buf = (unsigned char *) page;

Dawson