[Xen-devel] Re: [PATCH 14/15] [swiotlb] Move initialization (swiotlb_init) and its friends in swiotlb-default.c

[Chris Wright <chrisw@xxxxxxxxxxxx>]

* 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