All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tomas Henzl <thenzl@redhat.com>
To: Raghava Aditya Renukunta <RaghavaAditya.Renukunta@pmcs.com>,
	James.Bottomley@HansenPartnership.com,
	martin.petersen@oracle.com, linux-scsi@vger.kernel.org
Cc: Mahesh.Rajashekhara@pmcs.com, Murthy.Bhat@pmcs.com,
	Gana.Sridaran@pmcs.com, aacraid@pmc-sierra.com,
	Scott.Benesh@pmcs.com, jthumshirn@suse.de, shane.seymour@hpe.com,
	David.Carroll@pmcs.com
Subject: Re: [PATCH V3 1/9] aacraid: SCSI blk tag support
Date: Tue, 19 Jan 2016 16:33:58 +0100	[thread overview]
Message-ID: <569E5766.7090602@redhat.com> (raw)
In-Reply-To: <1452842182-684-2-git-send-email-RaghavaAditya.Renukunta@pmcs.com>

On 15.1.2016 08:16, 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 aac_fib_alloc_tag was
> created to 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.
>
> Changes in V2:
> Removed aac_fib_free_tag since it was a stub function
> Moved population of fib fields that are constant to aac_fib_setup
>
> Changes in V3:
> None
>
> Signed-off-by: Raghava Aditya Renukunta <RaghavaAditya.Renukunta@pmcs.com>
> ---
>  drivers/scsi/aacraid/aachba.c  | 27 ++++++++++++---------------
>  drivers/scsi/aacraid/aacraid.h |  1 +
>  drivers/scsi/aacraid/commsup.c | 32 +++++++++++++++++++++++++++++---
>  drivers/scsi/aacraid/dpcsup.c  |  2 --
>  drivers/scsi/aacraid/linit.c   |  2 ++
>  5 files changed, 44 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
> index e4c2437..7dfd0fa 100644
> --- a/drivers/scsi/aacraid/aachba.c
> +++ b/drivers/scsi/aacraid/aachba.c
> @@ -323,7 +323,6 @@ static inline int aac_valid_context(struct scsi_cmnd *scsicmd,
>  	if (unlikely(!scsicmd || !scsicmd->scsi_done)) {
>  		dprintk((KERN_WARNING "aac_valid_context: scsi command corrupt\n"));
>  		aac_fib_complete(fibptr);
> -		aac_fib_free(fibptr);
>  		return 0;
>  	}
>  	scsicmd->SCp.phase = AAC_OWNER_MIDLEVEL;
> @@ -331,7 +330,6 @@ static inline int aac_valid_context(struct scsi_cmnd *scsicmd,
>  	if (unlikely(!device || !scsi_device_online(device))) {
>  		dprintk((KERN_WARNING "aac_valid_context: scsi device corrupt\n"));
>  		aac_fib_complete(fibptr);
> -		aac_fib_free(fibptr);
>  		return 0;
>  	}
>  	return 1;
> @@ -541,7 +539,6 @@ static void get_container_name_callback(void *context, struct fib * fibptr)
>  	scsicmd->result = DID_OK << 16 | COMMAND_COMPLETE << 8 | SAM_STAT_GOOD;
>  
>  	aac_fib_complete(fibptr);
> -	aac_fib_free(fibptr);
>  	scsicmd->scsi_done(scsicmd);
>  }
>  
> @@ -557,7 +554,8 @@ static int aac_get_container_name(struct scsi_cmnd * scsicmd)
>  
>  	dev = (struct aac_dev *)scsicmd->device->host->hostdata;
>  
> -	if (!(cmd_fibcontext = aac_fib_alloc(dev)))
> +	cmd_fibcontext = aac_fib_alloc_tag(dev, scsicmd);
> +	if (!cmd_fibcontext)
>  		return -ENOMEM;

aac_fib_alloc_tag can't return NULL in the new implementation,
in some next series please remove all the needless tests

Reviewed-by: Tomas Henzl <thenzl@redhat.com>

Tomas

>  
>  	aac_fib_init(cmd_fibcontext);
> @@ -586,7 +584,6 @@ static int aac_get_container_name(struct scsi_cmnd * scsicmd)
>  
>  	printk(KERN_WARNING "aac_get_container_name: aac_fib_send failed with status: %d.\n", status);
>  	aac_fib_complete(cmd_fibcontext);
> -	aac_fib_free(cmd_fibcontext);
>  	return -1;
>  }
>  
> @@ -1024,7 +1021,6 @@ static void get_container_serial_callback(void *context, struct fib * fibptr)
>  	scsicmd->result = DID_OK << 16 | COMMAND_COMPLETE << 8 | SAM_STAT_GOOD;
>  
>  	aac_fib_complete(fibptr);
> -	aac_fib_free(fibptr);
>  	scsicmd->scsi_done(scsicmd);
>  }
>  
> @@ -1040,7 +1036,8 @@ static int aac_get_container_serial(struct scsi_cmnd * scsicmd)
>  
>  	dev = (struct aac_dev *)scsicmd->device->host->hostdata;
>  
> -	if (!(cmd_fibcontext = aac_fib_alloc(dev)))
> +	cmd_fibcontext = aac_fib_alloc_tag(dev, scsicmd);
> +	if (!cmd_fibcontext)
>  		return -ENOMEM;
>  
>  	aac_fib_init(cmd_fibcontext);
> @@ -1068,7 +1065,6 @@ static int aac_get_container_serial(struct scsi_cmnd * scsicmd)
>  
>  	printk(KERN_WARNING "aac_get_container_serial: aac_fib_send failed with status: %d.\n", status);
>  	aac_fib_complete(cmd_fibcontext);
> -	aac_fib_free(cmd_fibcontext);
>  	return -1;
>  }
>  
> @@ -1869,7 +1865,6 @@ static void io_callback(void *context, struct fib * fibptr)
>  		break;
>  	}
>  	aac_fib_complete(fibptr);
> -	aac_fib_free(fibptr);
>  
>  	scsicmd->scsi_done(scsicmd);
>  }
> @@ -1954,7 +1949,8 @@ static int aac_read(struct scsi_cmnd * scsicmd)
>  	/*
>  	 *	Alocate and initialize a Fib
>  	 */
> -	if (!(cmd_fibcontext = aac_fib_alloc(dev))) {
> +	cmd_fibcontext = aac_fib_alloc_tag(dev, scsicmd);
> +	if (!cmd_fibcontext) {
>  		printk(KERN_WARNING "aac_read: fib allocation failed\n");
>  		return -1;
>  	}
> @@ -2051,7 +2047,8 @@ static int aac_write(struct scsi_cmnd * scsicmd)
>  	/*
>  	 *	Allocate and initialize a Fib then setup a BlockWrite command
>  	 */
> -	if (!(cmd_fibcontext = aac_fib_alloc(dev))) {
> +	cmd_fibcontext = aac_fib_alloc_tag(dev, scsicmd);
> +	if (!cmd_fibcontext) {
>  		/* FIB temporarily unavailable,not catastrophic failure */
>  
>  		/* scsicmd->result = DID_ERROR << 16;
> @@ -2285,7 +2282,7 @@ static int aac_start_stop(struct scsi_cmnd *scsicmd)
>  	/*
>  	 *	Allocate and initialize a Fib
>  	 */
> -	cmd_fibcontext = aac_fib_alloc(aac);
> +	cmd_fibcontext = aac_fib_alloc_tag(aac, scsicmd);
>  	if (!cmd_fibcontext)
>  		return SCSI_MLQUEUE_HOST_BUSY;
>  
> @@ -3157,7 +3154,6 @@ static void aac_srb_callback(void *context, struct fib * fibptr)
>  	scsicmd->result |= le32_to_cpu(srbreply->scsi_status);
>  
>  	aac_fib_complete(fibptr);
> -	aac_fib_free(fibptr);
>  	scsicmd->scsi_done(scsicmd);
>  }
>  
> @@ -3187,9 +3183,10 @@ static int aac_send_srb_fib(struct scsi_cmnd* scsicmd)
>  	/*
>  	 *	Allocate and initialize a Fib then setup a BlockWrite command
>  	 */
> -	if (!(cmd_fibcontext = aac_fib_alloc(dev))) {
> +	cmd_fibcontext = aac_fib_alloc_tag(dev, scsicmd);
> +	if (!cmd_fibcontext)
>  		return -1;
> -	}
> +
>  	status = aac_adapter_scsi(cmd_fibcontext, scsicmd);
>  
>  	/*
> diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
> index 074878b..f51f0a0 100644
> --- a/drivers/scsi/aacraid/aacraid.h
> +++ b/drivers/scsi/aacraid/aacraid.h
> @@ -2114,6 +2114,7 @@ int aac_acquire_irq(struct aac_dev *dev);
>  void aac_free_irq(struct aac_dev *dev);
>  const char *aac_driverinfo(struct Scsi_Host *);
>  struct fib *aac_fib_alloc(struct aac_dev *dev);
> +struct fib *aac_fib_alloc_tag(struct aac_dev *dev, struct scsi_cmnd *scmd);
>  int aac_fib_setup(struct aac_dev *dev);
>  void aac_fib_map_free(struct aac_dev *dev);
>  void aac_fib_free(struct fib * context);
> diff --git a/drivers/scsi/aacraid/commsup.c b/drivers/scsi/aacraid/commsup.c
> index a1f90fe..6b286b3 100644
> --- a/drivers/scsi/aacraid/commsup.c
> +++ b/drivers/scsi/aacraid/commsup.c
> @@ -137,6 +137,7 @@ int aac_fib_setup(struct aac_dev * dev)
>  		i++, fibptr++)
>  	{
>  		fibptr->flags = 0;
> +		fibptr->size = sizeof(struct fib);
>  		fibptr->dev = dev;
>  		fibptr->hw_fib_va = hw_fib;
>  		fibptr->data = (void *) fibptr->hw_fib_va->data;
> @@ -156,13 +157,38 @@ int aac_fib_setup(struct aac_dev * dev)
>  	 */
>  	dev->fibs[dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB - 1].next = NULL;
>  	/*
> -	 *	Enable this to debug out of queue space
> -	 */
> -	dev->free_fib = &dev->fibs[0];
> +	*	Set 8 fibs aside for management tools
> +	*/
> +	dev->free_fib = &dev->fibs[dev->scsi_host_ptr->can_queue];
>  	return 0;
>  }
>  
>  /**
> + *	aac_fib_alloc_tag	-	allocate a fib
> + *	@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];
> +	/*
> +	 *	Null out fields that depend on being zero at the start of
> +	 *	each I/O
> +	 */
> +	fibptr->hw_fib_va->header.XferState = 0;
> +	fibptr->type = FSAFS_NTC_FIB_CONTEXT;
> +	fibptr->callback_data = NULL;
> +	fibptr->callback = NULL;
> +
> +	return fibptr;
> +}
> +
> +/**
>   *	aac_fib_alloc	-	allocate a fib
>   *	@dev: Adapter to allocate the fib for
>   *
> diff --git a/drivers/scsi/aacraid/dpcsup.c b/drivers/scsi/aacraid/dpcsup.c
> index da9d993..d677b52 100644
> --- a/drivers/scsi/aacraid/dpcsup.c
> +++ b/drivers/scsi/aacraid/dpcsup.c
> @@ -394,7 +394,6 @@ unsigned int aac_intr_normal(struct aac_dev *dev, u32 index,
>  				fib->callback(fib->callback_data, fib);
>  			} else {
>  				aac_fib_complete(fib);
> -				aac_fib_free(fib);
>  			}
>  		} else {
>  			unsigned long flagv;
> @@ -416,7 +415,6 @@ unsigned int aac_intr_normal(struct aac_dev *dev, u32 index,
>  				fib->done = 0;
>  				spin_unlock_irqrestore(&fib->event_lock, flagv);
>  				aac_fib_complete(fib);
> -				aac_fib_free(fib);
>  			}
>  
>  		}
> diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
> index 76eaa38..129a515 100644
> --- a/drivers/scsi/aacraid/linit.c
> +++ b/drivers/scsi/aacraid/linit.c
> @@ -454,6 +454,8 @@ static int aac_slave_configure(struct scsi_device *sdev)
>  	} else
>  		scsi_change_queue_depth(sdev, 1);
>  
> +		sdev->tagged_supported = 1;
> +
>  	return 0;
>  }
>  


  parent reply	other threads:[~2016-01-19 15:34 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-15  7:16 [PATCH V3 0/9] aacraid: Patchset for aacraid driver version 41052 Raghava Aditya Renukunta
2016-01-15  7:16 ` [PATCH V3 1/9] aacraid: SCSI blk tag support Raghava Aditya Renukunta
2016-01-15  0:17   ` Seymour, Shane M
2016-01-18 11:13   ` Johannes Thumshirn
2016-01-19 15:33   ` Tomas Henzl [this message]
2016-01-15  7:16 ` [PATCH V3 2/9] aacraid: Fix RRQ overload Raghava Aditya Renukunta
2016-01-15  7:16 ` [PATCH V3 3/9] aacraid: Added EEH support Raghava Aditya Renukunta
2016-01-18 11:16   ` Johannes Thumshirn
2016-01-23 22:55   ` kbuild test robot
2016-01-15  7:16 ` [PATCH V3 4/9] aacraid: Fix memory leak in aac_fib_map_free Raghava Aditya Renukunta
2016-01-15  7:16 ` [PATCH V3 5/9] aacraid: Set correct msix count for EEH recovery Raghava Aditya Renukunta
2016-01-18 11:18   ` Johannes Thumshirn
2016-01-15  7:16 ` [PATCH V3 6/9] aacraid: Fundamental reset support for Series 7 Raghava Aditya Renukunta
2016-01-19 15:43   ` Tomas Henzl
2016-01-15  7:16 ` [PATCH V3 7/9] aacraid: Fix AIF triggered IOP_RESET Raghava Aditya Renukunta
2016-01-14 23:35   ` Seymour, Shane M
2016-01-18 11:56   ` Johannes Thumshirn
2016-01-19 16:14   ` Tomas Henzl
2016-01-20 20:32     ` Raghava Aditya Renukunta
2016-01-22 13:14       ` Tomas Henzl
2016-01-23  5:07         ` Raghava Aditya Renukunta
2016-01-27  2:31         ` Martin K. Petersen
2016-01-27 18:02           ` Raghava Aditya Renukunta
2016-01-15  7:16 ` [PATCH V3 8/9] aacraid: Fix character device re-initialization Raghava Aditya Renukunta
2016-01-18 11:58   ` Johannes Thumshirn
2016-01-20 13:41   ` Tomas Henzl
2016-01-20 20:43     ` Raghava Aditya Renukunta
2016-01-22 13:08       ` Tomas Henzl
2016-01-23  5:07         ` Raghava Aditya Renukunta
2016-01-15  7:16 ` [PATCH V3 9/9] aacraid: Update driver version Raghava Aditya Renukunta

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=569E5766.7090602@redhat.com \
    --to=thenzl@redhat.com \
    --cc=David.Carroll@pmcs.com \
    --cc=Gana.Sridaran@pmcs.com \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=Mahesh.Rajashekhara@pmcs.com \
    --cc=Murthy.Bhat@pmcs.com \
    --cc=RaghavaAditya.Renukunta@pmcs.com \
    --cc=Scott.Benesh@pmcs.com \
    --cc=aacraid@pmc-sierra.com \
    --cc=jthumshirn@suse.de \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=shane.seymour@hpe.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.