Re: [PATCH 2/2] fbdev: Don't sort deferred-I/O pages by default

[Thomas Zimmermann <tzimmermann@suse.de>]

[-- Attachment #1.1: Type: text/plain, Size: 2179 bytes --]

Hi

Am 11.02.22 um 08:58 schrieb Dan Carpenter:
> On Thu, Feb 10, 2022 at 10:16:45PM +0100, Sam Ravnborg wrote:
>>> diff --git a/drivers/video/fbdev/core/fb_defio.c b/drivers/video/fbdev/core/fb_defio.c
>>> index 3727b1ca87b1..1f672cf253b2 100644
>>> --- a/drivers/video/fbdev/core/fb_defio.c
>>> +++ b/drivers/video/fbdev/core/fb_defio.c
>>> @@ -132,15 +132,20 @@ static vm_fault_t fb_deferred_io_mkwrite(struct vm_fault *vmf)
>>>   	if (!list_empty(&page->lru))
>>>   		goto page_already_added;
>>>   
>>> -	/* we loop through the pagelist before adding in order
>>> -	to keep the pagelist sorted */
>>> -	list_for_each_entry(cur, &fbdefio->pagelist, lru) {
>>> -		if (cur->index > page->index)
>>> -			break;
>>> +	if (fbdefio->sort_pagelist) {
>>> +		/*
>>> +		 * We loop through the pagelist before adding in order
>>> +		 * to keep the pagelist sorted.
>>> +		 */
>>> +		list_for_each_entry(cur, &fbdefio->pagelist, lru) {
>>> +			if (cur->index > page->index)
>>> +				break;
>>> +		}
>>> +		list_add_tail(&page->lru, &cur->lru);
>>> +	} else {
>>> +		list_add_tail(&page->lru, &fbdefio->pagelist);
>>>   	}
>> Bikeshedding - my personal style is to have the likely part first.
>> This makes reading the code easier.
> 
> I've thought about this quite a bit...  I guess my rule is to avoid
> negatives as much as possible so I prefer the original code.  My rules
> right now are:
> 
> 1) Always do error handling.  Don't do success handling.
> 2) Return as quickly as possible and pull the code in an indent.
> 3) Avoid negatives.  Never had negatives in the variable names.

 From what I know, CPUs' branch prediction prefers backward jumps (e.g., 
loops) but avoids forward jumps. Compilers arrange the code to optimize 
for this pattern. So I tend to put the exception or error handling into 
the if branch.  But I have no idea if that really makes a difference at 
runtime.

Best regards
Thomas

> 
> regards,
> dan carpenter

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]