Messages in this thread | | | Date | Fri, 20 Jan 2006 11:18:15 +0100 | From | Nick Piggin <> | Subject | Re: [patch] sg: simplify page_count manipulations |
| |
On Thu, Jan 19, 2006 at 02:05:25PM -0800, Andrew Morton wrote: > Nick Piggin <npiggin@suse.de> wrote: > > > > On Wed, Jan 18, 2006 at 07:59:37PM -0800, Andrew Morton wrote: > > > Nick Piggin <npiggin@suse.de> wrote: > > > > - /* N.B. correction _not_ applied to base page of each allocation */ > > > > - for (k = 0; k < rsv_schp->k_use_sg; ++k, ++sg) { > > > > - for (m = PAGE_SIZE; m < sg->length; m += PAGE_SIZE) { > > > > - page = sg->page; > > > > - if (startFinish) > > > > - get_page(page); > > > > - else { > > > > - if (page_count(page) > 0) > > > > - __put_page(page); > > > > - } > > > > - } > > > > - } > > > > -} > > > > > > What on earth is the above trying to do? The inner loop is a rather > > > complex way of doing atomic_add(&page->count, sg->length/PAGE_SIZE). One > > > suspects there's a missing "[m]" in there. > > > > > > > It does this on the first mmap of the device, in the hope that subsequent > > nopage, unmaps would not free the constituent pages in the scatterlist. > > > > But it's doing it wrongly, isn't it? Or am I completely nuts?
No I think you're right. I'm not sure why this doesn't oops but I thought it was the (main) reason others wanted to get rid of this convoluted code earlier on. I see nobody else is planning to do anything about it though, so I figure I must have missed the reason why it isn't a problem.
But either way I don't think the code actually _does_ anything, even if its bugginess doesn't actually lead to a bug.
Nick - 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/
| |