netdev - Re: [PATCH v2] net: fec: Use unlocked timecounter reads for saving state

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]

Date:   Tue, 30 Aug 2022 17:21:15 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Csókás Bence <csokas.bence@...lan.hu>
Cc:     netdev@...r.kernel.org, Richard Cochran <richardcochran@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, qiangqing.zhang@....com,
        kernel@...gutronix.de, Marc Kleine-Budde <mkl@...gutronix.de>
Subject: Re: [PATCH v2] net: fec: Use unlocked timecounter reads for saving
 state

On Tue, Aug 30, 2022 at 05:05:24PM +0200, Csókás Bence wrote:
> 
> 
> On 2022. 08. 30. 14:29, Andrew Lunn wrote:
> > > -	fec_ptp_gettime(&fep->ptp_caps, &fep->ptp_saved_state.ts_phc);
> > > +	if (preempt_count_equals(0)) {
> > 
> > ~/linux/drivers$ grep -r preempt_count_equals *
> > ~/linux/drivers$
> > 
> > No other driver plays games like this.
> > 
> > Why not unconditionally take the lock?
> 
> Because then we would be back at the original problem (see Marc's message):
> 
> | [   14.001542] BUG: sleeping function called from invalid context at
> kernel/locking/mutex.c:283
> 
> | [   14.010604] in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 13,
> name: kworker/0:1
> 
> | [   14.018737] preempt_count: 201, expected: 0
> 
> We cannot take a mutex in atomic context. However, we also don't *need to*
> take a mutex in atomic context.

You are not taking a mutex, you are taking a spinlock. You can do that
in atomic context. Can you protect everything which needs protecting
with a spinlock? And avoid sleeping...

     Andrew