diff mbox

[RESEND,0/1] AHCI: Optimize interrupt processing

Message ID 20130809082335.GA25306@dhcp-26-207.brq.redhat.com
State Not Applicable
Delegated to: David Miller
Headers show

Commit Message

Alexander Gordeev Aug. 9, 2013, 8:23 a.m. UTC 
On Mon, Jul 29, 2013 at 07:46:53AM -0400, Tejun Heo wrote:
> One thing which would probably be worthwhile tho is getting rid of the
> bitmap based qc tag allocator in libata.  That one is just borderline
> stupid to keep around on any setup which is supposed to be scalable.

Hi Tejun,

How about this approach?


> Thanks.
> 
> -- 
> tejun

Comments

Tejun Heo Aug. 9, 2013, 2:15 p.m. UTC | #1 
On Fri, Aug 09, 2013 at 10:23:35AM +0200, Alexander Gordeev wrote:
> On Mon, Jul 29, 2013 at 07:46:53AM -0400, Tejun Heo wrote:
> > One thing which would probably be worthwhile tho is getting rid of the
> > bitmap based qc tag allocator in libata.  That one is just borderline
> > stupid to keep around on any setup which is supposed to be scalable.
> 
> Hi Tejun,
> 
> How about this approach?

Haven't looked at it thoroughly and I still don't know anything about
blk-mq but it looks good on the first glance.

Thanks.
Jens Axboe Aug. 9, 2013, 2:24 p.m. UTC | #2 
On 08/09/2013 02:23 AM, Alexander Gordeev wrote:
> On Mon, Jul 29, 2013 at 07:46:53AM -0400, Tejun Heo wrote:
>> One thing which would probably be worthwhile tho is getting rid of the
>> bitmap based qc tag allocator in libata.  That one is just borderline
>> stupid to keep around on any setup which is supposed to be scalable.
> 
> Hi Tejun,
> 
> How about this approach?
> 
> @@ -5639,6 +5627,12 @@ struct ata_port *ata_port_alloc(struct ata_host *host)
>  	if (!ap)
>  		return NULL;
>  
> +	ap->qc_tags = blk_mq_init_tags(ATA_MAX_QUEUE, 1, NUMA_NO_NODE);
> +	if (!ap->qc_tags) {
> +		kfree(ap);
> +		return NULL;
> +	}

This should be blk_mq_init_tags(ATA_MAX_QUEUE - 1, 1, ...) since the
total depth is normal_tags + reserved_tags. Apart from that, I think it
looks alright based on a cursory look.
diff mbox

Patch

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index f218427..5c2a236 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -72,6 +72,8 @@ 
 #include "libata.h"
 #include "libata-transport.h"
 
+#include "../../block/blk-mq-tag.h"
+
 /* debounce timing parameters in msecs { interval, duration, timeout } */
 const unsigned long sata_deb_timing_normal[]		= {   5,  100, 2000 };
 const unsigned long sata_deb_timing_hotplug[]		= {  25,  500, 2000 };
@@ -1569,18 +1571,8 @@  unsigned ata_exec_internal_sg(struct ata_device *dev,
 
 	/* initialize internal qc */
 
-	/* XXX: Tag 0 is used for drivers with legacy EH as some
-	 * drivers choke if any other tag is given.  This breaks
-	 * ata_tag_internal() test for those drivers.  Don't use new
-	 * EH stuff without converting to it.
-	 */
-	if (ap->ops->error_handler)
-		tag = ATA_TAG_INTERNAL;
-	else
-		tag = 0;
-
-	if (test_and_set_bit(tag, &ap->qc_allocated))
-		BUG();
+	tag = blk_mq_get_tag(ap->qc_tags, GFP_ATOMIC, true);
+	BUG_ON(!ata_tag_internal(tag));
 	qc = __ata_qc_from_tag(ap, tag);
 
 	qc->tag = tag;
@@ -4733,21 +4725,17 @@  void swap_buf_le16(u16 *buf, unsigned int buf_words)
 static struct ata_queued_cmd *ata_qc_new(struct ata_port *ap)
 {
 	struct ata_queued_cmd *qc = NULL;
-	unsigned int i;
+	unsigned int tag;
 
 	/* no command while frozen */
 	if (unlikely(ap->pflags & ATA_PFLAG_FROZEN))
 		return NULL;
 
-	/* the last tag is reserved for internal command. */
-	for (i = 0; i < ATA_MAX_QUEUE - 1; i++)
-		if (!test_and_set_bit(i, &ap->qc_allocated)) {
-			qc = __ata_qc_from_tag(ap, i);
-			break;
-		}
+	tag = blk_mq_get_tag(ap->qc_tags, GFP_ATOMIC, false);
+	qc = __ata_qc_from_tag(ap, tag);
 
 	if (qc)
-		qc->tag = i;
+		qc->tag = tag;
 
 	return qc;
 }
@@ -4799,7 +4787,7 @@  void ata_qc_free(struct ata_queued_cmd *qc)
 	tag = qc->tag;
 	if (likely(ata_tag_valid(tag))) {
 		qc->tag = ATA_TAG_POISON;
-		clear_bit(tag, &ap->qc_allocated);
+		blk_mq_put_tag(ap->qc_tags, tag);
 	}
 }
 
@@ -5639,6 +5627,12 @@  struct ata_port *ata_port_alloc(struct ata_host *host)
 	if (!ap)
 		return NULL;
 
+	ap->qc_tags = blk_mq_init_tags(ATA_MAX_QUEUE, 1, NUMA_NO_NODE);
+	if (!ap->qc_tags) {
+		kfree(ap);
+		return NULL;
+	}
+
 	ap->pflags |= ATA_PFLAG_INITIALIZING | ATA_PFLAG_FROZEN;
 	ap->lock = &host->lock;
 	ap->print_id = -1;
@@ -5677,6 +5671,14 @@  struct ata_port *ata_port_alloc(struct ata_host *host)
 	return ap;
 }
 
+static void ata_port_free(struct ata_port *ap)
+{
+	kfree(ap->pmp_link);
+	kfree(ap->slave_link);
+	blk_mq_free_tags(ap->qc_tags);
+	kfree(ap);
+}
+
 static void ata_host_release(struct device *gendev, void *res)
 {
 	struct ata_host *host = dev_get_drvdata(gendev);
@@ -5691,9 +5693,7 @@  static void ata_host_release(struct device *gendev, void *res)
 		if (ap->scsi_host)
 			scsi_host_put(ap->scsi_host);
 
-		kfree(ap->pmp_link);
-		kfree(ap->slave_link);
-		kfree(ap);
+		ata_port_free(ap);
 		host->ports[i] = NULL;
 	}
 
diff --git a/include/linux/libata.h b/include/linux/libata.h
index eae7a05..4ff9494 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -126,7 +126,7 @@  enum {
 	ATA_DEF_QUEUE		= 1,
 	/* tag ATA_MAX_QUEUE - 1 is reserved for internal commands */
 	ATA_MAX_QUEUE		= 32,
-	ATA_TAG_INTERNAL	= ATA_MAX_QUEUE - 1,
+	ATA_TAG_INTERNAL	= 0,
 	ATA_SHORT_PAUSE		= 16,
 
 	ATAPI_MAX_DRAIN		= 16 << 10,
@@ -766,7 +766,7 @@  struct ata_port {
 	unsigned int		cbl;	/* cable type; ATA_CBL_xxx */
 
 	struct ata_queued_cmd	qcmd[ATA_MAX_QUEUE];
-	unsigned long		qc_allocated;
+	struct blk_mq_tags	*qc_tags;
 	unsigned int		qc_active;
 	int			nr_active_links; /* #links with active qcs */