From: Cornelia Huck <cohuck@redhat.com>
To: Halil Pasic <pasic@linux.ibm.com>
Cc: Dong Jia Shi <bjsdjshi@linux.ibm.com>,
linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
linux-s390@vger.kernel.org
Subject: Re: [PATCH 1/1] s390: vfio-ccw: push down unsupported IDA check
Date: Mon, 14 May 2018 17:01:35 +0200 [thread overview]
Message-ID: <20180514170135.2cab4f7d.cohuck@redhat.com> (raw)
In-Reply-To: <7bd8fc8b-0500-9905-a3a0-2ec56d3fc9f2@linux.ibm.com>
On Mon, 14 May 2018 16:44:29 +0200
Halil Pasic <pasic@linux.ibm.com> wrote:
> On 05/14/2018 04:00 PM, Cornelia Huck wrote:
> > On Mon, 14 May 2018 15:37:17 +0200
> > Halil Pasic <pasic@linux.ibm.com> wrote:
> >
> >> On 05/14/2018 01:55 PM, Cornelia Huck wrote:
> >>> On Wed, 9 May 2018 19:36:47 +0200
> >>> Halil Pasic <pasic@linux.ibm.com> wrote:
> >>>
> >>>> + /*
> >>>> + * 2k byte block IDAWs (fmt1 or fmt2) are not yet supported.
> >>>> + * There are however CPs that don't use IDA at all, and can
> >>>> + * benefit from not failing until failure is eminent.
> >>>
> >>> What about:
> >>>
> >>> "As we don't want to fail direct addressing even if the orb specified
> >>> one of the unsupported formats, we defer checking for IDAWs in
> >>> unsupported formats to here."
> >>
> >> Was the second sentence only confusing because of CP? I'm not perfectly
> >> satisfied with your version either:
> >> * 'fail direct addressing even if the orb specified one of the unsupported formats'
> >> I wanted to say: 'hey it does not matter what format for IDA the orb implies
> >> if the channel program does not use any IDA at all'. That could be paraphrased
> >> as channel programs using direct addressing exclusively. But failing the direct
> >> addressing does not fit for me.
> >
> > But that's effectively what happens now, no? We reject the orb out of
> > hand due to unsupported flags that do not have any relevance for the
> > channel program in that case.
>
> Yes, that's what happens now, except that we make the whole channel program fail,
> and not the direct addressing. But the comment should describe what happens
> with the patch applied.
Even more, it should describe _why_ it is done that way (the reason
being "we don't want to fail..."). That's where I've been coming from.
> >>> The patch looks sane, I have only issues with the description/comments.
> >>>
> >>
> >> Thanks for having a look. Please give me short feedback about the one
> >> open point and I will respin with the requested changes.
> >
> > Does anybody else have feedback?
> >
>
> Will wait a day or so. Dong Jia and Jason have already seen the patch, and
> they only complained about the text. Since that spin was mainly for the
> tested-by tags, and I stated that any substantial discussion should happen
> upstream, I ignored those complaints.
>
> So yes I will wait a bit so everybody can chime in.
Sounds good.
prev parent reply other threads:[~2018-05-14 15:01 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-05-09 17:36 [PATCH 1/1] s390: vfio-ccw: push down unsupported IDA check Halil Pasic
2018-05-14 11:55 ` Cornelia Huck
2018-05-14 13:37 ` Halil Pasic
2018-05-14 14:00 ` Cornelia Huck
2018-05-14 14:44 ` Halil Pasic
2018-05-14 15:01 ` Cornelia Huck [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180514170135.2cab4f7d.cohuck@redhat.com \
--to=cohuck@redhat.com \
--cc=bjsdjshi@linux.ibm.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-s390@vger.kernel.org \
--cc=pasic@linux.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).