[PATCH] s390 hypfs fixes for 2.6.18-rc1-mm1

[PATCH] s390 hypfs fixes for 2.6.18-rc1-mm1

[Badari Pulavarty]

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

Hi Micheal,

I made fixes to hypfs to fit new vfs ops interfaces. I am not sure if we 
really
need to vectorize aio interfaces, can you check and see if this is okay ?

And also, I am not sure what hypfs_aio_write() is actually doing.
It doesn't seem to be  doing with "buf" ?

(BTW - I have no way to verify these change. Can you give them a spin ?)

Thanks,
Badari




[-- Attachment #2: hypfs-fixes.patch --]
[-- Type: text/plain, Size: 1516 bytes --]

Vectorize aio interfaces to hypfs to fit new vfs ops interfaces.

Signed-off-by: Badari Pulavarty <pbadari@us.ibm.com>

Index: linux-2.6.18-rc1/arch/s390/hypfs/inode.c
===================================================================
--- linux-2.6.18-rc1.orig/arch/s390/hypfs/inode.c	2006-07-11 21:28:07.000000000 -0700
+++ linux-2.6.18-rc1/arch/s390/hypfs/inode.c	2006-07-12 21:18:44.000000000 -0700
@@ -134,12 +134,20 @@ static int hypfs_open(struct inode *inod
 	return 0;
 }
 
-static ssize_t hypfs_aio_read(struct kiocb *iocb, __user char *buf,
-			      size_t count, loff_t offset)
+static ssize_t hypfs_aio_read(struct kiocb *iocb, const struct iovec *iov,
+			      unsigned long nr_segs, loff_t offset)
 {
 	char *data;
 	size_t len;
 	struct file *filp = iocb->ki_filp;
+	/* XXX: temporary */
+	const struct __user *buf = iov[0].iov_base;
+	size_t count = iov[0].iov_len;
+
+	if (nr_segs != 1) {
+		count = -EINVAL;
+		goto out;
+	}
 
 	data = filp->private_data;
 	len = strlen(data);
@@ -158,12 +166,13 @@ static ssize_t hypfs_aio_read(struct kio
 out:
 	return count;
 }
-static ssize_t hypfs_aio_write(struct kiocb *iocb, const char __user *buf,
-			       size_t count, loff_t pos)
+static ssize_t hypfs_aio_write(struct kiocb *iocb, const struct iovec *iov,
+			      unsigned long nr_segs, loff_t offset)
 {
 	int rc;
 	struct super_block *sb;
 	struct hypfs_sb_info *fs_info;
+	size_t count = iov_length(iov, nr_segs);
 
 	sb = iocb->ki_filp->f_dentry->d_inode->i_sb;
 	fs_info = sb->s_fs_info;

Re: [PATCH] s390 hypfs fixes for 2.6.18-rc1-mm1

[Badari Pulavarty]

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

Badari Pulavarty wrote:
> Hi Micheal,
>
> I made fixes to hypfs to fit new vfs ops interfaces. I am not sure if 
> we really
> need to vectorize aio interfaces, can you check and see if this is okay ?
>
> And also, I am not sure what hypfs_aio_write() is actually doing.
> It doesn't seem to be  doing with "buf" ?
>
> (BTW - I have no way to verify these change. Can you give them a spin ?)
>
> Thanks,
> Badari

Here is the updated version ..



[-- Attachment #2: hypfs-fixes.patch --]
[-- Type: text/plain, Size: 1508 bytes --]

Vectorize aio interfaces to hypfs to fit new vfs ops interfaces.

Signed-off-by: Badari Pulavarty <pbadari@us.ibm.com>

Index: linux-2.6.18-rc1/arch/s390/hypfs/inode.c
===================================================================
--- linux-2.6.18-rc1.orig/arch/s390/hypfs/inode.c	2006-07-11 21:28:07.000000000 -0700
+++ linux-2.6.18-rc1/arch/s390/hypfs/inode.c	2006-07-12 21:31:43.000000000 -0700
@@ -134,12 +134,20 @@ static int hypfs_open(struct inode *inod
 	return 0;
 }
 
-static ssize_t hypfs_aio_read(struct kiocb *iocb, __user char *buf,
-			      size_t count, loff_t offset)
+static ssize_t hypfs_aio_read(struct kiocb *iocb, const struct iovec *iov,
+			      unsigned long nr_segs, loff_t offset)
 {
 	char *data;
 	size_t len;
 	struct file *filp = iocb->ki_filp;
+	/* XXX: temporary */
+	char __user *buf = iov[0].iov_base;
+	size_t count = iov[0].iov_len;
+
+	if (nr_segs != 1) {
+		count = -EINVAL;
+		goto out;
+	}
 
 	data = filp->private_data;
 	len = strlen(data);
@@ -158,12 +166,13 @@ static ssize_t hypfs_aio_read(struct kio
 out:
 	return count;
 }
-static ssize_t hypfs_aio_write(struct kiocb *iocb, const char __user *buf,
-			       size_t count, loff_t pos)
+static ssize_t hypfs_aio_write(struct kiocb *iocb, const struct iovec *iov,
+			      unsigned long nr_segs, loff_t offset)
 {
 	int rc;
 	struct super_block *sb;
 	struct hypfs_sb_info *fs_info;
+	size_t count = iov_length(iov, nr_segs);
 
 	sb = iocb->ki_filp->f_dentry->d_inode->i_sb;
 	fs_info = sb->s_fs_info;

Re: [PATCH] s390 hypfs fixes for 2.6.18-rc1-mm1

[Cedric Le Goater]

Badari Pulavarty wrote:
> Badari Pulavarty wrote:
>> Hi Micheal,
>>
>> I made fixes to hypfs to fit new vfs ops interfaces. I am not sure if
>> we really
>> need to vectorize aio interfaces, can you check and see if this is okay ?
>>
>> And also, I am not sure what hypfs_aio_write() is actually doing.
>> It doesn't seem to be  doing with "buf" ?
>>
>> (BTW - I have no way to verify these change. Can you give them a spin ?)
>>
>> Thanks,
>> Badari
> 
> Here is the updated version ..

indeed it compiles better :)

it boots fine. what kind of tests do you run ?

thanks,

C.

Re: [PATCH] s390 hypfs fixes for 2.6.18-rc1-mm1

[Badari Pulavarty]

On Thu, 2006-07-13 at 11:04 +0200, Cedric Le Goater wrote:
> Badari Pulavarty wrote:
> > Badari Pulavarty wrote:
> >> Hi Micheal,
> >>
> >> I made fixes to hypfs to fit new vfs ops interfaces. I am not sure if
> >> we really
> >> need to vectorize aio interfaces, can you check and see if this is okay ?
> >>
> >> And also, I am not sure what hypfs_aio_write() is actually doing.
> >> It doesn't seem to be  doing with "buf" ?
> >>
> >> (BTW - I have no way to verify these change. Can you give them a spin ?)
> >>
> >> Thanks,
> >> Badari
> > 
> > Here is the updated version ..
> 
> indeed it compiles better :)
> 
> it boots fine. what kind of tests do you run ?

I have no way of testing hypfs, since I don't even know what it does :(
Is it possible to test it for me ?

Thanks,
Badari

Re: [PATCH] s390 hypfs fixes for 2.6.18-rc1-mm1

[Andrew Morton]

On Wed, 12 Jul 2006 21:17:53 -0700
Badari Pulavarty <pbadari@us.ibm.com> wrote:

> +static ssize_t hypfs_aio_read(struct kiocb *iocb, const struct iovec *iov,
> +			      unsigned long nr_segs, loff_t offset)
>  {
>  	char *data;
>  	size_t len;
>  	struct file *filp = iocb->ki_filp;
> +	/* XXX: temporary */
> +	char __user *buf = iov[0].iov_base;
> +	size_t count = iov[0].iov_len;
> +
> +	if (nr_segs != 1) {
> +		count = -EINVAL;
> +		goto out;
> +	}

err, "temporary" things tend to become permanent.  What's the real fix?

Re: [PATCH] s390 hypfs fixes for 2.6.18-rc1-mm1

[Badari Pulavarty]

Andrew Morton wrote:
> On Wed, 12 Jul 2006 21:17:53 -0700
> Badari Pulavarty <pbadari@us.ibm.com> wrote:
>
>   
>> +static ssize_t hypfs_aio_read(struct kiocb *iocb, const struct iovec *iov,
>> +			      unsigned long nr_segs, loff_t offset)
>>  {
>>  	char *data;
>>  	size_t len;
>>  	struct file *filp = iocb->ki_filp;
>> +	/* XXX: temporary */
>> +	char __user *buf = iov[0].iov_base;
>> +	size_t count = iov[0].iov_len;
>> +
>> +	if (nr_segs != 1) {
>> +		count = -EINVAL;
>> +		goto out;
>> +	}
>>     
>
> err, "temporary" things tend to become permanent.  What's the real fix?
>   
I am not sure, if we really need to vectorize this method or not - 
meaning will this be ever called
with more than one items in the vector. 

Micheal, is it possible ? Can some one directly use AIO interface on 
hypfs ? If not, we can always
look at only first element and ignore rest of them. Otherwise, we need 
to iterate on all the elements
of the vector.

Thanks,
Badari

Re: [PATCH] s390 hypfs fixes for 2.6.18-rc1-mm1

[Michael Holzheu]

Badari Pulavarty <pbadari@us.ibm.com> wrote on 07/13/2006 04:31:00 PM:
> Andrew Morton wrote:
> > On Wed, 12 Jul 2006 21:17:53 -0700
> > Badari Pulavarty <pbadari@us.ibm.com> wrote:
> >

[snip]

> >
> > err, "temporary" things tend to become permanent.  What's the real fix?
> >
> I am not sure, if we really need to vectorize this method or not -
> meaning will this be ever called
> with more than one items in the vector.
>
> Micheal, is it possible ? Can some one directly use AIO interface on
> hypfs ? If not, we can always
> look at only first element and ignore rest of them. Otherwise, we need
> to iterate on all the elements
> of the vector.

Of course it is possible that someone uses AIO on hypfs files, but
normally the synchronous IO functions are used. I used the AIO
implementation together with do_sync_read/write() only because
it was not more effort regarding the implementation and we got
the AIO interface for free.

Nevertheless we probably should implement the complete
function.

Michael

Re: [PATCH] s390 hypfs fixes for 2.6.18-rc1-mm1

[Badari Pulavarty]

On Thu, 2006-07-13 at 17:41 +0200, Michael Holzheu wrote:
> Badari Pulavarty <pbadari@us.ibm.com> wrote on 07/13/2006 04:31:00 PM:
> > Andrew Morton wrote:
> > > On Wed, 12 Jul 2006 21:17:53 -0700
> > > Badari Pulavarty <pbadari@us.ibm.com> wrote:
> > >
> 
> [snip]
> 
> > >
> > > err, "temporary" things tend to become permanent.  What's the real fix?
> > >
> > I am not sure, if we really need to vectorize this method or not -
> > meaning will this be ever called
> > with more than one items in the vector.
> >
> > Micheal, is it possible ? Can some one directly use AIO interface on
> > hypfs ? If not, we can always
> > look at only first element and ignore rest of them. Otherwise, we need
> > to iterate on all the elements
> > of the vector.
> 
> Of course it is possible that someone uses AIO on hypfs files, but
> normally the synchronous IO functions are used. I used the AIO
> implementation together with do_sync_read/write() only because
> it was not more effort regarding the implementation and we got
> the AIO interface for free.
> 
> Nevertheless we probably should implement the complete
> function.

Well, if you say so.. I have no idea what hypfs provides,
if you think that some one can do vector and/or async
operations on these files, its worth adding the complexity.

I was inclined towards just dealing with only single entry
in the vector and fail if some one called with multiple vectors.
But, its your call. Here is the complete code - completely untested :(
Could you please compile and test it  ?

Thanks,
Badari

Vectorize aio interfaces to hypfs to fit new vfs ops interfaces.

Signed-off-by: Badari Pulavarty <pbadari@us.ibm.com>

Index: linux-2.6.18-rc1/arch/s390/hypfs/inode.c
===================================================================
--- linux-2.6.18-rc1.orig/arch/s390/hypfs/inode.c	2006-07-11 21:28:07.000000000 -0700
+++ linux-2.6.18-rc1/arch/s390/hypfs/inode.c	2006-07-13 10:47:07.000000000 -0700
@@ -134,36 +134,50 @@ static int hypfs_open(struct inode *inod
 	return 0;
 }
 
-static ssize_t hypfs_aio_read(struct kiocb *iocb, __user char *buf,
-			      size_t count, loff_t offset)
+static ssize_t hypfs_aio_read(struct kiocb *iocb, const struct iovec *iov,
+			      unsigned long nr_segs, loff_t offset)
 {
 	char *data;
 	size_t len;
 	struct file *filp = iocb->ki_filp;
+	int i, ret = 0;
 
 	data = filp->private_data;
 	len = strlen(data);
-	if (offset > len) {
-		count = 0;
-		goto out;
-	}
-	if (count > len - offset)
-		count = len - offset;
-	if (copy_to_user(buf, data + offset, count)) {
-		count = -EFAULT;
+	if (offset < len)
+		len -= offset;
+	else
 		goto out;
+
+	for (i=0; i < nr_segs; i++) {
+		char __user *buf = iov[i].iov_base;
+		size_t count = iov[i].iov_len;
+
+		if (count > len)
+			count = len;
+		if (copy_to_user(buf, data + offset, count)) {
+			if (!ret)
+				ret = -EFAULT;
+			break;
+		}
+		offset += count;
+		len -= count;
+		ret += count;
+		if (len == 0)
+			break;
 	}
-	iocb->ki_pos += count;
+	iocb->ki_pos = offset;
 	file_accessed(filp);
 out:
-	return count;
+	return ret;
 }
-static ssize_t hypfs_aio_write(struct kiocb *iocb, const char __user *buf,
-			       size_t count, loff_t pos)
+static ssize_t hypfs_aio_write(struct kiocb *iocb, const struct iovec *iov,
+			      unsigned long nr_segs, loff_t offset)
 {
 	int rc;
 	struct super_block *sb;
 	struct hypfs_sb_info *fs_info;
+	size_t count = iov_length(iov, nr_segs);
 
 	sb = iocb->ki_filp->f_dentry->d_inode->i_sb;
 	fs_info = sb->s_fs_info;

Re: [PATCH] s390 hypfs fixes for 2.6.18-rc1-mm1

[Michael Holzheu]

Hi Badari,

Badari Pulavarty <pbadari@us.ibm.com> wrote on 07/13/2006 07:39:04 PM:
> Well, if you say so.. I have no idea what hypfs provides,
> if you think that some one can do vector and/or async
> operations on these files, its worth adding the complexity.
>
> I was inclined towards just dealing with only single entry
> in the vector and fail if some one called with multiple vectors.
> But, its your call. Here is the complete code - completely untested :(
> Could you please compile and test it  ?
>
> Thanks,
> Badari
>

I tested your patch. It works fine! You can test hypfs only
on zSeries machines in LPAR mode. It provides acounting
information for the zSeries Hypervisor LPAR environment.

Thanks!

Michael


> Vectorize aio interfaces to hypfs to fit new vfs ops interfaces.
>
> Signed-off-by: Badari Pulavarty <pbadari@us.ibm.com>
>
> Index: linux-2.6.18-rc1/arch/s390/hypfs/inode.c
> ===================================================================
> --- linux-2.6.18-rc1.orig/arch/s390/hypfs/inode.c   2006-07-11 21:
> 28:07.000000000 -0700
> +++ linux-2.6.18-rc1/arch/s390/hypfs/inode.c   2006-07-13 10:47:07.
> 000000000 -0700
> @@ -134,36 +134,50 @@ static int hypfs_open(struct inode *inod
>     return 0;
>  }
>
> -static ssize_t hypfs_aio_read(struct kiocb *iocb, __user char *buf,
> -               size_t count, loff_t offset)
> +static ssize_t hypfs_aio_read(struct kiocb *iocb, const struct iovec
*iov,
> +               unsigned long nr_segs, loff_t offset)
>  {
>     char *data;
>     size_t len;
>     struct file *filp = iocb->ki_filp;
> +   int i, ret = 0;
>
>     data = filp->private_data;
>     len = strlen(data);
> -   if (offset > len) {
> -      count = 0;
> -      goto out;
> -   }
> -   if (count > len - offset)
> -      count = len - offset;
> -   if (copy_to_user(buf, data + offset, count)) {
> -      count = -EFAULT;
> +   if (offset < len)
> +      len -= offset;
> +   else
>        goto out;
> +
> +   for (i=0; i < nr_segs; i++) {
> +      char __user *buf = iov[i].iov_base;
> +      size_t count = iov[i].iov_len;
> +
> +      if (count > len)
> +         count = len;
> +      if (copy_to_user(buf, data + offset, count)) {
> +         if (!ret)
> +            ret = -EFAULT;
> +         break;
> +      }
> +      offset += count;
> +      len -= count;
> +      ret += count;
> +      if (len == 0)
> +         break;
>     }
> -   iocb->ki_pos += count;
> +   iocb->ki_pos = offset;
>     file_accessed(filp);
>  out:
> -   return count;
> +   return ret;
>  }
> -static ssize_t hypfs_aio_write(struct kiocb *iocb, const char __user
*buf,
> -                size_t count, loff_t pos)
> +static ssize_t hypfs_aio_write(struct kiocb *iocb, const struct iovec
*iov,
> +               unsigned long nr_segs, loff_t offset)
>  {
>     int rc;
>     struct super_block *sb;
>     struct hypfs_sb_info *fs_info;
> +   size_t count = iov_length(iov, nr_segs);
>
>     sb = iocb->ki_filp->f_dentry->d_inode->i_sb;
>     fs_info = sb->s_fs_info;
>
>
>

Re: [PATCH] s390 hypfs fixes for 2.6.18-rc1-mm1

[Michael Holzheu]

Hi Badari

Your new patch compiles and seems to work.

Badari Pulavarty <pbadari@us.ibm.com> wrote on 07/13/2006 06:17:53 AM:
> Badari Pulavarty wrote:
> > Hi Micheal,
> >
> > I made fixes to hypfs to fit new vfs ops interfaces. I am not sure if
> > we really
> > need to vectorize aio interfaces, can you check and see if this is okay
?
> > And also, I am not sure what hypfs_aio_write() is actually doing.
> > It doesn't seem to be  doing with "buf" ?

The hypfs write function currently is only used to trigger the update
process of hypfs. We just ignore the buffer.

Thanks

Michael