linux-kernel - Re: [PATCH 22/23] floppy: cleanup: do not iterate on current

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: <f5fb363f-8c2c-3aca-6b71-4a45544d067a@linux.com>
Date:   Fri, 10 Apr 2020 11:35:51 +0300
From:   Denis Efremov <efremov@...ux.com>
To:     Willy Tarreau <w@....eu>
Cc:     Jens Axboe <axboe@...nel.dk>, linux-block@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 22/23] floppy: cleanup: do not iterate on current_fdc in
 DMA grab/release functions

I see a couple of similar cycles in do_floppy_init:

for (i = 0; i < N_FDC; i++) {
        current_fdc = i;
        memset(&fdc_state[current_fdc], 0, sizeof(*fdc_state));
        fdc_state[current_fdc].dtr = -1;
        fdc_state[current_fdc].dor = 0x4;
...
}

for (i = 0; i < N_FDC; i++) {
        current_fdc = i;
        fdc_state[current_fdc].driver_version = FD_DRIVER_VERSION;
...
}

On 3/31/20 12:40 PM, Willy Tarreau wrote:
> Both floppy_grab_irq_and_dma() and floppy_release_irq_and_dma() used to
> iterate on the global variable while setting up or freeing resources.
> Now that they exclusively rely on functions which take the fdc as an
> argument, so let's not touch the global one anymore.
> 
> Signed-off-by: Willy Tarreau <w@....eu>
> ---
>  drivers/block/floppy.c | 39 ++++++++++++++++++++-------------------
>  1 file changed, 20 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
> index 8850baa3372a..77bb9a5fcd33 100644
> --- a/drivers/block/floppy.c
> +++ b/drivers/block/floppy.c
> @@ -4854,6 +4854,8 @@ static void floppy_release_regions(int fdc)
>  
>  static int floppy_grab_irq_and_dma(void)
>  {
> +	int fdc;
> +
>  	if (atomic_inc_return(&usage_count) > 1)
>  		return 0;
>  
> @@ -4881,24 +4883,24 @@ static int floppy_grab_irq_and_dma(void)
>  		}
>  	}
>  
> -	for (current_fdc = 0; current_fdc < N_FDC; current_fdc++) {
> -		if (fdc_state[current_fdc].address != -1) {
> -			if (floppy_request_regions(current_fdc))
> +	for (fdc = 0; fdc < N_FDC; fdc++) {
> +		if (fdc_state[fdc].address != -1) {
> +			if (floppy_request_regions(fdc))
>  				goto cleanup;
>  		}
>  	}
> -	for (current_fdc = 0; current_fdc < N_FDC; current_fdc++) {
> -		if (fdc_state[current_fdc].address != -1) {
> -			reset_fdc_info(current_fdc, 1);
> -			fdc_outb(fdc_state[current_fdc].dor, current_fdc, FD_DOR);
> +	for (fdc = 0; fdc < N_FDC; fdc++) {
> +		if (fdc_state[fdc].address != -1) {
> +			reset_fdc_info(fdc, 1);
> +			fdc_outb(fdc_state[fdc].dor, fdc, FD_DOR);
>  		}
>  	}
> -	current_fdc = 0;
> +
>  	set_dor(0, ~0, 8);	/* avoid immediate interrupt */
>  
> -	for (current_fdc = 0; current_fdc < N_FDC; current_fdc++)
> -		if (fdc_state[current_fdc].address != -1)
> -			fdc_outb(fdc_state[current_fdc].dor, current_fdc, FD_DOR);
> +	for (fdc = 0; fdc < N_FDC; fdc++)
> +		if (fdc_state[fdc].address != -1)
> +			fdc_outb(fdc_state[fdc].dor, fdc, FD_DOR);
>  	/*
>  	 * The driver will try and free resources and relies on us
>  	 * to know if they were allocated or not.
> @@ -4909,15 +4911,16 @@ static int floppy_grab_irq_and_dma(void)
>  cleanup:
>  	fd_free_irq();
>  	fd_free_dma();
> -	while (--current_fdc >= 0)
> -		floppy_release_regions(current_fdc);
> +	while (--fdc >= 0)
> +		floppy_release_regions(fdc);
> +	current_fdc = 0;
>  	atomic_dec(&usage_count);
>  	return -1;
>  }
>  
>  static void floppy_release_irq_and_dma(void)
>  {
> -	int old_fdc;
> +	int fdc;
>  #ifndef __sparc__
>  	int drive;
>  #endif
> @@ -4958,11 +4961,9 @@ static void floppy_release_irq_and_dma(void)
>  		pr_info("auxiliary floppy timer still active\n");
>  	if (work_pending(&floppy_work))
>  		pr_info("work still pending\n");
> -	old_fdc = current_fdc;
> -	for (current_fdc = 0; current_fdc < N_FDC; current_fdc++)
> -		if (fdc_state[current_fdc].address != -1)
> -			floppy_release_regions(current_fdc);
> -	current_fdc = old_fdc;
> +	for (fdc = 0; fdc < N_FDC; fdc++)
> +		if (fdc_state[fdc].address != -1)
> +			floppy_release_regions(fdc);
>  }
>  
>  #ifdef MODULE
>