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:   Fri, 25 Sep 2020 02:42:08 +0200
From:   Frederic Weisbecker <frederic@...nel.org>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     qianjun.kernel@...il.com, peterz@...radead.org, will@...nel.org,
        luto@...nel.org, linux-kernel@...r.kernel.org,
        laoar.shao@...il.com, qais.yousef@....com, urezki@...il.com
Subject: Re: [PATCH V7 4/4] softirq: Allow early break the softirq processing
 loop

On Thu, Sep 24, 2020 at 05:37:42PM +0200, Thomas Gleixner wrote:
> Subject: softirq; Prevent starvation of higher softirq vectors
[...]
> +	/*
> +	 * Word swap pending to move the not yet handled bits of the previous
> +	 * run first and then clear the duplicates in the newly raised ones.
> +	 */
> +	swahw32s(&cur_pending);
> +	pending = cur_pending & ~(cur_pending << SIRQ_PREV_SHIFT);
> +
>  	for_each_set_bit(vec_nr, &pending, NR_SOFTIRQS) {
>  		int prev_count;
>  
> +		vec_nr &= SIRQ_VECTOR_MASK;

Shouldn't NR_SOFTIRQS above protect from that?

>  		__clear_bit(vec_nr, &pending);
>  		kstat_incr_softirqs_this_cpu(vec_nr);
>
[...]
> +	} else {
> +		/*
> +		 * Retain the unprocessed bits and swap @cur_pending back
> +		 * into normal ordering
> +		 */
> +		cur_pending = (u32)pending;
> +		swahw32s(&cur_pending);
> +		/*
> +		 * If the previous bits are done move the low word of
> +		 * @pending into the high word so it's processed first.
> +		 */
> +		if (!(cur_pending & SIRQ_PREV_MASK))
> +			cur_pending <<= SIRQ_PREV_SHIFT;

If the previous bits are done and there is no timeout, should
we consider to restart a loop?

A common case would be to enter do_softirq() with RCU_SOFTIRQ set
in the SIRQ_PREV_MASK and NET_RX_SOFTIRQ set in the normal mask.

You would always end up processing the RCU_SOFTIRQ here and trigger
ksoftirqd for the NET_RX_SOFTIRQ.

Although that's probably no big deal as we should be already in ksoftirqd
if we processed prev bits. We are just going to iterate the kthread loop
instead of the do_softirq loop. Probably no real issue then...


>  
> +		/* Merge the newly pending ones into the low word */
> +		cur_pending |= new_pending;
> +	}
> +	set_softirq_pending(cur_pending);
>  	wakeup_softirqd();
>  out:
>  	lockdep_softirq_end(in_hardirq);

Powered by blists - more mailing lists