Re: sendfile and page usage statistics

On Wed, Oct 10, 2007 at 12:34:36PM +0300, Kostik Belousov wrote:
> On Wed, Oct 10, 2007 at 10:25:41AM +0400, Igor Sysoev wrote:
> > On Wed, Oct 10, 2007 at 04:14:44PM +1000, Antony Mawer wrote:
> > 
> > > On 10/10/2007 4:06 PM, Igor Sysoev wrote:
> > > >On Wed, Oct 10, 2007 at 09:57:48AM +0400, Igor Sysoev wrote:
> > > >
> > > >>On Tue, Oct 09, 2007 at 04:50:01PM +0400, Igor Sysoev wrote:
> > > >>
> > > >>>As I understand if sendfile() with hardware TX chsum or TSO are used,
> > > >>>then CPU does not touch file pages at all. So pagedaemon never set
> > > >>>PG_REFERENCED to vnode object pages while scanning them.
> > > >>>Does VM set PG_REFERENCED by another way ? Otherwise, often used files
> > > >>>that accessed by sendfile() only have small chances to be cached by VM.
> > > >>I have looked kern_sendfile() in CURRENT and it seems to me that
> > > >>PG_REFERENCED is never set when sendfile() gets cached vm_page.
> > > >>I have looked the code path:
> > > >>
> > > >>vm_page_grab()
> > > >>    vm_page_lookup()
> > > >>    vm_page_wire()
> > > >>sf_buf_alloc() on i386 and amd64
> > > >>
> > > >>and have not seen PG_REFERENCED.
> > > >
> > > >As I understand the following patch against uipc_syscalls.c
> > > >should fix the bug to some extent:
> > > >
> > > >                         if (pg->valid && vm_page_is_valid(pg, pgoff, 
> > > >                         xfsize))
> > > >+                                vm_page_flag_set(pg, PG_REFERENCED);
> > > >                                 VM_OBJECT_UNLOCK(obj);
> > > >                         else if (m != NULL)
> > > 
> > > You are missing opening/closing braces on your if() statement with this 
> > > change...
> > 
> > Oh, thank you! You have saved me from coredump.
> > This is why in my coding style I always use braces for single statement 
> > inside if().
> 
> Also, as a minimum you need to brace the vm_page_flag_set() with
> vm_page_lock_queues()/vm_page_unlock_queues().
> 
> But I think that the actual patch is wrong. The PG_REFERENCED bit shall
> be set when hardware access bit is set on pte. You patch would set it in
> advance.

To clarify my point that is badly worded above:
PG_REFERENCED shall be translated from PG_A (for i386/amd64) in pte when
the sf buffer is dissolved. This way, PG_REFERENCED will be set only
when the page was actually used, preventing page cache pollution.

The busdma, most likely, shall notify pmap on hw access to the page.