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] [day] [month] [year] [list]

Message-ID: <20070621045058.GG20172@localhost.sw.ru>
Date:	Thu, 21 Jun 2007 08:50:58 +0400
From:	Dmitry Monakhov <dmonakhov@...ru>
To:	Mingming Cao <cmm@...ibm.com>
Cc:	Alex Tomas <alex@...sterfs.com>, linux-ext4@...r.kernel.org
Subject: Re: delayed allocatiou result in Oops

On 16:46 Срд 20 Июн     , Mingming Cao wrote:
> On Wed, 2007-06-20 at 12:15 +0400, Alex Tomas wrote:
> > Mingming Cao wrote:
> > > Hmm, PageMappedToDisk is probably not sufficient enough for pagesize!
> > > =blocksize. Is that the reason we need page->private to pass the
> > > request?
> > 
> > PageMappedToDisk isn't enough in that case, definitely. bh is the way
> > to track state of each block (this is what i'm implementing now), but
> > I think current nobh version (per-page flags are used) is valuable.
> > 
> > > That's good to know, thanks for the update. So probably above error case
> > > handling will be addressed in the new version? 
> > 
> > well, you actually can move that SetPagePrivate() few lines above
> > (Dmitriy already tested this).
> 
> Like this?
Of course not, we may set page private bit only if 
space was successfully reserved by ext4_wb_reserve_space_page().
So patch looks like this:
diff --git a/fs/ext4/writeback.c b/fs/ext4/writeback.c
index 3e669d6..84e5029 100644
--- a/fs/ext4/writeback.c
+++ b/fs/ext4/writeback.c
@@ -904,9 +904,6 @@ int ext4_wb_commit_write(struct file *file, struct page *page,
 	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
@@ -918,6 +915,9 @@ int ext4_wb_commit_write(struct file *file, struct page *page,
 		if (err)
 			return err;
 	}
+	/* mark page private so that we get
+	 * called to invalidate/release page */
+	SetPagePrivate(page);
 
 	/* ok. block for this page is allocated already or it has
 	 * been reserved succesfully. so, user may use it */
-- 
1.5.2
> 
> Index: linux-2.6.22-rc5/fs/ext4/writeback.c
> ===================================================================
> --- linux-2.6.22-rc5.orig/fs/ext4/writeback.c	2007-06-20 16:41:26.000000000 -0700
> +++ linux-2.6.22-rc5/fs/ext4/writeback.c	2007-06-20 16:44:10.000000000 -0700
> @@ -918,16 +918,16 @@ int ext4_wb_commit_write(struct file *fi
>  	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;
> +		/* mark page private so that we get
> +		 * called to invalidate/release page */
> +		SetPagePrivate(page);
> +
>  		err = ext4_wb_reserve_space_page(page, 1);
>  		if (err)
>  			return err;
> 
> 
> -
> 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

-
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