Hello Alan,
Thank you for your review. Please see below for a comment.
On Tuesday, July 16, 2013 5:13 PM Alan Stern wrote:
> On Tue, 16 Jul 2013, Andrzej Pietrasiewicz wrote:
>
> > This is needed to prepare for configfs integration.
> >
> > So far the luns have been allocated during gadget's initialization,
> > based on the nluns module parameter's value; the exact number is known
> > when the gadget is initialized and that number of luns is allocated in
> > one go; they all will be used.
> >
> > When configfs is in place, the luns will be created one-by-one by the
> user.
> > Once the user is satisfied with the number of luns, they activate the
> > gadget. The number of luns must be <= FSG_MAX_LUN (currently 8), but
> > other than that it is not known up front and the user need not use
> > contiguous numbering (apart from the default lun #0). On the other
> > hand, the function code uses lun numbers to identify them and the
> > number needs to be used as an index into an array.
> >
> > Given the above, an array needs to be allocated, but it might happen
> > that
> > 7 out of its 8 elements will not be used. On my machine sizeof(struct
> > fsg_lun) == 462, so > 3k of memory is allocated but not used in the
> > worst case.
> >
> > By adding another level of indirection (allocating an array of
> > pointers to struct fsg_lun and then allocating individual luns instead
> > of an array of struct fsg_luns) at most 7 pointers are wasted, which is
> much less.
> >
> > This patch also changes some for/while loops to cope with the fact
> > that in the luns array some entries are potentially empty.
>
> > @@ -621,7 +621,7 @@ static int sleep_thread(struct fsg_common *common)
> >
> > static int do_read(struct fsg_common *common) {
> > - struct fsg_lun *curlun = common->curlun;
> > + struct fsg_lun **curlun = common->curlun;
>
> It would be much easier here and in other functions to instead do:
>
> + struct fsg_lun *curlun = *common->curlun;
>
> Then you wouldn't need to replace "curlun" with "*curlun" all over the
> place.
>
> Alternatively (I haven't checked in detail to see if this would work),
> keep common->curlun itself as a "struct fsg_lun *" instead of making it
> "struct fsg_lun **".
Yeah, these two ideas look attractive, I'll give them a try.
Thanks,
Andrzej
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
