Re: [PATCH 01/10] aacraid: SCSI blk tag support

[Johannes Thumshirn <jthumshirn@suse.de>]

On Tue, 2015-12-01 at 04:39 -0800, Raghava Aditya Renukunta wrote:
> From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
> 
> The method to allocate and free FIB's in the present code utilizes
> spinlocks.Multiple IO's have to wait on the spinlock to acquire or
> free fibs creating a performance bottleneck.
> 
> An alternative solution would be to use block layer tags to keep track
> of the fibs allocated and freed. To this end 2 functions
> aac_fib_alloc_tag and aac_fib_free_tag were created which utilize the
> blk layer tags to plug into the Fib pool.These functions are used
> exclusively in the IO path. 8 fibs are reserved for the use of AIF
> management software and utilize the previous spinlock based
> implementations.
> 
> Signed-off-by: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
> ---
>  drivers/scsi/aacraid/aachba.c  | 35 +++++++++++++++++-------------
>  drivers/scsi/aacraid/aacraid.h |  2 ++
>  drivers/scsi/aacraid/commsup.c | 49 +++++++++++++++++++++++++++++++++++++++-
> --
>  drivers/scsi/aacraid/dpcsup.c  |  4 ++--
>  drivers/scsi/aacraid/linit.c   |  2 ++
>  5 files changed, 72 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
> index e4c2437..06cbab8 100644
> --- a/drivers/scsi/aacraid/aachba.c
> +++ b/drivers/scsi/aacraid/aachba.c
> @@ -323,7 +323,7 @@ static inline int aac_valid_context(struct scsi_cmnd 

[...]

> 
>  	return 0;
>  }
>  
> @@ -166,6 +166,49 @@ int aac_fib_setup(struct aac_dev * dev)
>   *	aac_fib_alloc	-	allocate a fib

Nitpick/Typo aac_fib_alloc_tag

>   *	@dev: Adapter to allocate the fib for
>   *
> + *	Allocate a fib from the adapter fib pool using tags
> + *	from the blk layer.
> + */
> +
> +struct fib *aac_fib_alloc_tag(struct aac_dev *dev, struct scsi_cmnd *scmd)
> +{
> +	struct fib *fibptr;
> +
> +	fibptr = &dev->fibs[scmd->request->tag];
> +	/*
> +	 *	Set the proper node type code and node byte size
> +	 */
> +	fibptr->type = FSAFS_NTC_FIB_CONTEXT;
> +	fibptr->size = sizeof(struct fib);
> +	/*
> +	 *	Null out fields that depend on being zero at the start of
> +	 *	each I/O
> +	 */
> +	fibptr->hw_fib_va->header.XferState = 0;
> +	fibptr->flags = 0;
> +	fibptr->callback = NULL;
> +	fibptr->callback_data = NULL;
> +
> +	return fibptr;
> +}
> +
> +/**
> + *	aac_fib_free_tag	free a fib
> + *	@fibptr: fib to free up
> + *
> + *	Placeholder to free tag allocated fibs
> + *	Does not do anything
> + */
> +
> +void aac_fib_free_tag(struct fib *fibptr)
> +{
> +	(void)fibptr;
> +}
> +

I'm not sure if I like this placeholder. Sure it's a NOP and doesn't cost
anything but it's dead code. But if I'm the only one objecting here I'm fine
with it.

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html