On 20/08/15 09:10, Roger Pau Monn? wrote:

Hello,

Hi,

I have some comments regarding the commit message, IMHO it would be good
that a native English speaker reviews it too.

El 07/08/15 a les 18.46, Julien Grall ha escrit:

quoted

The PV block protocol is using 4KB page granularity. The goal of this
patch is to allow a Linux using 64KB page granularity using block
device on a non-modified Xen.

The block API is using segment which should at least be the size of a
Linux page. Therefore, the driver will have to break the page in chunk
of 4K before giving the page to the backend.

Breaking a 64KB segment in 4KB chunk will result to have some chunk with
no data.

I would rewrite the above line as:

When breaking a 64KB segment into 4KB chunks it is possible that some

Correct,

chunks are empty.

Sounds good, I will replace with it.

quoted

As the PV protocol always require to have data in the chunk, we
have to count the number of Xen page which will be in use and avoid to

                                  ^pages

quoted

sent empty chunk.

  ^and avoid sending empty chunks.

quoted

Note that, a pre-defined number of grant is reserved before preparing

                                     ^grants are

quoted

the request. This pre-defined number is based on the number and the
maximum size of the segments. If each segment contain a very small

                                                ^contains

quoted

amount of data, the driver may reserve too much grant (16 grant is

                                             ^many grants   ^grants are

quoted

reserved per segment with 64KB page granularity).

Futhermore, in the case of persistent grant we allocate one Linux page

                                        ^grants

quoted

per grant although only the 4KB of the page will be effectively use.

                             ^first                              ^in

quoted

This could be improved by share the page with multiple grants.

                            ^sharing

quoted

Signed-off-by: Julien Grall <redacted>

LGTM:

Acked-by: Roger Pau Monn? <roger.pau@citrix.com>

Thank you, I will fix all the typos in the next version.

Just one question.

[..]

quoted

+		gnttab_foreach_grant_in_range(sg_page(sg),
+					      sg->offset,
+					      sg->length,
+					      blkif_setup_rw_req_grant,
+					      &setup);

If I'm understanding this right, on x86 gnttab_foreach_grant_in_range is
only going to perform one iteration, since XEN_PAGE_SIZE == PAGE_SIZE.

Correct, it will only perform when iteration for x86 but also for arm32
and arm64 (when 4KB page is in use).

Regards,

-- 
Julien Grall