search for: cio_dma_pool_init

...r > > problem due to init not failing? > > > > Good question! > > I did a quick check. virtio_ccw_init() should be OK, because we don't > register the driver if allocation fails, so the thing is going to end > up dysfunctional as expected. > > If however cio_dma_pool_init() fails, then we end up with the same > problem with airqs, just on the !AIRQ_IV_CACHELINE code path. It can be > fixed analogously: make cio_dma_zalloc() fail all allocation if > cio_dma_pool_init() failed before. Ok, makes sense. > > The rest should be OK. > > > >...

...r > > problem due to init not failing? > > > > Good question! > > I did a quick check. virtio_ccw_init() should be OK, because we don't > register the driver if allocation fails, so the thing is going to end > up dysfunctional as expected. > > If however cio_dma_pool_init() fails, then we end up with the same > problem with airqs, just on the !AIRQ_IV_CACHELINE code path. It can be > fixed analogously: make cio_dma_zalloc() fail all allocation if > cio_dma_pool_init() failed before. Ok, makes sense. > > The rest should be OK. > > > >...

...; ret = register_pm_notifier(&css_power_notifier); > >> - if (ret) { > >> - unregister_reboot_notifier(&css_reboot_notifier); > >> - goto out_unregister; > >> - } > >> + if (ret) > >> + goto out_unregister_rn; > >> + ret = cio_dma_pool_init(); > >> + if (ret) > >> + goto out_unregister_rn; > > > > Don't you also need to unregister the pm notifier on failure here? > > Mmh, that was the original intention. Thanks! I suppose we could also move cio_dma_pool_init() right before the register_r...

..., normally I'd do the memset during alloc not during free - but maybe this makes more sense here with your usecase in mind. > @@ -1063,6 +1163,7 @@ static int __init css_bus_init(void) > unregister_reboot_notifier(&css_reboot_notifier); > goto out_unregister; > } > + cio_dma_pool_init(); This is too late for early devices (ccw console!).

..., normally I'd do the memset during alloc not during free - but maybe this makes more sense here with your usecase in mind. > @@ -1063,6 +1163,7 @@ static int __init css_bus_init(void) > unregister_reboot_notifier(&css_reboot_notifier); > goto out_unregister; > } > + cio_dma_pool_init(); This is too late for early devices (ccw console!).

...:18:10 +0200 (CEST) > Sebastian Ott <sebott at linux.ibm.com> wrote: > > > @@ -1063,6 +1163,7 @@ static int __init css_bus_init(void) > > > unregister_reboot_notifier(&css_reboot_notifier); > > > goto out_unregister; > > > } > > > + cio_dma_pool_init(); > > > > This is too late for early devices (ccw console!). > > You have already raised concern about this last time (thanks). I think, > I've addressed this issue: tje cio_dma_pool is only used by the airq > stuff. I don't think the ccw console needs it. P...

...:18:10 +0200 (CEST) > Sebastian Ott <sebott at linux.ibm.com> wrote: > > > @@ -1063,6 +1163,7 @@ static int __init css_bus_init(void) > > > unregister_reboot_notifier(&css_reboot_notifier); > > > goto out_unregister; > > > } > > > + cio_dma_pool_init(); > > > > This is too late for early devices (ccw console!). > > You have already raised concern about this last time (thanks). I think, > I've addressed this issue: tje cio_dma_pool is only used by the airq > stuff. I don't think the ccw console needs it. P...

...iff --git a/drivers/s390/cio/css.c b/drivers/s390/cio/css.c > > > index 6fc91d534af1..7901c8ed3597 100644 > > > --- a/drivers/s390/cio/css.c > > > +++ b/drivers/s390/cio/css.c > > > @@ -1182,6 +1182,7 @@ static int __init css_bus_init(void) > > > ret = cio_dma_pool_init(); > > > if (ret) > > > goto out_unregister_pmn; > > > + airq_init(); > > > > ...but don't check the return code here. Probably a pathological case, > > but shouldn't you handle that error as well? > > > > Tricky business....

...iff --git a/drivers/s390/cio/css.c b/drivers/s390/cio/css.c > > > index 6fc91d534af1..7901c8ed3597 100644 > > > --- a/drivers/s390/cio/css.c > > > +++ b/drivers/s390/cio/css.c > > > @@ -1182,6 +1182,7 @@ static int __init css_bus_init(void) > > > ret = cio_dma_pool_init(); > > > if (ret) > > > goto out_unregister_pmn; > > > + airq_init(); > > > > ...but don't check the return code here. Probably a pathological case, > > but shouldn't you handle that error as well? > > > > Tricky business....

...ged, 25 insertions(+), 12 deletions(-) Apologies if that already has been answered (and I missed it in my mail pile...), but two things had come to my mind previously: - CHSC... does anything need to be done there? Last time I asked: "Anyway, css_bus_init() uses some chscs early (before cio_dma_pool_init), so we could not use the pools there, even if we wanted to. Do chsc commands either work, or else fail benignly on a protected virt guest?" - PCI indicators... does this interact with any dma configuration on the pci device? (I know pci is not supported yet, and I don't really e...

...ged, 25 insertions(+), 12 deletions(-) Apologies if that already has been answered (and I missed it in my mail pile...), but two things had come to my mind previously: - CHSC... does anything need to be done there? Last time I asked: "Anyway, css_bus_init() uses some chscs early (before cio_dma_pool_init), so we could not use the pools there, even if we wanted to. Do chsc commands either work, or else fail benignly on a protected virt guest?" - PCI indicators... does this interact with any dma configuration on the pci device? (I know pci is not supported yet, and I don't really e...

...cio/css.c b/drivers/s390/cio/css.c > > > > index 6fc91d534af1..7901c8ed3597 100644 > > > > --- a/drivers/s390/cio/css.c > > > > +++ b/drivers/s390/cio/css.c > > > > @@ -1182,6 +1182,7 @@ static int __init css_bus_init(void) > > > > ret = cio_dma_pool_init(); > > > > if (ret) > > > > goto out_unregister_pmn; > > > > + airq_init(); > > > > > > ...but don't check the return code here. Probably a pathological case, > > > but shouldn't you handle that error as well? > >...

On Thu, 23 May 2019, Michael Mueller wrote: > +static void __init cio_dma_pool_init(void) > +{ > + /* No need to free up the resources: compiled in */ > + cio_dma_pool = cio_gp_dma_create(cio_get_dma_css_dev(), 1); This can return NULL. > +/** > + * Allocate dma memory from the css global pool. Intended for memory not > + * specific to any single device within...

...t; > > > > > > Good question! > > > > I did a quick check. virtio_ccw_init() should be OK, because we don't > > register the driver if allocation fails, so the thing is going to end > > up dysfunctional as expected. > > > > If however cio_dma_pool_init() fails, then we end up with the same > > problem with airqs, just on the !AIRQ_IV_CACHELINE code path. It can be > > fixed analogously: make cio_dma_zalloc() fail all allocation if > > cio_dma_pool_init() failed before. > > Ok, makes sense. v5 is out with the fixes. I h...

..._bus_init(void) > if (ret) > goto out_unregister; > ret = register_pm_notifier(&css_power_notifier); > - if (ret) { > - unregister_reboot_notifier(&css_reboot_notifier); > - goto out_unregister; > - } > + if (ret) > + goto out_unregister_rn; > + ret = cio_dma_pool_init(); > + if (ret) > + goto out_unregister_rn; Don't you also need to unregister the pm notifier on failure here? Other than that, I noticed only cosmetic issues; seems reasonable to me. > css_init_done = 1; > > /* Enable default isc for I/O subchannels. */ > isc_regi...

..._bus_init(void) > if (ret) > goto out_unregister; > ret = register_pm_notifier(&css_power_notifier); > - if (ret) { > - unregister_reboot_notifier(&css_reboot_notifier); > - goto out_unregister; > - } > + if (ret) > + goto out_unregister_rn; > + ret = cio_dma_pool_init(); > + if (ret) > + goto out_unregister_rn; Don't you also need to unregister the pm notifier on failure here? Other than that, I noticed only cosmetic issues; seems reasonable to me. > css_init_done = 1; > > /* Enable default isc for I/O subchannels. */ > isc_regi...

...} > -subsys_initcall(airq_init); (...) > diff --git a/drivers/s390/cio/css.c b/drivers/s390/cio/css.c > index 6fc91d534af1..7901c8ed3597 100644 > --- a/drivers/s390/cio/css.c > +++ b/drivers/s390/cio/css.c > @@ -1182,6 +1182,7 @@ static int __init css_bus_init(void) > ret = cio_dma_pool_init(); > if (ret) > goto out_unregister_pmn; > + airq_init(); ...but don't check the return code here. Probably a pathological case, but shouldn't you handle that error as well? > css_init_done = 1; > > /* Enable default isc for I/O subchannels. */

...} > -subsys_initcall(airq_init); (...) > diff --git a/drivers/s390/cio/css.c b/drivers/s390/cio/css.c > index 6fc91d534af1..7901c8ed3597 100644 > --- a/drivers/s390/cio/css.c > +++ b/drivers/s390/cio/css.c > @@ -1182,6 +1182,7 @@ static int __init css_bus_init(void) > ret = cio_dma_pool_init(); > if (ret) > goto out_unregister_pmn; > + airq_init(); ...but don't check the return code here. Probably a pathological case, but shouldn't you handle that error as well? > css_init_done = 1; > > /* Enable default isc for I/O subchannels. */

...struct subchannel *cio_probe_console(void); > diff --git a/drivers/s390/cio/css.c b/drivers/s390/cio/css.c > index e0f19f1e82a0..1b867c941b86 100644 > --- a/drivers/s390/cio/css.c > +++ b/drivers/s390/cio/css.c > @@ -1184,6 +1184,7 @@ static int __init css_bus_init(void) > ret = cio_dma_pool_init(); > if (ret) > goto out_unregister_pmn; > + airq_init(); Ignoring the return code here does not really hurt right now, but we probably want to change that if we want to consider failures in css initialization to be fatal. > css_init_done = 1; > > /* Enable default is...

...if that already has been answered (and I missed it in my mail > > pile...), but two things had come to my mind previously: > > > > - CHSC... does anything need to be done there? Last time I asked: > > "Anyway, css_bus_init() uses some chscs > > early (before cio_dma_pool_init), so we could not use the pools > > there, even if we wanted to. Do chsc commands either work, or else > > fail benignly on a protected virt guest?" > > Protected virt won't support all CHSC. The supported ones won't requre > use of shared memory. So we ar...