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?

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 */
 

> Thanks.
> 
> -- 
> tejun

-- 
Regards,
Alexander Gordeev
agord...@redhat.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to