linux-ext4 - Re: [PATCH] jbd jbd2: fix dio write returning EIO whentry_to_release

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 06 Aug 2008 21:07:24 -0400
From:	Chris Mason <chris.mason@...cle.com>
To:	Mingming Cao <cmm@...ibm.com>
Cc:	Jan Kara <jack@...e.cz>,
	Hisashi Hifumi <hifumi.hisashi@....ntt.co.jp>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-ext4@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH] jbd jbd2: fix dio write returning EIO
	whentry_to_release_page fails

On Wed, 2008-08-06 at 15:57 -0700, Mingming Cao wrote:
> > 
> > > Does it make more sense to fix do_launder_page to call into the FS on
> > > every page, and let the FS check for PageDirty on its own?  That way
> > > invalidate_inode_pages2_range basically gets its own private call into
> > > the FS that says wait around until this page is really free.
> >   That would certainly work as well. But IMHO waiting for ->writepage()
> > call to finish isn't really a big deal even in try_to_release_page() if
> > __GFP_FS (and __GFP_WAIT) is set. The only problem is that there is no
> > effective way to do so and so Hisashi used that "wait for b_count to drop"
> > which looks really scary and I don't like it as well.
> > 
> 
> I was  looking at the comment in invalidate_complete_page2(), which is
> now only called from DIO path, it saids
> 
> /*
>  * This is like invalidate_complete_page(), except it ignores the page's
>  * refcount.  We do this because invalidate_inode_pages2() needs
> stronger
>  * invalidation guarantees, and cannot afford to leave pages behind
> because
>  * shrink_page_list() has a temp ref on them, or because they're
> transiently
>  * sitting in the lru_cache_add() pagevecs.
>  */
> 
> 
> I am wondering why we need stronger invalidate hurantees for DIO->
> invalidate_inode_pages_range(),which force the page being removed from
> page cache? In case of bh is busy due to ext3 writeout,
> journal_try_to_free_buffers() could return different error number(EBUSY)
> to try_to_releasepage() (instead of EIO).  In that case,  could we just
> leave the page in the cache, clean pageuptodate() (to force later buffer
> read to read from disk) and then invalidate_complete_page2() return
> successfully? Any issue with this way?

Isn't this similar to the recent splice thread?  It seems like there are
places in the kernel that expect an up to date page they have a
reference against to stay up to date.

-chris


--
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