Re: [PATCH] fbdefio: add set_page_dirty handler to deferred IO FB

[Jaya Kumar]

On Tue, Aug 19, 2008 at 2:38 AM, Andrew Morton
<akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
> On Tue, 19 Aug 2008 07:02:45 +0100 Ian Campbell <ijc@xxxxxxxxxxxxxx> wrote:
>
>> +static int fb_deferred_io_set_page_dirty(struct page *page)
>> +{
>> +     if (!PageDirty(page))
>> +             SetPageDirty(page);
>> +     return 0;
>> +}
>
> <searches, finds the thread.  "kernel BUG at lib/radix-tree.c:473!">
>
> Is there actually any benefit in setting these pages dirty?  Or should
> this be an empty function?  I see claims in the above thread that this
> driver uses PG_dirty for optimising writeback but I can't immediately
> locate any code which actually does that.

Hi Andrew,

I hope I have understood your question. You are right that PG_dirty
isn't used directly in defio. The defio portion does use each
page_mkwrite callback to build a list of the pages of the framebuffer
that were written to and then passes that list to pvfb (in this case).
pvfb then optimizes writeback by interpreting that list according to
its framebuffer and sending it to its actual destination. I think
Markus's code in xenfb_deferred_io() and xenfb_send* is doing the
latter.

>
> I suspect that we just shouldn't be pretending that this is a regular
> anon/pagecache page to this extent.  Maybe a suitable fix would be to
> teach fb_deferred_io_fault() to instantiate the pte itself
> (vm_insert_page()) and then return VM_FAULT_NOPAGE?
>

Ok, I haven't understood all the details of vm_insert_page() yet. I
noticed Peter's message that this won't work with page_mkclean (which
I need to do so that we can re-receive fault notification after the
driver has done writeback). I see your remark and Ian's that it would
be better to try to do the mapping and a_ops setup at open time. I
think I have some ideas of how to do this without breaking fb code
much. I will try to work on this issue this weekend. I'm sorry that
I'm so slow.

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