David Miller <davem@xxxxxxxxxxxxx> writes:
> [ As I am trying to remove direct SKB list pointer accesses I am
> committing this to net-next. If this causes a lot of grief I
> can and will revert, just let me know. ]
>
> Instead of direct SKB list pointer accesses.
>
> The loops in this function had to be rewritten to accommodate this
> more easily.
>
> The first loop iterates now over the target list in the outer loop,
> and triggers an mmc data operation when the per-operation limits are
> hit.
>
> Then after the loops, if we have any residue, we trigger the last
> and final operation.
>
> For the page aligned workaround, where we have to copy the read data
> back into the original list of SKBs, we use a two-tiered loop. The
> outer loop stays the same and iterates over pktlist, and then we have
> an inner loop which uses skb_peek_next(). The break logic has been
> simplified because we know that the aggregate length of the SKBs in
> the source and destination lists are the same.
>
> This change also ends up fixing a bug, having to do with the
> maintainance of the seg_sz variable and how it drove the outermost
> loop. It begins as:
>
> seg_sz = target_list->qlen;
>
> ie. the number of packets in the target_list queue. The loop
> structure was then:
>
> while (seq_sz) {
> ...
> while (not at end of target_list) {
> ...
> sg_cnt++
> ...
> }
> ...
> seg_sz -= sg_cnt;
>
> The assumption built into that last statement is that sg_cnt counts
> how many packets from target_list have been fully processed by the
> inner loop. But this not true.
>
> If we hit one of the limits, such as the max segment size or the max
> request size, we will break and copy a partial packet then contine
> back up to the top of the outermost loop.
>
> With the new loops we don't have this problem as we don't guard the
> loop exit with a packet count, but instead use the progression of the
> pkt_next SKB through the list to the end. The general structure is:
>
> sg_cnt = 0;
> skb_queue_walk(target_list, pkt_next) {
> pkt_offset = 0;
> ...
> sg_cnt++;
> ...
> while (pkt_offset < pkt_next->len) {
> pkt_offset += sg_data_size;
> if (queued up max per request)
> mmc_submit_one();
> }
> }
> if (sg_cnt)
> mmc_submit_one();
>
> The variables that maintain where we are in the MMC command state such
> as req_sz, sg_cnt, and sgl are reset when we emit one of these full
> sized requests.
>
> Signed-off-by: David S. Miller <davem@xxxxxxxxxxxxx>
Looks good to me, thanks.
Acked-by: Kalle Valo <kvalo@xxxxxxxxxxxxxx>
--
Kalle Valo
