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

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

[Ivan Shapovalov]

Upstream commit 91b0abe36a7b2b3b02d7500925a5f8455334f0e5
"mm + fs: store shadow entries in page cache".

Signed-off-by: Ivan Shapovalov <intelfx100@gmail.com>
---

We have missed this one long ago...

 fs/reiser4/super_ops.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/reiser4/super_ops.c b/fs/reiser4/super_ops.c
index acff93a..7e319bb 100644
--- a/fs/reiser4/super_ops.c
+++ b/fs/reiser4/super_ops.c
@@ -214,7 +214,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);
-- 
2.3.0

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

[Edward Shishkin]

Thanks!
I'll backward port it...

Edward.

On 02/13/2015 01:49 PM, Ivan Shapovalov wrote:
> Upstream commit 91b0abe36a7b2b3b02d7500925a5f8455334f0e5
> "mm + fs: store shadow entries in page cache".
>
> Signed-off-by: Ivan Shapovalov <intelfx100@gmail.com>
> ---
>
> We have missed this one long ago...
>
>   fs/reiser4/super_ops.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/reiser4/super_ops.c b/fs/reiser4/super_ops.c
> index acff93a..7e319bb 100644
> --- a/fs/reiser4/super_ops.c
> +++ b/fs/reiser4/super_ops.c
> @@ -214,7 +214,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);

[PATCH] reiser4: missed patch in porting to 3.15

[Ivan Shapovalov]

A patch was missed in the days of 3.15...

Ivan Shapovalov (1):
  Adjust reiser4 for 3.15: replace truncate_inode_pages(..., 0) with truncate_inode_pages_final(...).

 plugin/file/cryptcompress.c | 2 --
 super_ops.c                 | 2 +-
 2 files changed, 1 insertion(+), 3 deletions(-)

-- 
2.10.0

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

[Ivan Shapovalov]

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);
-- 
2.10.0

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

[Ivan Shapovalov]

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

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())?

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.

-- 
Ivan Shapovalov / intelfx /

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

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

[Edward Shishkin]

OK

On 09/30/2016 08:43 AM, 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);

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

[Edward Shishkin]

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.

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

Edward.

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

[Ivan Shapovalov]

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

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

[Ivan Shapovalov]

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

On 2016-10-03 at 17:40 +0300, Ivan Shapovalov wrote:
> 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:
> > > > [...]
> > > 
> > > 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...

Looks like the answer is "no, everything is OK". If ->evict_inode()
races with remove_mapping() and shadow entries are installed into the
radix tree, the *_final() call will still catch them. By association
this means that this call is still needed.

(However, now I wonder how does this work with regular truncate and
whether the radix tree can hold shadow entries for already truncated
pages during an inode's lifetime, but this is a purely VFS/mm
question.)

-- 
Ivan Shapovalov / intelfx /

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

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

[Edward Shishkin]

Agreed,
we have to call  *_final()

Thanks,
Edward.

On 10/03/2016 09:45 PM, Ivan Shapovalov wrote:
> On 2016-10-03 at 17:40 +0300, Ivan Shapovalov wrote:
>> 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:
>>>>> [...]
>>>> 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...
> Looks like the answer is "no, everything is OK". If ->evict_inode()
> races with remove_mapping() and shadow entries are installed into the
> radix tree, the *_final() call will still catch them. By association
> this means that this call is still needed.
>
> (However, now I wonder how does this work with regular truncate and
> whether the radix tree can hold shadow entries for already truncated
> pages during an inode's lifetime, but this is a purely VFS/mm
> question.)
>