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@xxxxxxxxxxxx>
> > > ---
> > > 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:
signature.asc
Description: This is a digitally signed message part
