[PATCH] libceph: Fix ceph_tcp_sendpage()'s more boolean usage

[PATCH] libceph: Fix ceph_tcp_sendpage()'s more boolean usage

[Benoît Canet]

Spotted while hunting http://tracker.ceph.com/issues/10905.

From struct ceph_msg_data_cursor in include/linux/ceph/messenger.h:

bool	last_piece;	/* current is last piece */

In ceph_msg_data_next():

*last_piece = cursor->last_piece;

A call to ceph_msg_data_next() is followed by:

ret = ceph_tcp_sendpage(con->sock, page, page_offset,
			length, last_piece);

while ceph_tcp_sendpage() is:

static int ceph_tcp_sendpage(struct socket *sock, struct page *page,i
			     int offset, size_t size, bool more)

The logic is inverted: correct it.

Signed-off-by: Benoît Canet <benoit.canet@nodalink.com>
---
 net/ceph/messenger.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
index ec68cd3..eda06fd 100644
--- a/net/ceph/messenger.c
+++ b/net/ceph/messenger.c
@@ -1544,7 +1544,7 @@ static int write_partial_message_data(struct ceph_connection *con)
 		page = ceph_msg_data_next(&msg->cursor, &page_offset, &length,
 							&last_piece);
 		ret = ceph_tcp_sendpage(con->sock, page, page_offset,
-				      length, last_piece);
+				      length, !last_piece);
 		if (ret <= 0) {
 			if (do_datacrc)
 				msg->footer.data_crc = cpu_to_le32(crc);
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] libceph: Fix ceph_tcp_sendpage()'s more boolean usage

[Ilya Dryomov]

On Thu, Jun 25, 2015 at 7:56 PM, Benoît Canet <benoit.canet@nodalink.com> wrote:
> Spotted while hunting http://tracker.ceph.com/issues/10905.
>
> From struct ceph_msg_data_cursor in include/linux/ceph/messenger.h:
>
> bool    last_piece;     /* current is last piece */
>
> In ceph_msg_data_next():
>
> *last_piece = cursor->last_piece;
>
> A call to ceph_msg_data_next() is followed by:
>
> ret = ceph_tcp_sendpage(con->sock, page, page_offset,
>                         length, last_piece);
>
> while ceph_tcp_sendpage() is:
>
> static int ceph_tcp_sendpage(struct socket *sock, struct page *page,i
>                              int offset, size_t size, bool more)
>
> The logic is inverted: correct it.
>
> Signed-off-by: Benoît Canet <benoit.canet@nodalink.com>
> ---
>  net/ceph/messenger.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
> index ec68cd3..eda06fd 100644
> --- a/net/ceph/messenger.c
> +++ b/net/ceph/messenger.c
> @@ -1544,7 +1544,7 @@ static int write_partial_message_data(struct ceph_connection *con)
>                 page = ceph_msg_data_next(&msg->cursor, &page_offset, &length,
>                                                         &last_piece);
>                 ret = ceph_tcp_sendpage(con->sock, page, page_offset,
> -                                     length, last_piece);
> +                                     length, !last_piece);
>                 if (ret <= 0) {
>                         if (do_datacrc)
>                                 msg->footer.data_crc = cpu_to_le32(crc);

Good catch.  Applied.

Thanks,

                Ilya
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] libceph: Fix ceph_tcp_sendpage()'s more boolean usage

[Alex Elder]

On 06/25/2015 11:56 AM, Benoît Canet wrote:
> Spotted while hunting http://tracker.ceph.com/issues/10905.
>
>  From struct ceph_msg_data_cursor in include/linux/ceph/messenger.h:
>
> bool	last_piece;	/* current is last piece */
>
> In ceph_msg_data_next():
>
> *last_piece = cursor->last_piece;
>
> A call to ceph_msg_data_next() is followed by:
>
> ret = ceph_tcp_sendpage(con->sock, page, page_offset,
> 			length, last_piece);
>
> while ceph_tcp_sendpage() is:
>
> static int ceph_tcp_sendpage(struct socket *sock, struct page *page,i
> 			     int offset, size_t size, bool more)
>
> The logic is inverted: correct it.

Whoops.  I'm surprised we haven't had more trouble from this.
This should go to stable too.

Looks good.

Reviewed-by: Alex Elder <elder@linaro.org>

>
> Signed-off-by: Benoît Canet <benoit.canet@nodalink.com>
> ---
>   net/ceph/messenger.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
> index ec68cd3..eda06fd 100644
> --- a/net/ceph/messenger.c
> +++ b/net/ceph/messenger.c
> @@ -1544,7 +1544,7 @@ static int write_partial_message_data(struct ceph_connection *con)
>   		page = ceph_msg_data_next(&msg->cursor, &page_offset, &length,
>   							&last_piece);
>   		ret = ceph_tcp_sendpage(con->sock, page, page_offset,
> -				      length, last_piece);
> +				      length, !last_piece);
>   		if (ret <= 0) {
>   			if (do_datacrc)
>   				msg->footer.data_crc = cpu_to_le32(crc);
>

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html