linux-ext4 - Re: delayed allocatiou result in Oops

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <1182214878.3711.21.camel@dyn9047017103.beaverton.ibm.com>
Date:	Mon, 18 Jun 2007 18:01:17 -0700
From:	Mingming Cao <cmm@...ibm.com>
To:	Dmitriy Monakhov <dmonakhov@...ru>
Cc:	Alex Tomas <alex@...sterfs.com>, linux-ext4@...r.kernel.org
Subject: Re: delayed allocatiou result in Oops

On Sat, 2007-06-16 at 12:14 +0400, Dmitriy Monakhov wrote:
> On 16:16 Птн 15 Июн     , Mingming Cao wrote:
> > I hit almost the same issue today also, but with different error #, and
> > one more kernel oops, when run fsstress on x86_64. 
> > 
> > EXT4-fs: writeback error = -2
> > EXT4-fs: writeback error = -2
> This error never happens in writeback in my case, only ENOSPC.
I see why, error -2 is ENOENT, your kernel already patched with your
previus patch to fix the return error from ext4_reserve_global() from
ENOENT to ENOSPC.

> > > > I've digged this a litle bit with folowig results:
> > > > 
> > > > int block_read_full_page(struct page *page, get_block_t *get_block)
> > > > {
> > > > ...
> > > > 1914:	if (!page_has_buffers(page)) <<< page_has_buffers(page) == true 
> > > > 		create_empty_buffers(page, blocksize, 0);
> > > > 	head = page_buffers(page); <<<<  page_buffers(page) == NULL  
> > > > <<<i've add debug info here:
> > > > <<< page->flags == 100000000000821
> > > > <<< PagePrivate(page) == 1, (page)->private == NULL
> > > > <<< So we have private page without buffers, it is WRONG.
> > > > 

Right, I think the problem is the PagePrivate() is being set without a
valid page->private pointer.

The PagePrivate flag is being set by delayed allocation in the
ext4_wb_commit_write().

Looking at ext4-delayed-allocation.patch

+int ext4_wb_commit_write(struct file *file, struct page *page,
+                            unsigned from, unsigned to)
+{
+       loff_t pos = ((loff_t)page->index << PAGE_CACHE_SHIFT) + to;
+       struct inode *inode = page->mapping->host;
+       int err = 0;
+
+       wb_debug("commit page %lu (%u-%u) for inode %lu\n",
+                       page->index, from, to, inode->i_ino);
+
+       /* mark page private so that we get
+        * called to invalidate/release page */
+       SetPagePrivate(page);
+
+       if (!PageBooked(page) && !PageMappedToDisk(page)) {
+               /* ->prepare_write() observed that block for this
+                * page hasn't been allocated yet. there fore it
+                * asked to reserve block for later allocation */
+               BUG_ON(page->private == 0);
+               page->private = 0;
+               err = ext4_wb_reserve_space_page(page, 1);
+               if (err)
+                       return err;
+       }


It sets the PagePrivate flag at the commit_write() time.  And the page-
>private is cleared to 0. page->private is being set to 1 at
ext4_wb_prepare_write():

+int ext4_wb_prepare_write(struct file *file, struct page *page,
+                             unsigned from, unsigned to)
+{
+       struct inode *inode = page->mapping->host;
+       struct buffer_head bh, *bhw = &bh;
+       int err = 0;
+
+       wb_debug("prepare page %lu (%u-%u) for inode %lu\n",
+                       page->index, from, to, page->mapping->host->i_ino);
+
+       /* if page is uptodate this means that ->prepare_write() has
+        * been called on page before and page is mapped to disk or
+        * we did reservation. page is protected and nobody can
+        * access it. hence, it safe to use page->private to pass
+        * flag that ->commit_write() has to reserve blocks. because
+        * an error may occur after ->prepare_write() we should not
+        * reserve block here. it's better to do in ->commit_write()
+        * when we're sure page is to be written */
+       page->private = 0;
+       if (!PageUptodate(page)) {
+               /* first write to this page */
+               bh.b_state = 0;
+               err = ext4_get_block(inode, page->index, bhw, 0);
+               if (err)
+                       return err;
+               if (!buffer_mapped(bhw)) {
+                       /* this block isn't allocated yet, reserve space */
+                       wb_debug("reserve space for new block\n");
+                       page->private = 1;
+                       ext4_wb_clear_page(page, from, to);
+                       ClearPageMappedToDisk(page);



>>From the comments it says the page->private is set to 1 to letting
commit_write know that it needs block reservation, but I don't see the
page->private value being checked in ext4_wb_commit_write(). Instead,
the PageMappedToDisk(page) flag is being checked.

Alex, can you clarify the use of page->private and PagePrivate flag
here? Do we still need the page->private for delayed allocation, with
PageBooked flag and PageMappedToDisk page flag?

Thanks,

Mingming

-
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html