netdev - Re: [PATCH] Remove cli()/sti() calls from HYSDN driver.

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, 2 May 2008 15:15:29 +0200
From:	Karsten Keil <kkeil@...e.de>
To:	Mark Asselstine <mark.asselstine@...driver.com>
Cc:	kernel-janitors@...r.kernel.org, netdev@...r.kernel.org,
	asselsm@...il.com
Subject: Re: [PATCH] Remove cli()/sti() calls from HYSDN driver.

On Thu, May 01, 2008 at 04:16:52PM -0400, Mark Asselstine wrote:
> >From looking at this driver the use of cli()/sti() within the do/while
> was a way to ensure interrupts were only disabled for short periods of
> time while the bulk of the time interrupts were free to occur. The
> use of the spin lock has eliminated the need to play with interrupts
> in this way while still allowing for IO to be protected.
> 
> The remaining 3 sti() calls seem unneeded now that at no other point
> in the driver is there a call to cli().
> 
> Signed-off-by: Mark Asselstine <mark.asselstine@...driver.com>
  Acked-by: Karsten Keil <kkeil@...e.de>
> ---
>  drivers/isdn/hysdn/boardergo.c |   14 +++++---------
>  1 files changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/isdn/hysdn/boardergo.c b/drivers/isdn/hysdn/boardergo.c
> index 6cdbad3..3eb096f 100644
> --- a/drivers/isdn/hysdn/boardergo.c
> +++ b/drivers/isdn/hysdn/boardergo.c
> @@ -64,10 +64,11 @@ ergo_interrupt(int intno, void *dev_id)
>  }				/* ergo_interrupt */
>  
>  /******************************************************************************/
> -/* ergo_irq_bh is the function called by the immediate kernel task list after */
> -/* being activated with queue_task and no interrupts active. This task is the */
> -/* only one handling data transfer from or to the card after booting. The task */
> -/* may be queued from everywhere (interrupts included).                       */
> +/* ergo_irq_bh will be called as part of the kernel clearing its shared work  */
> +/* queue sometime after a call to schedule_work has been made passing our     */
> +/* work_struct. This task is the only one handling data transfer from or to   */
> +/* the card after booting. The task may be queued from everywhere             */
> +/* (interrupts included).                                                     */
>  /******************************************************************************/
>  static void
>  ergo_irq_bh(struct work_struct *ugli_api)
> @@ -90,7 +91,6 @@ ergo_irq_bh(struct work_struct *ugli_api)
>  	card->hw_lock = 1;	/* we now lock the hardware */
>  
>  	do {
> -		sti();		/* reenable other ints */
>  		again = 0;	/* assume loop not to be repeated */
>  
>  		if (!dpr->ToHyFlag) {
> @@ -110,7 +110,6 @@ ergo_irq_bh(struct work_struct *ugli_api)
>  				again = 1;	/* restart loop */
>  			}
>  		}		/* a message has arrived for us */
> -		cli();		/* no further ints */
>  		if (again) {
>  			dpr->ToHyInt = 1;
>  			dpr->ToPcInt = 1;	/* interrupt to E1 for all cards */
> @@ -242,7 +241,6 @@ ergo_writebootimg(struct HYSDN_CARD *card, unsigned char *buf,
>  		byteout(card->iobase + PCI9050_USER_IO, PCI9050_E1_RUN);	/* start E1 processor */
>  		/* the interrupts are still masked */
>  
> -		sti();
>  		msleep_interruptible(20);		/* Timeout 20ms */
>  
>  		if (((tDpramBootSpooler *) card->dpram)->Len != DPRAM_SPOOLER_DATA_SIZE) {
> @@ -276,7 +274,6 @@ ergo_writebootseq(struct HYSDN_CARD *card, unsigned char *buf, int len)
>  	dst = sp->Data;		/* point to data in spool structure */
>  	buflen = sp->Len;	/* maximum len of spooled data */
>  	wr_mirror = sp->WrPtr;	/* only once read */
> -	sti();
>  
>  	/* try until all bytes written or error */
>  	i = 0x1000;		/* timeout value */
> @@ -380,7 +377,6 @@ ergo_waitpofready(struct HYSDN_CARD *card)
>  #endif /* CONFIG_HYSDN_CAPI */
>  			return (0);	/* success */
>  		}		/* data has arrived */
> -		sti();
>  		msleep_interruptible(50);		/* Timeout 50ms */
>  	}			/* wait until timeout */
>  
> -- 
> 1.5.5.1

-- 
Karsten Keil
SuSE Labs
ISDN and VOIP development
SUSE LINUX Products GmbH, Maxfeldstr.5 90409 Nuernberg, GF: Markus Rex, HRB 16746 (AG Nuernberg)
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html