* Konrad Rzeszutek Wilk (konrad.wilk@xxxxxxxxxx) wrote:
> > > +void __init
> > > +swiotlb_init(int verbose)
> > > +{
> > > + swiotlb_register_engine(&swiotlb_ops);
> > > + swiotlb_init_with_default_size(&swiotlb_ops, 64 * (1<<20),
> > > + verbose); /* default to 64MB */
> > > +}
> >
> > I'd expect the swiotlb-default file to have only private impl. of the
> > swiotlb_engine. Shouldn't this and the init stay in swiotlb.c? Also,
>
> Hmm, were you thinking that it might make sense to pass in
> a swiotlb_ops to swiotlb_init so that it can make the right assignments?
Yeah, something like that.
> The reason why I stuck here was that the swiotlb_ops needed to be
> visible to this function, and having it in swiotlb.c would mean it must
> now include the header definition for swiotlb-defualt.h.
And part of that need is because the allocator (effectively
common/global) is writing to impl. private data, like ->nslabs. But if
you move that back, then this may not be an issue.
> > would you ever call swiotlb_init w/out register_engine, why not move
> > register to the swiotlb_init?
>
> In essence combine swiotlb_register_engine with
> swiotlb_init_with_default_size?
Yep.
> There would still be a need for late call mechanism.
> Perhaps having two variants of swiotlb_init?: swiotlb_early_init(struct
> swiotlb_engine *swiotlb_ops) and swiotlb_late_init(struct swiotlb_engine
> *swiotlb_ops)?
That's basically what we have now, swiotlb{,_late}_init_with_default_size,
so seems reasonable to me.
> Or perhaps just pass in an argument: swiotlb_init(int late)?
>
> Furthermore have this new swiotlb_init detect if some of the fields
> (start ,end, overflow_buffer) have been allocated and if so skip the
> default allocation altogether?
That would keep the allocate, ->release, allocate cycle from happening
(which seems odd when it's the same sizes and same core allocation).
Part of why I thought there was too much moved into the impl. private
engine structure.
thanks,
-chris
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|