lkml.org 
[lkml]   [2008]   [Oct]   [23]   [last100]   RSS Feed
Views: [more markup]  [less markup]  [headers]  [forward] 
 
Messages in this thread
    /
    DateThu, 23 Oct 2008 08:37:16 +0200
    FromJens Axboe <>
    SubjectRe: [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
    
    
    
    
    \
     
     \ /
      Last update: 2008-10-23 08:41    [from the cache]
    ©2003-2008