reiser4: porting to 3.16: any reason ->aio_read() of struct file_operations has been left out?

reiser4: porting to 3.16: any reason ->aio_read() of struct file_operations has been left out?

[Ivan Shapovalov]

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

From `git log` I've seen that VFS people intend to replace ->aio_read() and
->aio_write() of struct file_operations with new methods ->read_iter() and
->write_iter().

(Along with a couple of related new helpers, differing from previous just in
calling _iter methods instead of aio_ ones.)

From other filesystems it seems that these are simple drop-in replacements
(however, well, I have zero familiarity with VFS). So here is a question:
is there any intentional reason that generic_file_aio_write() is not used
in reiser4?

What follows is a simple patch that I've currently applied to my own kernel
(seems to be the only significant vfs change affecting filesystems), however,
I fear that these code-paths are not generally used, so my "works for me"
isn't really representative. Could you please clarify the situation here?

From 81172835255a01718c2c256942d5887825a0cd7a Mon Sep 17 00:00:00 2001
From: Ivan Shapovalov <intelfx100@gmail.com>
Date: Tue, 19 Aug 2014 14:33:35 +0400
Subject: [PATCH] Adjust reiser4 to 3.16: ->{read,write}_iter of struct
 file_operations.

1. ->aio_{read,write} of struct file_operations are being replaced with ->{read,write}_iter.
2. do_sync_{read,write} are being replaced with new_sync_{read,write}.
3. generic_file_splice_write is being replaced with iter_file_splice_write.

Signed-off-by: Ivan Shapovalov <intelfx100@gmail.com>
---
 fs/reiser4/plugin/file/cryptcompress.c | 2 +-
 fs/reiser4/plugin/file/file.c          | 2 +-
 fs/reiser4/plugin/object.c             | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/reiser4/plugin/file/cryptcompress.c b/fs/reiser4/plugin/file/cryptcompress.c
index 8af388d..b0109fb 100644
--- a/fs/reiser4/plugin/file/cryptcompress.c
+++ b/fs/reiser4/plugin/file/cryptcompress.c
@@ -2964,7 +2964,7 @@ ssize_t read_cryptcompress(struct file * file, char __user *buf, size_t size,
 		reiser4_exit_context(ctx);
 		return result;
 	}
-	result = do_sync_read(file, buf, size, off);
+	result = new_sync_read(file, buf, size, off);
 
 	context_set_commit_async(ctx);
 	reiser4_exit_context(ctx);
diff --git a/fs/reiser4/plugin/file/file.c b/fs/reiser4/plugin/file/file.c
index 94029cd..e65c48d 100644
--- a/fs/reiser4/plugin/file/file.c
+++ b/fs/reiser4/plugin/file/file.c
@@ -1752,7 +1752,7 @@ ssize_t read_unix_file(struct file *file, char __user *buf,
 	switch (uf_info->container) {
 	case UF_CONTAINER_EXTENTS:
 		if (!reiser4_inode_get_flag(inode, REISER4_PART_MIXED)) {
-			result = do_sync_read(file, buf, read_amount, off);
+			result = new_sync_read(file, buf, read_amount, off);
 			break;
 		}
 	case UF_CONTAINER_TAILS:
diff --git a/fs/reiser4/plugin/object.c b/fs/reiser4/plugin/object.c
index 553f1e2..e431e1f 100644
--- a/fs/reiser4/plugin/object.c
+++ b/fs/reiser4/plugin/object.c
@@ -188,7 +188,7 @@ static struct file_operations regular_file_f_ops = {
 	.llseek = generic_file_llseek,
 	.read = reiser4_read_dispatch,
 	.write = reiser4_write_dispatch,
-	.aio_read = generic_file_aio_read,
+	.read_iter = generic_file_read_iter,
 	.unlocked_ioctl = reiser4_ioctl_dispatch,
 #ifdef CONFIG_COMPAT
 	.compat_ioctl = reiser4_ioctl_dispatch,
@@ -198,7 +198,7 @@ static struct file_operations regular_file_f_ops = {
 	.release = reiser4_release_dispatch,
 	.fsync = reiser4_sync_file_common,
 	.splice_read = generic_file_splice_read,
-	.splice_write = generic_file_splice_write
+	.splice_write = iter_file_splice_write
 };
 static struct address_space_operations regular_file_a_ops = {
 	.writepage = reiser4_writepage,
-- 
2.0.4

Thanks,
-- 
Ivan Shapovalov / intelfx /

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

Re: reiser4: porting to 3.16: any reason ->aio_read() of struct file_operations has been left out?

[Edward Shishkin]

On 08/20/2014 12:32 AM, Ivan Shapovalov wrote:
>  From `git log` I've seen that VFS people intend to replace ->aio_read() and
> ->aio_write() of struct file_operations with new methods ->read_iter() and
> ->write_iter().
>
> (Along with a couple of related new helpers, differing from previous just in
> calling _iter methods instead of aio_ ones.)
>
>  From other filesystems it seems that these are simple drop-in replacements
> (however, well, I have zero familiarity with VFS). So here is a question:
> is there any intentional reason that generic_file_aio_write() is not used
> in reiser4?


Currently reiser4 is a set of two filesystems which differ in methods
of handling regular files. For VFS we provide "dispatchers", which pass
management to appropriate plugin (UNIX_FILE or CRYPTCOMPRESS).

UNIX_FILE plugin doesn't use generic write for performance reasons
(I'll try to find the respective mailing thread). CRYPTCOMPRESS doesn't
use it for compatibility reasons: I don't know how how to rewrite it
gracefully using the generic write method.

Edward.


>
> What follows is a simple patch that I've currently applied to my own kernel
> (seems to be the only significant vfs change affecting filesystems), however,
> I fear that these code-paths are not generally used, so my "works for me"
> isn't really representative. Could you please clarify the situation here?
>
>  From 81172835255a01718c2c256942d5887825a0cd7a Mon Sep 17 00:00:00 2001
> From: Ivan Shapovalov <intelfx100@gmail.com>
> Date: Tue, 19 Aug 2014 14:33:35 +0400
> Subject: [PATCH] Adjust reiser4 to 3.16: ->{read,write}_iter of struct
>   file_operations.
>
> 1. ->aio_{read,write} of struct file_operations are being replaced with ->{read,write}_iter.
> 2. do_sync_{read,write} are being replaced with new_sync_{read,write}.
> 3. generic_file_splice_write is being replaced with iter_file_splice_write.
>
> Signed-off-by: Ivan Shapovalov <intelfx100@gmail.com>
> ---
>   fs/reiser4/plugin/file/cryptcompress.c | 2 +-
>   fs/reiser4/plugin/file/file.c          | 2 +-
>   fs/reiser4/plugin/object.c             | 4 ++--
>   3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/fs/reiser4/plugin/file/cryptcompress.c b/fs/reiser4/plugin/file/cryptcompress.c
> index 8af388d..b0109fb 100644
> --- a/fs/reiser4/plugin/file/cryptcompress.c
> +++ b/fs/reiser4/plugin/file/cryptcompress.c
> @@ -2964,7 +2964,7 @@ ssize_t read_cryptcompress(struct file * file, char __user *buf, size_t size,
>   		reiser4_exit_context(ctx);
>   		return result;
>   	}
> -	result = do_sync_read(file, buf, size, off);
> +	result = new_sync_read(file, buf, size, off);
>   
>   	context_set_commit_async(ctx);
>   	reiser4_exit_context(ctx);
> diff --git a/fs/reiser4/plugin/file/file.c b/fs/reiser4/plugin/file/file.c
> index 94029cd..e65c48d 100644
> --- a/fs/reiser4/plugin/file/file.c
> +++ b/fs/reiser4/plugin/file/file.c
> @@ -1752,7 +1752,7 @@ ssize_t read_unix_file(struct file *file, char __user *buf,
>   	switch (uf_info->container) {
>   	case UF_CONTAINER_EXTENTS:
>   		if (!reiser4_inode_get_flag(inode, REISER4_PART_MIXED)) {
> -			result = do_sync_read(file, buf, read_amount, off);
> +			result = new_sync_read(file, buf, read_amount, off);
>   			break;
>   		}
>   	case UF_CONTAINER_TAILS:
> diff --git a/fs/reiser4/plugin/object.c b/fs/reiser4/plugin/object.c
> index 553f1e2..e431e1f 100644
> --- a/fs/reiser4/plugin/object.c
> +++ b/fs/reiser4/plugin/object.c
> @@ -188,7 +188,7 @@ static struct file_operations regular_file_f_ops = {
>   	.llseek = generic_file_llseek,
>   	.read = reiser4_read_dispatch,
>   	.write = reiser4_write_dispatch,
> -	.aio_read = generic_file_aio_read,
> +	.read_iter = generic_file_read_iter,
>   	.unlocked_ioctl = reiser4_ioctl_dispatch,
>   #ifdef CONFIG_COMPAT
>   	.compat_ioctl = reiser4_ioctl_dispatch,
> @@ -198,7 +198,7 @@ static struct file_operations regular_file_f_ops = {
>   	.release = reiser4_release_dispatch,
>   	.fsync = reiser4_sync_file_common,
>   	.splice_read = generic_file_splice_read,
> -	.splice_write = generic_file_splice_write
> +	.splice_write = iter_file_splice_write
>   };
>   static struct address_space_operations regular_file_a_ops = {
>   	.writepage = reiser4_writepage,

Re: reiser4: porting to 3.16: any reason ->aio_read() of struct file_operations has been left out?

[Ivan Shapovalov]

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

On Wednesday 20 August 2014 at 01:39:42, Edward Shishkin wrote:	
> 
> On 08/20/2014 12:32 AM, Ivan Shapovalov wrote:
> >  From `git log` I've seen that VFS people intend to replace ->aio_read() and
> > ->aio_write() of struct file_operations with new methods ->read_iter() and
> > ->write_iter().
> >
> > (Along with a couple of related new helpers, differing from previous just in
> > calling _iter methods instead of aio_ ones.)
> >
> >  From other filesystems it seems that these are simple drop-in replacements
> > (however, well, I have zero familiarity with VFS). So here is a question:
> > is there any intentional reason that generic_file_aio_write() is not used
> > in reiser4?
> 
> 
> Currently reiser4 is a set of two filesystems which differ in methods
> of handling regular files. For VFS we provide "dispatchers", which pass
> management to appropriate plugin (UNIX_FILE or CRYPTCOMPRESS).
> 
> UNIX_FILE plugin doesn't use generic write for performance reasons
> (I'll try to find the respective mailing thread). CRYPTCOMPRESS doesn't
> use it for compatibility reasons: I don't know how how to rewrite it
> gracefully using the generic write method.
> 
> Edward.


Thanks for explanation! So, does this patch make any sense?

-- 
Ivan Shapovalov / intelfx /

> >
> > What follows is a simple patch that I've currently applied to my own kernel
> > (seems to be the only significant vfs change affecting filesystems), however,
> > I fear that these code-paths are not generally used, so my "works for me"
> > isn't really representative. Could you please clarify the situation here?
> >
> >  From 81172835255a01718c2c256942d5887825a0cd7a Mon Sep 17 00:00:00 2001
> > From: Ivan Shapovalov <intelfx100@gmail.com>
> > Date: Tue, 19 Aug 2014 14:33:35 +0400
> > Subject: [PATCH] Adjust reiser4 to 3.16: ->{read,write}_iter of struct
> >   file_operations.
> >
> > 1. ->aio_{read,write} of struct file_operations are being replaced with ->{read,write}_iter.
> > 2. do_sync_{read,write} are being replaced with new_sync_{read,write}.
> > 3. generic_file_splice_write is being replaced with iter_file_splice_write.
> >
> > Signed-off-by: Ivan Shapovalov <intelfx100@gmail.com>
> > ---
> >   fs/reiser4/plugin/file/cryptcompress.c | 2 +-
> >   fs/reiser4/plugin/file/file.c          | 2 +-
> >   fs/reiser4/plugin/object.c             | 4 ++--
> >   3 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/reiser4/plugin/file/cryptcompress.c b/fs/reiser4/plugin/file/cryptcompress.c
> > index 8af388d..b0109fb 100644
> > --- a/fs/reiser4/plugin/file/cryptcompress.c
> > +++ b/fs/reiser4/plugin/file/cryptcompress.c
> > @@ -2964,7 +2964,7 @@ ssize_t read_cryptcompress(struct file * file, char __user *buf, size_t size,
> >   		reiser4_exit_context(ctx);
> >   		return result;
> >   	}
> > -	result = do_sync_read(file, buf, size, off);
> > +	result = new_sync_read(file, buf, size, off);
> >   
> >   	context_set_commit_async(ctx);
> >   	reiser4_exit_context(ctx);
> > diff --git a/fs/reiser4/plugin/file/file.c b/fs/reiser4/plugin/file/file.c
> > index 94029cd..e65c48d 100644
> > --- a/fs/reiser4/plugin/file/file.c
> > +++ b/fs/reiser4/plugin/file/file.c
> > @@ -1752,7 +1752,7 @@ ssize_t read_unix_file(struct file *file, char __user *buf,
> >   	switch (uf_info->container) {
> >   	case UF_CONTAINER_EXTENTS:
> >   		if (!reiser4_inode_get_flag(inode, REISER4_PART_MIXED)) {
> > -			result = do_sync_read(file, buf, read_amount, off);
> > +			result = new_sync_read(file, buf, read_amount, off);
> >   			break;
> >   		}
> >   	case UF_CONTAINER_TAILS:
> > diff --git a/fs/reiser4/plugin/object.c b/fs/reiser4/plugin/object.c
> > index 553f1e2..e431e1f 100644
> > --- a/fs/reiser4/plugin/object.c
> > +++ b/fs/reiser4/plugin/object.c
> > @@ -188,7 +188,7 @@ static struct file_operations regular_file_f_ops = {
> >   	.llseek = generic_file_llseek,
> >   	.read = reiser4_read_dispatch,
> >   	.write = reiser4_write_dispatch,
> > -	.aio_read = generic_file_aio_read,
> > +	.read_iter = generic_file_read_iter,
> >   	.unlocked_ioctl = reiser4_ioctl_dispatch,
> >   #ifdef CONFIG_COMPAT
> >   	.compat_ioctl = reiser4_ioctl_dispatch,
> > @@ -198,7 +198,7 @@ static struct file_operations regular_file_f_ops = {
> >   	.release = reiser4_release_dispatch,
> >   	.fsync = reiser4_sync_file_common,
> >   	.splice_read = generic_file_splice_read,
> > -	.splice_write = generic_file_splice_write
> > +	.splice_write = iter_file_splice_write
> >   };
> >   static struct address_space_operations regular_file_a_ops = {
> >   	.writepage = reiser4_writepage,
> 

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

Re: reiser4: porting to 3.16: any reason ->aio_read() of struct file_operations has been left out?

[Edward Shishkin]

On 08/20/2014 10:34 PM, Ivan Shapovalov wrote:
> On Wednesday 20 August 2014 at 01:39:42, Edward Shishkin wrote:	
>> On 08/20/2014 12:32 AM, Ivan Shapovalov wrote:
>>>   From `git log` I've seen that VFS people intend to replace ->aio_read() and
>>> ->aio_write() of struct file_operations with new methods ->read_iter() and
>>> ->write_iter().
>>>
>>> (Along with a couple of related new helpers, differing from previous just in
>>> calling _iter methods instead of aio_ ones.)
>>>
>>>   From other filesystems it seems that these are simple drop-in replacements
>>> (however, well, I have zero familiarity with VFS). So here is a question:
>>> is there any intentional reason that generic_file_aio_write() is not used
>>> in reiser4?
>>
>> Currently reiser4 is a set of two filesystems which differ in methods
>> of handling regular files. For VFS we provide "dispatchers", which pass
>> management to appropriate plugin (UNIX_FILE or CRYPTCOMPRESS).
>>
>> UNIX_FILE plugin doesn't use generic write for performance reasons
>> (I'll try to find the respective mailing thread). CRYPTCOMPRESS doesn't
>> use it for compatibility reasons: I don't know how how to rewrite it
>> gracefully using the generic write method.
>>
>> Edward.
>
> Thanks for explanation! So, does this patch make any sense?


I haven't looked at this carefully yet, but likely it is correct.

Thanks,
Edward.

Re: reiser4: porting to 3.16: any reason ->aio_read() of struct file_operations has been left out?

[Ivan Shapovalov]

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

On Thursday 21 August 2014 at 00:30:45, Edward Shishkin wrote:	
> 
> On 08/20/2014 10:34 PM, Ivan Shapovalov wrote:
> > On Wednesday 20 August 2014 at 01:39:42, Edward Shishkin wrote:	
> >> On 08/20/2014 12:32 AM, Ivan Shapovalov wrote:
> >>>   From `git log` I've seen that VFS people intend to replace ->aio_read() and
> >>> ->aio_write() of struct file_operations with new methods ->read_iter() and
> >>> ->write_iter().
> >>>
> >>> (Along with a couple of related new helpers, differing from previous just in
> >>> calling _iter methods instead of aio_ ones.)
> >>>
> >>>   From other filesystems it seems that these are simple drop-in replacements
> >>> (however, well, I have zero familiarity with VFS). So here is a question:
> >>> is there any intentional reason that generic_file_aio_write() is not used
> >>> in reiser4?
> >>
> >> Currently reiser4 is a set of two filesystems which differ in methods
> >> of handling regular files. For VFS we provide "dispatchers", which pass
> >> management to appropriate plugin (UNIX_FILE or CRYPTCOMPRESS).
> >>
> >> UNIX_FILE plugin doesn't use generic write for performance reasons
> >> (I'll try to find the respective mailing thread). CRYPTCOMPRESS doesn't
> >> use it for compatibility reasons: I don't know how how to rewrite it
> >> gracefully using the generic write method.
> >>
> >> Edward.
> >
> > Thanks for explanation! So, does this patch make any sense?
> 
> 
> I haven't looked at this carefully yet, but likely it is correct.
> 
> Thanks,
> Edward.

Turned out it isn't.. The iter_file_splice_write() requires ->write_iter
to be set, or a NULL dereference happens.

At first I've thought that we're out of luck and will need to use the fallback
splice implementation (default_file_splice_write), but just setting
->write_iter to generic_file_write_iter strangely worked.

(By "it works" I mean "splice finishes successfully and does not cause data
corruptions").

Could you please comment on this? :)

Thanks,
-- 
Ivan Shapovalov / intelfx /

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

Re: reiser4: porting to 3.16: any reason ->aio_read() of struct file_operations has been left out?

[Edward Shishkin]

On 08/21/2014 05:05 PM, Ivan Shapovalov wrote:
> On Thursday 21 August 2014 at 00:30:45, Edward Shishkin wrote:	
>> On 08/20/2014 10:34 PM, Ivan Shapovalov wrote:
>>> On Wednesday 20 August 2014 at 01:39:42, Edward Shishkin wrote:	
>>>> On 08/20/2014 12:32 AM, Ivan Shapovalov wrote:
>>>>>    From `git log` I've seen that VFS people intend to replace ->aio_read() and
>>>>> ->aio_write() of struct file_operations with new methods ->read_iter() and
>>>>> ->write_iter().
>>>>>
>>>>> (Along with a couple of related new helpers, differing from previous just in
>>>>> calling _iter methods instead of aio_ ones.)
>>>>>
>>>>>    From other filesystems it seems that these are simple drop-in replacements
>>>>> (however, well, I have zero familiarity with VFS). So here is a question:
>>>>> is there any intentional reason that generic_file_aio_write() is not used
>>>>> in reiser4?
>>>> Currently reiser4 is a set of two filesystems which differ in methods
>>>> of handling regular files. For VFS we provide "dispatchers", which pass
>>>> management to appropriate plugin (UNIX_FILE or CRYPTCOMPRESS).
>>>>
>>>> UNIX_FILE plugin doesn't use generic write for performance reasons
>>>> (I'll try to find the respective mailing thread). CRYPTCOMPRESS doesn't
>>>> use it for compatibility reasons: I don't know how how to rewrite it
>>>> gracefully using the generic write method.
>>>>
>>>> Edward.
>>> Thanks for explanation! So, does this patch make any sense?
>>
>> I haven't looked at this carefully yet, but likely it is correct.
>>
>> Thanks,
>> Edward.
> Turned out it isn't.. The iter_file_splice_write() requires ->write_iter
> to be set, or a NULL dereference happens.
>
> At first I've thought that we're out of luck and will need to use the fallback
> splice implementation (default_file_splice_write), but just setting
> ->write_iter to generic_file_write_iter strangely worked.
>
> (By "it works" I mean "splice finishes successfully and does not cause data
> corruptions").
>
> Could you please comment on this? :)


I'll try to take a look at weekends..

Thanks,
Edward.

Re: reiser4: porting to 3.16: any reason ->aio_read() of struct file_operations has been left out?

[Edward Shishkin]

On 08/21/2014 05:05 PM, Ivan Shapovalov wrote:
> On Thursday 21 August 2014 at 00:30:45, Edward Shishkin wrote:	
>> On 08/20/2014 10:34 PM, Ivan Shapovalov wrote:
>>> On Wednesday 20 August 2014 at 01:39:42, Edward Shishkin wrote:	
>>>> On 08/20/2014 12:32 AM, Ivan Shapovalov wrote:
>>>>>    From `git log` I've seen that VFS people intend to replace ->aio_read() and
>>>>> ->aio_write() of struct file_operations with new methods ->read_iter() and
>>>>> ->write_iter().
>>>>>
>>>>> (Along with a couple of related new helpers, differing from previous just in
>>>>> calling _iter methods instead of aio_ ones.)
>>>>>
>>>>>    From other filesystems it seems that these are simple drop-in replacements
>>>>> (however, well, I have zero familiarity with VFS). So here is a question:
>>>>> is there any intentional reason that generic_file_aio_write() is not used
>>>>> in reiser4?
>>>> Currently reiser4 is a set of two filesystems which differ in methods
>>>> of handling regular files. For VFS we provide "dispatchers", which pass
>>>> management to appropriate plugin (UNIX_FILE or CRYPTCOMPRESS).
>>>>
>>>> UNIX_FILE plugin doesn't use generic write for performance reasons
>>>> (I'll try to find the respective mailing thread). CRYPTCOMPRESS doesn't
>>>> use it for compatibility reasons: I don't know how how to rewrite it
>>>> gracefully using the generic write method.
>>>>
>>>> Edward.
>>> Thanks for explanation! So, does this patch make any sense?
>>
>> I haven't looked at this carefully yet, but likely it is correct.
>>
>> Thanks,
>> Edward.


I have looked at VFS changes 3.15->3.16.

generic_file_aio_read()  was replaced with generic_file_read_iter()
Respectively, all its users (including us) now call ->read_iter().

Since we don't implement our own ->aio_read(), we should use
new_sync_read() instead of do_sync_read().

So, everything looks OK with our read/write.


> Turned out it isn't.. The iter_file_splice_write() requires ->write_iter
> to be set, or a NULL dereference happens.
>
> At first I've thought that we're out of luck and will need to use the fallback
> splice implementation (default_file_splice_write), but just setting
> ->write_iter to generic_file_write_iter strangely worked.
>
> (By "it works" I mean "splice finishes successfully and does not cause data
> corruptions").
>
> Could you please comment on this? :)


generic_file_splice_write() did:

->write_begin()
memcpy();
->write_end()

iter_file_splice_write() with generic_file_write_iter() does:

->write_begin()
copy_from_user();
->write_end()

So the question is why copy_from_user() works instead
of memcpy()? TBH, I don't know. If interesting, ask VFS
folks and tell us then ;)

Thanks,
Edward.

Re: reiser4: porting to 3.16: any reason ->aio_read() of struct file_operations has been left out?

[Edward Shishkin]

On 08/21/2014 05:05 PM, Ivan Shapovalov wrote:
> On Thursday 21 August 2014 at 00:30:45, Edward Shishkin wrote:	
>> On 08/20/2014 10:34 PM, Ivan Shapovalov wrote:
>>> On Wednesday 20 August 2014 at 01:39:42, Edward Shishkin wrote:	
>>>> On 08/20/2014 12:32 AM, Ivan Shapovalov wrote:
>>>>>    From `git log` I've seen that VFS people intend to replace ->aio_read() and
>>>>> ->aio_write() of struct file_operations with new methods ->read_iter() and
>>>>> ->write_iter().
>>>>>
>>>>> (Along with a couple of related new helpers, differing from previous just in
>>>>> calling _iter methods instead of aio_ ones.)
>>>>>
>>>>>    From other filesystems it seems that these are simple drop-in replacements
>>>>> (however, well, I have zero familiarity with VFS). So here is a question:
>>>>> is there any intentional reason that generic_file_aio_write() is not used
>>>>> in reiser4?
>>>> Currently reiser4 is a set of two filesystems which differ in methods
>>>> of handling regular files. For VFS we provide "dispatchers", which pass
>>>> management to appropriate plugin (UNIX_FILE or CRYPTCOMPRESS).
>>>>
>>>> UNIX_FILE plugin doesn't use generic write for performance reasons
>>>> (I'll try to find the respective mailing thread).


This mailing thread sheds the light on why we don't want to use generic 
write:
(this in only FYI, don't try to make friendship between reiser4 and VFS :))

http://markmail.org/message/ydzplf6bxokfgkch#query:+page:1+mid:appoj72tdeop3f7f+state:results


>>>>   CRYPTCOMPRESS doesn't
>>>> use it for compatibility reasons: I don't know how how to rewrite it
>>>> gracefully using the generic write method.
>>>>
>>>> Edward.
>>> Thanks for explanation! So, does this patch make any sense?
>>
>> I haven't looked at this carefully yet, but likely it is correct.
>>
>> Thanks,
>> Edward.
> Turned out it isn't.. The iter_file_splice_write() requires ->write_iter
> to be set, or a NULL dereference happens.
>
> At first I've thought that we're out of luck and will need to use the fallback
> splice implementation (default_file_splice_write), but just setting
> ->write_iter to generic_file_write_iter strangely worked.
>
> (By "it works" I mean "splice finishes successfully and does not cause data
> corruptions").
>
> Could you please comment on this? :)
>
> Thanks,

Re: reiser4: porting to 3.16: any reason ->aio_read() of struct file_operations has been left out?

[Ivan Shapovalov]

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

On Thursday 21 August 2014 at 19:05:32, Ivan Shapovalov wrote:	
> On Thursday 21 August 2014 at 00:30:45, Edward Shishkin wrote:	
> > 
> > On 08/20/2014 10:34 PM, Ivan Shapovalov wrote:
> > > On Wednesday 20 August 2014 at 01:39:42, Edward Shishkin wrote:	
> > >> On 08/20/2014 12:32 AM, Ivan Shapovalov wrote:
> > >>>   From `git log` I've seen that VFS people intend to replace ->aio_read() and
> > >>> ->aio_write() of struct file_operations with new methods ->read_iter() and
> > >>> ->write_iter().
> > >>>
> > >>> (Along with a couple of related new helpers, differing from previous just in
> > >>> calling _iter methods instead of aio_ ones.)
> > >>>
> > >>>   From other filesystems it seems that these are simple drop-in replacements
> > >>> (however, well, I have zero familiarity with VFS). So here is a question:
> > >>> is there any intentional reason that generic_file_aio_write() is not used
> > >>> in reiser4?
> > >>
> > >> Currently reiser4 is a set of two filesystems which differ in methods
> > >> of handling regular files. For VFS we provide "dispatchers", which pass
> > >> management to appropriate plugin (UNIX_FILE or CRYPTCOMPRESS).
> > >>
> > >> UNIX_FILE plugin doesn't use generic write for performance reasons
> > >> (I'll try to find the respective mailing thread). CRYPTCOMPRESS doesn't
> > >> use it for compatibility reasons: I don't know how how to rewrite it
> > >> gracefully using the generic write method.
> > >>
> > >> Edward.
> > >
> > > Thanks for explanation! So, does this patch make any sense?
> > 
> > 
> > I haven't looked at this carefully yet, but likely it is correct.
> > 
> > Thanks,
> > Edward.
> 
> Turned out it isn't.. The iter_file_splice_write() requires ->write_iter
> to be set, or a NULL dereference happens.
> 
> At first I've thought that we're out of luck and will need to use the fallback
> splice implementation (default_file_splice_write), but just setting
> ->write_iter to generic_file_write_iter strangely worked.
> 
> (By "it works" I mean "splice finishes successfully and does not cause data
> corruptions").

...aand... it doesn't. At least on reg40 (sic, NOT ccreg40):
1) `kde-cp` and FF 32.0.3's downloader produce empty files;
2) some applications (at least two games: luftrausers and x-plane 10, both
proprietary) deadlock somewhere inside of generic_file_write_iter().

Removing ".splice_write = iter_file_splice_write" and
".write_iter = generic_file_write_iter" fixes the described cases.

Unfortunately, no time to dig deeper (MIPT is evil). Hopefully you will
look into it someday... :)

Thanks,
-- 
Ivan Shapovalov / intelfx /

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

Re: reiser4: porting to 3.16: any reason ->aio_read() of struct file_operations has been left out?

[Edward Shishkin]

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


On 10/09/2014 08:55 PM, Ivan Shapovalov wrote:
> On Thursday 21 August 2014 at 19:05:32, Ivan Shapovalov wrote:	
>> On Thursday 21 August 2014 at 00:30:45, Edward Shishkin wrote:	
>>> On 08/20/2014 10:34 PM, Ivan Shapovalov wrote:
>>>> On Wednesday 20 August 2014 at 01:39:42, Edward Shishkin wrote:	
>>>>> On 08/20/2014 12:32 AM, Ivan Shapovalov wrote:
>>>>>>    From `git log` I've seen that VFS people intend to replace ->aio_read() and
>>>>>> ->aio_write() of struct file_operations with new methods ->read_iter() and
>>>>>> ->write_iter().
>>>>>>
>>>>>> (Along with a couple of related new helpers, differing from previous just in
>>>>>> calling _iter methods instead of aio_ ones.)
>>>>>>
>>>>>>    From other filesystems it seems that these are simple drop-in replacements
>>>>>> (however, well, I have zero familiarity with VFS). So here is a question:
>>>>>> is there any intentional reason that generic_file_aio_write() is not used
>>>>>> in reiser4?
>>>>> Currently reiser4 is a set of two filesystems which differ in methods
>>>>> of handling regular files. For VFS we provide "dispatchers", which pass
>>>>> management to appropriate plugin (UNIX_FILE or CRYPTCOMPRESS).
>>>>>
>>>>> UNIX_FILE plugin doesn't use generic write for performance reasons
>>>>> (I'll try to find the respective mailing thread). CRYPTCOMPRESS doesn't
>>>>> use it for compatibility reasons: I don't know how how to rewrite it
>>>>> gracefully using the generic write method.
>>>>>
>>>>> Edward.
>>>> Thanks for explanation! So, does this patch make any sense?
>>>
>>> I haven't looked at this carefully yet, but likely it is correct.
>>>
>>> Thanks,
>>> Edward.
>> Turned out it isn't.. The iter_file_splice_write() requires ->write_iter
>> to be set, or a NULL dereference happens.
>>
>> At first I've thought that we're out of luck and will need to use the fallback
>> splice implementation (default_file_splice_write), but just setting
>> ->write_iter to generic_file_write_iter strangely worked.
>>
>> (By "it works" I mean "splice finishes successfully and does not cause data
>> corruptions").
> ...aand... it doesn't. At least on reg40 (sic, NOT ccreg40):
> 1) `kde-cp` and FF 32.0.3's downloader produce empty files;
> 2) some applications (at least two games: luftrausers and x-plane 10, both
> proprietary) deadlock somewhere inside of generic_file_write_iter().
>
> Removing ".splice_write = iter_file_splice_write" and
> ".write_iter = generic_file_write_iter" fixes the described cases.


Thanks for the observation,
so the final port will look like this (attached)


>
> Unfortunately, no time to dig deeper (MIPT is evil). Hopefully you will
> look into it someday... :)


Yup, sure, study is the first deal.

Thanks,
Edward.

[-- Attachment #2: reiser4-port-for-3.16.patch --]
[-- Type: text/x-patch, Size: 1966 bytes --]

Only in linux-3.15/fs/reiser4/: context.h~
diff -u -r linux-3.15/fs/reiser4/plugin/file/cryptcompress.c linux-3.16.1/fs/reiser4/plugin/file/cryptcompress.c
--- linux-3.15/fs/reiser4/plugin/file/cryptcompress.c	2014-08-24 14:10:36.186139150 +0200
+++ linux-3.16.1/fs/reiser4/plugin/file/cryptcompress.c	2014-09-25 11:43:01.000000000 +0200
@@ -2964,7 +2964,7 @@
 		reiser4_exit_context(ctx);
 		return result;
 	}
-	result = do_sync_read(file, buf, size, off);
+	result = new_sync_read(file, buf, size, off);
 
 	context_set_commit_async(ctx);
 	reiser4_exit_context(ctx);
diff -u -r linux-3.15/fs/reiser4/plugin/file/file.c linux-3.16.1/fs/reiser4/plugin/file/file.c
--- linux-3.15/fs/reiser4/plugin/file/file.c	2014-08-24 14:10:36.188139156 +0200
+++ linux-3.16.1/fs/reiser4/plugin/file/file.c	2014-09-25 11:43:01.000000000 +0200
@@ -1752,7 +1752,7 @@
 	switch (uf_info->container) {
 	case UF_CONTAINER_EXTENTS:
 		if (!reiser4_inode_get_flag(inode, REISER4_PART_MIXED)) {
-			result = do_sync_read(file, buf, read_amount, off);
+			result = new_sync_read(file, buf, read_amount, off);
 			break;
 		}
 	case UF_CONTAINER_TAILS:
diff -u -r linux-3.15/fs/reiser4/plugin/object.c linux-3.16.1/fs/reiser4/plugin/object.c
--- linux-3.15/fs/reiser4/plugin/object.c	2014-08-24 14:10:36.200139193 +0200
+++ linux-3.16.1/fs/reiser4/plugin/object.c	2014-09-25 11:43:01.000000000 +0200
@@ -127,7 +127,7 @@
 	.llseek = generic_file_llseek,
 	.read = reiser4_read_dispatch,
 	.write = reiser4_write_dispatch,
-	.aio_read = generic_file_aio_read,
+	.read_iter = generic_file_read_iter,
 	.unlocked_ioctl = reiser4_ioctl_dispatch,
 #ifdef CONFIG_COMPAT
 	.compat_ioctl = reiser4_ioctl_dispatch,
@@ -137,7 +138,6 @@
 	.release = reiser4_release_dispatch,
 	.fsync = reiser4_sync_file_common,
 	.splice_read = generic_file_splice_read,
-	.splice_write = generic_file_splice_write
 };
 static struct address_space_operations regular_file_a_ops = {
 	.writepage = reiser4_writepage,