linux-ext4 - RE: [PATCH 3/5] ext4: No need to truncate pagecache twice in collapse range

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: <002101cf5ac7$76d97bf0$648c73d0$@samsung.com>
Date:	Fri, 18 Apr 2014 14:31:35 +0900
From:	Namjae Jeon <namjae.jeon@...sung.com>
To:	'Lukáš Czerner' <lczerner@...hat.com>
Cc:	'linux-ext4' <linux-ext4@...r.kernel.org>
Subject: RE: [PATCH 3/5] ext4: No need to truncate pagecache twice in collapse
 range

> On Thu, 17 Apr 2014, Namjae Jeon wrote:
> 
> > Date: Thu, 17 Apr 2014 09:41:45 +0900
> > From: Namjae Jeon <namjae.jeon@...sung.com>
> > To: Lukáš Czerner <lczerner@...hat.com>
> > Cc: linux-ext4 <linux-ext4@...r.kernel.org>
> > Subject: RE: [PATCH 3/5] ext4: No need to truncate pagecache twice in collapse
> >      range
> >
> > >
> > > We're already calling truncate_pagecache_range() before we attempt to
> > > do any actual job so there is not need to truncate pagecache once more
> > > using truncate_setsize() after we're finished.
> > >
> > > Remove truncate_setsize() and replace it just with i_size_write() note
> > > that we're holding appropriate locks.
> > >
> > > Signed-off-by: Lukas Czerner <lczerner@...hat.com>
> >
> > Hi Lukas.
> >
> > I added this code by getting rewiew from Hugh.
> > Plz see the disscusion beween Hugh and Dave.
> >
> > Hugh: But your case is different: collapse is much closer to truncation,
> >  and if you do not unmap the private COW'ed pages, then pages left
> >  behind beyond the EOF will break the spec that requires SIGBUS when
> >  touching there, and pages within EOF will be confusingly derived
> >  from file data now belonging to another offset or none (move these
> >  pages within the user address space? no, I don't think anon_vmas
> >  would allow that, and there may be no right place to move them).
> >
> > Dave: See above - we never leave pages beyond the new EOF because setting
> >  the new EOF is a truncate operation that calls
> >  truncate_setsize(inode, newsize).
> >
> > Hugh: Right, thanks, I now see the truncate_setsize() in the xfs case -
> >  though not in the ext4 case, which looks as if it's just doing an
> >  i_size_write() afterwards.
> >
> > Dave: So that's a bug in the ext4 code ;)
> >
> > truncate_setsize is not needed in case Hugh pointed out ?
> >
> > Thanks!
> 
> That is true, we need to make sure that the page cache is coherent
> with what's on disk. But we've already done that before releasing
> the blocks. As I mention in the comment we're doing
> truncate_pagecache_range() before removing any space. That's exactly
> how it's supposed to be used. See comment in
> truncate_pagecache_range().
> 
> However as I noticed we do not actually need to use
> truncate_pagecache_range(), but rather truncate_pagecache() so I can
> change that in my patch.
Hi Lukas.

Will you change that in your patch ?
Actually, I am waiting for this one..
Thanks.
> 
> Does that make sense to everyone ?
> 
> Thanks!
> -Lukas

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