Re: [PATCH v2 1/2] s390/cio: Convert ccw_io_region to pointer

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 27 Sep 2018 09:05:20 -0400
Eric Farman <farman@xxxxxxxxxxxxx> wrote:

> 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.  :)

Yes, it makes sense to just keep the flags for now. We can still change
it easily on top.



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux