[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v4 1/8] swiotlb: make io_tlb_default_mem local to swiotlb.c
On Thu, 20 Jul 2023 08:37:44 +0200 Christoph Hellwig <hch@xxxxxx> wrote: > On Thu, Jul 13, 2023 at 05:23:12PM +0200, Petr Tesarik wrote: > > From: Petr Tesarik <petr.tesarik.ext@xxxxxxxxxx> > > > > SWIOTLB implementation details should not be exposed to the rest of the > > kernel. This will allow to make changes to the implementation without > > modifying non-swiotlb code. > > > > To avoid breaking existing users, provide helper functions for the few > > required fields. > > > > As a bonus, using a helper function to initialize struct device allows to > > get rid of an #ifdef in driver core. > > > > Signed-off-by: Petr Tesarik <petr.tesarik.ext@xxxxxxxxxx> > > --- > > arch/arm/xen/mm.c | 2 +- > > arch/mips/pci/pci-octeon.c | 2 +- > > arch/x86/kernel/pci-dma.c | 2 +- > > drivers/base/core.c | 4 +--- > > drivers/xen/swiotlb-xen.c | 2 +- > > include/linux/swiotlb.h | 25 +++++++++++++++++++++++- > > kernel/dma/swiotlb.c | 39 +++++++++++++++++++++++++++++++++++++- > > 7 files changed, 67 insertions(+), 9 deletions(-) > > > > diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c > > index 3d826c0b5fee..0f32c14eb786 100644 > > --- a/arch/arm/xen/mm.c > > +++ b/arch/arm/xen/mm.c > > @@ -125,7 +125,7 @@ static int __init xen_mm_init(void) > > return 0; > > > > /* we can work with the default swiotlb */ > > - if (!io_tlb_default_mem.nslabs) { > > + if (!is_swiotlb_allocated()) { > > rc = swiotlb_init_late(swiotlb_size_or_default(), > > xen_swiotlb_gfp(), NULL); > > if (rc < 0) > > I'd much rather move the already initialized check into > swiotlb_init_late, which is a much cleaer interface. > > > /* we can work with the default swiotlb */ > > - if (!io_tlb_default_mem.nslabs) { > > + if (!is_swiotlb_allocated()) { > > int rc = swiotlb_init_late(swiotlb_size_or_default(), > > GFP_KERNEL, xen_swiotlb_fixup); > > if (rc < 0) > > .. and would take care of this one as well. Oh, you're right! These are the only two places that look at io_tlb_default_mem.nslabs, and all they need is to avoid double initialization. Makes perfect sense to move it inside swiotlb_init_late(). > > +bool is_swiotlb_allocated(void) > > +{ > > + return !!io_tlb_default_mem.nslabs; > > Nit: no need for the !!, we can rely on the implicit promotion to > bool. But with the suggestion above the need for this helper > should go away anyway. Eh, yes. I initially declared the return type as int and then forgot to change the return statement. But as you say, the whole function will go away entirely. Petr T
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |