netdev - Re: [PATCH net-next 2/3] net: mvmdio: Avoid excessive sleeps in polled mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87a5qq9wow.fsf@waldekranz.com>
Date: Mon, 04 Dec 2023 09:52:15 +0100
From: Tobias Waldekranz <tobias@...dekranz.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: davem@...emloft.net, andrew@...n.ch, gregory.clement@...tlin.com,
 sebastian.hesselbarth@...il.com, robh+dt@...nel.org,
 krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org,
 netdev@...r.kernel.org
Subject: Re: [PATCH net-next 2/3] net: mvmdio: Avoid excessive sleeps in
 polled mode

On lör, dec 02, 2023 at 12:45, Jakub Kicinski <kuba@...nel.org> wrote:
> On Fri,  1 Dec 2023 18:35:44 +0100 Tobias Waldekranz wrote:
>> @@ -94,23 +88,24 @@ static int orion_mdio_wait_ready(const struct orion_mdio_ops *ops,
>>  				 struct mii_bus *bus)
>>  {
>>  	struct orion_mdio_dev *dev = bus->priv;
>> -	unsigned long timeout = usecs_to_jiffies(MVMDIO_SMI_TIMEOUT);
>> -	unsigned long end = jiffies + timeout;
>> -	int timedout = 0;
>> +	unsigned long end, timeout;
>> +	int done, timedout;
>>  
>> -	while (1) {
>> -	        if (ops->is_done(dev))
>> +	if (dev->err_interrupt <= 0) {
>> +		if (!read_poll_timeout_atomic(ops->is_done, done, done, 2,
>> +					      MVMDIO_SMI_TIMEOUT, false, dev))
>>  			return 0;
>> -	        else if (timedout)
>> -			break;
>> -
>> -	        if (dev->err_interrupt <= 0) {
>> -			usleep_range(ops->poll_interval_min,
>> -				     ops->poll_interval_max);
>> +	} else {
>> +		timeout = usecs_to_jiffies(MVMDIO_SMI_TIMEOUT);
>> +		end = jiffies + timeout;
>> +		timedout = 0;
>> +
>> +		while (1) {
>> +			if (ops->is_done(dev))
>> +				return 0;
>> +			else if (timedout)
>> +				break;
>>  
>> -			if (time_is_before_jiffies(end))
>> -				++timedout;
>> -	        } else {
>>  			/* wait_event_timeout does not guarantee a delay of at
>>  			 * least one whole jiffie, so timeout must be no less
>>  			 * than two.
>
> drivers/net/ethernet/marvell/mvmdio.c:91:16: warning: variable 'end' set but not used [-Wunused-but-set-variable]
>    91 |         unsigned long end, timeout;
>       |                       ^

Rookie mistake, sorry about that.

Looking at it again, I think I was too scared of touching the original
interrupt path, as I have no means of testing it (no hardware). I will
try to simplify this in v2, and hope that someone else can test it.