[PATCH 0/2] libata tag patches - Kernel

This is a discussion on [PATCH 0/2] libata tag patches - Kernel ; Hi, Two tag related patches for libata. -- Jens Axboe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@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 to Thread
Results 1 to 13 of 13

Thread: [PATCH 0/2] libata tag patches

  1. [PATCH 0/2] libata tag patches

    Hi,

    Two tag related patches for libata.

    --
    Jens Axboe


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

  2. Re: [PATCH 1/2] libata: get rid of ATA_MAX_QUEUE loop in ata_qc_complete_multiple()

    Jens Axboe wrote:
    > We very rarely (if ever) complete more than one command in the
    > sactive mask at the time, even for extremely high IO rates. So
    > looping over the entire range of possible tags is pointless,
    > instead use __ffs() to just find the completed tags directly.
    >
    > Signed-off-by: Jens Axboe
    > ---
    > drivers/ata/libata-core.c | 15 +++++++++------
    > 1 files changed, 9 insertions(+), 6 deletions(-)
    >
    > diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
    > index 1ee9499..c3c53e7 100644
    > --- a/drivers/ata/libata-core.c
    > +++ b/drivers/ata/libata-core.c
    > @@ -4799,9 +4799,9 @@ void ata_qc_complete(struct ata_queued_cmd *qc)
    > */
    > int ata_qc_complete_multiple(struct ata_port *ap, u32 qc_active)
    > {
    > + unsigned int i = 0;
    > int nr_done = 0;
    > u32 done_mask;
    > - int i;
    >
    > done_mask = ap->qc_active ^ qc_active;
    >
    > @@ -4811,16 +4811,19 @@ int ata_qc_complete_multiple(struct ata_port *ap, u32 qc_active)
    > return -EINVAL;
    > }
    >
    > - for (i = 0; i < ATA_MAX_QUEUE; i++) {
    > + while (done_mask) {
    > struct ata_queued_cmd *qc;
    > + unsigned int next = __ffs(done_mask);
    >
    > - if (!(done_mask & (1 << i)))
    > - continue;
    > -
    > - if ((qc = ata_qc_from_tag(ap, i))) {
    > + qc = ata_qc_from_tag(ap, i + next);
    > + if (qc) {
    > ata_qc_complete(qc);
    > nr_done++;
    > }
    > + if (++next >= ATA_MAX_QUEUE)
    > + break;


    If you think about it, this statement is equivalent to

    if (ap->qc_active ^ qc_active == (1 << (ATA_MAX_QUEUE - 1)))

    To fix this, you could say

    if (++next + i >= ATA_MAX_QUEUE)

    but perhaps it would be even more efficient (or not much worse) to skip
    this check entirely.

    Regards,

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

  3. Re: [PATCH 2/2] libata: switch to using block layer tagging support

    Jens Axboe wrote:
    > libata currently has a pretty dumb ATA_MAX_QUEUE loop for finding
    > a free tag to use. Instead of fixing that up, convert libata to
    > using block layer tagging - gets rid of code in libata, and is also
    > much faster.
    >
    > Signed-off-by: Jens Axboe
    > ---
    > drivers/ata/libata-core.c | 66 ++++----------------------------------------
    > drivers/ata/libata-scsi.c | 10 +++++-
    > drivers/ata/libata.h | 19 +++++++++++-
    > include/linux/libata.h | 1 -
    > 4 files changed, 31 insertions(+), 65 deletions(-)


    Just to be sure, I take it this patch series is for 2.6.29?

    Seems nice to have, but since it wasn't ready for the merge window
    opening...

    Jeff



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

  4. Re: [PATCH 2/2] libata: switch to using block layer tagging support

    Jens Axboe wrote:
    > libata currently has a pretty dumb ATA_MAX_QUEUE loop for finding
    > a free tag to use. Instead of fixing that up, convert libata to
    > using block layer tagging - gets rid of code in libata, and is also
    > much faster.
    >
    > Signed-off-by: Jens Axboe


    Acked-by: Tejun Heo

    Thanks.

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

  5. Re: [PATCH 1/2] libata: get rid of ATA_MAX_QUEUE loop in ata_qc_complete_multiple()

    Jens Axboe wrote:
    > @@ -4811,16 +4811,19 @@ int ata_qc_complete_multiple(struct ata_port *ap, u32 qc_active)
    > return -EINVAL;
    > }
    >
    > - for (i = 0; i < ATA_MAX_QUEUE; i++) {
    > + while (done_mask) {
    > struct ata_queued_cmd *qc;
    > + unsigned int next = __ffs(done_mask);
    >
    > - if (!(done_mask & (1 << i)))
    > - continue;
    > -
    > - if ((qc = ata_qc_from_tag(ap, i))) {
    > + qc = ata_qc_from_tag(ap, i + next);
    > + if (qc) {
    > ata_qc_complete(qc);
    > nr_done++;
    > }
    > + if (++next >= ATA_MAX_QUEUE)
    > + break;
    > + i += next;
    > + done_mask >>= next;


    Shouldn't this be...

    i += next + 1;
    if (i >= ATA_MAX_QUEUE)
    break;

    or better...

    while (done_mask) {
    struct ata_queued_cmd *qc;
    unsigned int next = __ffs(done_mask);

    tag += next;
    if ((qc = ata_qc_from_tag(ap, tag))) {
    ata_qc_complete(qc);
    nr_done++;
    }
    next++;
    tag += next;
    done_mask >>= next;
    }

    done_mask is guaranteed to be zero at when tag reaches ATA_MAX_QUEUE.

    Thanks.

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

  6. Re: [PATCH 2/2] libata: switch to using block layer tagging support

    On Wed, Oct 22 2008, Jeff Garzik wrote:
    > Jens Axboe wrote:
    > >libata currently has a pretty dumb ATA_MAX_QUEUE loop for finding
    > >a free tag to use. Instead of fixing that up, convert libata to
    > >using block layer tagging - gets rid of code in libata, and is also
    > >much faster.
    > >
    > >Signed-off-by: Jens Axboe
    > >---
    > > drivers/ata/libata-core.c | 66
    > > ++++----------------------------------------
    > > drivers/ata/libata-scsi.c | 10 +++++-
    > > drivers/ata/libata.h | 19 +++++++++++-
    > > include/linux/libata.h | 1 -
    > > 4 files changed, 31 insertions(+), 65 deletions(-)

    >
    > Just to be sure, I take it this patch series is for 2.6.29?


    Sure yes, it was written and tested just the other day :-)

    > Seems nice to have, but since it wasn't ready for the merge window
    > opening...


    Indeed, but it can certainly wait for 2.6.29.

    --
    Jens Axboe

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

  7. Re: [PATCH 1/2] libata: get rid of ATA_MAX_QUEUE loop in ata_qc_complete_multiple()

    On Thu, Oct 23 2008, Tejun Heo wrote:
    > Jens Axboe wrote:
    > > @@ -4811,16 +4811,19 @@ int ata_qc_complete_multiple(struct ata_port *ap, u32 qc_active)
    > > return -EINVAL;
    > > }
    > >
    > > - for (i = 0; i < ATA_MAX_QUEUE; i++) {
    > > + while (done_mask) {
    > > struct ata_queued_cmd *qc;
    > > + unsigned int next = __ffs(done_mask);
    > >
    > > - if (!(done_mask & (1 << i)))
    > > - continue;
    > > -
    > > - if ((qc = ata_qc_from_tag(ap, i))) {
    > > + qc = ata_qc_from_tag(ap, i + next);
    > > + if (qc) {
    > > ata_qc_complete(qc);
    > > nr_done++;
    > > }
    > > + if (++next >= ATA_MAX_QUEUE)
    > > + break;
    > > + i += next;
    > > + done_mask >>= next;

    >
    > Shouldn't this be...
    >
    > i += next + 1;
    > if (i >= ATA_MAX_QUEUE)
    > break;
    >
    > or better...
    >
    > while (done_mask) {
    > struct ata_queued_cmd *qc;
    > unsigned int next = __ffs(done_mask);
    >
    > tag += next;
    > if ((qc = ata_qc_from_tag(ap, tag))) {
    > ata_qc_complete(qc);
    > nr_done++;
    > }
    > next++;
    > tag += next;
    > done_mask >>= next;
    > }
    >
    > done_mask is guaranteed to be zero at when tag reaches ATA_MAX_QUEUE.


    That does indeed look a lot cleaner. I think it was rewritten at some
    point and kept some of the logic for not passing 0 into __ffs, but it's
    clearly pointless now.

    I'll send out a revised patch when it's tested.

    --
    Jens Axboe

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

  8. Re: [PATCH 1/2] libata: get rid of ATA_MAX_QUEUE loop in ata_qc_complete_multiple()

    On Thu, Oct 23 2008, Tejun Heo wrote:
    > while (done_mask) {
    > struct ata_queued_cmd *qc;
    > unsigned int next = __ffs(done_mask);
    >
    > tag += next;
    > if ((qc = ata_qc_from_tag(ap, tag))) {
    > ata_qc_complete(qc);
    > nr_done++;
    > }
    > next++;
    > tag += next;
    > done_mask >>= next;
    > }


    That doesn't work (you're adding next to tag twice), it needs a little
    tweak:

    while (done_mask) {
    struct ata_queued_cmd *qc;
    unsigned int next = __ffs(done_mask);

    if ((qc = ata_qc_from_tag(ap, tag + next))) {
    ata_qc_complete(qc);
    nr_done++;
    }
    next++;
    tag += next;
    done_mask >>= next;
    }

    and I think it should work. Not tested yet :-)

    --
    Jens Axboe

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

  9. Re: [PATCH 1/2] libata: get rid of ATA_MAX_QUEUE loop in ata_qc_complete_multiple()

    On Wed, Oct 22 2008, Elias Oltmanns wrote:
    > Jens Axboe wrote:
    > > We very rarely (if ever) complete more than one command in the
    > > sactive mask at the time, even for extremely high IO rates. So
    > > looping over the entire range of possible tags is pointless,
    > > instead use __ffs() to just find the completed tags directly.
    > >
    > > Signed-off-by: Jens Axboe
    > > ---
    > > drivers/ata/libata-core.c | 15 +++++++++------
    > > 1 files changed, 9 insertions(+), 6 deletions(-)
    > >
    > > diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
    > > index 1ee9499..c3c53e7 100644
    > > --- a/drivers/ata/libata-core.c
    > > +++ b/drivers/ata/libata-core.c
    > > @@ -4799,9 +4799,9 @@ void ata_qc_complete(struct ata_queued_cmd *qc)
    > > */
    > > int ata_qc_complete_multiple(struct ata_port *ap, u32 qc_active)
    > > {
    > > + unsigned int i = 0;
    > > int nr_done = 0;
    > > u32 done_mask;
    > > - int i;
    > >
    > > done_mask = ap->qc_active ^ qc_active;
    > >
    > > @@ -4811,16 +4811,19 @@ int ata_qc_complete_multiple(struct ata_port *ap, u32 qc_active)
    > > return -EINVAL;
    > > }
    > >
    > > - for (i = 0; i < ATA_MAX_QUEUE; i++) {
    > > + while (done_mask) {
    > > struct ata_queued_cmd *qc;
    > > + unsigned int next = __ffs(done_mask);
    > >
    > > - if (!(done_mask & (1 << i)))
    > > - continue;
    > > -
    > > - if ((qc = ata_qc_from_tag(ap, i))) {
    > > + qc = ata_qc_from_tag(ap, i + next);
    > > + if (qc) {
    > > ata_qc_complete(qc);
    > > nr_done++;
    > > }
    > > + if (++next >= ATA_MAX_QUEUE)
    > > + break;

    >
    > If you think about it, this statement is equivalent to
    >
    > if (ap->qc_active ^ qc_active == (1 << (ATA_MAX_QUEUE - 1)))
    >
    > To fix this, you could say
    >
    > if (++next + i >= ATA_MAX_QUEUE)
    >
    > but perhaps it would be even more efficient (or not much worse) to skip
    > this check entirely.


    Yeah, the check should just be killed, that's the version I posted in
    the reply to Tejun as well.

    --
    Jens Axboe

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

  10. Re: [PATCH 1/2] libata: get rid of ATA_MAX_QUEUE loop in ata_qc_complete_multiple()

    On Thu, Oct 23 2008, Jens Axboe wrote:
    > On Thu, Oct 23 2008, Tejun Heo wrote:
    > > while (done_mask) {
    > > struct ata_queued_cmd *qc;
    > > unsigned int next = __ffs(done_mask);
    > >
    > > tag += next;
    > > if ((qc = ata_qc_from_tag(ap, tag))) {
    > > ata_qc_complete(qc);
    > > nr_done++;
    > > }
    > > next++;
    > > tag += next;
    > > done_mask >>= next;
    > > }

    >
    > That doesn't work (you're adding next to tag twice), it needs a little
    > tweak:
    >
    > while (done_mask) {
    > struct ata_queued_cmd *qc;
    > unsigned int next = __ffs(done_mask);
    >
    > if ((qc = ata_qc_from_tag(ap, tag + next))) {
    > ata_qc_complete(qc);
    > nr_done++;
    > }
    > next++;
    > tag += next;
    > done_mask >>= next;
    > }
    >
    > and I think it should work. Not tested yet :-)


    Pondered some more, and it can't work. The problem is that if we
    complete tag 31, we attempt to shift done_mask down by 32 bits. On a
    32-bit arch, that's not defined. So we DO need a check like the existing
    one, or something similar.

    So I don't think we need to make changes to this patch either, at least
    unless one of you can come up with a better check that avoids a branch.

    --
    Jens Axboe

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

  11. Re: [PATCH 1/2] libata: get rid of ATA_MAX_QUEUE loop in ata_qc_complete_multiple()

    Jens Axboe wrote:
    > On Thu, Oct 23 2008, Jens Axboe wrote:
    >> On Thu, Oct 23 2008, Tejun Heo wrote:

    >
    >> > while (done_mask) {
    >> > struct ata_queued_cmd *qc;
    >> > unsigned int next = __ffs(done_mask);
    >> >
    >> > tag += next;
    >> > if ((qc = ata_qc_from_tag(ap, tag))) {
    >> > ata_qc_complete(qc);
    >> > nr_done++;
    >> > }
    >> > next++;
    >> > tag += next;
    >> > done_mask >>= next;
    >> > }

    >>
    >> That doesn't work (you're adding next to tag twice), it needs a little
    >> tweak:
    >>
    >> while (done_mask) {
    >> struct ata_queued_cmd *qc;
    >> unsigned int next = __ffs(done_mask);
    >>
    >> if ((qc = ata_qc_from_tag(ap, tag + next))) {
    >> ata_qc_complete(qc);
    >> nr_done++;
    >> }
    >> next++;
    >> tag += next;
    >> done_mask >>= next;
    >> }
    >>
    >> and I think it should work. Not tested yet :-)

    >
    > Pondered some more, and it can't work. The problem is that if we
    > complete tag 31, we attempt to shift done_mask down by 32 bits. On a
    > 32-bit arch, that's not defined. So we DO need a check like the existing
    > one, or something similar.
    >
    > So I don't think we need to make changes to this patch either, at least
    > unless one of you can come up with a better check that avoids a branch.


    What about a switch outside the while loop:

    if (done_mask == ATA_MAX_QUEUE >> 1) {
    if ((qc = ata_qc_from_tag(ap, ATA_MAX_QUEUE >> 1))) {
    ata_qc_complete(qc);
    nr_done = 1;
    }
    } else
    while (done_mask)
    ...

    Alternatively, you could just alter tag and done_mask (tag =
    ATA_MAX_QUEUE >> 2, done_mask = 2) and enter the while loop
    unconditionally. But then, you claimed that there will hardly ever be
    more than one command to complete, so my suggestions will probably not
    improve anything in real life.

    Regards,

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

  12. Re: [PATCH 2/2] libata: switch to using block layer tagging support

    On Wed, Oct 22, 2008 at 09:40:43AM +0200, Jens Axboe wrote:
    > libata currently has a pretty dumb ATA_MAX_QUEUE loop for finding
    > a free tag to use. Instead of fixing that up, convert libata to
    > using block layer tagging - gets rid of code in libata, and is also
    > much faster.
    >
    > Signed-off-by: Jens Axboe


    Unfortunately this change breaks SATA for me, bisecting picked out this
    commit especially.

    Before:

    sata_sil 0000:00:01.0: Applying R_ERR on DMA activate FIS errata fix
    scsi0 : sata_sil
    scsi1 : sata_sil
    ata1: SATA max UDMA/100 mmio m512@0xfd000200 tf 0xfd000280 irq 66
    ata2: SATA max UDMA/100 mmio m512@0xfd000200 tf 0xfd0002c0 irq 66
    ata1: SATA link up 1.5 Gbps (SStatus 113 SControl 310)
    ata1.00: ATA-7: SAMSUNG HM080JI, YC100-02, max UDMA7
    ata1.00: 156368016 sectors, multi 0: LBA48 NCQ (depth 0/32)
    ata1.00: configured for UDMA/100
    ata2: SATA link down (SStatus 0 SControl 310)
    scsi 0:0:0:0: Direct-Access ATA SAMSUNG HM080JI YC10 PQ: 0 ANSI: 5
    sd 0:0:0:0: [sda] 156368016 512-byte hardware sectors: (80.0 GB/74.5 GiB)
    sd 0:0:0:0: [sda] Write Protect is off
    sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
    sd 0:0:0:0: [sda] 156368016 512-byte hardware sectors: (80.0 GB/74.5 GiB)
    sd 0:0:0:0: [sda] Write Protect is off
    sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
    sda: sda1 sda2
    sd 0:0:0:0: [sda] Attached SCSI disk

    After:

    sata_sil 0000:00:01.0: Applying R_ERR on DMA activate FIS errata fix
    scsi0 : sata_sil
    scsi1 : sata_sil
    ata1: SATA max UDMA/100 mmio m512@0xfd000200 tf 0xfd000280 irq 66
    ata2: SATA max UDMA/100 mmio m512@0xfd000200 tf 0xfd0002c0 irq 66
    ata1: SATA link up 1.5 Gbps (SStatus 113 SControl 310)
    ata1.00: ATA-7: SAMSUNG HM080JI, YC100-02, max UDMA7
    ata1.00: 156368016 sectors, multi 0: LBA48 NCQ (depth 0/32)
    ata1.00: configured for UDMA/100
    ata2: SATA link down (SStatus 0 SControl 310)
    scsi 0:0:0:0: Direct-Access ATA SAMSUNG HM080JI YC10 PQ: 0 ANSI: 5
    sd 0:0:0:0: [sda] 156368016 512-byte hardware sectors: (80.0 GB/74.5 GiB)
    sd 0:0:0:0: [sda] Write Protect is off
    sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
    sd 0:0:0:0: [sda] 156368016 512-byte hardware sectors: (80.0 GB/74.5 GiB)
    sd 0:0:0:0: [sda] Write Protect is off
    sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
    sda:

    Where it hangs until it times out after 180 seconds.

    Built with:

    CONFIG_ATA=y
    CONFIG_SATA_PMP=y
    CONFIG_ATA_SFF=y
    CONFIG_SATA_SIL=y

    I have no idea where to start looking at this, so hopefully someone has some
    suggestions :-)
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  13. Re: [PATCH 1/2] libata: get rid of ATA_MAX_QUEUE loop in ata_qc_complete_multiple()

    On Thu, Oct 23 2008, Elias Oltmanns wrote:
    > Jens Axboe wrote:
    > > On Thu, Oct 23 2008, Jens Axboe wrote:
    > >> On Thu, Oct 23 2008, Tejun Heo wrote:

    > >
    > >> > while (done_mask) {
    > >> > struct ata_queued_cmd *qc;
    > >> > unsigned int next = __ffs(done_mask);
    > >> >
    > >> > tag += next;
    > >> > if ((qc = ata_qc_from_tag(ap, tag))) {
    > >> > ata_qc_complete(qc);
    > >> > nr_done++;
    > >> > }
    > >> > next++;
    > >> > tag += next;
    > >> > done_mask >>= next;
    > >> > }
    > >>
    > >> That doesn't work (you're adding next to tag twice), it needs a little
    > >> tweak:
    > >>
    > >> while (done_mask) {
    > >> struct ata_queued_cmd *qc;
    > >> unsigned int next = __ffs(done_mask);
    > >>
    > >> if ((qc = ata_qc_from_tag(ap, tag + next))) {
    > >> ata_qc_complete(qc);
    > >> nr_done++;
    > >> }
    > >> next++;
    > >> tag += next;
    > >> done_mask >>= next;
    > >> }
    > >>
    > >> and I think it should work. Not tested yet :-)

    > >
    > > Pondered some more, and it can't work. The problem is that if we
    > > complete tag 31, we attempt to shift done_mask down by 32 bits. On a
    > > 32-bit arch, that's not defined. So we DO need a check like the existing
    > > one, or something similar.
    > >
    > > So I don't think we need to make changes to this patch either, at least
    > > unless one of you can come up with a better check that avoids a branch.

    >
    > What about a switch outside the while loop:
    >
    > if (done_mask == ATA_MAX_QUEUE >> 1) {
    > if ((qc = ata_qc_from_tag(ap, ATA_MAX_QUEUE >> 1))) {
    > ata_qc_complete(qc);
    > nr_done = 1;
    > }
    > } else
    > while (done_mask)
    > ...
    >
    > Alternatively, you could just alter tag and done_mask (tag =
    > ATA_MAX_QUEUE >> 2, done_mask = 2) and enter the while loop
    > unconditionally. But then, you claimed that there will hardly ever be
    > more than one command to complete, so my suggestions will probably not
    > improve anything in real life.


    Honestly, I think the current check is a lot cleaner then.

    --
    Jens Axboe

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@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 to Thread