On 09/27/2018 07:13 AM, Cornelia Huck wrote:
On Fri, 21 Sep 2018 22:40:12 +0200
Eric Farman <farman@xxxxxxxxxxxxx> wrote:
In the event that we want to change the layout of the ccw_io_region in the
future[1], it might be easier to work with it as a pointer within the
vfio_ccw_private struct rather than an embedded struct.
[1] https://patchwork.kernel.org/comment/22228541/
Signed-off-by: Eric Farman <farman@xxxxxxxxxxxxx>
---
drivers/s390/cio/vfio_ccw_drv.c | 12 +++++++++++-
drivers/s390/cio/vfio_ccw_fsm.c | 6 +++---
drivers/s390/cio/vfio_ccw_ops.c | 4 ++--
drivers/s390/cio/vfio_ccw_private.h | 2 +-
4 files changed, 17 insertions(+), 7 deletions(-)
@@ -114,6 +114,14 @@ static int vfio_ccw_sch_probe(struct subchannel *sch)
private = kzalloc(sizeof(*private), GFP_KERNEL | GFP_DMA);
if (!private)
return -ENOMEM;
+
+ private->io_region = kzalloc(sizeof(*private->io_region),
+ GFP_KERNEL | GFP_DMA);
Any particular reason for this to be under 2G? Looking through the
code, I did not spot a place where we feed things in there directly to
an interface that is 2G sensitive.
I'm inclined to just keep it for now (as the structure it was taken
from was also allocated below 2G), but this might be a remainder of the
original implementation that was not using idals (and we might be able
to get rid of GFP_DMA here).
I suspect you are right and it can be removed, since I don't see any
reason it needs to be. But I don't know the history here, so I just
kept the same allocation. I guess it makes sense to look into that
cleanup here. But I did say this was a quick attempt. :)
But maybe I'm just missing coffee :)
Now that is the real tragedy right here!
+ if (!private->io_region) {
+ kfree(private);
+ return -ENOMEM;
+ }
+
private->sch = sch;
dev_set_drvdata(&sch->dev, private);
