* RE: Kernel 5.5.4 build fail for BPF-selftests with latest LLVM
From: Bird, Tim @ 2020-02-20 17:02 UTC (permalink / raw)
To: Jesper Dangaard Brouer, shuah
Cc: Alexei Starovoitov, Daniel Díaz, Andrii Nakryiko,
Andrii Nakryiko, netdev@vger.kernel.org, BPF-dev-list,
Daniel Borkmann, David Miller, LKML, Greg Kroah-Hartman,
Anders Roxell, Toke Høiland-Jørgensen,
open list:KERNEL SELFTEST FRAMEWORK
In-Reply-To: <20200220173740.7a3f9ad7@carbon>
> -----Original Message-----
> From: Jesper Dangaard Brouer
>
> On Wed, 19 Feb 2020 17:47:23 -0700
> shuah <shuah@kernel.org> wrote:
>
> > On 2/19/20 5:27 PM, Alexei Starovoitov wrote:
> > > On Wed, Feb 19, 2020 at 03:59:41PM -0600, Daniel Díaz wrote:
> > >>>
> > >>> When I download a specific kernel release, how can I know what LLVM
> > >>> git-hash or version I need (to use BPF-selftests)?
> > >
> > > as discussed we're going to add documentation-like file that will
> > > list required commits in tools.
> > > This will be enforced for future llvm/pahole commits.
> > >
> > >>> Do you think it is reasonable to require end-users to compile their own
> > >>> bleeding edge version of LLVM, to use BPF-selftests?
> > >
> > > absolutely.
Is it just the BPF-selftests that require the bleeding edge version of LLVM,
or do BPF features themselves need the latest LLVM. If the latter, then this
is quite worrisome, and I fear the BPF developers are getting ahead of themselves.
We don't usually have a kernel dependency on the latest compiler version (some
recent security fixes are an anomaly). In fact deprecating support for older compiler
versions has been quite slow and methodical over the years.
It's quite dangerous to be baking stuff into the kernel that depends on features
from compilers that haven't even made it to release yet.
I'm sorry, but I'm coming into the middle of this thread. Can you please explain
what the features are in the latest LLVM that are required for BPF-selftests?
-- Tim
> > + linux-kselftest@vger.kernel.org
> >
> > End-users in this context are users and not necessarily developers.
>
> I agree. And I worry that we are making it increasingly hard for
> non-developer users.
>
>
> > > If a developer wants to send a patch they must run all selftests and
> > > all of them must pass in their environment.
> > > "but I'm adding a tracing feature and don't care about networking tests
> > > failing"... is not acceptable.
> >
> > This is a reasonable expectation when a developers sends bpf patches.
>
> Sure. I have several versions on LLVM that I've compiled manually.
>
> > >
> > >>> I do hope that some end-users of BPF-selftests will be CI-systems.
> > >>> That also implies that CI-system maintainers need to constantly do
> > >>> "latest built from sources" of LLVM git-tree to keep up. Is that a
> > >>> reasonable requirement when buying a CI-system in the cloud?
> > >
> > > "buying CI-system in the cloud" ?
> > > If I could buy such system I would pay for it out of my own pocket to save
> > > maintainer's and developer's time.
>
> And Daniel Díaz want to provide his help below (to tests it on arch
> that you likely don't even have access to). That sounds like a good
> offer, and you don't even have to pay.
>
> > >
> > >> We [1] are end users of kselftests and many other test suites [2]. We
> > >> run all of our testing on every git-push on linux-stable-rc, mainline,
> > >> and linux-next -- approximately 1 million tests per week. We have a
> > >> dedicated engineering team looking after this CI infrastructure and
> > >> test results, and as such, I can wholeheartedly echo Jesper's
> > >> sentiment here: We would really like to help kernel maintainers and
> > >> developers by automatically testing their code in real hardware, but
> > >> the BPF kselftests are difficult to work with from a CI perspective.
> > >> We have caught and reported [3] many [4] build [5] failures [6] in the
> > >> past for libbpf/Perf, but building is just one of the pieces. We are
> > >> unable to run the entire BPF kselftests because only a part of the
> > >> code builds, so our testing is very limited there.
> > >>
> > >> We hope that this situation can be improved and that our and everyone
> > >> else's automated testing can help you guys too. For this to work out,
> > >> we need some help.
> > >
> >
> > It would be helpful understand what "help" is in this context.
> >
> > > I don't understand what kind of help you need. Just install the
> > > latest tools.
>
> I admire that you want to push *everybody* forward to use the latest
> LLVM, but saying latest is LLVM devel git tree HEAD is too extreme.
> I can support saying latest LLVM release is required.
>
> As soon as your LLVM patches are accepted into llvm-git-tree, you will
> add some BPF selftests that util this. Then CI-systems pull latest
> bpf-next they will start to fail to compile BPF-selftests, and CI
> stops. Now you want to force CI-system maintainer to recompile LLVM
> from git. This will likely take some time. Until that happens
> CI-system doesn't catch stuff. E.g. I really want the ARM tests that
> Linaro can run for us (which isn't run before you apply patches...).
>
>
> > What would be helpful is to write bpf tests such that older tests that
> > worked on older llvm versions continue to work and with some indication
> > on which tests require new bleeding edge tools.
> >
> > > Both the latest llvm and the latest pahole are required.
> >
> > It would be helpful if you can elaborate why latest tools are a
> > requirement.
> >
> > > If by 'help' you mean to tweak selftests to skip tests then it's a nack.
> > > We have human driven CI. Every developer must run selftests/bpf before
> > > emailing the patches. Myself and Daniel run them as well before applying.
> > > These manual runs is the only thing that keeps bpf tree going.
> > > If selftests get to skip tests humans will miss those errors.
> > > When I don't see '0 SKIPPED, 0 FAILED' I go and investigate.
> > > Anything but zero is a path to broken kernels.
> > >
> > > Imagine the tests would get skipped when pahole is too old.
> > > That would mean all of the kernel features from year 2019
> > > would get skipped. Is there a point of running such selftests?
> > > I think the value is not just zero. The value is negative.
> > > Such selftests that run old stuff would give false believe
> > > that they do something meaningful.
> > > "but CI can do build only tests"... If 'helping' such CI means hurting the
> > > key developer/maintainer workflow such CI is on its own.
> > >
> >
> > Skipping tests will be useless. I am with you on that. However,
> > figuring out how to maintain some level of backward compatibility
> > to run at least older tests and warn users to upgrade would be
> > helpful.
>
> What I propose is that a BPF-selftest that use a new LLVM feature,
> should return FAIL (or perhaps SKIP), when it is compiled with say one
> release old LLVM. This will allow new-tests to show up in CI-systems
> reports as FAIL, and give everybody breathing room to upgrade their LLVM
> compiler.
>
> > I suspect currently users are ignoring bpf failures because they
> > are unable to keep up with the requirement to install newer tools
> > to run the tests. This isn't great either.
>
> Yes, my worry is also that we are simply making it too difficult for
> non-developer users to run these tests. And I specifically want to
> attract CI-systems to run these. And especially Linaro, who have
> dedicated engineering team looking after their CI infrastructure, and
> they explicitly in this email confirm my worry.
>
>
> > Users that care are sharing their pain to see if they can get some
> > help or explanation on why new tools are required every so often.
> > I don't think everybody understands why. :)
>
> --
> Best regards,
> Jesper Dangaard Brouer
> MSc.CS, Principal Kernel Engineer at Red Hat
> LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH] Set default CONNECTIVITY_CHECK_URIS to https://www.yoctoproject.org/
From: Jean-Marie LEMETAYER @ 2020-02-20 16:52 UTC (permalink / raw)
To: Richard Purdie; +Cc: OE-core
In-Reply-To: <18c55700df9ee84ea44fbffc93ba4e9f3c0e1663.camel@linuxfoundation.org>
On Feb 20, 2020, at 4:27 PM, Richard Purdie richard.purdie@linuxfoundation.org wrote:
> On Thu, 2020-02-20 at 10:03 -0500, Nataliya Korovkina wrote:
>> Signed-off-by: Nataliya Korovkina <malus.brandywine@gmail.com>
>> ---
>> meta/conf/distro/include/default-distrovars.inc | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/meta/conf/distro/include/default-distrovars.inc
>> b/meta/conf/distro/include/default-distrovars.inc
>> index 433d4b6651..6c9155b2ae 100644
>> --- a/meta/conf/distro/include/default-distrovars.inc
>> +++ b/meta/conf/distro/include/default-distrovars.inc
>> @@ -48,4 +48,4 @@ KERNEL_IMAGETYPES ??= "${KERNEL_IMAGETYPE}"
>> # fetch from the network (and warn you if not). To disable the test
>> set
>> # the variable to be empty.
>> # Git example url: git://git.yoctoproject.org/yocto-firewall-
>> test;protocol=git;rev=master
>> -CONNECTIVITY_CHECK_URIS ?= "https://www.example.com/"
>> +CONNECTIVITY_CHECK_URIS ?= "https://www.yoctoproject.org/"
>
> Why would that be better?
>
> There have been several requests to change this however I'm not sure we
> can find one perfect url we can use :(
Hi Nataliya,
I am one of those who made a request:
http://lists.openembedded.org/pipermail/openembedded-core/2020-January/291926.html
Since I have created 3 pages on GitHub pages, GitLab pages and Bitbucket pages:
- https://connectivitycheck.github.io
- https://connectivitycheck.github.io
- https://connectivitycheck.bitbucket.io
I think we could use them in a mirrored way like Richard have suggested (in my thread).
I cannot do it right now, but this is definitely on my roadmap
because I still see the issue regularly.
Best Regards,
Jean-Marie
^ permalink raw reply
* RE: [Xen-devel] [RFC PATCH v3 06/12] xen-blkfront: add callbacks for PM suspend and hibernation
From: Durrant, Paul @ 2020-02-20 17:01 UTC (permalink / raw)
To: Roger Pau Monné
Cc: Agarwal, Anchal, Valentin, Eduardo, len.brown@intel.com,
peterz@infradead.org, benh@kernel.crashing.org, x86@kernel.org,
linux-mm@kvack.org, pavel@ucw.cz, hpa@zytor.com,
tglx@linutronix.de, sstabellini@kernel.org, fllinden@amaozn.com,
Kamata, Munehisa, mingo@redhat.com,
xen-devel@lists.xenproject.org, Singh, Balbir, axboe@kernel.dk,
konrad.wilk@oracle.com, bp@alien8.de, boris.ostrovsky@oracle.com,
jgross@suse.com, netdev@vger.kernel.org, linux-pm@vger.kernel.org,
rjw@rjwysocki.net, linux-kernel@vger.kernel.org,
vkuznets@redhat.com, davem@davemloft.net, Woodhouse, David
In-Reply-To: <20200220164839.GR4679@Air-de-Roger>
> -----Original Message-----
> From: Roger Pau Monné <roger.pau@citrix.com>
> Sent: 20 February 2020 16:49
> To: Durrant, Paul <pdurrant@amazon.co.uk>
> Cc: Agarwal, Anchal <anchalag@amazon.com>; Valentin, Eduardo
> <eduval@amazon.com>; len.brown@intel.com; peterz@infradead.org;
> benh@kernel.crashing.org; x86@kernel.org; linux-mm@kvack.org;
> pavel@ucw.cz; hpa@zytor.com; tglx@linutronix.de; sstabellini@kernel.org;
> fllinden@amaozn.com; Kamata, Munehisa <kamatam@amazon.com>;
> mingo@redhat.com; xen-devel@lists.xenproject.org; Singh, Balbir
> <sblbir@amazon.com>; axboe@kernel.dk; konrad.wilk@oracle.com;
> bp@alien8.de; boris.ostrovsky@oracle.com; jgross@suse.com;
> netdev@vger.kernel.org; linux-pm@vger.kernel.org; rjw@rjwysocki.net;
> linux-kernel@vger.kernel.org; vkuznets@redhat.com; davem@davemloft.net;
> Woodhouse, David <dwmw@amazon.co.uk>
> Subject: Re: [Xen-devel] [RFC PATCH v3 06/12] xen-blkfront: add callbacks
> for PM suspend and hibernation
>
> On Thu, Feb 20, 2020 at 04:23:13PM +0000, Durrant, Paul wrote:
> > > -----Original Message-----
> > > From: Roger Pau Monné <roger.pau@citrix.com>
> > > Sent: 20 February 2020 15:45
> > > To: Durrant, Paul <pdurrant@amazon.co.uk>
> > > Cc: Agarwal, Anchal <anchalag@amazon.com>; Valentin, Eduardo
> > > <eduval@amazon.com>; len.brown@intel.com; peterz@infradead.org;
> > > benh@kernel.crashing.org; x86@kernel.org; linux-mm@kvack.org;
> > > pavel@ucw.cz; hpa@zytor.com; tglx@linutronix.de;
> sstabellini@kernel.org;
> > > fllinden@amaozn.com; Kamata, Munehisa <kamatam@amazon.com>;
> > > mingo@redhat.com; xen-devel@lists.xenproject.org; Singh, Balbir
> > > <sblbir@amazon.com>; axboe@kernel.dk; konrad.wilk@oracle.com;
> > > bp@alien8.de; boris.ostrovsky@oracle.com; jgross@suse.com;
> > > netdev@vger.kernel.org; linux-pm@vger.kernel.org; rjw@rjwysocki.net;
> > > linux-kernel@vger.kernel.org; vkuznets@redhat.com;
> davem@davemloft.net;
> > > Woodhouse, David <dwmw@amazon.co.uk>
> > > Subject: Re: [Xen-devel] [RFC PATCH v3 06/12] xen-blkfront: add
> callbacks
> > > for PM suspend and hibernation
> > >
> > > On Thu, Feb 20, 2020 at 08:54:36AM +0000, Durrant, Paul wrote:
> > > > > -----Original Message-----
> > > > > From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf
> Of
> > > > > Roger Pau Monné
> > > > > Sent: 20 February 2020 08:39
> > > > > To: Agarwal, Anchal <anchalag@amazon.com>
> > > > > Cc: Valentin, Eduardo <eduval@amazon.com>; len.brown@intel.com;
> > > > > peterz@infradead.org; benh@kernel.crashing.org; x86@kernel.org;
> linux-
> > > > > mm@kvack.org; pavel@ucw.cz; hpa@zytor.com; tglx@linutronix.de;
> > > > > sstabellini@kernel.org; fllinden@amaozn.com; Kamata, Munehisa
> > > > > <kamatam@amazon.com>; mingo@redhat.com; xen-
> > > devel@lists.xenproject.org;
> > > > > Singh, Balbir <sblbir@amazon.com>; axboe@kernel.dk;
> > > > > konrad.wilk@oracle.com; bp@alien8.de; boris.ostrovsky@oracle.com;
> > > > > jgross@suse.com; netdev@vger.kernel.org; linux-pm@vger.kernel.org;
> > > > > rjw@rjwysocki.net; linux-kernel@vger.kernel.org;
> vkuznets@redhat.com;
> > > > > davem@davemloft.net; Woodhouse, David <dwmw@amazon.co.uk>
> > > > > Subject: Re: [Xen-devel] [RFC PATCH v3 06/12] xen-blkfront: add
> > > callbacks
> > > > > for PM suspend and hibernation
> > > > >
> > > > > Thanks for this work, please see below.
> > > > >
> > > > > On Wed, Feb 19, 2020 at 06:04:24PM +0000, Anchal Agarwal wrote:
> > > > > > On Tue, Feb 18, 2020 at 10:16:11AM +0100, Roger Pau Monné wrote:
> > > > > > > On Mon, Feb 17, 2020 at 11:05:53PM +0000, Anchal Agarwal
> wrote:
> > > > > > > > On Mon, Feb 17, 2020 at 11:05:09AM +0100, Roger Pau Monné
> wrote:
> > > > > > > > > On Fri, Feb 14, 2020 at 11:25:34PM +0000, Anchal Agarwal
> > > wrote:
> > > > > > > > Quiescing the queue seemed a better option here as we want
> to
> > > make
> > > > > sure ongoing
> > > > > > > > requests dispatches are totally drained.
> > > > > > > > I should accept that some of these notion is borrowed from
> how
> > > nvme
> > > > > freeze/unfreeze
> > > > > > > > is done although its not apple to apple comparison.
> > > > > > >
> > > > > > > That's fine, but I would still like to requests that you use
> the
> > > same
> > > > > > > logic (as much as possible) for both the Xen and the PM
> initiated
> > > > > > > suspension.
> > > > > > >
> > > > > > > So you either apply this freeze/unfreeze to the Xen suspension
> > > (and
> > > > > > > drop the re-issuing of requests on resume) or adapt the same
> > > approach
> > > > > > > as the Xen initiated suspension. Keeping two completely
> different
> > > > > > > approaches to suspension / resume on blkfront is not suitable
> long
> > > > > > > term.
> > > > > > >
> > > > > > I agree with you on overhaul of xen suspend/resume wrt blkfront
> is a
> > > > > good
> > > > > > idea however, IMO that is a work for future and this patch
> series
> > > should
> > > > > > not be blocked for it. What do you think?
> > > > >
> > > > > It's not so much that I think an overhaul of suspend/resume in
> > > > > blkfront is needed, it's just that I don't want to have two
> completely
> > > > > different suspend/resume paths inside blkfront.
> > > > >
> > > > > So from my PoV I think the right solution is to either use the
> same
> > > > > code (as much as possible) as it's currently used by Xen initiated
> > > > > suspend/resume, or to also switch Xen initiated suspension to use
> the
> > > > > newly introduced code.
> > > > >
> > > > > Having two different approaches to suspend/resume in the same
> driver
> > > > > is a recipe for disaster IMO: it adds complexity by forcing
> developers
> > > > > to take into account two different suspend/resume approaches when
> > > > > there's no need for it.
> > > >
> > > > I disagree. S3 or S4 suspend/resume (or perhaps we should call them
> > > power state transitions to avoid confusion) are quite different from
> Xen
> > > suspend/resume.
> > > > Power state transitions ought to be, and indeed are, visible to the
> > > software running inside the guest. Applications, as well as drivers,
> can
> > > receive notification and take whatever action they deem appropriate.
> > > > Xen suspend/resume OTOH is used when a guest is migrated and the
> code
> > > should go to all lengths possible to make any software running inside
> the
> > > guest (other than Xen specific enlightened code, such as PV drivers)
> > > completely unaware that anything has actually happened.
> > >
> > > So from what you say above PM state transitions are notified to all
> > > drivers, and Xen suspend/resume is only notified to PV drivers, and
> > > here we are speaking about blkfront which is a PV driver, and should
> > > get notified in both cases. So I'm unsure why the same (or at least
> > > very similar) approach can't be used in both cases.
> > >
> > > The suspend/resume approach proposed by this patch is completely
> > > different than the one used by a xenbus initiated suspend/resume, and
> > > I don't see a technical reason that warrants this difference.
> > >
> >
> > Within an individual PV driver it may well be ok to use common
> mechanisms for connecting to the backend but issues will arise if any
> subsequent action is visible to the guest. E.g. a network frontend needs
> to issue gratuitous ARPs without anything else in the network stack (or
> monitoring the network stack) knowing that it has happened.
> >
> > > I'm not saying that the approach used here is wrong, it's just that I
> > > don't see the point in having two different ways to do suspend/resume
> > > in the same driver, unless there's a technical reason for it, which I
> > > don't think has been provided.
> >
> > The technical justification is that the driver needs to know what kind
> of suspend or resume it is doing, so that it doesn't do the wrong thing.
> There may also be differences in the state of the system e.g. in Windows,
> at least some of the resume-from-xen-suspend code runs with interrupts
> disabled (which is necessary to make sure enough state is restored before
> things become visible to other kernel code).
> >
> > >
> > > I would be fine with switching xenbus initiated suspend/resume to also
> > > use the approach proposed here: freeze the queues and drain the shared
> > > rings before suspending.
> > >
> >
> > I think abstracting away at the xenbus level to some degree is probably
> feasible, but some sort of flag should be passed to the individual drivers
> so they know what circumstances they are operating under.
> >
> > > > So, whilst it may be possible to use common routines to, for
> example,
> > > re-establish PV frontend/backend communication, PV frontend code
> should be
> > > acutely aware of the circumstances they are operating in. I can cite
> > > example code in the Windows PV driver, which have supported guest
> S3/S4
> > > power state transitions since day 1.
> > >
> > > Hm, please bear with me, as I'm not sure I fully understand. Why isn't
> > > the current suspend/resume logic suitable for PM transitions?
> > >
> >
> > I don’t know the details for Linux but it may well be to do with
> assumptions made about the system e.g. the ability to block waiting for
> something to happen on another CPU (which may have already been quiesced
> in a PM context).
> >
> > > As said above, I'm happy to switch xenbus initiated suspend/resume to
> > > use the logic in this patch, but unless there's a technical reason for
> > > it I don't see why blkfront should have two completely different
> > > approaches to suspend/resume depending on whether it's a PM or a
> > > xenbus state change.
> > >
> >
> > Hopefully what I said above illustrates why it may not be 100% common.
>
> Yes, that's fine. I don't expect it to be 100% common (as I guess
> that the hooks will have different prototypes), but I expect
> that routines can be shared, and that the approach taken can be the
> same.
>
> For example one necessary difference will be that xenbus initiated
> suspend won't close the PV connection, in case suspension fails. On PM
> suspend you seem to always close the connection beforehand, so you
> will always have to re-negotiate on resume even if suspension failed.
>
> What I'm mostly worried about is the different approach to ring
> draining. Ie: either xenbus is changed to freeze the queues and drain
> the shared rings, or PM uses the already existing logic of not
> flushing the rings an re-issuing in-flight requests on resume.
>
Yes, that's needs consideration. I don’t think the same semantic can be suitable for both. E.g. in a xen-suspend we need to freeze with as little processing as possible to avoid dirtying RAM late in the migration cycle, and we know that in-flight data can wait. But in a transition to S4 we need to make sure that at least all the in-flight blkif requests get completed, since they probably contain bits of the guest's memory image and that's not going to get saved any other way.
Paul
^ permalink raw reply
* Re: [PATCH 00/12] drm: Put drm_display_mode on diet
From: Emil Velikov @ 2020-02-20 17:01 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Intel Graphics Development, ML dri-devel
In-Reply-To: <20200220142759.GA13686@intel.com>
On Thu, 20 Feb 2020 at 14:28, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
>
> On Thu, Feb 20, 2020 at 01:21:03PM +0000, Emil Velikov wrote:
> > On Wed, 19 Feb 2020 at 20:35, Ville Syrjala
> > <ville.syrjala@linux.intel.com> wrote:
> > >
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > struct drm_display_mode is extremely fat. Put it on diet.
> > >
> > > Some stats for the whole series:
> > >
> > > 64bit sizeof(struct drm_display_mode):
> > > 200 -> 136 bytes (-32%)
> > >
> > > 64bit bloat-o-meter -c drm.ko:
> > > add/remove: 1/0 grow/shrink: 29/47 up/down: 893/-1544 (-651)
> > > Function old new delta
> > > ...
> > > Total: Before=189430, After=188779, chg -0.34%
> > > add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0 (0)
> > > Data old new delta
> > > Total: Before=11667, After=11667, chg +0.00%
> > > add/remove: 0/0 grow/shrink: 0/5 up/down: 0/-16896 (-16896)
> > > RO Data old new delta
> > > edid_4k_modes 1000 680 -320
> > > edid_est_modes 3400 2312 -1088
> > > edid_cea_modes_193 5400 3672 -1728
> > > drm_dmt_modes 17600 11968 -5632
> > > edid_cea_modes_1 25400 17272 -8128
> > > Total: Before=71239, After=54343, chg -23.72%
> > >
> > >
> > > 64bit bloat-o-meter drm.ko:
> > > add/remove: 1/0 grow/shrink: 29/52 up/down: 893/-18440 (-17547)
> > > ...
> > > Total: Before=272336, After=254789, chg -6.44%
> > >
> > >
> > > 32bit sizeof(struct drm_display_mode):
> > > 184 -> 120 bytes (-34%)
> > >
> > > 32bit bloat-o-meter -c drm.ko
> > > add/remove: 1/0 grow/shrink: 19/21 up/down: 743/-1368 (-625)
> > > Function old new delta
> > > ...
> > > Total: Before=172359, After=171734, chg -0.36%
> > > add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0 (0)
> > > Data old new delta
> > > Total: Before=4227, After=4227, chg +0.00%
> > > add/remove: 0/0 grow/shrink: 0/5 up/down: 0/-16896 (-16896)
> > > RO Data old new delta
> > > edid_4k_modes 920 600 -320
> > > edid_est_modes 3128 2040 -1088
> > > edid_cea_modes_193 4968 3240 -1728
> > > drm_dmt_modes 16192 10560 -5632
> > > edid_cea_modes_1 23368 15240 -8128
> > > Total: Before=59230, After=42334, chg -28.53%
> > >
> > > 32bit bloat-o-meter drm.ko:
> > > add/remove: 1/0 grow/shrink: 19/26 up/down: 743/-18264 (-17521)
> > > ...
> > > Total: Before=235816, After=218295, chg -7.43%
> > >
> > >
> > > Some ideas for further reduction:
> > > - Convert mode->name to a pointer (saves 24/28 bytes in the
> > > struct but would often require a heap alloc for the name (though
> > > typical mode name is <10 bytes so still overall win perhaps)
> > > - Get rid of mode->name entirely? I guess setcrtc & co. is the only
> > > place where we have to preserve the user provided name, elsewhere
> > > could pehaps just generate on demand? Not sure how tricky this
> > > would get.
> >
> > The series does some great work, with future work reaching the cache
> > line for 64bit.
> > Doing much more than that might be an overkill IMHO.
> >
> > In particular, if we change DRM_DISPLAY_MODE_LEN to 24 we get there,
> > avoiding the heap alloc/calc on demand fun.
> > While also ensuring the name is sufficiently large for the next decade or so.
>
> Unfortunately it's part of the uabi. So can't change it without some
> risk of userspace breakage.
>
Right the define is in the uABI. More importantly userspace can
provide a drm_mode_modeinfo blob, with a name[32], which we cannot
store in the kernel drm_display_mode::name[24].
One might get away with returning -EINVAL if the actual name given by
the user is > 24.
Since you've found a better way, there's on point in risking it.
-Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [Intel-gfx] [PATCH 00/12] drm: Put drm_display_mode on diet
From: Emil Velikov @ 2020-02-20 17:01 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Intel Graphics Development, ML dri-devel
In-Reply-To: <20200220142759.GA13686@intel.com>
On Thu, 20 Feb 2020 at 14:28, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
>
> On Thu, Feb 20, 2020 at 01:21:03PM +0000, Emil Velikov wrote:
> > On Wed, 19 Feb 2020 at 20:35, Ville Syrjala
> > <ville.syrjala@linux.intel.com> wrote:
> > >
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > struct drm_display_mode is extremely fat. Put it on diet.
> > >
> > > Some stats for the whole series:
> > >
> > > 64bit sizeof(struct drm_display_mode):
> > > 200 -> 136 bytes (-32%)
> > >
> > > 64bit bloat-o-meter -c drm.ko:
> > > add/remove: 1/0 grow/shrink: 29/47 up/down: 893/-1544 (-651)
> > > Function old new delta
> > > ...
> > > Total: Before=189430, After=188779, chg -0.34%
> > > add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0 (0)
> > > Data old new delta
> > > Total: Before=11667, After=11667, chg +0.00%
> > > add/remove: 0/0 grow/shrink: 0/5 up/down: 0/-16896 (-16896)
> > > RO Data old new delta
> > > edid_4k_modes 1000 680 -320
> > > edid_est_modes 3400 2312 -1088
> > > edid_cea_modes_193 5400 3672 -1728
> > > drm_dmt_modes 17600 11968 -5632
> > > edid_cea_modes_1 25400 17272 -8128
> > > Total: Before=71239, After=54343, chg -23.72%
> > >
> > >
> > > 64bit bloat-o-meter drm.ko:
> > > add/remove: 1/0 grow/shrink: 29/52 up/down: 893/-18440 (-17547)
> > > ...
> > > Total: Before=272336, After=254789, chg -6.44%
> > >
> > >
> > > 32bit sizeof(struct drm_display_mode):
> > > 184 -> 120 bytes (-34%)
> > >
> > > 32bit bloat-o-meter -c drm.ko
> > > add/remove: 1/0 grow/shrink: 19/21 up/down: 743/-1368 (-625)
> > > Function old new delta
> > > ...
> > > Total: Before=172359, After=171734, chg -0.36%
> > > add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0 (0)
> > > Data old new delta
> > > Total: Before=4227, After=4227, chg +0.00%
> > > add/remove: 0/0 grow/shrink: 0/5 up/down: 0/-16896 (-16896)
> > > RO Data old new delta
> > > edid_4k_modes 920 600 -320
> > > edid_est_modes 3128 2040 -1088
> > > edid_cea_modes_193 4968 3240 -1728
> > > drm_dmt_modes 16192 10560 -5632
> > > edid_cea_modes_1 23368 15240 -8128
> > > Total: Before=59230, After=42334, chg -28.53%
> > >
> > > 32bit bloat-o-meter drm.ko:
> > > add/remove: 1/0 grow/shrink: 19/26 up/down: 743/-18264 (-17521)
> > > ...
> > > Total: Before=235816, After=218295, chg -7.43%
> > >
> > >
> > > Some ideas for further reduction:
> > > - Convert mode->name to a pointer (saves 24/28 bytes in the
> > > struct but would often require a heap alloc for the name (though
> > > typical mode name is <10 bytes so still overall win perhaps)
> > > - Get rid of mode->name entirely? I guess setcrtc & co. is the only
> > > place where we have to preserve the user provided name, elsewhere
> > > could pehaps just generate on demand? Not sure how tricky this
> > > would get.
> >
> > The series does some great work, with future work reaching the cache
> > line for 64bit.
> > Doing much more than that might be an overkill IMHO.
> >
> > In particular, if we change DRM_DISPLAY_MODE_LEN to 24 we get there,
> > avoiding the heap alloc/calc on demand fun.
> > While also ensuring the name is sufficiently large for the next decade or so.
>
> Unfortunately it's part of the uabi. So can't change it without some
> risk of userspace breakage.
>
Right the define is in the uABI. More importantly userspace can
provide a drm_mode_modeinfo blob, with a name[32], which we cannot
store in the kernel drm_display_mode::name[24].
One might get away with returning -EINVAL if the actual name given by
the user is > 24.
Since you've found a better way, there's on point in risking it.
-Emil
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply
* provide in-place uncached remapping for dma-direct (resend)
From: Christoph Hellwig @ 2020-02-20 17:01 UTC (permalink / raw)
To: Jonas Bonn, Stefan Kristiansson, Stafford Horne
Cc: Mark Rutland, linux-arch, Robin Murphy, linux-kernel, iommu,
openrisc, Will Deacon, linux-arm-kernel, Marek Szyprowski
Hi all,
this series provides support for remapping places uncached in-place in
the generic dma-direct code, and moves openrisc over from its own
in-place remapping scheme. The arm64 folks also had interest in such
a scheme to avoid problems with speculating into cache aliases.
Also all architectures that always use small page mappings for the
kernel and have non-coherent DMA should look into enabling this
scheme, as it is much more efficient than the vmap remapping.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 1/2] dma-mapping: support setting memory uncached in place
From: Christoph Hellwig @ 2020-02-20 17:01 UTC (permalink / raw)
To: Jonas Bonn, Stefan Kristiansson, Stafford Horne
Cc: Mark Rutland, linux-arch, Robin Murphy, linux-kernel, iommu,
openrisc, Will Deacon, linux-arm-kernel, Marek Szyprowski
In-Reply-To: <20200220170139.387354-1-hch@lst.de>
We currently only support remapping memory as uncached through vmap
or a magic uncached segment provided by some architectures. But there
is a simpler and much better way available on some architectures where
we can just remap the memory in place. The advantages are:
1) no aliasing is possible, which prevents speculating into the
cached alias
2) there is no need to allocate new ptes and thus no need for a special
pre-allocated pool of memory that can be used with GFP_ATOMIC DMA
allocations
The downside is that architectures must provide a way to set arbitrary
pages uncached in the kernel mapping, which might not be possible on
architecture that have a special implicit kernel mapping, and requires
splitting of huge page kernel mappings where they exist.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
include/linux/dma-noncoherent.h | 3 +++
kernel/dma/Kconfig | 8 ++++++++
kernel/dma/direct.c | 28 ++++++++++++++++++----------
3 files changed, 29 insertions(+), 10 deletions(-)
diff --git a/include/linux/dma-noncoherent.h b/include/linux/dma-noncoherent.h
index ca9b5770caee..0820ec58f119 100644
--- a/include/linux/dma-noncoherent.h
+++ b/include/linux/dma-noncoherent.h
@@ -111,4 +111,7 @@ static inline void arch_dma_prep_coherent(struct page *page, size_t size)
void *uncached_kernel_address(void *addr);
void *cached_kernel_address(void *addr);
+int arch_dma_set_uncached(void *cpu_addr, size_t size);
+void arch_dma_clear_uncached(void *cpu_addr, size_t size);
+
#endif /* _LINUX_DMA_NONCOHERENT_H */
diff --git a/kernel/dma/Kconfig b/kernel/dma/Kconfig
index 4c103a24e380..7bc0b77f1243 100644
--- a/kernel/dma/Kconfig
+++ b/kernel/dma/Kconfig
@@ -83,6 +83,14 @@ config DMA_DIRECT_REMAP
bool
select DMA_REMAP
+#
+# Should be selected if the architecture can remap memory from the page
+# allocator and CMA as uncached and provides the arch_dma_set_uncached and
+# arch_dma_clear_uncached helpers
+#
+config ARCH_HAS_DMA_SET_UNCACHED
+ bool
+
config DMA_CMA
bool "DMA Contiguous Memory Allocator"
depends on HAVE_DMA_CONTIGUOUS && CMA
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 6af7ae83c4ad..73fe65a4cbc0 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -169,11 +169,8 @@ void *dma_direct_alloc_pages(struct device *dev, size_t size,
ret = dma_common_contiguous_remap(page, PAGE_ALIGN(size),
dma_pgprot(dev, PAGE_KERNEL, attrs),
__builtin_return_address(0));
- if (!ret) {
- dma_free_contiguous(dev, page, size);
- return ret;
- }
-
+ if (!ret)
+ goto out_free_pages;
memset(ret, 0, size);
goto done;
}
@@ -186,8 +183,7 @@ void *dma_direct_alloc_pages(struct device *dev, size_t size,
* so log an error and fail.
*/
dev_info(dev, "Rejecting highmem page from CMA.\n");
- dma_free_contiguous(dev, page, size);
- return NULL;
+ goto out_free_pages;
}
ret = page_address(page);
@@ -196,10 +192,15 @@ void *dma_direct_alloc_pages(struct device *dev, size_t size,
memset(ret, 0, size);
- if (IS_ENABLED(CONFIG_ARCH_HAS_UNCACHED_SEGMENT) &&
- dma_alloc_need_uncached(dev, attrs)) {
+ if (dma_alloc_need_uncached(dev, attrs)) {
arch_dma_prep_coherent(page, size);
- ret = uncached_kernel_address(ret);
+
+ if (IS_ENABLED(CONFIG_ARCH_HAS_DMA_SET_UNCACHED)) {
+ if (!arch_dma_set_uncached(ret, size))
+ goto out_free_pages;
+ } else if (IS_ENABLED(CONFIG_ARCH_HAS_UNCACHED_SEGMENT)) {
+ ret = uncached_kernel_address(ret);
+ }
}
done:
if (force_dma_unencrypted(dev))
@@ -207,6 +208,9 @@ void *dma_direct_alloc_pages(struct device *dev, size_t size,
else
*dma_handle = phys_to_dma(dev, page_to_phys(page));
return ret;
+out_free_pages:
+ dma_free_contiguous(dev, page, size);
+ return NULL;
}
void dma_direct_free_pages(struct device *dev, size_t size, void *cpu_addr,
@@ -230,6 +234,8 @@ void dma_direct_free_pages(struct device *dev, size_t size, void *cpu_addr,
if (IS_ENABLED(CONFIG_DMA_REMAP) && is_vmalloc_addr(cpu_addr))
vunmap(cpu_addr);
+ else if (IS_ENABLED(CONFIG_ARCH_HAS_DMA_SET_UNCACHED))
+ arch_dma_clear_uncached(cpu_addr, size);
dma_free_contiguous(dev, dma_direct_to_page(dev, dma_addr), size);
}
@@ -238,6 +244,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
dma_addr_t *dma_handle, gfp_t gfp, unsigned long attrs)
{
if (!IS_ENABLED(CONFIG_ARCH_HAS_UNCACHED_SEGMENT) &&
+ !IS_ENABLED(CONFIG_ARCH_HAS_DMA_SET_UNCACHED) &&
!IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) &&
dma_alloc_need_uncached(dev, attrs))
return arch_dma_alloc(dev, size, dma_handle, gfp, attrs);
@@ -248,6 +255,7 @@ void dma_direct_free(struct device *dev, size_t size,
void *cpu_addr, dma_addr_t dma_addr, unsigned long attrs)
{
if (!IS_ENABLED(CONFIG_ARCH_HAS_UNCACHED_SEGMENT) &&
+ !IS_ENABLED(CONFIG_ARCH_HAS_DMA_SET_UNCACHED) &&
!IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) &&
dma_alloc_need_uncached(dev, attrs))
arch_dma_free(dev, size, cpu_addr, dma_addr, attrs);
--
2.24.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 1/2] dma-mapping: support setting memory uncached in place
From: Christoph Hellwig @ 2020-02-20 17:01 UTC (permalink / raw)
To: Jonas Bonn, Stefan Kristiansson, Stafford Horne
Cc: Mark Rutland, linux-arch, Robin Murphy, linux-kernel, iommu,
openrisc, Will Deacon, linux-arm-kernel
In-Reply-To: <20200220170139.387354-1-hch@lst.de>
We currently only support remapping memory as uncached through vmap
or a magic uncached segment provided by some architectures. But there
is a simpler and much better way available on some architectures where
we can just remap the memory in place. The advantages are:
1) no aliasing is possible, which prevents speculating into the
cached alias
2) there is no need to allocate new ptes and thus no need for a special
pre-allocated pool of memory that can be used with GFP_ATOMIC DMA
allocations
The downside is that architectures must provide a way to set arbitrary
pages uncached in the kernel mapping, which might not be possible on
architecture that have a special implicit kernel mapping, and requires
splitting of huge page kernel mappings where they exist.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
include/linux/dma-noncoherent.h | 3 +++
kernel/dma/Kconfig | 8 ++++++++
kernel/dma/direct.c | 28 ++++++++++++++++++----------
3 files changed, 29 insertions(+), 10 deletions(-)
diff --git a/include/linux/dma-noncoherent.h b/include/linux/dma-noncoherent.h
index ca9b5770caee..0820ec58f119 100644
--- a/include/linux/dma-noncoherent.h
+++ b/include/linux/dma-noncoherent.h
@@ -111,4 +111,7 @@ static inline void arch_dma_prep_coherent(struct page *page, size_t size)
void *uncached_kernel_address(void *addr);
void *cached_kernel_address(void *addr);
+int arch_dma_set_uncached(void *cpu_addr, size_t size);
+void arch_dma_clear_uncached(void *cpu_addr, size_t size);
+
#endif /* _LINUX_DMA_NONCOHERENT_H */
diff --git a/kernel/dma/Kconfig b/kernel/dma/Kconfig
index 4c103a24e380..7bc0b77f1243 100644
--- a/kernel/dma/Kconfig
+++ b/kernel/dma/Kconfig
@@ -83,6 +83,14 @@ config DMA_DIRECT_REMAP
bool
select DMA_REMAP
+#
+# Should be selected if the architecture can remap memory from the page
+# allocator and CMA as uncached and provides the arch_dma_set_uncached and
+# arch_dma_clear_uncached helpers
+#
+config ARCH_HAS_DMA_SET_UNCACHED
+ bool
+
config DMA_CMA
bool "DMA Contiguous Memory Allocator"
depends on HAVE_DMA_CONTIGUOUS && CMA
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 6af7ae83c4ad..73fe65a4cbc0 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -169,11 +169,8 @@ void *dma_direct_alloc_pages(struct device *dev, size_t size,
ret = dma_common_contiguous_remap(page, PAGE_ALIGN(size),
dma_pgprot(dev, PAGE_KERNEL, attrs),
__builtin_return_address(0));
- if (!ret) {
- dma_free_contiguous(dev, page, size);
- return ret;
- }
-
+ if (!ret)
+ goto out_free_pages;
memset(ret, 0, size);
goto done;
}
@@ -186,8 +183,7 @@ void *dma_direct_alloc_pages(struct device *dev, size_t size,
* so log an error and fail.
*/
dev_info(dev, "Rejecting highmem page from CMA.\n");
- dma_free_contiguous(dev, page, size);
- return NULL;
+ goto out_free_pages;
}
ret = page_address(page);
@@ -196,10 +192,15 @@ void *dma_direct_alloc_pages(struct device *dev, size_t size,
memset(ret, 0, size);
- if (IS_ENABLED(CONFIG_ARCH_HAS_UNCACHED_SEGMENT) &&
- dma_alloc_need_uncached(dev, attrs)) {
+ if (dma_alloc_need_uncached(dev, attrs)) {
arch_dma_prep_coherent(page, size);
- ret = uncached_kernel_address(ret);
+
+ if (IS_ENABLED(CONFIG_ARCH_HAS_DMA_SET_UNCACHED)) {
+ if (!arch_dma_set_uncached(ret, size))
+ goto out_free_pages;
+ } else if (IS_ENABLED(CONFIG_ARCH_HAS_UNCACHED_SEGMENT)) {
+ ret = uncached_kernel_address(ret);
+ }
}
done:
if (force_dma_unencrypted(dev))
@@ -207,6 +208,9 @@ void *dma_direct_alloc_pages(struct device *dev, size_t size,
else
*dma_handle = phys_to_dma(dev, page_to_phys(page));
return ret;
+out_free_pages:
+ dma_free_contiguous(dev, page, size);
+ return NULL;
}
void dma_direct_free_pages(struct device *dev, size_t size, void *cpu_addr,
@@ -230,6 +234,8 @@ void dma_direct_free_pages(struct device *dev, size_t size, void *cpu_addr,
if (IS_ENABLED(CONFIG_DMA_REMAP) && is_vmalloc_addr(cpu_addr))
vunmap(cpu_addr);
+ else if (IS_ENABLED(CONFIG_ARCH_HAS_DMA_SET_UNCACHED))
+ arch_dma_clear_uncached(cpu_addr, size);
dma_free_contiguous(dev, dma_direct_to_page(dev, dma_addr), size);
}
@@ -238,6 +244,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
dma_addr_t *dma_handle, gfp_t gfp, unsigned long attrs)
{
if (!IS_ENABLED(CONFIG_ARCH_HAS_UNCACHED_SEGMENT) &&
+ !IS_ENABLED(CONFIG_ARCH_HAS_DMA_SET_UNCACHED) &&
!IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) &&
dma_alloc_need_uncached(dev, attrs))
return arch_dma_alloc(dev, size, dma_handle, gfp, attrs);
@@ -248,6 +255,7 @@ void dma_direct_free(struct device *dev, size_t size,
void *cpu_addr, dma_addr_t dma_addr, unsigned long attrs)
{
if (!IS_ENABLED(CONFIG_ARCH_HAS_UNCACHED_SEGMENT) &&
+ !IS_ENABLED(CONFIG_ARCH_HAS_DMA_SET_UNCACHED) &&
!IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) &&
dma_alloc_need_uncached(dev, attrs))
arch_dma_free(dev, size, cpu_addr, dma_addr, attrs);
--
2.24.1
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply related
* Re: 回覆:skeleton
From: Brad Bishop @ 2020-02-20 17:01 UTC (permalink / raw)
To: Jeff Chan
Cc: OpenBMC Maillist
In-Reply-To: <-sifmi5quxtt6-92q1n5-wyg6ssnts50d-w3mxqcpkbefz-35tf4ixc78y0-97rd5o-shjcwj2at2netjz0rthkxa46-tez059-yjezs9-f6thaysp4k1b6xrwf-6pqhrjbvxja7-b3yx80-86m5ir-5rqmyn.1582160771127@email.android.com>
> On Feb 19, 2020, at 8:06 PM, Jeff Chan <ckimchan17@gmail.com> wrote:
>
> Hi Brad,
> Thanks for reply, I checked github.com/openbmc/skeleton, most of them released 2 years ago, and the title said "will be replaced...", is it still good to adopt it?
No I would not recommend that. Skeleton covers a lot of BMC functions - which one in particular are you interested in?
>
> Jeff
>
>
> -------- 原始郵件 --------
> 寄件者: Brad Bishop <bradleyb@fuzziesquirrel.com>
> 日期: 2020年2月14日 週五 02:39
> 收件人: Jeff Chan <ckimchan17@gmail.com>
> 副本: OpenBMC Maillist <openbmc@lists.ozlabs.org>
> 主旨: Re: skeleton
>
>
> > On Feb 11, 2020, at 6:29 AM, Jeff Chan <ckimchan17@gmail.com> wrote:
> >
> > Hi,
> > As the description in skeleton github, it will be replaced with proper implementation, what's the up to date implementation? where can I find those docs or samples?
> >
> > Jeff
>
> Hi Jeff
>
> Most of skeleton has been rewritten. skeleton covers a lot of BMC functions - which one in particular are you interested in?
>
> thx -brad
^ permalink raw reply
* provide in-place uncached remapping for dma-direct (resend)
From: Christoph Hellwig @ 2020-02-20 17:01 UTC (permalink / raw)
To: Jonas Bonn, Stefan Kristiansson, Stafford Horne
Cc: Mark Rutland, linux-arch, Robin Murphy, linux-kernel, iommu,
openrisc, Will Deacon, linux-arm-kernel
Hi all,
this series provides support for remapping places uncached in-place in
the generic dma-direct code, and moves openrisc over from its own
in-place remapping scheme. The arm64 folks also had interest in such
a scheme to avoid problems with speculating into cache aliases.
Also all architectures that always use small page mappings for the
kernel and have non-coherent DMA should look into enabling this
scheme, as it is much more efficient than the vmap remapping.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply
* [PATCH 2/2] openrisc: use the generic in-place uncached DMA allocator
From: Christoph Hellwig @ 2020-02-20 17:01 UTC (permalink / raw)
To: Jonas Bonn, Stefan Kristiansson, Stafford Horne
Cc: Mark Rutland, linux-arch, Robin Murphy, linux-kernel, iommu,
openrisc, Will Deacon, linux-arm-kernel
In-Reply-To: <20200220170139.387354-1-hch@lst.de>
Switch openrisc to use the dma-direct allocator and just provide the
hooks for setting memory uncached or cached.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/openrisc/Kconfig | 1 +
arch/openrisc/kernel/dma.c | 51 +++++---------------------------------
2 files changed, 7 insertions(+), 45 deletions(-)
diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig
index 1928e061ff96..041fff4326dc 100644
--- a/arch/openrisc/Kconfig
+++ b/arch/openrisc/Kconfig
@@ -7,6 +7,7 @@
config OPENRISC
def_bool y
select ARCH_32BIT_OFF_T
+ select ARCH_HAS_DMA_SET_UNCACHED
select ARCH_HAS_SYNC_DMA_FOR_DEVICE
select OF
select OF_EARLY_FLATTREE
diff --git a/arch/openrisc/kernel/dma.c b/arch/openrisc/kernel/dma.c
index adec711ad39d..c73d2b3ae267 100644
--- a/arch/openrisc/kernel/dma.c
+++ b/arch/openrisc/kernel/dma.c
@@ -11,8 +11,6 @@
* Copyright (C) 2010-2011 Jonas Bonn <jonas@southpole.se>
*
* DMA mapping callbacks...
- * As alloc_coherent is the only DMA callback being used currently, that's
- * the only thing implemented properly. The rest need looking into...
*/
#include <linux/dma-noncoherent.h>
@@ -67,62 +65,25 @@ static const struct mm_walk_ops clear_nocache_walk_ops = {
.pte_entry = page_clear_nocache,
};
-/*
- * Alloc "coherent" memory, which for OpenRISC means simply uncached.
- *
- * This function effectively just calls __get_free_pages, sets the
- * cache-inhibit bit on those pages, and makes sure that the pages are
- * flushed out of the cache before they are used.
- *
- * If the NON_CONSISTENT attribute is set, then this function just
- * returns "normal", cachable memory.
- *
- * There are additional flags WEAK_ORDERING and WRITE_COMBINE to take
- * into consideration here, too. All current known implementations of
- * the OR1K support only strongly ordered memory accesses, so that flag
- * is being ignored for now; uncached but write-combined memory is a
- * missing feature of the OR1K.
- */
-void *
-arch_dma_alloc(struct device *dev, size_t size, dma_addr_t *dma_handle,
- gfp_t gfp, unsigned long attrs)
+int arch_dma_set_uncached(void *cpu_addr, size_t size)
{
- unsigned long va;
- void *page;
-
- page = alloc_pages_exact(size, gfp | __GFP_ZERO);
- if (!page)
- return NULL;
-
- /* This gives us the real physical address of the first page. */
- *dma_handle = __pa(page);
-
- va = (unsigned long)page;
+ unsigned long va = (unsigned long)cpu_addr;
/*
* We need to iterate through the pages, clearing the dcache for
* them and setting the cache-inhibit bit.
*/
- if (walk_page_range(&init_mm, va, va + size, &set_nocache_walk_ops,
- NULL)) {
- free_pages_exact(page, size);
- return NULL;
- }
-
- return (void *)va;
+ return walk_page_range(&init_mm, va, va + size, &set_nocache_walk_ops,
+ NULL);
}
-void
-arch_dma_free(struct device *dev, size_t size, void *vaddr,
- dma_addr_t dma_handle, unsigned long attrs)
+void arch_dma_clear_uncached(void *cpu_addr, size_t size)
{
- unsigned long va = (unsigned long)vaddr;
+ unsigned long va = (unsigned long)cpu_addr;
/* walk_page_range shouldn't be able to fail here */
WARN_ON(walk_page_range(&init_mm, va, va + size,
&clear_nocache_walk_ops, NULL));
-
- free_pages_exact(vaddr, size);
}
void arch_sync_dma_for_device(phys_addr_t addr, size_t size,
--
2.24.1
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply related
* [PATCH 1/2] dma-mapping: support setting memory uncached in place
From: Christoph Hellwig @ 2020-02-20 17:01 UTC (permalink / raw)
To: Jonas Bonn, Stefan Kristiansson, Stafford Horne
Cc: Marek Szyprowski, Robin Murphy, Will Deacon, Mark Rutland,
openrisc, iommu, linux-arm-kernel, linux-arch, linux-kernel
In-Reply-To: <20200220170139.387354-1-hch@lst.de>
We currently only support remapping memory as uncached through vmap
or a magic uncached segment provided by some architectures. But there
is a simpler and much better way available on some architectures where
we can just remap the memory in place. The advantages are:
1) no aliasing is possible, which prevents speculating into the
cached alias
2) there is no need to allocate new ptes and thus no need for a special
pre-allocated pool of memory that can be used with GFP_ATOMIC DMA
allocations
The downside is that architectures must provide a way to set arbitrary
pages uncached in the kernel mapping, which might not be possible on
architecture that have a special implicit kernel mapping, and requires
splitting of huge page kernel mappings where they exist.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
include/linux/dma-noncoherent.h | 3 +++
kernel/dma/Kconfig | 8 ++++++++
kernel/dma/direct.c | 28 ++++++++++++++++++----------
3 files changed, 29 insertions(+), 10 deletions(-)
diff --git a/include/linux/dma-noncoherent.h b/include/linux/dma-noncoherent.h
index ca9b5770caee..0820ec58f119 100644
--- a/include/linux/dma-noncoherent.h
+++ b/include/linux/dma-noncoherent.h
@@ -111,4 +111,7 @@ static inline void arch_dma_prep_coherent(struct page *page, size_t size)
void *uncached_kernel_address(void *addr);
void *cached_kernel_address(void *addr);
+int arch_dma_set_uncached(void *cpu_addr, size_t size);
+void arch_dma_clear_uncached(void *cpu_addr, size_t size);
+
#endif /* _LINUX_DMA_NONCOHERENT_H */
diff --git a/kernel/dma/Kconfig b/kernel/dma/Kconfig
index 4c103a24e380..7bc0b77f1243 100644
--- a/kernel/dma/Kconfig
+++ b/kernel/dma/Kconfig
@@ -83,6 +83,14 @@ config DMA_DIRECT_REMAP
bool
select DMA_REMAP
+#
+# Should be selected if the architecture can remap memory from the page
+# allocator and CMA as uncached and provides the arch_dma_set_uncached and
+# arch_dma_clear_uncached helpers
+#
+config ARCH_HAS_DMA_SET_UNCACHED
+ bool
+
config DMA_CMA
bool "DMA Contiguous Memory Allocator"
depends on HAVE_DMA_CONTIGUOUS && CMA
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 6af7ae83c4ad..73fe65a4cbc0 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -169,11 +169,8 @@ void *dma_direct_alloc_pages(struct device *dev, size_t size,
ret = dma_common_contiguous_remap(page, PAGE_ALIGN(size),
dma_pgprot(dev, PAGE_KERNEL, attrs),
__builtin_return_address(0));
- if (!ret) {
- dma_free_contiguous(dev, page, size);
- return ret;
- }
-
+ if (!ret)
+ goto out_free_pages;
memset(ret, 0, size);
goto done;
}
@@ -186,8 +183,7 @@ void *dma_direct_alloc_pages(struct device *dev, size_t size,
* so log an error and fail.
*/
dev_info(dev, "Rejecting highmem page from CMA.\n");
- dma_free_contiguous(dev, page, size);
- return NULL;
+ goto out_free_pages;
}
ret = page_address(page);
@@ -196,10 +192,15 @@ void *dma_direct_alloc_pages(struct device *dev, size_t size,
memset(ret, 0, size);
- if (IS_ENABLED(CONFIG_ARCH_HAS_UNCACHED_SEGMENT) &&
- dma_alloc_need_uncached(dev, attrs)) {
+ if (dma_alloc_need_uncached(dev, attrs)) {
arch_dma_prep_coherent(page, size);
- ret = uncached_kernel_address(ret);
+
+ if (IS_ENABLED(CONFIG_ARCH_HAS_DMA_SET_UNCACHED)) {
+ if (!arch_dma_set_uncached(ret, size))
+ goto out_free_pages;
+ } else if (IS_ENABLED(CONFIG_ARCH_HAS_UNCACHED_SEGMENT)) {
+ ret = uncached_kernel_address(ret);
+ }
}
done:
if (force_dma_unencrypted(dev))
@@ -207,6 +208,9 @@ void *dma_direct_alloc_pages(struct device *dev, size_t size,
else
*dma_handle = phys_to_dma(dev, page_to_phys(page));
return ret;
+out_free_pages:
+ dma_free_contiguous(dev, page, size);
+ return NULL;
}
void dma_direct_free_pages(struct device *dev, size_t size, void *cpu_addr,
@@ -230,6 +234,8 @@ void dma_direct_free_pages(struct device *dev, size_t size, void *cpu_addr,
if (IS_ENABLED(CONFIG_DMA_REMAP) && is_vmalloc_addr(cpu_addr))
vunmap(cpu_addr);
+ else if (IS_ENABLED(CONFIG_ARCH_HAS_DMA_SET_UNCACHED))
+ arch_dma_clear_uncached(cpu_addr, size);
dma_free_contiguous(dev, dma_direct_to_page(dev, dma_addr), size);
}
@@ -238,6 +244,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
dma_addr_t *dma_handle, gfp_t gfp, unsigned long attrs)
{
if (!IS_ENABLED(CONFIG_ARCH_HAS_UNCACHED_SEGMENT) &&
+ !IS_ENABLED(CONFIG_ARCH_HAS_DMA_SET_UNCACHED) &&
!IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) &&
dma_alloc_need_uncached(dev, attrs))
return arch_dma_alloc(dev, size, dma_handle, gfp, attrs);
@@ -248,6 +255,7 @@ void dma_direct_free(struct device *dev, size_t size,
void *cpu_addr, dma_addr_t dma_addr, unsigned long attrs)
{
if (!IS_ENABLED(CONFIG_ARCH_HAS_UNCACHED_SEGMENT) &&
+ !IS_ENABLED(CONFIG_ARCH_HAS_DMA_SET_UNCACHED) &&
!IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) &&
dma_alloc_need_uncached(dev, attrs))
arch_dma_free(dev, size, cpu_addr, dma_addr, attrs);
--
2.24.1
^ permalink raw reply related
* [PATCH 2/2] openrisc: use the generic in-place uncached DMA allocator
From: Christoph Hellwig @ 2020-02-20 17:01 UTC (permalink / raw)
To: Jonas Bonn, Stefan Kristiansson, Stafford Horne
Cc: Marek Szyprowski, Robin Murphy, Will Deacon, Mark Rutland,
openrisc, iommu, linux-arm-kernel, linux-arch, linux-kernel
In-Reply-To: <20200220170139.387354-1-hch@lst.de>
Switch openrisc to use the dma-direct allocator and just provide the
hooks for setting memory uncached or cached.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/openrisc/Kconfig | 1 +
arch/openrisc/kernel/dma.c | 51 +++++---------------------------------
2 files changed, 7 insertions(+), 45 deletions(-)
diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig
index 1928e061ff96..041fff4326dc 100644
--- a/arch/openrisc/Kconfig
+++ b/arch/openrisc/Kconfig
@@ -7,6 +7,7 @@
config OPENRISC
def_bool y
select ARCH_32BIT_OFF_T
+ select ARCH_HAS_DMA_SET_UNCACHED
select ARCH_HAS_SYNC_DMA_FOR_DEVICE
select OF
select OF_EARLY_FLATTREE
diff --git a/arch/openrisc/kernel/dma.c b/arch/openrisc/kernel/dma.c
index adec711ad39d..c73d2b3ae267 100644
--- a/arch/openrisc/kernel/dma.c
+++ b/arch/openrisc/kernel/dma.c
@@ -11,8 +11,6 @@
* Copyright (C) 2010-2011 Jonas Bonn <jonas@southpole.se>
*
* DMA mapping callbacks...
- * As alloc_coherent is the only DMA callback being used currently, that's
- * the only thing implemented properly. The rest need looking into...
*/
#include <linux/dma-noncoherent.h>
@@ -67,62 +65,25 @@ static const struct mm_walk_ops clear_nocache_walk_ops = {
.pte_entry = page_clear_nocache,
};
-/*
- * Alloc "coherent" memory, which for OpenRISC means simply uncached.
- *
- * This function effectively just calls __get_free_pages, sets the
- * cache-inhibit bit on those pages, and makes sure that the pages are
- * flushed out of the cache before they are used.
- *
- * If the NON_CONSISTENT attribute is set, then this function just
- * returns "normal", cachable memory.
- *
- * There are additional flags WEAK_ORDERING and WRITE_COMBINE to take
- * into consideration here, too. All current known implementations of
- * the OR1K support only strongly ordered memory accesses, so that flag
- * is being ignored for now; uncached but write-combined memory is a
- * missing feature of the OR1K.
- */
-void *
-arch_dma_alloc(struct device *dev, size_t size, dma_addr_t *dma_handle,
- gfp_t gfp, unsigned long attrs)
+int arch_dma_set_uncached(void *cpu_addr, size_t size)
{
- unsigned long va;
- void *page;
-
- page = alloc_pages_exact(size, gfp | __GFP_ZERO);
- if (!page)
- return NULL;
-
- /* This gives us the real physical address of the first page. */
- *dma_handle = __pa(page);
-
- va = (unsigned long)page;
+ unsigned long va = (unsigned long)cpu_addr;
/*
* We need to iterate through the pages, clearing the dcache for
* them and setting the cache-inhibit bit.
*/
- if (walk_page_range(&init_mm, va, va + size, &set_nocache_walk_ops,
- NULL)) {
- free_pages_exact(page, size);
- return NULL;
- }
-
- return (void *)va;
+ return walk_page_range(&init_mm, va, va + size, &set_nocache_walk_ops,
+ NULL);
}
-void
-arch_dma_free(struct device *dev, size_t size, void *vaddr,
- dma_addr_t dma_handle, unsigned long attrs)
+void arch_dma_clear_uncached(void *cpu_addr, size_t size)
{
- unsigned long va = (unsigned long)vaddr;
+ unsigned long va = (unsigned long)cpu_addr;
/* walk_page_range shouldn't be able to fail here */
WARN_ON(walk_page_range(&init_mm, va, va + size,
&clear_nocache_walk_ops, NULL));
-
- free_pages_exact(vaddr, size);
}
void arch_sync_dma_for_device(phys_addr_t addr, size_t size,
--
2.24.1
^ permalink raw reply related
* Re: [dpdk-dev] [PATCH v2] app/testpmd: fix return value in portlist parser
From: Ferruh Yigit @ 2020-02-20 17:01 UTC (permalink / raw)
To: Hariprasad Govindharajan, Wenzhuo Lu, Jingjing Wu,
Bernard Iremonger
Cc: dev, stephen, david.marchand
In-Reply-To: <1582213402-18941-1-git-send-email-hariprasad.govindharajan@intel.com>
On 2/20/2020 3:43 PM, Hariprasad Govindharajan wrote:
> The function parse_port_list() is designed to return
> unsigned int value. After sanitizing the inputs,
> it is returning -1. Changed it to return 0.
>
> Fixes: 2df00d562d20 ("app/testpmd: add --portlist option")
> Cc: hariprasad.govindharajan@intel.com
>
> Signed-off-by: Hariprasad Govindharajan <hariprasad.govindharajan@intel.com>
> ---
> app/test-pmd/config.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 9d95202..91db508 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -2642,7 +2642,7 @@ parse_port_list(const char *list, unsigned int *values, unsigned int maxsize)
> unsigned int marked[maxsize];
>
> if (list == NULL || values == NULL)
> - return -1;
> + return 0;
Can you please clarify what is the expected return value on error in the
function comment?
>
> for (i = 0; i < (int)maxsize; i++)
> marked[i] = 0;
>
^ permalink raw reply
* [PATCH 2/2] openrisc: use the generic in-place uncached DMA allocator
From: Christoph Hellwig @ 2020-02-20 17:01 UTC (permalink / raw)
To: Jonas Bonn, Stefan Kristiansson, Stafford Horne
Cc: Mark Rutland, linux-arch, Robin Murphy, linux-kernel, iommu,
openrisc, Will Deacon, linux-arm-kernel, Marek Szyprowski
In-Reply-To: <20200220170139.387354-1-hch@lst.de>
Switch openrisc to use the dma-direct allocator and just provide the
hooks for setting memory uncached or cached.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/openrisc/Kconfig | 1 +
arch/openrisc/kernel/dma.c | 51 +++++---------------------------------
2 files changed, 7 insertions(+), 45 deletions(-)
diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig
index 1928e061ff96..041fff4326dc 100644
--- a/arch/openrisc/Kconfig
+++ b/arch/openrisc/Kconfig
@@ -7,6 +7,7 @@
config OPENRISC
def_bool y
select ARCH_32BIT_OFF_T
+ select ARCH_HAS_DMA_SET_UNCACHED
select ARCH_HAS_SYNC_DMA_FOR_DEVICE
select OF
select OF_EARLY_FLATTREE
diff --git a/arch/openrisc/kernel/dma.c b/arch/openrisc/kernel/dma.c
index adec711ad39d..c73d2b3ae267 100644
--- a/arch/openrisc/kernel/dma.c
+++ b/arch/openrisc/kernel/dma.c
@@ -11,8 +11,6 @@
* Copyright (C) 2010-2011 Jonas Bonn <jonas@southpole.se>
*
* DMA mapping callbacks...
- * As alloc_coherent is the only DMA callback being used currently, that's
- * the only thing implemented properly. The rest need looking into...
*/
#include <linux/dma-noncoherent.h>
@@ -67,62 +65,25 @@ static const struct mm_walk_ops clear_nocache_walk_ops = {
.pte_entry = page_clear_nocache,
};
-/*
- * Alloc "coherent" memory, which for OpenRISC means simply uncached.
- *
- * This function effectively just calls __get_free_pages, sets the
- * cache-inhibit bit on those pages, and makes sure that the pages are
- * flushed out of the cache before they are used.
- *
- * If the NON_CONSISTENT attribute is set, then this function just
- * returns "normal", cachable memory.
- *
- * There are additional flags WEAK_ORDERING and WRITE_COMBINE to take
- * into consideration here, too. All current known implementations of
- * the OR1K support only strongly ordered memory accesses, so that flag
- * is being ignored for now; uncached but write-combined memory is a
- * missing feature of the OR1K.
- */
-void *
-arch_dma_alloc(struct device *dev, size_t size, dma_addr_t *dma_handle,
- gfp_t gfp, unsigned long attrs)
+int arch_dma_set_uncached(void *cpu_addr, size_t size)
{
- unsigned long va;
- void *page;
-
- page = alloc_pages_exact(size, gfp | __GFP_ZERO);
- if (!page)
- return NULL;
-
- /* This gives us the real physical address of the first page. */
- *dma_handle = __pa(page);
-
- va = (unsigned long)page;
+ unsigned long va = (unsigned long)cpu_addr;
/*
* We need to iterate through the pages, clearing the dcache for
* them and setting the cache-inhibit bit.
*/
- if (walk_page_range(&init_mm, va, va + size, &set_nocache_walk_ops,
- NULL)) {
- free_pages_exact(page, size);
- return NULL;
- }
-
- return (void *)va;
+ return walk_page_range(&init_mm, va, va + size, &set_nocache_walk_ops,
+ NULL);
}
-void
-arch_dma_free(struct device *dev, size_t size, void *vaddr,
- dma_addr_t dma_handle, unsigned long attrs)
+void arch_dma_clear_uncached(void *cpu_addr, size_t size)
{
- unsigned long va = (unsigned long)vaddr;
+ unsigned long va = (unsigned long)cpu_addr;
/* walk_page_range shouldn't be able to fail here */
WARN_ON(walk_page_range(&init_mm, va, va + size,
&clear_nocache_walk_ops, NULL));
-
- free_pages_exact(vaddr, size);
}
void arch_sync_dma_for_device(phys_addr_t addr, size_t size,
--
2.24.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [OpenRISC] [PATCH 2/2] openrisc: use the generic in-place uncached DMA allocator
From: Christoph Hellwig @ 2020-02-20 17:01 UTC (permalink / raw)
To: openrisc
In-Reply-To: <20200220170139.387354-1-hch@lst.de>
Switch openrisc to use the dma-direct allocator and just provide the
hooks for setting memory uncached or cached.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/openrisc/Kconfig | 1 +
arch/openrisc/kernel/dma.c | 51 +++++---------------------------------
2 files changed, 7 insertions(+), 45 deletions(-)
diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig
index 1928e061ff96..041fff4326dc 100644
--- a/arch/openrisc/Kconfig
+++ b/arch/openrisc/Kconfig
@@ -7,6 +7,7 @@
config OPENRISC
def_bool y
select ARCH_32BIT_OFF_T
+ select ARCH_HAS_DMA_SET_UNCACHED
select ARCH_HAS_SYNC_DMA_FOR_DEVICE
select OF
select OF_EARLY_FLATTREE
diff --git a/arch/openrisc/kernel/dma.c b/arch/openrisc/kernel/dma.c
index adec711ad39d..c73d2b3ae267 100644
--- a/arch/openrisc/kernel/dma.c
+++ b/arch/openrisc/kernel/dma.c
@@ -11,8 +11,6 @@
* Copyright (C) 2010-2011 Jonas Bonn <jonas@southpole.se>
*
* DMA mapping callbacks...
- * As alloc_coherent is the only DMA callback being used currently, that's
- * the only thing implemented properly. The rest need looking into...
*/
#include <linux/dma-noncoherent.h>
@@ -67,62 +65,25 @@ static const struct mm_walk_ops clear_nocache_walk_ops = {
.pte_entry = page_clear_nocache,
};
-/*
- * Alloc "coherent" memory, which for OpenRISC means simply uncached.
- *
- * This function effectively just calls __get_free_pages, sets the
- * cache-inhibit bit on those pages, and makes sure that the pages are
- * flushed out of the cache before they are used.
- *
- * If the NON_CONSISTENT attribute is set, then this function just
- * returns "normal", cachable memory.
- *
- * There are additional flags WEAK_ORDERING and WRITE_COMBINE to take
- * into consideration here, too. All current known implementations of
- * the OR1K support only strongly ordered memory accesses, so that flag
- * is being ignored for now; uncached but write-combined memory is a
- * missing feature of the OR1K.
- */
-void *
-arch_dma_alloc(struct device *dev, size_t size, dma_addr_t *dma_handle,
- gfp_t gfp, unsigned long attrs)
+int arch_dma_set_uncached(void *cpu_addr, size_t size)
{
- unsigned long va;
- void *page;
-
- page = alloc_pages_exact(size, gfp | __GFP_ZERO);
- if (!page)
- return NULL;
-
- /* This gives us the real physical address of the first page. */
- *dma_handle = __pa(page);
-
- va = (unsigned long)page;
+ unsigned long va = (unsigned long)cpu_addr;
/*
* We need to iterate through the pages, clearing the dcache for
* them and setting the cache-inhibit bit.
*/
- if (walk_page_range(&init_mm, va, va + size, &set_nocache_walk_ops,
- NULL)) {
- free_pages_exact(page, size);
- return NULL;
- }
-
- return (void *)va;
+ return walk_page_range(&init_mm, va, va + size, &set_nocache_walk_ops,
+ NULL);
}
-void
-arch_dma_free(struct device *dev, size_t size, void *vaddr,
- dma_addr_t dma_handle, unsigned long attrs)
+void arch_dma_clear_uncached(void *cpu_addr, size_t size)
{
- unsigned long va = (unsigned long)vaddr;
+ unsigned long va = (unsigned long)cpu_addr;
/* walk_page_range shouldn't be able to fail here */
WARN_ON(walk_page_range(&init_mm, va, va + size,
&clear_nocache_walk_ops, NULL));
-
- free_pages_exact(vaddr, size);
}
void arch_sync_dma_for_device(phys_addr_t addr, size_t size,
--
2.24.1
^ permalink raw reply related
* [PATCH 2/2] openrisc: use the generic in-place uncached DMA allocator
From: Christoph Hellwig @ 2020-02-20 17:01 UTC (permalink / raw)
To: Jonas Bonn, Stefan Kristiansson, Stafford Horne
Cc: Mark Rutland, linux-arch, Robin Murphy, linux-kernel, iommu,
openrisc, Will Deacon, linux-arm-kernel, Marek Szyprowski
In-Reply-To: <20200220170139.387354-1-hch@lst.de>
Switch openrisc to use the dma-direct allocator and just provide the
hooks for setting memory uncached or cached.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/openrisc/Kconfig | 1 +
arch/openrisc/kernel/dma.c | 51 +++++---------------------------------
2 files changed, 7 insertions(+), 45 deletions(-)
diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig
index 1928e061ff96..041fff4326dc 100644
--- a/arch/openrisc/Kconfig
+++ b/arch/openrisc/Kconfig
@@ -7,6 +7,7 @@
config OPENRISC
def_bool y
select ARCH_32BIT_OFF_T
+ select ARCH_HAS_DMA_SET_UNCACHED
select ARCH_HAS_SYNC_DMA_FOR_DEVICE
select OF
select OF_EARLY_FLATTREE
diff --git a/arch/openrisc/kernel/dma.c b/arch/openrisc/kernel/dma.c
index adec711ad39d..c73d2b3ae267 100644
--- a/arch/openrisc/kernel/dma.c
+++ b/arch/openrisc/kernel/dma.c
@@ -11,8 +11,6 @@
* Copyright (C) 2010-2011 Jonas Bonn <jonas@southpole.se>
*
* DMA mapping callbacks...
- * As alloc_coherent is the only DMA callback being used currently, that's
- * the only thing implemented properly. The rest need looking into...
*/
#include <linux/dma-noncoherent.h>
@@ -67,62 +65,25 @@ static const struct mm_walk_ops clear_nocache_walk_ops = {
.pte_entry = page_clear_nocache,
};
-/*
- * Alloc "coherent" memory, which for OpenRISC means simply uncached.
- *
- * This function effectively just calls __get_free_pages, sets the
- * cache-inhibit bit on those pages, and makes sure that the pages are
- * flushed out of the cache before they are used.
- *
- * If the NON_CONSISTENT attribute is set, then this function just
- * returns "normal", cachable memory.
- *
- * There are additional flags WEAK_ORDERING and WRITE_COMBINE to take
- * into consideration here, too. All current known implementations of
- * the OR1K support only strongly ordered memory accesses, so that flag
- * is being ignored for now; uncached but write-combined memory is a
- * missing feature of the OR1K.
- */
-void *
-arch_dma_alloc(struct device *dev, size_t size, dma_addr_t *dma_handle,
- gfp_t gfp, unsigned long attrs)
+int arch_dma_set_uncached(void *cpu_addr, size_t size)
{
- unsigned long va;
- void *page;
-
- page = alloc_pages_exact(size, gfp | __GFP_ZERO);
- if (!page)
- return NULL;
-
- /* This gives us the real physical address of the first page. */
- *dma_handle = __pa(page);
-
- va = (unsigned long)page;
+ unsigned long va = (unsigned long)cpu_addr;
/*
* We need to iterate through the pages, clearing the dcache for
* them and setting the cache-inhibit bit.
*/
- if (walk_page_range(&init_mm, va, va + size, &set_nocache_walk_ops,
- NULL)) {
- free_pages_exact(page, size);
- return NULL;
- }
-
- return (void *)va;
+ return walk_page_range(&init_mm, va, va + size, &set_nocache_walk_ops,
+ NULL);
}
-void
-arch_dma_free(struct device *dev, size_t size, void *vaddr,
- dma_addr_t dma_handle, unsigned long attrs)
+void arch_dma_clear_uncached(void *cpu_addr, size_t size)
{
- unsigned long va = (unsigned long)vaddr;
+ unsigned long va = (unsigned long)cpu_addr;
/* walk_page_range shouldn't be able to fail here */
WARN_ON(walk_page_range(&init_mm, va, va + size,
&clear_nocache_walk_ops, NULL));
-
- free_pages_exact(vaddr, size);
}
void arch_sync_dma_for_device(phys_addr_t addr, size_t size,
--
2.24.1
^ permalink raw reply related
* [OpenRISC] [PATCH 1/2] dma-mapping: support setting memory uncached in place
From: Christoph Hellwig @ 2020-02-20 17:01 UTC (permalink / raw)
To: openrisc
In-Reply-To: <20200220170139.387354-1-hch@lst.de>
We currently only support remapping memory as uncached through vmap
or a magic uncached segment provided by some architectures. But there
is a simpler and much better way available on some architectures where
we can just remap the memory in place. The advantages are:
1) no aliasing is possible, which prevents speculating into the
cached alias
2) there is no need to allocate new ptes and thus no need for a special
pre-allocated pool of memory that can be used with GFP_ATOMIC DMA
allocations
The downside is that architectures must provide a way to set arbitrary
pages uncached in the kernel mapping, which might not be possible on
architecture that have a special implicit kernel mapping, and requires
splitting of huge page kernel mappings where they exist.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
include/linux/dma-noncoherent.h | 3 +++
kernel/dma/Kconfig | 8 ++++++++
kernel/dma/direct.c | 28 ++++++++++++++++++----------
3 files changed, 29 insertions(+), 10 deletions(-)
diff --git a/include/linux/dma-noncoherent.h b/include/linux/dma-noncoherent.h
index ca9b5770caee..0820ec58f119 100644
--- a/include/linux/dma-noncoherent.h
+++ b/include/linux/dma-noncoherent.h
@@ -111,4 +111,7 @@ static inline void arch_dma_prep_coherent(struct page *page, size_t size)
void *uncached_kernel_address(void *addr);
void *cached_kernel_address(void *addr);
+int arch_dma_set_uncached(void *cpu_addr, size_t size);
+void arch_dma_clear_uncached(void *cpu_addr, size_t size);
+
#endif /* _LINUX_DMA_NONCOHERENT_H */
diff --git a/kernel/dma/Kconfig b/kernel/dma/Kconfig
index 4c103a24e380..7bc0b77f1243 100644
--- a/kernel/dma/Kconfig
+++ b/kernel/dma/Kconfig
@@ -83,6 +83,14 @@ config DMA_DIRECT_REMAP
bool
select DMA_REMAP
+#
+# Should be selected if the architecture can remap memory from the page
+# allocator and CMA as uncached and provides the arch_dma_set_uncached and
+# arch_dma_clear_uncached helpers
+#
+config ARCH_HAS_DMA_SET_UNCACHED
+ bool
+
config DMA_CMA
bool "DMA Contiguous Memory Allocator"
depends on HAVE_DMA_CONTIGUOUS && CMA
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 6af7ae83c4ad..73fe65a4cbc0 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -169,11 +169,8 @@ void *dma_direct_alloc_pages(struct device *dev, size_t size,
ret = dma_common_contiguous_remap(page, PAGE_ALIGN(size),
dma_pgprot(dev, PAGE_KERNEL, attrs),
__builtin_return_address(0));
- if (!ret) {
- dma_free_contiguous(dev, page, size);
- return ret;
- }
-
+ if (!ret)
+ goto out_free_pages;
memset(ret, 0, size);
goto done;
}
@@ -186,8 +183,7 @@ void *dma_direct_alloc_pages(struct device *dev, size_t size,
* so log an error and fail.
*/
dev_info(dev, "Rejecting highmem page from CMA.\n");
- dma_free_contiguous(dev, page, size);
- return NULL;
+ goto out_free_pages;
}
ret = page_address(page);
@@ -196,10 +192,15 @@ void *dma_direct_alloc_pages(struct device *dev, size_t size,
memset(ret, 0, size);
- if (IS_ENABLED(CONFIG_ARCH_HAS_UNCACHED_SEGMENT) &&
- dma_alloc_need_uncached(dev, attrs)) {
+ if (dma_alloc_need_uncached(dev, attrs)) {
arch_dma_prep_coherent(page, size);
- ret = uncached_kernel_address(ret);
+
+ if (IS_ENABLED(CONFIG_ARCH_HAS_DMA_SET_UNCACHED)) {
+ if (!arch_dma_set_uncached(ret, size))
+ goto out_free_pages;
+ } else if (IS_ENABLED(CONFIG_ARCH_HAS_UNCACHED_SEGMENT)) {
+ ret = uncached_kernel_address(ret);
+ }
}
done:
if (force_dma_unencrypted(dev))
@@ -207,6 +208,9 @@ void *dma_direct_alloc_pages(struct device *dev, size_t size,
else
*dma_handle = phys_to_dma(dev, page_to_phys(page));
return ret;
+out_free_pages:
+ dma_free_contiguous(dev, page, size);
+ return NULL;
}
void dma_direct_free_pages(struct device *dev, size_t size, void *cpu_addr,
@@ -230,6 +234,8 @@ void dma_direct_free_pages(struct device *dev, size_t size, void *cpu_addr,
if (IS_ENABLED(CONFIG_DMA_REMAP) && is_vmalloc_addr(cpu_addr))
vunmap(cpu_addr);
+ else if (IS_ENABLED(CONFIG_ARCH_HAS_DMA_SET_UNCACHED))
+ arch_dma_clear_uncached(cpu_addr, size);
dma_free_contiguous(dev, dma_direct_to_page(dev, dma_addr), size);
}
@@ -238,6 +244,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
dma_addr_t *dma_handle, gfp_t gfp, unsigned long attrs)
{
if (!IS_ENABLED(CONFIG_ARCH_HAS_UNCACHED_SEGMENT) &&
+ !IS_ENABLED(CONFIG_ARCH_HAS_DMA_SET_UNCACHED) &&
!IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) &&
dma_alloc_need_uncached(dev, attrs))
return arch_dma_alloc(dev, size, dma_handle, gfp, attrs);
@@ -248,6 +255,7 @@ void dma_direct_free(struct device *dev, size_t size,
void *cpu_addr, dma_addr_t dma_addr, unsigned long attrs)
{
if (!IS_ENABLED(CONFIG_ARCH_HAS_UNCACHED_SEGMENT) &&
+ !IS_ENABLED(CONFIG_ARCH_HAS_DMA_SET_UNCACHED) &&
!IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) &&
dma_alloc_need_uncached(dev, attrs))
arch_dma_free(dev, size, cpu_addr, dma_addr, attrs);
--
2.24.1
^ permalink raw reply related
* [PATCH 1/2] dma-mapping: support setting memory uncached in place
From: Christoph Hellwig @ 2020-02-20 17:01 UTC (permalink / raw)
To: Jonas Bonn, Stefan Kristiansson, Stafford Horne
Cc: Mark Rutland, linux-arch, Robin Murphy, linux-kernel, iommu,
openrisc, Will Deacon, linux-arm-kernel, Marek Szyprowski
In-Reply-To: <20200220170139.387354-1-hch@lst.de>
We currently only support remapping memory as uncached through vmap
or a magic uncached segment provided by some architectures. But there
is a simpler and much better way available on some architectures where
we can just remap the memory in place. The advantages are:
1) no aliasing is possible, which prevents speculating into the
cached alias
2) there is no need to allocate new ptes and thus no need for a special
pre-allocated pool of memory that can be used with GFP_ATOMIC DMA
allocations
The downside is that architectures must provide a way to set arbitrary
pages uncached in the kernel mapping, which might not be possible on
architecture that have a special implicit kernel mapping, and requires
splitting of huge page kernel mappings where they exist.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
include/linux/dma-noncoherent.h | 3 +++
kernel/dma/Kconfig | 8 ++++++++
kernel/dma/direct.c | 28 ++++++++++++++++++----------
3 files changed, 29 insertions(+), 10 deletions(-)
diff --git a/include/linux/dma-noncoherent.h b/include/linux/dma-noncoherent.h
index ca9b5770caee..0820ec58f119 100644
--- a/include/linux/dma-noncoherent.h
+++ b/include/linux/dma-noncoherent.h
@@ -111,4 +111,7 @@ static inline void arch_dma_prep_coherent(struct page *page, size_t size)
void *uncached_kernel_address(void *addr);
void *cached_kernel_address(void *addr);
+int arch_dma_set_uncached(void *cpu_addr, size_t size);
+void arch_dma_clear_uncached(void *cpu_addr, size_t size);
+
#endif /* _LINUX_DMA_NONCOHERENT_H */
diff --git a/kernel/dma/Kconfig b/kernel/dma/Kconfig
index 4c103a24e380..7bc0b77f1243 100644
--- a/kernel/dma/Kconfig
+++ b/kernel/dma/Kconfig
@@ -83,6 +83,14 @@ config DMA_DIRECT_REMAP
bool
select DMA_REMAP
+#
+# Should be selected if the architecture can remap memory from the page
+# allocator and CMA as uncached and provides the arch_dma_set_uncached and
+# arch_dma_clear_uncached helpers
+#
+config ARCH_HAS_DMA_SET_UNCACHED
+ bool
+
config DMA_CMA
bool "DMA Contiguous Memory Allocator"
depends on HAVE_DMA_CONTIGUOUS && CMA
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 6af7ae83c4ad..73fe65a4cbc0 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -169,11 +169,8 @@ void *dma_direct_alloc_pages(struct device *dev, size_t size,
ret = dma_common_contiguous_remap(page, PAGE_ALIGN(size),
dma_pgprot(dev, PAGE_KERNEL, attrs),
__builtin_return_address(0));
- if (!ret) {
- dma_free_contiguous(dev, page, size);
- return ret;
- }
-
+ if (!ret)
+ goto out_free_pages;
memset(ret, 0, size);
goto done;
}
@@ -186,8 +183,7 @@ void *dma_direct_alloc_pages(struct device *dev, size_t size,
* so log an error and fail.
*/
dev_info(dev, "Rejecting highmem page from CMA.\n");
- dma_free_contiguous(dev, page, size);
- return NULL;
+ goto out_free_pages;
}
ret = page_address(page);
@@ -196,10 +192,15 @@ void *dma_direct_alloc_pages(struct device *dev, size_t size,
memset(ret, 0, size);
- if (IS_ENABLED(CONFIG_ARCH_HAS_UNCACHED_SEGMENT) &&
- dma_alloc_need_uncached(dev, attrs)) {
+ if (dma_alloc_need_uncached(dev, attrs)) {
arch_dma_prep_coherent(page, size);
- ret = uncached_kernel_address(ret);
+
+ if (IS_ENABLED(CONFIG_ARCH_HAS_DMA_SET_UNCACHED)) {
+ if (!arch_dma_set_uncached(ret, size))
+ goto out_free_pages;
+ } else if (IS_ENABLED(CONFIG_ARCH_HAS_UNCACHED_SEGMENT)) {
+ ret = uncached_kernel_address(ret);
+ }
}
done:
if (force_dma_unencrypted(dev))
@@ -207,6 +208,9 @@ void *dma_direct_alloc_pages(struct device *dev, size_t size,
else
*dma_handle = phys_to_dma(dev, page_to_phys(page));
return ret;
+out_free_pages:
+ dma_free_contiguous(dev, page, size);
+ return NULL;
}
void dma_direct_free_pages(struct device *dev, size_t size, void *cpu_addr,
@@ -230,6 +234,8 @@ void dma_direct_free_pages(struct device *dev, size_t size, void *cpu_addr,
if (IS_ENABLED(CONFIG_DMA_REMAP) && is_vmalloc_addr(cpu_addr))
vunmap(cpu_addr);
+ else if (IS_ENABLED(CONFIG_ARCH_HAS_DMA_SET_UNCACHED))
+ arch_dma_clear_uncached(cpu_addr, size);
dma_free_contiguous(dev, dma_direct_to_page(dev, dma_addr), size);
}
@@ -238,6 +244,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
dma_addr_t *dma_handle, gfp_t gfp, unsigned long attrs)
{
if (!IS_ENABLED(CONFIG_ARCH_HAS_UNCACHED_SEGMENT) &&
+ !IS_ENABLED(CONFIG_ARCH_HAS_DMA_SET_UNCACHED) &&
!IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) &&
dma_alloc_need_uncached(dev, attrs))
return arch_dma_alloc(dev, size, dma_handle, gfp, attrs);
@@ -248,6 +255,7 @@ void dma_direct_free(struct device *dev, size_t size,
void *cpu_addr, dma_addr_t dma_addr, unsigned long attrs)
{
if (!IS_ENABLED(CONFIG_ARCH_HAS_UNCACHED_SEGMENT) &&
+ !IS_ENABLED(CONFIG_ARCH_HAS_DMA_SET_UNCACHED) &&
!IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) &&
dma_alloc_need_uncached(dev, attrs))
arch_dma_free(dev, size, cpu_addr, dma_addr, attrs);
--
2.24.1
^ permalink raw reply related
* [OpenRISC] provide in-place uncached remapping for dma-direct (resend)
From: Christoph Hellwig @ 2020-02-20 17:01 UTC (permalink / raw)
To: openrisc
Hi all,
this series provides support for remapping places uncached in-place in
the generic dma-direct code, and moves openrisc over from its own
in-place remapping scheme. The arm64 folks also had interest in such
a scheme to avoid problems with speculating into cache aliases.
Also all architectures that always use small page mappings for the
kernel and have non-coherent DMA should look into enabling this
scheme, as it is much more efficient than the vmap remapping.
^ permalink raw reply
* Re: [PATCH 2/2] MAINTAINERS: Set MIPS status to Odd Fixes
From: YunQiang Su @ 2020-02-20 17:01 UTC (permalink / raw)
To: Zhou Yanjie
Cc: Thomas Bogendoerfer, Paul Burton, linux-mips, linux-kernel,
wayne.sun, chris.wang, Yunqiang Su, dongsheng.qiu, xuwanhao
In-Reply-To: <5E4E7E3E.6070608@wanyeetech.com>
Zhou Yanjie <zhouyanjie@wanyeetech.com> 于2020年2月20日周四 下午8:40写道:
>
> Hi,
>
> CC people from Ingenic Semi and Wanyee Tech.
>
> On 2020年02月20日 20:11, YunQiang Su wrote:
> > CC people from NeoCore and CIP United, and my Wave Computing's mail address.
> >
> > Thomas Bogendoerfer <tsbogend@alpha.franken.de> 于2020年2月20日周四 下午7:23写道:
> >> On Wed, Feb 19, 2020 at 11:17:30AM -0800, Paul Burton wrote:
> >>> My time with MIPS the company has reached its end, and so at best I'll
> >>> have little time spend on maintaining arch/mips/. Reflect that in
> >>> MAINTAINERS by changing status to Odd Fixes. Hopefully this might spur
> >>> the involvement of someone with more time, but even if not it should
> >>> help serve to avoid unrealistic expectations.
> >> I'd like to jump in as MIPS maintainer. I'm doing Linux MIPS kernel
> > It is a great news that you are willing to act as maintainer as Linux-MIPS.
> >
> >> development since ages (started with an Olivetti M700 and kernel version
> >> 2.x) and right now time for doing the jobs isn't issue:-)
> >>
> > I noticed that you are mainly working some old machines.
> > And recently years, there are some new machines from Ingenic, Loongson, MTK etc.
> > MIPS Inc also have some MIPSr6 IPs.
> > I think that you need some of these machines.
>
> I can provide some new Ingenic platform machines as a gift to Thomas.
> Ingenic X1000 can be provided in a short time, it has been directly
> supported by kernel 5.6.
> X1830 and X2000 will be available later.
>
> > In the last years, we see that the single maintainer is not enough as people may
> > quite busy.
> > Do you think that we need co-maintainers?
Paul Cercueil also has interest about it. That's so cool.
As a suggestion, I think that we can have a maintainers team:
Option1: Paul Cercueil/Thomas Bogendoerfer/Jiaxun Yang
Option2: Thomas Bogendoerfer/Paul Cercueil/Jiaxun Yang
any idea?
background:
1. PaulC works lots for Ingenic support.
2. Thomas works lots for some SGI IP and some other hardware
3. Jiaxun works lots for Loongson hardware.
> >
> >> Thomas.
> >>
> >> --
> >> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> >> good idea. [ RFC1925, 2.3 ]
> >
> >
>
--
YunQiang Su
^ permalink raw reply
* provide in-place uncached remapping for dma-direct (resend)
From: Christoph Hellwig @ 2020-02-20 17:01 UTC (permalink / raw)
To: Jonas Bonn, Stefan Kristiansson, Stafford Horne
Cc: Marek Szyprowski, Robin Murphy, Will Deacon, Mark Rutland,
openrisc, iommu, linux-arm-kernel, linux-arch, linux-kernel
Hi all,
this series provides support for remapping places uncached in-place in
the generic dma-direct code, and moves openrisc over from its own
in-place remapping scheme. The arm64 folks also had interest in such
a scheme to avoid problems with speculating into cache aliases.
Also all architectures that always use small page mappings for the
kernel and have non-coherent DMA should look into enabling this
scheme, as it is much more efficient than the vmap remapping.
^ permalink raw reply
* Re: [PATCH v3 00/12] Enable per-file/directory DAX operations V3
From: Darrick J. Wong @ 2020-02-20 17:00 UTC (permalink / raw)
To: Ira Weiny
Cc: Jeff Moyer, Dan Williams, Linux Kernel Mailing List,
Alexander Viro, Dave Chinner, Christoph Hellwig,
Theodore Y. Ts'o, Jan Kara, linux-ext4, linux-xfs,
linux-fsdevel
In-Reply-To: <20200220164957.GB20772@iweiny-DESK2.sc.intel.com>
On Thu, Feb 20, 2020 at 08:49:57AM -0800, Ira Weiny wrote:
> On Thu, Feb 20, 2020 at 08:30:24AM -0800, Darrick J. Wong wrote:
> > On Thu, Feb 20, 2020 at 08:20:28AM -0800, Ira Weiny wrote:
> > > On Tue, Feb 18, 2020 at 03:54:30PM -0800, 'Ira Weiny' wrote:
> > > > On Tue, Feb 18, 2020 at 09:22:58AM -0500, Jeff Moyer wrote:
> > > > > Ira Weiny <ira.weiny@intel.com> writes:
> > > > > > If my disassembly of read_pages is correct it looks like readpage is null which
> > > > > > makes sense because all files should be IS_DAX() == true due to the mount option...
> > > > > >
> > > > > > But tracing code indicates that the patch:
> > > > > >
> > > > > > fs: remove unneeded IS_DAX() check
> > > > > >
> > > > > > ... may be the culprit and the following fix may work...
> > > > > >
> > > > > > diff --git a/mm/filemap.c b/mm/filemap.c
> > > > > > index 3a7863ba51b9..7eaf74a2a39b 100644
> > > > > > --- a/mm/filemap.c
> > > > > > +++ b/mm/filemap.c
> > > > > > @@ -2257,7 +2257,7 @@ generic_file_read_iter(struct kiocb *iocb, struct iov_iter *iter)
> > > > > > if (!count)
> > > > > > goto out; /* skip atime */
> > > > > >
> > > > > > - if (iocb->ki_flags & IOCB_DIRECT) {
> > > > > > + if (iocb->ki_flags & IOCB_DIRECT || IS_DAX(inode)) {
> > > > > > struct file *file = iocb->ki_filp;
> > > > > > struct address_space *mapping = file->f_mapping;
> > > > > > struct inode *inode = mapping->host;
> > > > >
> > > > > Well, you'll have to up-level the inode variable instantiation,
> > > > > obviously. That solves this particular issue.
> > > >
> > > > Well... This seems to be a random issue. I've had BMC issues with
> > > > my server most of the day... But even with this patch I still get the failure
> > > > in read_pages(). :-/
> > > >
> > > > And I have gotten it to both succeed and fail with qemu... :-/
> > >
> > > ... here is the fix. I made the change in xfs_diflags_to_linux() early on with
> > > out factoring in the flag logic changes we have agreed upon...
> > >
> > > diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> > > index 62d9f622bad1..d592949ad396 100644
> > > --- a/fs/xfs/xfs_ioctl.c
> > > +++ b/fs/xfs/xfs_ioctl.c
> > > @@ -1123,11 +1123,11 @@ xfs_diflags_to_linux(
> > > inode->i_flags |= S_NOATIME;
> > > else
> > > inode->i_flags &= ~S_NOATIME;
> > > - if (xflags & FS_XFLAG_DAX)
> > > +
> > > + if (xfs_inode_enable_dax(ip))
> > > inode->i_flags |= S_DAX;
> > > else
> > > inode->i_flags &= ~S_DAX;
> > > -
> > > }
> > >
> > > But the one thing which tripped me up, and concerns me, is we have 2 functions
> > > which set the inode flags.
> > >
> > > xfs_diflags_to_iflags()
> > > xfs_diflags_to_linux()
> > >
> > > xfs_diflags_to_iflags() is geared toward initialization but logically they do
> > > the same thing. I see no reason to keep them separate. Does anyone?
> > >
> > > Based on this find, the discussion on behavior in this thread, and the comments
> > > from Dave I'm reworking the series because the flag check/set functions have
> > > all changed and I really want to be as clear as possible with both the patches
> > > and the resulting code.[*] So v4 should be out today including attempting to
> > > document what we have discussed here and being as clear as possible on the
> > > behavior. :-D
> > >
> > > Thanks so much for testing this!
> > >
> > > Ira
> > >
> > > [*] I will probably throw in a patch to remove xfs_diflags_to_iflags() as I
> > > really don't see a reason to keep it.
> > >
> >
> > I prefer you keep the one in xfs_iops.c since ioctls are a higher level
> > function than general inode operations.
>
> Makes sense. Do you prefer the xfs_diflags_to_iflags() name as well?
I don't really care one way or another, so ... iflags wins by arbitrary
choice! 8)
--D
> Ira
>
> >
> > --D
^ permalink raw reply
* Re: [PATCH 1/2] mm: move force_dma_unencrypted() to mem_encrypt.h
From: Christian Borntraeger @ 2020-02-20 17:00 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Halil Pasic, Michael S. Tsirkin, Jason Wang, Marek Szyprowski,
Robin Murphy, linux-s390, virtualization, linux-kernel, iommu,
Janosch Frank, Viktor Mihajlovski, Cornelia Huck, Ram Pai,
Thiago Jung Bauermann, David Gibson, Lendacky, Thomas,
Michael Mueller
In-Reply-To: <20200220163135.GA13192@lst.de>
On 20.02.20 17:31, Christoph Hellwig wrote:
> On Thu, Feb 20, 2020 at 05:23:20PM +0100, Christian Borntraeger wrote:
>> >From a users perspective it makes absolutely perfect sense to use the
>> bounce buffers when they are NEEDED.
>> Forcing the user to specify iommu_platform just because you need bounce buffers
>> really feels wrong. And obviously we have a severe performance issue
>> because of the indirections.
>
> The point is that the user should not have to specify iommu_platform.
I (and Halil) agree on that. From a user perspective we want to
have the right thing in the guest without any command option. The iommu_plaform
thing was indeed a first step to make things work.
I might mis-read Halils patch, but isnt one effect of this patch set
that things WILL work without iommu_platform?
> We need to make sure any new hypervisor (especially one that might require
> bounce buffering) always sets it, as was a rather bogus legacy hack
> that isn't extensibe for cases that for example require bounce buffering.
^ permalink raw reply
* Re: [PATCH 1/2] mm: move force_dma_unencrypted() to mem_encrypt.h
From: Christian Borntraeger @ 2020-02-20 17:00 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-s390, Janosch Frank, Michael S. Tsirkin, Jason Wang,
Cornelia Huck, Ram Pai, linux-kernel, virtualization, Halil Pasic,
iommu, David Gibson, Michael Mueller, Lendacky, Thomas,
Viktor Mihajlovski, Robin Murphy
In-Reply-To: <20200220163135.GA13192@lst.de>
On 20.02.20 17:31, Christoph Hellwig wrote:
> On Thu, Feb 20, 2020 at 05:23:20PM +0100, Christian Borntraeger wrote:
>> >From a users perspective it makes absolutely perfect sense to use the
>> bounce buffers when they are NEEDED.
>> Forcing the user to specify iommu_platform just because you need bounce buffers
>> really feels wrong. And obviously we have a severe performance issue
>> because of the indirections.
>
> The point is that the user should not have to specify iommu_platform.
I (and Halil) agree on that. From a user perspective we want to
have the right thing in the guest without any command option. The iommu_plaform
thing was indeed a first step to make things work.
I might mis-read Halils patch, but isnt one effect of this patch set
that things WILL work without iommu_platform?
> We need to make sure any new hypervisor (especially one that might require
> bounce buffering) always sets it, as was a rather bogus legacy hack
> that isn't extensibe for cases that for example require bounce buffering.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.