2011-05-15 05:32:02

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 1/3] bsg: fix bsg_poll() to return POLLOUT properly

POLLOUT should be returned only if bd->queued_cmds < bd->max_queue
so that bsg_alloc_command() can proceed.

Signed-off-by: Namhyung Kim <[email protected]>
---
block/bsg.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/block/bsg.c b/block/bsg.c
index 0c8b64a16484..c4f49e255751 100644
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -878,7 +878,7 @@ static unsigned int bsg_poll(struct file *file, poll_table *wait)
spin_lock_irq(&bd->lock);
if (!list_empty(&bd->done_list))
mask |= POLLIN | POLLRDNORM;
- if (bd->queued_cmds >= bd->max_queue)
+ if (bd->queued_cmds < bd->max_queue)
mask |= POLLOUT;
spin_unlock_irq(&bd->lock);

--
1.7.5


2011-05-15 05:32:25

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 2/3] bsg: remove unnecessary conditional expressions

Second condition in OR always implies first condition is false
thus bytes_read in the second is not needed. The same goes to
bytes_written.

Signed-off-by: Namhyung Kim <[email protected]>
---
block/bsg.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/bsg.c b/block/bsg.c
index c4f49e255751..b7e42ad55361 100644
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -606,7 +606,7 @@ bsg_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
ret = __bsg_read(buf, count, bd, NULL, &bytes_read);
*ppos = bytes_read;

- if (!bytes_read || (bytes_read && err_block_err(ret)))
+ if (!bytes_read || err_block_err(ret))
bytes_read = ret;

return bytes_read;
@@ -686,7 +686,7 @@ bsg_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
/*
* return bytes written on non-fatal errors
*/
- if (!bytes_written || (bytes_written && err_block_err(ret)))
+ if (!bytes_written || err_block_err(ret))
bytes_written = ret;

dprintk("%s: returning %Zd\n", bd->name, bytes_written);
--
1.7.5

2011-05-15 05:32:12

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 3/3] bsg: fix address space warning from sparse

copy_from/to_user() and blk_rq_map_user() want __user pointer.
This patch fixes following warnings from sparse:

CHECK block/bsg.c
block/bsg.c:185:38: warning: incorrect type in argument 2 (different address spaces)
block/bsg.c:185:38: expected void const [noderef] <asn:1>*from
block/bsg.c:185:38: got void *<noident>
block/bsg.c:295:58: warning: incorrect type in argument 4 (different address spaces)
block/bsg.c:295:58: expected void [noderef] <asn:1>*<noident>
block/bsg.c:295:58: got void *[assigned] dxferp
block/bsg.c:311:52: warning: incorrect type in argument 4 (different address spaces)
block/bsg.c:311:52: expected void [noderef] <asn:1>*<noident>
block/bsg.c:311:52: got void *[assigned] dxferp
block/bsg.c:448:37: warning: incorrect type in argument 1 (different address spaces)
block/bsg.c:448:37: expected void [noderef] <asn:1>*dst
block/bsg.c:448:37: got void *<noident>

Signed-off-by: Namhyung Kim <[email protected]>
---
block/bsg.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/block/bsg.c b/block/bsg.c
index b7e42ad55361..702f1316bb8f 100644
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -182,7 +182,7 @@ static int blk_fill_sgv4_hdr_rq(struct request_queue *q, struct request *rq,
return -ENOMEM;
}

- if (copy_from_user(rq->cmd, (void *)(unsigned long)hdr->request,
+ if (copy_from_user(rq->cmd, (void __user *)(unsigned long)hdr->request,
hdr->request_len))
return -EFAULT;

@@ -249,7 +249,7 @@ bsg_map_hdr(struct bsg_device *bd, struct sg_io_v4 *hdr, fmode_t has_write_perm,
struct request *rq, *next_rq = NULL;
int ret, rw;
unsigned int dxfer_len;
- void *dxferp = NULL;
+ void __user *dxferp = NULL;
struct bsg_class_device *bcd = &q->bsg_dev;

/* if the LLD has been removed then the bsg_unregister_queue will
@@ -291,7 +291,7 @@ bsg_map_hdr(struct bsg_device *bd, struct sg_io_v4 *hdr, fmode_t has_write_perm,
rq->next_rq = next_rq;
next_rq->cmd_type = rq->cmd_type;

- dxferp = (void*)(unsigned long)hdr->din_xferp;
+ dxferp = (void __user *)(unsigned long)hdr->din_xferp;
ret = blk_rq_map_user(q, next_rq, NULL, dxferp,
hdr->din_xfer_len, GFP_KERNEL);
if (ret)
@@ -300,10 +300,10 @@ bsg_map_hdr(struct bsg_device *bd, struct sg_io_v4 *hdr, fmode_t has_write_perm,

if (hdr->dout_xfer_len) {
dxfer_len = hdr->dout_xfer_len;
- dxferp = (void*)(unsigned long)hdr->dout_xferp;
+ dxferp = (void __user *)(unsigned long)hdr->dout_xferp;
} else if (hdr->din_xfer_len) {
dxfer_len = hdr->din_xfer_len;
- dxferp = (void*)(unsigned long)hdr->din_xferp;
+ dxferp = (void __user *)(unsigned long)hdr->din_xferp;
} else
dxfer_len = 0;

@@ -445,7 +445,7 @@ static int blk_complete_sgv4_hdr_rq(struct request *rq, struct sg_io_v4 *hdr,
int len = min_t(unsigned int, hdr->max_response_len,
rq->sense_len);

- ret = copy_to_user((void*)(unsigned long)hdr->response,
+ ret = copy_to_user((void __user *)(unsigned long)hdr->response,
rq->sense, len);
if (!ret)
hdr->response_len = len;
--
1.7.5

2011-05-18 19:31:01

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH 1/3] bsg: fix bsg_poll() to return POLLOUT properly

On 2011-05-18 20:09, FUJITA Tomonori wrote:
> On Sun, 15 May 2011 14:31:51 +0900
> Namhyung Kim <[email protected]> wrote:
>
>> POLLOUT should be returned only if bd->queued_cmds < bd->max_queue
>> so that bsg_alloc_command() can proceed.
>>
>> Signed-off-by: Namhyung Kim <[email protected]>
>> ---
>> block/bsg.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> All the three patches look good.
>
> Acked-by: FUJITA Tomonori <[email protected]>

You can add mine, too.

Acked-by: Jens Axboe <[email protected]>

--
Jens Axboe