Hi Andy,
You are right, the function names are a bit off. I see the following
options:
1) Rename these functions to map_sg_to_data_area() and
unmap_sg_from_data_area(). Even though these functions allocate,
deallocate space in the data area and possibly copy to/from it, their
purpose is to map/unmap the scatterlist to the data area. Later on, we
can improve these functions to actually map the data, and not copy them.
2) Rename these functions to alloc_and_scatter_data_area() and
gather_and_free_data_area(). These names describe exactly what these
functions do.
3) Split them into a pair of functions that allocate/free data area, and
another pair that copy between the data area and the sg. I didn't go for
this one because I didn't want to divert from your original
implementation. Such approach would also have to iterate over the
scatter/gather list twice.
Thanks,
Ilias
On Wed, Apr 22, 2015 at 12:41PM, Andy Grover wrote:
> On 04/22/2015 03:15 AM, Ilias Tsitsimpis wrote:
> >Introduce map_cmd_ring()/unmap_cmd_ring() functions that
> >allocate/deallocate space from the command ring and copy data from/to a
> >given scatter-gather list. These functions are needed so the next patch,
> >introducing support for bidirectional commands in TCMU, can use the same
> >code path both for 't_data_sg' and for 't_bidi_data_sg'.
> >
> >Signed-off-by: Ilias Tsitsimpis <iliastsi@xxxxxxxxxxx>
> >Signed-off-by: Vangelis Koukis <vkoukis@xxxxxxxxxxx>
>
> The function names "{map,unmap}_cmd_ring" seem a little off. We might get
> away with it but I think at some point we're going to want to do some actual
> mapping of pages to the data area and don't want to be confusing.
>
> First, cmd_ring should really be data_area.
>
> But also, "map" is really allocating space in the data area and possibly
> copying into it, and "unmap" is possibly copying out of it and then freeing
> data area space. Calling them copy_{to,from}_data_area also isn't really
> accurate. I can't think of any good opposing verbs for these function names.
>
> How would things work out if we broke things up so we had a pair of
> functions to allocate/free data area space, and another pair to copy
> to/from, between the (possibly discontiguous) data_area and the sg?
>
> Regards -- Andy
Attachment:
signature.asc
Description: Digital signature
