Re: [PATCH] Adjust reiser4 for 3.15: replace truncate_inode_pages(..., 0) with truncate_inode_pages_final(...).

[Ivan Shapovalov <intelfx@intelfx.name>]

[-- Attachment #1: Type: text/plain, Size: 3349 bytes --]

On 2016-10-03 at 16:29 +0200, Edward Shishkin wrote:
> On 09/30/2016 08:47 AM, Ivan Shapovalov wrote:
> > On 2016-09-30 at 09:43 +0300, Ivan Shapovalov wrote:
> > > Upstream commit 91b0abe36a7b2b3b02d7500925a5f8455334f0e5
> > > "mm + fs: store shadow entries in page cache".
> > > 
> > > Moreover, the truncate_inode_pages(..., 0) in
> > > delete_object_cryptcompress()
> > > is not needed at all.
> > > 
> > > Signed-off-by: Ivan Shapovalov <intelfx@intelfx.name>
> > > ---
> > >   plugin/file/cryptcompress.c | 2 --
> > >   super_ops.c                 | 2 +-
> > >   2 files changed, 1 insertion(+), 3 deletions(-)
> > > 
> > > diff --git a/plugin/file/cryptcompress.c
> > > b/plugin/file/cryptcompress.c
> > > index 59d8df8..5433de9 100644
> > > --- a/plugin/file/cryptcompress.c
> > > +++ b/plugin/file/cryptcompress.c
> > > @@ -3595,8 +3595,6 @@ int delete_object_cryptcompress(struct
> > > inode
> > > *inode)
> > >   			(unsigned long
> > > long)get_inode_oid(inode),
> > >   			result);
> > >   	}
> > > -	truncate_inode_pages(inode->i_mapping, 0);
> > > -	assert("edward-1487", pages_truncate_ok(inode, 0));
> > >   	/* and remove stat data */
> > >   	return reiser4_delete_object_common(inode);
> > >   }
> > > diff --git a/super_ops.c b/super_ops.c
> > > index 73c18f2..697580c 100644
> > > --- a/super_ops.c
> > > +++ b/super_ops.c
> > > @@ -215,7 +215,7 @@ static void reiser4_evict_inode(struct inode
> > > *inode)
> > >   			fplug->delete_object(inode);
> > >   	}
> > >   
> > > -	truncate_inode_pages(&inode->i_data, 0);
> > > +	truncate_inode_pages_final(&inode->i_data);
> > >   	inode->i_blocks = 0;
> > >   	clear_inode(inode);
> > >   	reiser4_exit_context(ctx);
> > 
> > BTW, this raises a question: in the ->evict_inode path, are we ever
> > allowed to call plain truncate_inode_pages() (i. e. not *_final())?
> 
> Actually, I would like to see a kind of assertion 1487 instead:
> everything should be already truncated at that point.

Do you mean that the assertion should go instead of
truncate_inode_pages_final()? Doesn't that function contain extra
logic, beyond removing pages?

Moreover, that function seems to contain extra logic _before_ actually
going off and truncating pages. This makes me wonder: doesn't this mean
that we _must not ever_ call regular truncate_inode_pages() from
inside ->evict_inode()?

I do not know VFS enough to answer these kinds of stupid questions...

> 
> > 
> > The ->delete_object plugin methods do this as part of their logic,
> > actually. At least the cryptcompress plugin calls
> > truncate_inode_pages(..., new_size) at the end of
> > prune_cryptcompress(), however I suspect that the regular file
> > plugin
> > also does this deep inside reiser4's guts.
> 
> File body is truncated item-by-item from right to left. For each item
> its ->kill_hook() method is called. It is responsible for truncating
> attached pages. Bodies of cryptcompress files are not connected:
> there can be pages without "parent" items (in the case of holes), so
> in addition we call truncate_inode_pages() in prune_cryptcompress()
> to kill those pages.
> 
Understood. Thanks for explanation!
(the above question is still in effect...)

-- 
Ivan Shapovalov / intelfx /

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]