[PATCH] RDMA/cxgb4: Add default_llseek to debugfs files.

[PATCH] RDMA/cxgb4: Add default_llseek to debugfs files.

[Steve Wise]

This resolves a linux-next merge issue.

Signed-off-by: Steve Wise <swise@opengridcomputing.com>
---

 drivers/infiniband/hw/cxgb4/device.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/infiniband/hw/cxgb4/device.c b/drivers/infiniband/hw/cxgb4/device.c
index 986cfd7..b254357 100644
--- a/drivers/infiniband/hw/cxgb4/device.c
+++ b/drivers/infiniband/hw/cxgb4/device.c
@@ -182,6 +182,7 @@ static const struct file_operations qp_debugfs_fops = {
 	.open    = qp_open,
 	.release = qp_release,
 	.read    = debugfs_read,
+	.llseek  = default_llseek,
 };
 
 static int dump_stag(int id, void *p, void *data)
@@ -255,6 +256,7 @@ static const struct file_operations stag_debugfs_fops = {
 	.open    = stag_open,
 	.release = stag_release,
 	.read    = debugfs_read,
+	.llseek  = default_llseek,
 };
 
 static int setup_debugfs(struct c4iw_dev *devp)

Re: [PATCH] RDMA/cxgb4: Add default_llseek to debugfs files.

[Roland Dreier]

 > @@ -182,6 +182,7 @@ static const struct file_operations qp_debugfs_fops = {
 >  	.open    = qp_open,
 >  	.release = qp_release,
 >  	.read    = debugfs_read,
 > +	.llseek  = default_llseek,
 >  };

I think this could actually be generic_file_llseek (right, Arnd?).

Also could qp_read() and other read methods be simplified by using
simple_read_from_buffer()?  I don't see anything fancy there, seems like
the read methods could become one-liners that way.

 - R.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] RDMA/cxgb4: Add default_llseek to debugfs files.

[Steve Wise]

On 09/29/2010 12:19 PM, Roland Dreier wrote:
>   >  @@ -182,6 +182,7 @@ static const struct file_operations qp_debugfs_fops = {
>   >   	.open    = qp_open,
>   >   	.release = qp_release,
>   >   	.read    = debugfs_read,
>   >  +	.llseek  = default_llseek,
>   >   };
>
> I think this could actually be generic_file_llseek (right, Arnd?).
>
> Also could qp_read() and other read methods be simplified by using
> simple_read_from_buffer()?  I don't see anything fancy there, seems like
> the read methods could become one-liners that way.
>
>    

I assume you mean debugs_read()...Like this?

static ssize_t debugfs_read(struct file *file, char __user *buf, size_t 
count,
                             loff_t *ppos)
{
         struct c4iw_debugfs_data *d = file->private_data;

         return simple_read_from_buffer(buf, ppos, d->buf, d->pos);
}


Steve.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] RDMA/cxgb4: Add default_llseek to debugfs files.

[Roland Dreier]

 > I assume you mean debugs_read()...Like this?

Sorry, yeah, was looking at the old code.  But yes, that code seems a
lot simpler.

Re: [PATCH] RDMA/cxgb4: Add default_llseek to debugfs files.

[Steve Wise]

On 09/29/2010 12:41 PM, Roland Dreier wrote:
>   >  I assume you mean debugs_read()...Like this?
>
> Sorry, yeah, was looking at the old code.  But yes, that code seems a
> lot simpler.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>    

I'll change it, test it,  and send you a patch.

Re: [PATCH] RDMA/cxgb4: Add default_llseek to debugfs files.

[Roland Dreier]

 > I'll change it, test it,  and send you a patch.

great, thanks.

Re: [PATCH] RDMA/cxgb4: Add default_llseek to debugfs files.

[Arnd Bergmann]

On Wednesday 29 September 2010 19:19:32 Roland Dreier wrote:
>  > @@ -182,6 +182,7 @@ static const struct file_operations qp_debugfs_fops = {
>  >      .open    = qp_open,
>  >      .release = qp_release,
>  >      .read    = debugfs_read,
>  > +    .llseek  = default_llseek,
>  >  };
> 
> I think this could actually be generic_file_llseek (right, Arnd?).

The main difference between default_llseek and generic_file_llseek
is that default_llseek doesn't care about the maximum file size
of the underlying file system, which is ULONG_MAX on debugfs,
so they are equivalent.

In general, the preferred one is no_llseek for those files where
you know you do not need to seek. If you do, I'd use default_llseek
for character devices and generic_file_llseek for file systems
that set the s_maxbytes.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] RDMA/cxgb4: Add default_llseek to debugfs files.

[Roland Dreier]

 > The main difference between default_llseek and generic_file_llseek
 > is that default_llseek doesn't care about the maximum file size
 > of the underlying file system, which is ULONG_MAX on debugfs,
 > so they are equivalent.

I thought default_llseek also takes the BKL still?

 > In general, the preferred one is no_llseek for those files where
 > you know you do not need to seek. If you do, I'd use default_llseek
 > for character devices and generic_file_llseek for file systems
 > that set the s_maxbytes.

The case in question is for debugfs files, so we should use
generic_file_llseek, right?

 - R.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] RDMA/cxgb4: Add default_llseek to debugfs files.

[Arnd Bergmann]

On Wednesday 29 September 2010 21:21:14 Roland Dreier wrote:
> 
>  > The main difference between default_llseek and generic_file_llseek
>  > is that default_llseek doesn't care about the maximum file size
>  > of the underlying file system, which is ULONG_MAX on debugfs,
>  > so they are equivalent.
> 
> I thought default_llseek also takes the BKL still?

Not any more in linux-next.

>  > In general, the preferred one is no_llseek for those files where
>  > you know you do not need to seek. If you do, I'd use default_llseek
>  > for character devices and generic_file_llseek for file systems
>  > that set the s_maxbytes.
> 
> The case in question is for debugfs files, so we should use
> generic_file_llseek, right?

Yes, but it's a very weak "should". My automatic conversion script
uses default_llseek.

	Arnd

Re: [PATCH] RDMA/cxgb4: Add default_llseek to debugfs files.

[Roland Dreier]

 > Yes, but it's a very weak "should". My automatic conversion script
 > uses default_llseek.

OK, I'll just take steve's patch that does default_llseek if it really
doesn't matter.

 - R.

Re: [PATCH] RDMA/cxgb4: Add default_llseek to debugfs files.

[Roland Dreier]

 > --- a/drivers/infiniband/hw/cxgb4/device.c
 > +++ b/drivers/infiniband/hw/cxgb4/device.c
 > @@ -182,6 +182,7 @@ static const struct file_operations qp_debugfs_fops = {
 >  	.open    = qp_open,
 >  	.release = qp_release,
 >  	.read    = debugfs_read,
 > +	.llseek  = default_llseek,
 >  };
 >  
 >  static int dump_stag(int id, void *p, void *data)
 > @@ -255,6 +256,7 @@ static const struct file_operations stag_debugfs_fops = {
 >  	.open    = stag_open,
 >  	.release = stag_release,
 >  	.read    = debugfs_read,
 > +	.llseek  = default_llseek,
 >  };
 >  
 >  static int setup_debugfs(struct c4iw_dev *devp)
 > 
 > 

Steve, can you also do a quick test to make sure this works OK with
generic_file_llseek instead of default_llseek?

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html