On Tue, 19 Nov 2019 10:16:30 -0500 Eric Farman <farman@xxxxxxxxxxxxx> wrote: > On 11/19/19 8:00 AM, Cornelia Huck wrote: > > On Fri, 15 Nov 2019 03:56:13 +0100 > > Eric Farman <farman@xxxxxxxxxxxxx> wrote: > > > >> From: Farhan Ali <alifm@xxxxxxxxxxxxx> > >> > >> The subchannel logical path mask (lpm) would have the most > >> up to date information of channel paths that are logically > >> available for the subchannel. > >> > >> Signed-off-by: Farhan Ali <alifm@xxxxxxxxxxxxx> > >> Signed-off-by: Eric Farman <farman@xxxxxxxxxxxxx> > >> --- > >> > >> Notes: > >> v0->v1: [EF] > >> - None; however I am greatly confused by this one. Thoughts? > > > > I think it's actually wrong. > > > >> > >> drivers/s390/cio/vfio_ccw_cp.c | 4 +--- > >> 1 file changed, 1 insertion(+), 3 deletions(-) > >> > >> diff --git a/drivers/s390/cio/vfio_ccw_cp.c b/drivers/s390/cio/vfio_ccw_cp.c > >> index 3645d1720c4b..d4a86fb9d162 100644 > >> --- a/drivers/s390/cio/vfio_ccw_cp.c > >> +++ b/drivers/s390/cio/vfio_ccw_cp.c > >> @@ -779,9 +779,7 @@ union orb *cp_get_orb(struct channel_program *cp, u32 intparm, u8 lpm) > >> orb->cmd.intparm = intparm; > >> orb->cmd.fmt = 1; > >> orb->cmd.key = PAGE_DEFAULT_KEY >> 4; > >> - > >> - if (orb->cmd.lpm == 0) > >> - orb->cmd.lpm = lpm; > > > > In the end, the old code will use the lpm from the subchannel > > structure, if userspace did not supply anything to be used. > > > >> + orb->cmd.lpm = lpm; > > > > The new code will always discard any lpm userspace has supplied and > > replace it with the lpm from the subchannel structure. This feels > > wrong; what if the I/O is supposed to be restricted to a subset of the > > paths? > > I had the same opinion, but didn't want to flat-out discard it from his > series without a second look. :) :) > > > > > If we want to include the current value of the subchannel lpm in the > > requests, we probably want to AND the masks instead. > > Then we'd be on the hook to return some sort of error if the result is > zero. Is it better to just send it to hw as-is, and let the response > come back naturally? (Which is what we do today.) But if a chpid is logically varied off, it is removed from the lpm, right? Therefore, the caller really should get a 'no path' indication back, shouldn't it? > > > > >> > >> chain = list_first_entry(&cp->ccwchain_list, struct ccwchain, next); > >> cpa = chain->ch_ccw; > > >
