Re: Warning on memory offline (possible in migration ?)

Re: Warning on memory offline (possible in migration ?)

[KAMEZAWA Hiroyuki]

On Thu, 17 Apr 2008 09:19:30 +0900
KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> > I'd expect that you could reproduce this by disabling readahead with
> > fadvise(POSIX_FADV_RANDOM) and then issuing the above four reads.
> > 
> Thank you for advice. I'll try.
> 
(Added lkml to CC:)

What happens:
  When I do memory offline on ia64/NUMA box, __set_page_dirty_buffers() printed
  out WARNINGS because the page under migration is not up-to-date.

Following is my investigation.

Assume 16k pages / 4 buffers of 4096bytes block (ext3).
4 buffers on a page of ext3.

At page offlining, we can find a page which is not up-to-date.
But all buffers of the page seems up-to-date.

buffers on a page by prink().
    buffer 0, block_nr= some vaule, state= BH_uptodate | BH_Req| BH_Mapped
    buffer 1, block_nr= -1,         state= BH_uptodate
    buffer 2, block_nr= -1,         state= BH_uptodate
    buffer 3, block_nr= -1,         state= BH_uptodate

It seems no I/O for 3 buffers. It's because the page is the last page of inode
and blocks for buffer[1,2,3] is not assgined.
(maybe BH_uptodate is set by block_write_full_page().

Adding below check can hide the warning....but I can't say this is correct.
Can we set this page dirty silently in this case ? 

===
+
+static int check_fragment_page(struct page *page, struct address_space *mapping
)
+{
+       struct inode *inode = mapping->host;
+       unsigned long lastblock, coverblock;
+
+       if (!page_has_buffers(page))
+               return 0;
+
+       lastblock = (i_size_read(inode) - 1) >> inode->i_blkbits;
+       coverblock = (page->index + 1) << (PAGE_SHIFT - inode->i_blkbits);
+
+       return coverblock > lastblock;
+}
+
+
+
 static int __set_page_dirty(struct page *page,
                struct address_space *mapping, int warn)
 {
@@ -717,7 +734,9 @@ static int __set_page_dirty(struct page

        write_lock_irq(&mapping->tree_lock);
        if (page->mapping) {    /* Race with truncate? */
-               WARN_ON_ONCE(warn && !PageUptodate(page));
+               WARN_ON_ONCE(warn
+                            && !PageUptodate(page)
+                            && !check_fragment_page(page, mapping));

                if (mapping_cap_account_dirty(mapping)) {
                        __inc_zone_page_state(page, NR_FILE_DIRTY);
==

Thanks,
-Kame

Re: Warning on memory offline (possible in migration ?)

[Andrew Morton]

On Thu, 17 Apr 2008 15:38:18 +0900 KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:

> On Thu, 17 Apr 2008 09:19:30 +0900
> KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> > > I'd expect that you could reproduce this by disabling readahead with
> > > fadvise(POSIX_FADV_RANDOM) and then issuing the above four reads.
> > > 
> > Thank you for advice. I'll try.
> > 
> (Added lkml to CC:)
> 
> What happens:
>   When I do memory offline on ia64/NUMA box, __set_page_dirty_buffers() printed
>   out WARNINGS because the page under migration is not up-to-date.

The warning is in __set_page_dirty().

> Following is my investigation.
> 
> Assume 16k pages / 4 buffers of 4096bytes block (ext3).
> 4 buffers on a page of ext3.
> 
> At page offlining, we can find a page which is not up-to-date.
> But all buffers of the page seems up-to-date.
> 
> buffers on a page by prink().
>     buffer 0, block_nr= some vaule, state= BH_uptodate | BH_Req| BH_Mapped
>     buffer 1, block_nr= -1,         state= BH_uptodate
>     buffer 2, block_nr= -1,         state= BH_uptodate
>     buffer 3, block_nr= -1,         state= BH_uptodate
> 
> It seems no I/O for 3 buffers. It's because the page is the last page of inode
> and blocks for buffer[1,2,3] is not assgined.
> (maybe BH_uptodate is set by block_write_full_page().
> 
> Adding below check can hide the warning....but I can't say this is correct.
> Can we set this page dirty silently in this case ? 
> 
> ===
> +
> +static int check_fragment_page(struct page *page, struct address_space *mapping
> )
> +{
> +       struct inode *inode = mapping->host;
> +       unsigned long lastblock, coverblock;
> +
> +       if (!page_has_buffers(page))
> +               return 0;
> +
> +       lastblock = (i_size_read(inode) - 1) >> inode->i_blkbits;
> +       coverblock = (page->index + 1) << (PAGE_SHIFT - inode->i_blkbits);
> +
> +       return coverblock > lastblock;
> +}
> +
> +
> +
>  static int __set_page_dirty(struct page *page,
>                 struct address_space *mapping, int warn)
>  {
> @@ -717,7 +734,9 @@ static int __set_page_dirty(struct page
> 
>         write_lock_irq(&mapping->tree_lock);
>         if (page->mapping) {    /* Race with truncate? */
> -               WARN_ON_ONCE(warn && !PageUptodate(page));
> +               WARN_ON_ONCE(warn
> +                            && !PageUptodate(page)
> +                            && !check_fragment_page(page, mapping));
> 
>                 if (mapping_cap_account_dirty(mapping)) {
>                         __inc_zone_page_state(page, NR_FILE_DIRTY);
> ==

The warning is just wrong, I think.  We don't nowmally hit it because
write() will use mark_buffer_dirty() which supresses the warning and mmaped
pages are uptodate.

Nick?

Re: Warning on memory offline (possible in migration ?)

[KAMEZAWA Hiroyuki]

On Wed, 16 Apr 2008 23:43:03 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Thu, 17 Apr 2008 15:38:18 +0900 KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> 
> > On Thu, 17 Apr 2008 09:19:30 +0900
> > KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> > > > I'd expect that you could reproduce this by disabling readahead with
> > > > fadvise(POSIX_FADV_RANDOM) and then issuing the above four reads.
> > > > 
> > > Thank you for advice. I'll try.
> > > 
> > (Added lkml to CC:)
> > 
> > What happens:
> >   When I do memory offline on ia64/NUMA box, __set_page_dirty_buffers() printed
> >   out WARNINGS because the page under migration is not up-to-date.
> 
> The warning is in __set_page_dirty().
> 
Sorry, __set_page_dirty() in fs/buffer.c 

Thanks,
-Kame