* Re: [PATCH v2 0/6] NFS: Add support for the v4.2 READ_PLUS operation
From: Chuck Lever @ 2020-02-20 18:29 UTC (permalink / raw)
To: Anna Schumaker; +Cc: Trond.Myklebust, Linux NFS Mailing List
In-Reply-To: <c20cd5a113d19b8287bcc3db9c1b594ecb0e61e4.camel@gmail.com>
> On Feb 20, 2020, at 1:25 PM, Anna Schumaker <schumaker.anna@gmail.com> wrote:
>
> On Thu, 2020-02-20 at 12:40 -0500, Chuck Lever wrote:
>> Hi Anna-
>>
>>> On Feb 14, 2020, at 4:12 PM, schumaker.anna@gmail.com wrote:
>>>
>>> From: Anna Schumaker <Anna.Schumaker@Netapp.com>
>>>
>>> These patches add client support for the READ_PLUS operation, which
>>> breaks read requests into several "data" and "hole" segments when
>>> replying to the client. I also add a "noreadplus" mount option to allow
>>> users to disable the new operation if it becomes a problem, similar to
>>> the "nordirplus" mount option that we already have.
>>
>> Hrm, I went looking for the patch that adds "noreadplus", but I
>> don't see it in this series?
>
> You suggested dropping that patch in the v1 posting and waiting to see if
> anybody asks for it.
Yes, recalling that now.
I requested that because I don't like to add administrative interfaces
if there isn't a clear need. I think we might have one now!
>> Wondering if, to start off, the default should be "noreadplus"
>> until our feet are under us. Just a thought.
>
> I could re-add the patch with this as the default if that's the way everybody
> wants to go.
Yep, I'm interested in other opinions too.
> Anna
>
>>
>>
>>> Here are the results of some performance tests I ran on Netapp lab
>>> machines. I tested by reading various 2G files from a few different
>>> undelying filesystems and across several NFS versions. I used the
>>> `vmtouch` utility to make sure files were only cached when we wanted
>>> them to be. In addition to 100% data and 100% hole cases, I also tested
>>> with files that alternate between data and hole segments. These files
>>> have either 4K, 8K, 16K, or 32K segment sizes and start with either data
>>> or hole segments. So the file mixed-4d has a 4K segment size beginning
>>> with a data segment, but mixed-32h hase 32K segments beginning with a
>>> hole. The units are in seconds, with the first number for each NFS
>>> version being the uncached read time and the second number is for when
>>> the file is cached on the server.
>>>
>>> ext4 | v3 | v4.0 | v4.1 | v4.2
>>> |
>>> ----------|-----------------|-----------------|-----------------|-----------
>>> ------|
>>> data | 22.909 : 18.253 | 22.934 : 18.252 | 22.902 : 18.253 | 23.485 :
>>> 18.253 |
>>> hole | 18.256 : 18.253 | 18.255 : 18.252 | 18.256 : 18.253 | 0.708
>>> : 0.709 |
>>> mixed-4d | 28.261 : 18.253 | 29.616 : 18.252 | 28.341 : 18.252 | 24.508
>>> : 9.150 |
>>> mixed-8d | 27.956 : 18.253 | 28.404 : 18.252 | 28.320 : 18.252 | 23.967
>>> : 9.140 |
>>> mixed-16d | 28.172 : 18.253 | 27.946 : 18.252 | 27.627 : 18.252 | 23.043
>>> : 9.134 |
>>> mixed-32d | 25.350 : 18.253 | 24.406 : 18.252 | 24.384 : 18.253 | 20.698
>>> : 9.132 |
>>> mixed-4h | 28.913 : 18.253 | 28.564 : 18.252 | 27.996 : 18.252 | 21.837
>>> : 9.150 |
>>> mixed-8h | 28.625 : 18.253 | 27.833 : 18.252 | 27.798 : 18.253 | 21.710
>>> : 9.140 |
>>> mixed-16h | 27.975 : 18.253 | 27.662 : 18.252 | 27.795 : 18.253 | 20.585
>>> : 9.134 |
>>> mixed-32h | 25.958 : 18.253 | 25.491 : 18.252 | 24.856 : 18.252 | 21.018
>>> : 9.132 |
>>>
>>> xfs | v3 | v4.0 | v4.1 | v4.2
>>> |
>>> ----------|-----------------|-----------------|-----------------|-----------
>>> ------|
>>> data | 22.041 : 18.253 | 22.618 : 18.252 | 23.067 : 18.253 | 23.496 :
>>> 18.253 |
>>> hole | 18.256 : 18.253 | 18.255 : 18.252 | 18.256 : 18.253 | 0.723
>>> : 0.708 |
>>> mixed-4d | 29.417 : 18.253 | 28.503 : 18.252 | 28.671 : 18.253 | 24.957
>>> : 9.150 |
>>> mixed-8d | 29.080 : 18.253 | 29.401 : 18.252 | 29.251 : 18.252 | 24.625
>>> : 9.140 |
>>> mixed-16d | 27.638 : 18.253 | 28.606 : 18.252 | 27.871 : 18.253 | 25.511
>>> : 9.135 |
>>> mixed-32d | 24.967 : 18.253 | 25.239 : 18.252 | 25.434 : 18.252 | 21.728
>>> : 9.132 |
>>> mixed-4h | 34.816 : 18.253 | 36.243 : 18.252 | 35.837 : 18.252 | 32.332
>>> : 9.150 |
>>> mixed-8h | 43.469 : 18.253 | 44.009 : 18.252 | 43.810 : 18.253 | 37.962
>>> : 9.140 |
>>> mixed-16h | 29.280 : 18.253 | 28.563 : 18.252 | 28.241 : 18.252 | 22.116
>>> : 9.134 |
>>> mixed-32h | 29.428 : 18.253 | 29.378 : 18.252 | 28.808 : 18.253 | 27.378
>>> : 9.134 |
>>>
>>> btrfs | v3 | v4.0 | v4.1 | v4.2
>>> |
>>> ----------|-----------------|-----------------|-----------------|-----------
>>> ------|
>>> data | 25.547 : 18.253 | 25.053 : 18.252 | 24.209 : 18.253 | 32.121 :
>>> 18.253 |
>>> hole | 18.256 : 18.253 | 18.255 : 18.252 | 18.256 : 18.252 | 0.702
>>> : 0.724 |
>>> mixed-4d | 19.016 : 18.253 | 18.822 : 18.252 | 18.955 : 18.253 | 18.697
>>> : 9.150 |
>>> mixed-8d | 19.186 : 18.253 | 19.444 : 18.252 | 18.841 : 18.253 | 18.452
>>> : 9.140 |
>>> mixed-16d | 18.480 : 18.253 | 19.010 : 18.252 | 19.167 : 18.252 | 16.000
>>> : 9.134 |
>>> mixed-32d | 18.635 : 18.253 | 18.565 : 18.252 | 18.550 : 18.252 | 15.930
>>> : 9.132 |
>>> mixed-4h | 19.079 : 18.253 | 18.990 : 18.252 | 19.157 : 18.253 | 27.834
>>> : 9.150 |
>>> mixed-8h | 18.613 : 18.253 | 19.234 : 18.252 | 18.616 : 18.253 | 20.177
>>> : 9.140 |
>>> mixed-16h | 18.590 : 18.253 | 19.221 : 18.252 | 19.654 : 18.253 | 17.273
>>> : 9.135 |
>>> mixed-32h | 18.768 : 18.253 | 19.122 : 18.252 | 18.535 : 18.252 | 15.791
>>> : 9.132 |
>>>
>>> ext3 | v3 | v4.0 | v4.1 | v4.2
>>> |
>>> ----------|-----------------|-----------------|-----------------|-----------
>>> ------|
>>> data | 34.292 : 18.253 | 33.810 : 18.252 | 33.450 : 18.253 | 33.390 :
>>> 18.254 |
>>> hole | 18.256 : 18.253 | 18.255 : 18.252 | 18.256 : 18.253 | 0.718
>>> : 0.728 |
>>> mixed-4d | 46.818 : 18.253 | 47.140 : 18.252 | 48.385 : 18.253 | 42.887
>>> : 9.150 |
>>> mixed-8d | 58.554 : 18.253 | 59.277 : 18.252 | 59.673 : 18.253 | 56.760
>>> : 9.140 |
>>> mixed-16d | 44.631 : 18.253 | 44.291 : 18.252 | 44.729 : 18.253 | 40.237
>>> : 9.135 |
>>> mixed-32d | 39.110 : 18.253 | 38.735 : 18.252 | 38.902 : 18.252 | 35.270
>>> : 9.132 |
>>> mixed-4h | 56.396 : 18.253 | 56.387 : 18.252 | 56.573 : 18.253 | 67.661
>>> : 9.150 |
>>> mixed-8h | 58.483 : 18.253 | 58.484 : 18.252 | 59.099 : 18.253 | 77.958
>>> : 9.140 |
>>> mixed-16h | 42.511 : 18.253 | 42.338 : 18.252 | 42.356 : 18.252 | 51.805
>>> : 9.135 |
>>> mixed-32h | 38.419 : 18.253 | 38.504 : 18.252 | 38.643 : 18.252 | 40.411
>>> : 9.132 |
>>>
>>>
>>> Changes since v1:
>>> - Rebase to 5.6-rc1
>>> - Drop the mount option patch for now
>>> - Fix fallback to READ when the server doesn't support READ_PLUS
>>>
>>> Any questions?
>>> Anna
>>>
>>>
>>> Anna Schumaker (6):
>>> SUNRPC: Split out a function for setting current page
>>> SUNRPC: Add the ability to expand holes in data pages
>>> SUNRPC: Add the ability to shift data to a specific offset
>>> NFS: Add READ_PLUS data segment support
>>> NFS: Add READ_PLUS hole segment decoding
>>> NFS: Decode multiple READ_PLUS segments
>>>
>>> fs/nfs/nfs42xdr.c | 169 +++++++++++++++++++++++++
>>> fs/nfs/nfs4proc.c | 43 ++++++-
>>> fs/nfs/nfs4xdr.c | 1 +
>>> include/linux/nfs4.h | 2 +-
>>> include/linux/nfs_fs_sb.h | 1 +
>>> include/linux/nfs_xdr.h | 2 +-
>>> include/linux/sunrpc/xdr.h | 2 +
>>> net/sunrpc/xdr.c | 244 ++++++++++++++++++++++++++++++++++++-
>>> 8 files changed, 457 insertions(+), 7 deletions(-)
>>>
>>> --
>>> 2.25.0
>>>
>>
>> --
>> Chuck Lever
>> chucklever@gmail.com
--
Chuck Lever
chucklever@gmail.com
^ permalink raw reply
* Re: [PATCH v2 4/6] NFS: Add READ_PLUS data segment support
From: Anna Schumaker @ 2020-02-20 18:28 UTC (permalink / raw)
To: Chuck Lever; +Cc: Trond.Myklebust, Linux NFS Mailing List
In-Reply-To: <CD23428B-4105-4A52-864D-73CFA64E4551@oracle.com>
On Thu, 2020-02-20 at 09:55 -0500, Chuck Lever wrote:
> > On Feb 20, 2020, at 9:42 AM, Anna Schumaker <schumaker.anna@gmail.com>
> > wrote:
> >
> > On Fri, 2020-02-14 at 17:28 -0500, Chuck Lever wrote:
> > > > On Feb 14, 2020, at 4:12 PM, schumaker.anna@gmail.com wrote:
> > > >
> > > > From: Anna Schumaker <Anna.Schumaker@Netapp.com>
> > > >
> > > > This patch adds client support for decoding a single NFS4_CONTENT_DATA
> > > > segment returned by the server. This is the simplest implementation
> > > > possible, since it does not account for any hole segments in the reply.
> > > >
> > > > Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
> > > > ---
> > > > fs/nfs/nfs42xdr.c | 138 ++++++++++++++++++++++++++++++++++++++
> > > > fs/nfs/nfs4proc.c | 43 +++++++++++-
> > > > fs/nfs/nfs4xdr.c | 1 +
> > > > include/linux/nfs4.h | 2 +-
> > > > include/linux/nfs_fs_sb.h | 1 +
> > > > include/linux/nfs_xdr.h | 2 +-
> > > > 6 files changed, 182 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/fs/nfs/nfs42xdr.c b/fs/nfs/nfs42xdr.c
> > > > index c03f3246d6c5..bf118ecabe2c 100644
> > > > --- a/fs/nfs/nfs42xdr.c
> > > > +++ b/fs/nfs/nfs42xdr.c
> > > > @@ -45,6 +45,15 @@
> > > > #define encode_deallocate_maxsz (op_encode_hdr_maxsz + \
> > > > encode_fallocate_maxsz)
> > > > #define decode_deallocate_maxsz (op_decode_hdr_maxsz)
> > > > +#define encode_read_plus_maxsz (op_encode_hdr_maxsz + \
> > > > + encode_stateid_maxsz + 3)
> > > > +#define NFS42_READ_PLUS_SEGMENT_SIZE (1 /* data_content4 */ + \
> > > > + 2 /* data_info4.di_offset */ +
> > > > \
> > > > + 2 /* data_info4.di_length */)
> > > > +#define decode_read_plus_maxsz (op_decode_hdr_maxsz + \
> > > > + 1 /* rpr_eof */ + \
> > > > + 1 /* rpr_contents count */ + \
> > > > + NFS42_READ_PLUS_SEGMENT_SIZE)
> > > > #define encode_seek_maxsz (op_encode_hdr_maxsz + \
> > > > encode_stateid_maxsz + \
> > > > 2 /* offset */ + \
> > > > @@ -128,6 +137,14 @@
> > > > decode_putfh_maxsz + \
> > > > decode_deallocate_maxsz + \
> > > > decode_getattr_maxsz)
> > > > +#define NFS4_enc_read_plus_sz (compound_encode_hdr_maxsz + \
> > > > + encode_sequence_maxsz + \
> > > > + encode_putfh_maxsz + \
> > > > + encode_read_plus_maxsz)
> > > > +#define NFS4_dec_read_plus_sz (compound_decode_hdr_maxsz + \
> > > > + decode_sequence_maxsz + \
> > > > + decode_putfh_maxsz + \
> > > > + decode_read_plus_maxsz)
> > > > #define NFS4_enc_seek_sz (compound_encode_hdr_maxsz + \
> > > > encode_sequence_maxsz + \
> > > > encode_putfh_maxsz + \
> > > > @@ -252,6 +269,16 @@ static void encode_deallocate(struct xdr_stream
> > > > *xdr,
> > > > encode_fallocate(xdr, args);
> > > > }
> > > >
> > > > +static void encode_read_plus(struct xdr_stream *xdr,
> > > > + const struct nfs_pgio_args *args,
> > > > + struct compound_hdr *hdr)
> > > > +{
> > > > + encode_op_hdr(xdr, OP_READ_PLUS, decode_read_plus_maxsz, hdr);
> > > > + encode_nfs4_stateid(xdr, &args->stateid);
> > > > + encode_uint64(xdr, args->offset);
> > > > + encode_uint32(xdr, args->count);
> > > > +}
> > > > +
> > > > static void encode_seek(struct xdr_stream *xdr,
> > > > const struct nfs42_seek_args *args,
> > > > struct compound_hdr *hdr)
> > > > @@ -446,6 +473,29 @@ static void nfs4_xdr_enc_deallocate(struct rpc_rqst
> > > > *req,
> > > > encode_nops(&hdr);
> > > > }
> > > >
> > > > +/*
> > > > + * Encode READ_PLUS request
> > > > + */
> > > > +static void nfs4_xdr_enc_read_plus(struct rpc_rqst *req,
> > > > + struct xdr_stream *xdr,
> > > > + const void *data)
> > > > +{
> > > > + const struct nfs_pgio_args *args = data;
> > > > + struct compound_hdr hdr = {
> > > > + .minorversion = nfs4_xdr_minorversion(&args->seq_args),
> > > > + };
> > > > +
> > > > + encode_compound_hdr(xdr, req, &hdr);
> > > > + encode_sequence(xdr, &args->seq_args, &hdr);
> > > > + encode_putfh(xdr, args->fh, &hdr);
> > > > + encode_read_plus(xdr, args, &hdr);
> > > > +
> > > > + rpc_prepare_reply_pages(req, args->pages, args->pgbase,
> > > > + args->count, hdr.replen);
> > > > + req->rq_rcv_buf.flags |= XDRBUF_READ;
> > >
> > > IMO this line is incorrect.
> >
> > You're right, this line causes problems for RDMA with READ_PLUS. I added it
> > to
> > match how the other xdr read encoders were set up
>
> Ja, I think just removing that line should be sufficient.
> Better would be replacing it with a comment explaining
> why this encoder does not set XDRBUF_READ. :-)
>
>
> > > RFC 8267 Section 6.1 does not list any part of the result of READ_PLUS
> > > as DDP-eligible. There's no way for a client to know how to set up
> > > Write chunks, unless it knows exactly where the file's holes are in
> > > advance. Even then... racy.
> > >
> > > Just curious, have you tried READ_PLUS with proto=rdma ?
> >
> > I haven't done in-depth performance testing, but I have been able to run it.
>
> We should figure out whether that will have a regressive
> impact on NFS/RDMA workloads. I expect that it will, but
> the client can always set up the Reply chunk so that the
> READ payload fits precisely in an RDMA segment that lines
> up with page cache pages. That mitigates some impact.
>
> If your patch set already changes NFSv4.2 mounts to always
> use READ_PLUS in place of READ, it might be prudent for the
> "proto=rdma" mount option to also set "noreadplus", at least
> for the time being.
I can make this change.
>
> The down-side here is that would make NFSv4.2 on RDMA
> unable to recognize holes in files the same way as it
> does on TCP, and that's a pretty significant variation
> in behavior. Does "noreadplus" even deal with that?
Setting "noreadplus" just causes the client to use the READ operation instead,
so there should be no difference between v4.1 and v4.2 if the option is set.
Anna
>
>
> > Anna
> >
> > >
> > > > + encode_nops(&hdr);
> > > > +}
> > > > +
> > > > /*
> > > > * Encode SEEK request
> > > > */
> > > > @@ -694,6 +744,67 @@ static int decode_deallocate(struct xdr_stream
> > > > *xdr,
> > > > struct nfs42_falloc_res *re
> > > > return decode_op_hdr(xdr, OP_DEALLOCATE);
> > > > }
> > > >
> > > > +static uint32_t decode_read_plus_data(struct xdr_stream *xdr, struct
> > > > nfs_pgio_res *res,
> > > > + uint32_t *eof)
> > > > +{
> > > > + __be32 *p;
> > > > + uint32_t count, recvd;
> > > > + uint64_t offset;
> > > > +
> > > > + p = xdr_inline_decode(xdr, 8 + 4);
> > > > + if (unlikely(!p))
> > > > + return -EIO;
> > > > +
> > > > + p = xdr_decode_hyper(p, &offset);
> > > > + count = be32_to_cpup(p);
> > > > + if (count == 0)
> > > > + return 0;
> > > > +
> > > > + recvd = xdr_read_pages(xdr, count);
> > > > + if (count > recvd) {
> > > > + dprintk("NFS: server cheating in read reply: "
> > > > + "count %u > recvd %u\n", count, recvd);
> > > > + count = recvd;
> > > > + *eof = 0;
> > > > + }
> > > > +
> > > > + return count;
> > > > +}
> > > > +
> > > > +static int decode_read_plus(struct xdr_stream *xdr, struct nfs_pgio_res
> > > > *res)
> > > > +{
> > > > + __be32 *p;
> > > > + uint32_t count, eof, segments, type;
> > > > + int status;
> > > > +
> > > > + status = decode_op_hdr(xdr, OP_READ_PLUS);
> > > > + if (status)
> > > > + return status;
> > > > +
> > > > + p = xdr_inline_decode(xdr, 4 + 4);
> > > > + if (unlikely(!p))
> > > > + return -EIO;
> > > > +
> > > > + eof = be32_to_cpup(p++);
> > > > + segments = be32_to_cpup(p++);
> > > > + if (segments == 0)
> > > > + return 0;
> > > > +
> > > > + p = xdr_inline_decode(xdr, 4);
> > > > + if (unlikely(!p))
> > > > + return -EIO;
> > > > +
> > > > + type = be32_to_cpup(p++);
> > > > + if (type == NFS4_CONTENT_DATA)
> > > > + count = decode_read_plus_data(xdr, res, &eof);
> > > > + else
> > > > + return -EINVAL;
> > > > +
> > > > + res->eof = eof;
> > > > + res->count = count;
> > > > + return 0;
> > > > +}
> > > > +
> > > > static int decode_seek(struct xdr_stream *xdr, struct nfs42_seek_res
> > > > *res)
> > > > {
> > > > int status;
> > > > @@ -870,6 +981,33 @@ static int nfs4_xdr_dec_deallocate(struct rpc_rqst
> > > > *rqstp,
> > > > return status;
> > > > }
> > > >
> > > > +/*
> > > > + * Decode READ_PLUS request
> > > > + */
> > > > +static int nfs4_xdr_dec_read_plus(struct rpc_rqst *rqstp,
> > > > + struct xdr_stream *xdr,
> > > > + void *data)
> > > > +{
> > > > + struct nfs_pgio_res *res = data;
> > > > + struct compound_hdr hdr;
> > > > + int status;
> > > > +
> > > > + status = decode_compound_hdr(xdr, &hdr);
> > > > + if (status)
> > > > + goto out;
> > > > + status = decode_sequence(xdr, &res->seq_res, rqstp);
> > > > + if (status)
> > > > + goto out;
> > > > + status = decode_putfh(xdr);
> > > > + if (status)
> > > > + goto out;
> > > > + status = decode_read_plus(xdr, res);
> > > > + if (!status)
> > > > + status = res->count;
> > > > +out:
> > > > + return status;
> > > > +}
> > > > +
> > > > /*
> > > > * Decode SEEK request
> > > > */
> > > > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> > > > index 95d07a3dc5d1..ed3ec8c36273 100644
> > > > --- a/fs/nfs/nfs4proc.c
> > > > +++ b/fs/nfs/nfs4proc.c
> > > > @@ -69,6 +69,10 @@
> > > >
> > > > #include "nfs4trace.h"
> > > >
> > > > +#ifdef CONFIG_NFS_V4_2
> > > > +#include "nfs42.h"
> > > > +#endif /* CONFIG_NFS_V4_2 */
> > > > +
> > > > #define NFSDBG_FACILITY NFSDBG_PROC
> > > >
> > > > #define NFS4_BITMASK_SZ 3
> > > > @@ -5199,28 +5203,60 @@ static bool nfs4_read_stateid_changed(struct
> > > > rpc_task *task,
> > > > return true;
> > > > }
> > > >
> > > > +static bool nfs4_read_plus_not_supported(struct rpc_task *task,
> > > > + struct nfs_pgio_header *hdr)
> > > > +{
> > > > + struct nfs_server *server = NFS_SERVER(hdr->inode);
> > > > + struct rpc_message *msg = &task->tk_msg;
> > > > +
> > > > + if (msg->rpc_proc == &nfs4_procedures[NFSPROC4_CLNT_READ_PLUS]
> > > > &&
> > > > + server->caps & NFS_CAP_READ_PLUS && task->tk_status ==
> > > > -ENOTSUPP) {
> > > > + server->caps &= ~NFS_CAP_READ_PLUS;
> > > > + msg->rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_READ];
> > > > + rpc_restart_call_prepare(task);
> > > > + return true;
> > > > + }
> > > > + return false;
> > > > +}
> > > > +
> > > > static int nfs4_read_done(struct rpc_task *task, struct nfs_pgio_header
> > > > *hdr)
> > > > {
> > > > -
> > > > dprintk("--> %s\n", __func__);
> > > >
> > > > if (!nfs4_sequence_done(task, &hdr->res.seq_res))
> > > > return -EAGAIN;
> > > > if (nfs4_read_stateid_changed(task, &hdr->args))
> > > > return -EAGAIN;
> > > > + if (nfs4_read_plus_not_supported(task, hdr))
> > > > + return -EAGAIN;
> > > > if (task->tk_status > 0)
> > > > nfs_invalidate_atime(hdr->inode);
> > > > return hdr->pgio_done_cb ? hdr->pgio_done_cb(task, hdr) :
> > > > nfs4_read_done_cb(task, hdr);
> > > > }
> > > >
> > > > +#ifdef CONFIG_NFS_V4_2
> > > > +static void nfs42_read_plus_support(struct nfs_server *server, struct
> > > > rpc_message *msg)
> > > > +{
> > > > + if (server->caps & NFS_CAP_READ_PLUS)
> > > > + msg->rpc_proc =
> > > > &nfs4_procedures[NFSPROC4_CLNT_READ_PLUS];
> > > > + else
> > > > + msg->rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_READ];
> > > > +}
> > > > +#else
> > > > +static void nfs42_read_plus_support(struct nfs_server *server, struct
> > > > rpc_message *msg)
> > > > +{
> > > > + msg->rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_READ];
> > > > +}
> > > > +#endif /* CONFIG_NFS_V4_2 */
> > > > +
> > > > static void nfs4_proc_read_setup(struct nfs_pgio_header *hdr,
> > > > struct rpc_message *msg)
> > > > {
> > > > hdr->timestamp = jiffies;
> > > > if (!hdr->pgio_done_cb)
> > > > hdr->pgio_done_cb = nfs4_read_done_cb;
> > > > - msg->rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_READ];
> > > > + nfs42_read_plus_support(NFS_SERVER(hdr->inode), msg);
> > > > nfs4_init_sequence(&hdr->args.seq_args, &hdr->res.seq_res, 0,
> > > > 0);
> > > > }
> > > >
> > > > @@ -9970,7 +10006,8 @@ static const struct nfs4_minor_version_ops
> > > > nfs_v4_2_minor_ops = {
> > > > | NFS_CAP_SEEK
> > > > | NFS_CAP_LAYOUTSTATS
> > > > | NFS_CAP_CLONE
> > > > - | NFS_CAP_LAYOUTERROR,
> > > > + | NFS_CAP_LAYOUTERROR
> > > > + | NFS_CAP_READ_PLUS,
> > > > .init_client = nfs41_init_client,
> > > > .shutdown_client = nfs41_shutdown_client,
> > > > .match_stateid = nfs41_match_stateid,
> > > > diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
> > > > index 47817ef0aadb..68b2917d0537 100644
> > > > --- a/fs/nfs/nfs4xdr.c
> > > > +++ b/fs/nfs/nfs4xdr.c
> > > > @@ -7584,6 +7584,7 @@ const struct rpc_procinfo nfs4_procedures[] = {
> > > > PROC42(COPY_NOTIFY, enc_copy_notify, dec_copy_notify)
> > > > ,
> > > > PROC(LOOKUPP, enc_lookupp, dec_lookupp),
> > > > PROC42(LAYOUTERROR, enc_layouterror, dec_layouterror)
> > > > ,
> > > > + PROC42(READ_PLUS, enc_read_plus, dec_read_plus),
> > > > };
> > > >
> > > > static unsigned int nfs_version4_counts[ARRAY_SIZE(nfs4_procedures)];
> > > > diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
> > > > index 82d8fb422092..c1eeef52545c 100644
> > > > --- a/include/linux/nfs4.h
> > > > +++ b/include/linux/nfs4.h
> > > > @@ -540,8 +540,8 @@ enum {
> > > >
> > > > NFSPROC4_CLNT_LOOKUPP,
> > > > NFSPROC4_CLNT_LAYOUTERROR,
> > > > -
> > > > NFSPROC4_CLNT_COPY_NOTIFY,
> > > > + NFSPROC4_CLNT_READ_PLUS,
> > > > };
> > > >
> > > > /* nfs41 types */
> > > > diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
> > > > index 465fa98258a3..11248c5a7b24 100644
> > > > --- a/include/linux/nfs_fs_sb.h
> > > > +++ b/include/linux/nfs_fs_sb.h
> > > > @@ -281,5 +281,6 @@ struct nfs_server {
> > > > #define NFS_CAP_OFFLOAD_CANCEL (1U << 25)
> > > > #define NFS_CAP_LAYOUTERROR (1U << 26)
> > > > #define NFS_CAP_COPY_NOTIFY (1U << 27)
> > > > +#define NFS_CAP_READ_PLUS (1U << 28)
> > > >
> > > > #endif
> > > > diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> > > > index 94c77ed55ce1..8efbf3d8b263 100644
> > > > --- a/include/linux/nfs_xdr.h
> > > > +++ b/include/linux/nfs_xdr.h
> > > > @@ -655,7 +655,7 @@ struct nfs_pgio_args {
> > > > struct nfs_pgio_res {
> > > > struct nfs4_sequence_res seq_res;
> > > > struct nfs_fattr * fattr;
> > > > - __u32 count;
> > > > + __u64 count;
> > > > __u32 op_status;
> > > > union {
> > > > struct {
> > > > --
> > > > 2.25.0
> > > >
> > >
> > > --
> > > Chuck Lever
>
> --
> Chuck Lever
>
>
>
^ permalink raw reply
* Re: [PATCH v6 2/7] MIPS: JZ4780: Introduce SMP support.
From: Paul Cercueil @ 2020-02-20 18:27 UTC (permalink / raw)
To: 周琰杰 (Zhou Yanjie)
Cc: linux-mips, linux-kernel, linux-clk, devicetree, tglx, ralf,
paulburton, jiaxun.yang, chenhc, sboyd, mturquette, mark.rutland,
robh+dt, daniel.lezcano, geert+renesas, krzk, ebiederm,
miquel.raynal, keescook, sernia.zhou, zhenwenjin, dongsheng.qiu
In-Reply-To: <1582215889-113034-4-git-send-email-zhouyanjie@wanyeetech.com>
Hi Zhou,
Le ven., févr. 21, 2020 at 00:24, 周琰杰 (Zhou Yanjie)
<zhouyanjie@wanyeetech.com> a écrit :
> Forward port smp support from kernel 3.18.3 of CI20_linux
> to upstream kernel 5.6.
>
> Tested-by: H. Nikolaus Schaller <hns@goldelico.com>
> Tested-by: Paul Boddie <paul@boddie.org.uk>
> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
> Reviewed-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
>
> Notes:
> v1->v2:
> 1.Remove unnecessary "plat_irq_dispatch(void)" in irq-ingenic.c.
> 2.Add a timeout check for "jz4780_boot_secondary()" to avoid a
> dead loop.
> 3.Replace hard code in smp.c with macro.
>
> v2->v3:
> 1.Remove unnecessary "extern void (*r4k_blast_dcache)(void)" in
> smp.c.
> 2.Use "for_each_of_cpu_node" instead "for_each_compatible_node"
> in smp.c.
> 3.Use "of_cpu_node_to_id" instead "of_property_read_u32_index" in
> smp.c.
> 4.Move LCR related operations to jz4780-cgu.c.
>
> v3->v4:
> Rebase on top of kernel 5.6-rc1.
>
> v4->v5:
> 1.Splitting changes involving "jz4780-cgu.c" into separate commit.
> 2.Use "request_irq()" replace "setup_irq()".
>
> v5->v6:
> In order to have a kernel that works on multiple SoCs at the same
> time, use "IS_ENABLED()" replace "#ifdef".
>
> arch/mips/include/asm/mach-jz4740/smp.h | 81 ++++++++++
> arch/mips/jz4740/Kconfig | 3 +
> arch/mips/jz4740/Makefile | 5 +
> arch/mips/jz4740/prom.c | 4 +
> arch/mips/jz4740/smp-entry.S | 57 +++++++
> arch/mips/jz4740/smp.c | 254
> ++++++++++++++++++++++++++++++++
> arch/mips/kernel/idle.c | 36 ++++-
> 7 files changed, 439 insertions(+), 1 deletion(-)
> create mode 100644 arch/mips/include/asm/mach-jz4740/smp.h
> create mode 100644 arch/mips/jz4740/smp-entry.S
> create mode 100644 arch/mips/jz4740/smp.c
>
> diff --git a/arch/mips/include/asm/mach-jz4740/smp.h
> b/arch/mips/include/asm/mach-jz4740/smp.h
> new file mode 100644
> index 00000000..af3c21b
> --- /dev/null
> +++ b/arch/mips/include/asm/mach-jz4740/smp.h
> @@ -0,0 +1,81 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2013, Paul Burton <paul.burton@imgtec.com>
> + * JZ4780 SMP definitions
> + */
> +
> +#ifndef __MIPS_ASM_MACH_JZ4740_JZ4780_SMP_H__
> +#define __MIPS_ASM_MACH_JZ4740_JZ4780_SMP_H__
> +
> +#define read_c0_corectrl() __read_32bit_c0_register($12, 2)
> +#define write_c0_corectrl(val) __write_32bit_c0_register($12, 2,
> val)
> +
> +#define read_c0_corestatus() __read_32bit_c0_register($12, 3)
> +#define write_c0_corestatus(val) __write_32bit_c0_register($12, 3,
> val)
> +
> +#define read_c0_reim() __read_32bit_c0_register($12, 4)
> +#define write_c0_reim(val) __write_32bit_c0_register($12, 4, val)
> +
> +#define read_c0_mailbox0() __read_32bit_c0_register($20, 0)
> +#define write_c0_mailbox0(val) __write_32bit_c0_register($20, 0,
> val)
> +
> +#define read_c0_mailbox1() __read_32bit_c0_register($20, 1)
> +#define write_c0_mailbox1(val) __write_32bit_c0_register($20, 1,
> val)
> +
> +#define smp_clr_pending(mask) do { \
> + unsigned int stat; \
> + stat = read_c0_corestatus(); \
> + stat &= ~((mask) & 0xff); \
> + write_c0_corestatus(stat); \
> + } while (0)
> +
> +/*
> + * Core Control register
> + */
> +#define CORECTRL_SLEEP1M_SHIFT 17
> +#define CORECTRL_SLEEP1M (_ULCAST_(0x1) << CORECTRL_SLEEP1M_SHIFT)
> +#define CORECTRL_SLEEP0M_SHIFT 16
> +#define CORECTRL_SLEEP0M (_ULCAST_(0x1) << CORECTRL_SLEEP0M_SHIFT)
> +#define CORECTRL_RPC1_SHIFT 9
> +#define CORECTRL_RPC1 (_ULCAST_(0x1) << CORECTRL_RPC1_SHIFT)
> +#define CORECTRL_RPC0_SHIFT 8
> +#define CORECTRL_RPC0 (_ULCAST_(0x1) << CORECTRL_RPC0_SHIFT)
> +#define CORECTRL_SWRST1_SHIFT 1
> +#define CORECTRL_SWRST1 (_ULCAST_(0x1) << CORECTRL_SWRST1_SHIFT)
> +#define CORECTRL_SWRST0_SHIFT 0
> +#define CORECTRL_SWRST0 (_ULCAST_(0x1) << CORECTRL_SWRST0_SHIFT)
> +
> +/*
> + * Core Status register
> + */
> +#define CORESTATUS_SLEEP1_SHIFT 17
> +#define CORESTATUS_SLEEP1 (_ULCAST_(0x1) << CORESTATUS_SLEEP1_SHIFT)
> +#define CORESTATUS_SLEEP0_SHIFT 16
> +#define CORESTATUS_SLEEP0 (_ULCAST_(0x1) << CORESTATUS_SLEEP0_SHIFT)
> +#define CORESTATUS_IRQ1P_SHIFT 9
> +#define CORESTATUS_IRQ1P (_ULCAST_(0x1) << CORESTATUS_IRQ1P_SHIFT)
> +#define CORESTATUS_IRQ0P_SHIFT 8
> +#define CORESTATUS_IRQ0P (_ULCAST_(0x1) << CORESTATUS_IRQ8P_SHIFT)
> +#define CORESTATUS_MIRQ1P_SHIFT 1
> +#define CORESTATUS_MIRQ1P (_ULCAST_(0x1) << CORESTATUS_MIRQ1P_SHIFT)
> +#define CORESTATUS_MIRQ0P_SHIFT 0
> +#define CORESTATUS_MIRQ0P (_ULCAST_(0x1) << CORESTATUS_MIRQ0P_SHIFT)
> +
> +/*
> + * Reset Entry & IRQ Mask register
> + */
> +#define REIM_ENTRY_SHIFT 16
> +#define REIM_ENTRY (_ULCAST_(0xffff) << REIM_ENTRY_SHIFT)
> +#define REIM_IRQ1M_SHIFT 9
> +#define REIM_IRQ1M (_ULCAST_(0x1) << REIM_IRQ1M_SHIFT)
> +#define REIM_IRQ0M_SHIFT 8
> +#define REIM_IRQ0M (_ULCAST_(0x1) << REIM_IRQ0M_SHIFT)
> +#define REIM_MBOXIRQ1M_SHIFT 1
> +#define REIM_MBOXIRQ1M (_ULCAST_(0x1) << REIM_MBOXIRQ1M_SHIFT)
> +#define REIM_MBOXIRQ0M_SHIFT 0
> +#define REIM_MBOXIRQ0M (_ULCAST_(0x1) << REIM_MBOXIRQ0M_SHIFT)
> +
> +extern void jz4780_smp_init(void);
> +extern void jz4780_secondary_cpu_entry(void);
> +
> +#endif /* __MIPS_ASM_MACH_JZ4740_JZ4780_SMP_H__ */
> diff --git a/arch/mips/jz4740/Kconfig b/arch/mips/jz4740/Kconfig
> index 412d2fa..0239597 100644
> --- a/arch/mips/jz4740/Kconfig
> +++ b/arch/mips/jz4740/Kconfig
> @@ -34,9 +34,12 @@ config MACH_JZ4770
>
> config MACH_JZ4780
> bool
> + select GENERIC_CLOCKEVENTS_BROADCAST if SMP
> select MIPS_CPU_SCACHE
> + select NR_CPUS_DEFAULT_2
> select SYS_HAS_CPU_MIPS32_R2
> select SYS_SUPPORTS_HIGHMEM
> + select SYS_SUPPORTS_SMP
>
> config MACH_X1000
> bool
> diff --git a/arch/mips/jz4740/Makefile b/arch/mips/jz4740/Makefile
> index 6de14c0..0a0f024 100644
> --- a/arch/mips/jz4740/Makefile
> +++ b/arch/mips/jz4740/Makefile
> @@ -12,3 +12,8 @@ CFLAGS_setup.o =
> -I$(src)/../../../scripts/dtc/libfdt
> # PM support
>
> obj-$(CONFIG_PM) += pm.o
> +
> +# SMP support
> +
> +obj-$(CONFIG_SMP) += smp.o
> +obj-$(CONFIG_SMP) += smp-entry.o
> diff --git a/arch/mips/jz4740/prom.c b/arch/mips/jz4740/prom.c
> index ff4555c..026259e 100644
> --- a/arch/mips/jz4740/prom.c
> +++ b/arch/mips/jz4740/prom.c
> @@ -8,10 +8,14 @@
>
> #include <asm/bootinfo.h>
> #include <asm/fw/fw.h>
> +#include <asm/mach-jz4740/smp.h>
>
> void __init prom_init(void)
> {
> fw_init_cmdline();
> +
> + if (IS_ENABLED(CONFIG_MACH_JZ4780) && IS_ENABLED(CONFIG_SMP))
> + jz4780_smp_init();
I guess it's OK for a first version, but SMP should really be decoupled
from the SoC version. A SMP-enabled kernel should be able to also
support non-SMP SoCs.
> }
>
> void __init prom_free_prom_memory(void)
> diff --git a/arch/mips/jz4740/smp-entry.S
> b/arch/mips/jz4740/smp-entry.S
> new file mode 100644
> index 00000000..20049a3
> --- /dev/null
> +++ b/arch/mips/jz4740/smp-entry.S
> @@ -0,0 +1,57 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2013, Paul Burton <paul.burton@imgtec.com>
> + * JZ4780 SMP entry point
> + */
> +
> +#include <asm/addrspace.h>
> +#include <asm/asm.h>
> +#include <asm/asmmacro.h>
> +#include <asm/cacheops.h>
> +#include <asm/mipsregs.h>
> +
> +#define CACHE_SIZE (32 * 1024)
> +#define CACHE_LINESIZE 32
> +
> +.extern jz4780_cpu_entry_sp
> +.extern jz4780_cpu_entry_gp
> +
> +.section .text.smp-entry
> +.balign 0x10000
> +.set noreorder
> +LEAF(jz4780_secondary_cpu_entry)
> + mtc0 zero, CP0_CAUSE
> +
> + li t0, ST0_CU0
> + mtc0 t0, CP0_STATUS
> +
> + /* cache setup */
> + li t0, KSEG0
> + ori t1, t0, CACHE_SIZE
> + mtc0 zero, CP0_TAGLO, 0
> +1: cache Index_Store_Tag_I, 0(t0)
> + cache Index_Store_Tag_D, 0(t0)
> + bne t0, t1, 1b
> + addiu t0, t0, CACHE_LINESIZE
> +
> + /* kseg0 cache attribute */
> + mfc0 t0, CP0_CONFIG, 0
> + ori t0, t0, CONF_CM_CACHABLE_NONCOHERENT
> + mtc0 t0, CP0_CONFIG, 0
> +
> + /* pagemask */
> + mtc0 zero, CP0_PAGEMASK, 0
> +
> + /* retrieve sp */
> + la t0, jz4780_cpu_entry_sp
> + lw sp, 0(t0)
> +
> + /* retrieve gp */
> + la t0, jz4780_cpu_entry_gp
> + lw gp, 0(t0)
> +
> + /* jump to the kernel in kseg0 */
> + la t0, smp_bootstrap
> + jr t0
> + nop
> + END(jz4780_secondary_cpu_entry)
> diff --git a/arch/mips/jz4740/smp.c b/arch/mips/jz4740/smp.c
> new file mode 100644
> index 00000000..5fe0cf3
> --- /dev/null
> +++ b/arch/mips/jz4740/smp.c
> @@ -0,0 +1,254 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2013, Paul Burton <paul.burton@imgtec.com>
> + * JZ4780 SMP
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/of.h>
> +#include <linux/sched.h>
> +#include <linux/sched/task_stack.h>
> +#include <linux/smp.h>
> +#include <linux/tick.h>
> +#include <asm/mach-jz4740/smp.h>
> +#include <asm/smp-ops.h>
> +
> +static struct clk *cpu_clock_gates[CONFIG_NR_CPUS] = { NULL };
> +
> +u32 jz4780_cpu_entry_sp;
> +u32 jz4780_cpu_entry_gp;
> +
> +static struct cpumask cpu_running;
This is not used anywhere, is it?
> +
> +static DEFINE_SPINLOCK(smp_lock);
> +
> +static irqreturn_t mbox_handler(int irq, void *dev_id)
> +{
> + int cpu = smp_processor_id();
> + u32 action, status;
> +
> + spin_lock(&smp_lock);
> +
> + switch (cpu) {
> + case 0:
> + action = read_c0_mailbox0();
> + write_c0_mailbox0(0);
> + break;
> + case 1:
> + action = read_c0_mailbox1();
> + write_c0_mailbox1(0);
> + break;
> + default:
> + panic("unhandled cpu %d!", cpu);
XBurst has 4 mailboxes, maybe add them here?
> + }
> +
> + /* clear pending mailbox interrupt */
> + status = read_c0_corestatus();
> + status &= ~(CORESTATUS_MIRQ0P << cpu);
> + write_c0_corestatus(status);
> +
> + spin_unlock(&smp_lock);
> +
> + if (action & SMP_RESCHEDULE_YOURSELF)
> + scheduler_ipi();
> + if (action & SMP_CALL_FUNCTION)
> + generic_smp_call_function_interrupt();
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void jz4780_smp_setup(void)
> +{
> + u32 addr, reim;
> + int cpu;
> +
> + reim = read_c0_reim();
> +
> + for (cpu = 0; cpu < NR_CPUS; cpu++) {
> + __cpu_number_map[cpu] = cpu;
> + __cpu_logical_map[cpu] = cpu;
> + set_cpu_possible(cpu, true);
> + }
> +
> + /* mask mailbox interrupts for this core */
> + reim &= ~REIM_MBOXIRQ0M;
> + write_c0_reim(reim);
> +
> + /* clear mailboxes & pending mailbox IRQs */
> + write_c0_mailbox0(0);
> + write_c0_mailbox1(0);
> + write_c0_corestatus(0);
> +
> + /* set reset entry point */
> + addr = KSEG1ADDR((u32)&jz4780_secondary_cpu_entry);
> + WARN_ON(addr & ~REIM_ENTRY);
> + reim &= ~REIM_ENTRY;
> + reim |= addr & REIM_ENTRY;
> +
> + /* unmask mailbox interrupts for this core */
> + reim |= REIM_MBOXIRQ0M;
> + write_c0_reim(reim);
> + set_c0_status(STATUSF_IP3);
> + irq_enable_hazard();
> +
> + cpumask_set_cpu(cpu, &cpu_running);
> +}
> +
> +static void jz4780_smp_prepare_cpus(unsigned int max_cpus)
> +{
> + struct device_node *cpu_node;
> + unsigned cpu, ctrl;
> + int err;
> +
> + /* setup the mailbox IRQ */
> + err = request_irq(MIPS_CPU_IRQ_BASE + 3, mbox_handler,
> + IRQF_PERCPU | IRQF_NO_THREAD, "core mailbox", NULL);
> + if (err)
> + pr_err("request_irq() on core mailbox failed\n");
> +
> + init_cpu_present(cpu_possible_mask);
jz4780_smp_setup() marks CPUs 0..NR_CPUS as possible, NR_CPUS being a
config option that can be anywhere between 2 and 256. This basically
says that up to 256 CPUs are actually present.
> +
> + ctrl = read_c0_corectrl();
> +
> + for (cpu = 0; cpu < max_cpus; cpu++) {
> + /* use reset entry point from REIM register */
> + ctrl |= CORECTRL_RPC0 << cpu;
max_cpus is just NR_CPUS, so this does not work. You should move this
in the "for_each_of_cpu_node" below.
> + }
> +
> + for_each_of_cpu_node(cpu_node) {
> + cpu = of_cpu_node_to_id(cpu_node);
> + if (cpu < 0) {
> + pr_err("Failed to read index of %s\n",
> + cpu_node->full_name);
> + continue;
> + }
> +
> + cpu_clock_gates[cpu] = of_clk_get(cpu_node, 0);
> + if (IS_ERR(cpu_clock_gates[cpu])) {
> + cpu_clock_gates[cpu] = NULL;
> + continue;
> + }
> +
> + err = clk_prepare(cpu_clock_gates[cpu]);
> + if (err)
> + pr_err("Failed to prepare CPU clock gate\n");
> + }
> +
> + write_c0_corectrl(ctrl);
> +}
> +
> +static int jz4780_boot_secondary(int cpu, struct task_struct *idle)
> +{
> + unsigned long flags;
> + u32 ctrl;
> +
> + spin_lock_irqsave(&smp_lock, flags);
> +
> + /* ensure the core is in reset */
> + ctrl = read_c0_corectrl();
> + ctrl |= CORECTRL_SWRST0 << cpu;
> + write_c0_corectrl(ctrl);
> +
> + /* ungate core clock */
> + if (cpu_clock_gates[cpu])
> + clk_enable(cpu_clock_gates[cpu]);
> +
> + /* set entry sp/gp register values */
> + jz4780_cpu_entry_sp = __KSTK_TOS(idle);
> + jz4780_cpu_entry_gp = (u32)task_thread_info(idle);
> + smp_wmb();
> +
> + /* take the core out of reset */
> + ctrl &= ~(CORECTRL_SWRST0 << cpu);
> + write_c0_corectrl(ctrl);
> +
> + cpumask_set_cpu(cpu, &cpu_running);
> +
> + spin_unlock_irqrestore(&smp_lock, flags);
> +
> + return 0;
> +}
> +
> +static void jz4780_init_secondary(void)
> +{
> +}
> +
> +static void jz4780_smp_finish(void)
> +{
> + u32 reim;
> +
> + spin_lock(&smp_lock);
> +
> + /* unmask mailbox interrupts for this core */
> + reim = read_c0_reim();
> + reim |= REIM_MBOXIRQ0M << smp_processor_id();
> + write_c0_reim(reim);
> +
> + spin_unlock(&smp_lock);
> +
> + /* unmask interrupts for this core */
> + change_c0_status(ST0_IM, STATUSF_IP3 | STATUSF_IP2 |
> + STATUSF_IP1 | STATUSF_IP0);
> + irq_enable_hazard();
> +
> + /* force broadcast timer */
> + tick_broadcast_force();
> +}
> +
> +static void jz4780_send_ipi_single_locked(int cpu, unsigned int
> action)
> +{
> + u32 mbox;
> +
> + switch (cpu) {
> + case 0:
> + mbox = read_c0_mailbox0();
> + write_c0_mailbox0(mbox | action);
> + break;
> + case 1:
> + mbox = read_c0_mailbox1();
> + write_c0_mailbox1(mbox | action);
> + break;
Same as above - there are 4 mailboxes.
> + default:
> + panic("unhandled cpu %d!", cpu);
> + }
> +}
> +
> +static void jz4780_send_ipi_single(int cpu, unsigned int action)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(&smp_lock, flags);
> + jz4780_send_ipi_single_locked(cpu, action);
> + spin_unlock_irqrestore(&smp_lock, flags);
> +}
> +
> +static void jz4780_send_ipi_mask(const struct cpumask *mask,
> + unsigned int action)
> +{
> + unsigned long flags;
> + int cpu;
> +
> + spin_lock_irqsave(&smp_lock, flags);
> +
> + for_each_cpu(cpu, mask)
> + jz4780_send_ipi_single_locked(cpu, action);
> +
> + spin_unlock_irqrestore(&smp_lock, flags);
> +}
> +
> +static struct plat_smp_ops jz4780_smp_ops = {
> + .send_ipi_single = jz4780_send_ipi_single,
> + .send_ipi_mask = jz4780_send_ipi_mask,
> + .init_secondary = jz4780_init_secondary,
> + .smp_finish = jz4780_smp_finish,
> + .boot_secondary = jz4780_boot_secondary,
> + .smp_setup = jz4780_smp_setup,
> + .prepare_cpus = jz4780_smp_prepare_cpus,
> +};
> +
> +void jz4780_smp_init(void)
This one can be marked __init.
> +{
> + register_smp_ops(&jz4780_smp_ops);
> +}
> diff --git a/arch/mips/kernel/idle.c b/arch/mips/kernel/idle.c
> index 37f8e78..a3afd98 100644
> --- a/arch/mips/kernel/idle.c
> +++ b/arch/mips/kernel/idle.c
> @@ -18,6 +18,7 @@
> #include <asm/cpu-type.h>
> #include <asm/idle.h>
> #include <asm/mipsregs.h>
> +#include <asm/r4kcache.h>
>
> /*
> * Not all of the MIPS CPUs have the "wait" instruction available.
> Moreover,
> @@ -88,6 +89,34 @@ static void __cpuidle rm7k_wait_irqoff(void)
> }
>
> /*
> + * The Ingenic jz4780 SMP variant has to write back dirty cache
> lines before
> + * executing wait. The CPU & cache clock will be gated until we
> return from
> + * the wait, and if another core attempts to access data from our
> data cache
> + * during this time then it will lock up.
> + */
> +void jz4780_smp_wait_irqoff(void)
> +{
> + unsigned long pending = read_c0_cause() & read_c0_status() &
> CAUSEF_IP;
> +
> + /*
> + * Going to idle has a significant overhead due to the cache flush
> so
> + * try to avoid it if we'll immediately be woken again due to an
> IRQ.
> + */
> + if (!need_resched() && !pending) {
> + r4k_blast_dcache();
> +
> + __asm__(
> + " .set push \n"
> + " .set mips3 \n"
> + " sync \n"
> + " wait \n"
> + " .set pop \n");
> + }
> +
> + local_irq_enable();
> +}
> +
> +/*
> * Au1 'wait' is only useful when the 32kHz counter is used as timer,
> * since coreclock (and the cp0 counter) stops upon executing it.
> Only an
> * interrupt can wake it, so they must be enabled before entering
> idle modes.
> @@ -172,7 +201,6 @@ void __init check_wait(void)
> case CPU_CAVIUM_OCTEON_PLUS:
> case CPU_CAVIUM_OCTEON2:
> case CPU_CAVIUM_OCTEON3:
> - case CPU_XBURST:
> case CPU_LOONGSON32:
> case CPU_XLR:
> case CPU_XLP:
> @@ -246,6 +274,12 @@ void __init check_wait(void)
> cpu_wait = r4k_wait;
> */
> break;
> + case CPU_XBURST:
> + if (IS_ENABLED(CONFIG_MACH_JZ4780) &&
> + IS_ENABLED(CONFIG_SMP) && (NR_CPUS > 1))
> + cpu_wait = jz4780_smp_wait_irqoff;
> + else
> + cpu_wait = r4k_wait;
> default:
> break;
> }
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH v2 3/4] PCI: brcmstb: Wait for Raspberry Pi's firmware when present
From: Florian Fainelli @ 2020-02-20 18:27 UTC (permalink / raw)
To: Nicolas Saenz Julienne, Florian Fainelli, linux-kernel,
bcm-kernel-feedback-list, Lorenzo Pieralisi, Andrew Murray
Cc: tim.gover, gregkh, linux-usb, linux-rpi-kernel, linux-pci,
Bjorn Helgaas, linux-arm-kernel, wahrenst
In-Reply-To: <cab8c0d70fd30c49579199d002b81b87ed34a920.camel@suse.de>
On 2/20/2020 10:14 AM, Nicolas Saenz Julienne wrote:
> On Wed, 2020-02-19 at 11:21 -0800, Florian Fainelli wrote:
>> On 2/19/20 4:39 AM, Nicolas Saenz Julienne wrote:
>>> xHCI's PCI fixup, run at the end of pcie-brcmstb's probe, depends on
>>> RPi4's VideoCore firmware interface to be up and running. It's possible
>>> for both initializations to race, so make sure it's available prior
>>> starting.
>>>
>>> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
>>
>> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
>>
>> It does not look like there is something making sure that
>> CONFIG_RASPBERRYPI_FIRMWARE is being selected or depended on, should we
>> have a "default XHCI_PCI" added to drivers/firmware/Kconfig?
>
> I think having that would enable the firmware interface for all XHCI_PCI users,
> which isn't ideal. The firmware call has stubs for the case the firmware
> interace isn't compiled, so no problem there. Ultimately we want to enable
> CONFIG_RASPBERRYPI_FIRMWARE only when the built image targets the RPi4
> (reglardless of being arm64/arm32). But I don't think that's feasible.
It would enable the driver, which is only functional if the matching
Device Tree node is present, that seems like a reasonable price to pay
for a multiplatform kernel. After all, this is a functional dependency
for the Pi4.
--
Florian
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 3/4] PCI: brcmstb: Wait for Raspberry Pi's firmware when present
From: Florian Fainelli @ 2020-02-20 18:27 UTC (permalink / raw)
To: Nicolas Saenz Julienne, Florian Fainelli, linux-kernel,
bcm-kernel-feedback-list, Lorenzo Pieralisi, Andrew Murray
Cc: linux-usb, linux-rpi-kernel, linux-arm-kernel, gregkh, tim.gover,
linux-pci, wahrenst, Bjorn Helgaas
In-Reply-To: <cab8c0d70fd30c49579199d002b81b87ed34a920.camel@suse.de>
On 2/20/2020 10:14 AM, Nicolas Saenz Julienne wrote:
> On Wed, 2020-02-19 at 11:21 -0800, Florian Fainelli wrote:
>> On 2/19/20 4:39 AM, Nicolas Saenz Julienne wrote:
>>> xHCI's PCI fixup, run at the end of pcie-brcmstb's probe, depends on
>>> RPi4's VideoCore firmware interface to be up and running. It's possible
>>> for both initializations to race, so make sure it's available prior
>>> starting.
>>>
>>> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
>>
>> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
>>
>> It does not look like there is something making sure that
>> CONFIG_RASPBERRYPI_FIRMWARE is being selected or depended on, should we
>> have a "default XHCI_PCI" added to drivers/firmware/Kconfig?
>
> I think having that would enable the firmware interface for all XHCI_PCI users,
> which isn't ideal. The firmware call has stubs for the case the firmware
> interace isn't compiled, so no problem there. Ultimately we want to enable
> CONFIG_RASPBERRYPI_FIRMWARE only when the built image targets the RPi4
> (reglardless of being arm64/arm32). But I don't think that's feasible.
It would enable the driver, which is only functional if the matching
Device Tree node is present, that seems like a reasonable price to pay
for a multiplatform kernel. After all, this is a functional dependency
for the Pi4.
--
Florian
^ permalink raw reply
* BUG: git clean -d cannot remove files from read-only directories
From: Adam Milazzo @ 2020-02-20 18:27 UTC (permalink / raw)
To: git@vger.kernel.org
Repro steps:
1. git init
2. mkdir d
3. touch d/a
4. chmod -w d
5. git clean -fd
Actual result:
Git doesn't remove anything, saying "warning: failed to remove d/a".
Expected result:
Git should remove the subdirectory 'd' along with its contents. Note that git can remove a read-only file (touch b; chmod -w b; git clean -f) with no problem.
Environment:
git version 2.7.4 on GNU/Linux
Why this is important:
* This has a significant impact in real scenarios: the go language, when using modules, will download referenced modules and place them in read-only directories inside a cache directory, potentially inside the git repository. These cached modules can't be cleaned up by git clean. (Furthermore, git clean then returns a failure status code, which causes our build system to fail. I don't want to ignore the failure code.)
* There is no simple and reliable way to find out which directories I would need to chmod u+w in order to allow git to delete their contents. I'd have to effectively replicate git's algorithm for determining tracked files. "git clean" is invoked at the root of the repository and the repository contains many projects worked on by different teams, so there's no fixed list of things I can manually chmod to make "git clean" work reliably.
* It seems inconsistent for "git clean" to be able to remove read-only files but not files from read-only directories.
^ permalink raw reply
* [PATCH v2 0/4] msm/gpu/a6xx: use the DMA-API for GMU memory allocations
From: Jordan Crouse @ 2020-02-20 18:26 UTC (permalink / raw)
To: linux-arm-msm
Cc: smasetty, John Stultz, Bjorn Andersson, Sean Paul, devicetree,
Stephen Boyd, Douglas Anderson, linux-kernel, dri-devel,
Rob Herring, Rob Clark, David Airlie, Andy Gross, Mark Rutland,
freedreno, Daniel Vetter
When CONFIG_INIT_ON_ALLOC_DEFAULT_ON the GMU memory allocator runs afoul of
cache coherency issues because it is mapped as write-combine without clearing
the cache after it was zeroed.
Rather than duplicate the hacky workaround we use in the GEM allocator for the
same reason it turns out that we don't need to have a bespoke memory allocator
for the GMU anyway. It uses a flat, global address space and there are only
two relatively minor allocations anyway. In short, this is essentially what the
DMA API was created for so replace a bunch of memory management code with two
calls to allocate and free DMA memory and we're fine.
The only wrinkle is that the memory allocations need to be in a very specific
location in the GMU virtual address space so in order to get the iova allocator
to do the right thing we need to specify the dma-ranges property in the device
tree for the GMU node. Since we've not yet converted the GMU bindings over to
YAML two patches quickly turn into four but at the end of it we have at least
one bindings file converted to YAML and 99 less lines of code to worry about.
v2: Fix the example bindings for dma-ranges - the third item is the size
Pass false to of_dma_configure so that it fails probe if the DMA region is not
set up.
Jordan Crouse (4):
dt-bindings: display: msm: Convert GMU bindings to YAML
dt-bindings: display: msm: Add required dma-range property
arm64: dts: sdm845: Set the virtual address range for GMU allocations
drm/msm/a6xx: Use the DMA API for GMU memory objects
.../devicetree/bindings/display/msm/gmu.txt | 116 -----------------
.../devicetree/bindings/display/msm/gmu.yaml | 140 +++++++++++++++++++++
arch/arm64/boot/dts/qcom/sdm845.dtsi | 2 +
drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 112 ++---------------
drivers/gpu/drm/msm/adreno/a6xx_gmu.h | 5 +-
5 files changed, 153 insertions(+), 222 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/display/msm/gmu.txt
create mode 100644 Documentation/devicetree/bindings/display/msm/gmu.yaml
--
2.7.4
^ permalink raw reply
* [PATCH v2 1/4] dt-bindings: display: msm: Convert GMU bindings to YAML
From: Jordan Crouse @ 2020-02-20 18:26 UTC (permalink / raw)
To: linux-arm-msm
Cc: smasetty, John Stultz, Sean Paul, devicetree, linux-kernel,
dri-devel, Rob Herring, Rob Clark, David Airlie, Mark Rutland,
freedreno, Daniel Vetter
In-Reply-To: <1582223216-23459-1-git-send-email-jcrouse@codeaurora.org>
Convert display/msm/gmu.txt to display/msm/gmu.yaml and remove the old
text bindings.
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
.../devicetree/bindings/display/msm/gmu.txt | 116 ------------------
.../devicetree/bindings/display/msm/gmu.yaml | 130 +++++++++++++++++++++
2 files changed, 130 insertions(+), 116 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/display/msm/gmu.txt
create mode 100644 Documentation/devicetree/bindings/display/msm/gmu.yaml
diff --git a/Documentation/devicetree/bindings/display/msm/gmu.txt b/Documentation/devicetree/bindings/display/msm/gmu.txt
deleted file mode 100644
index bf9c7a2..0000000
--- a/Documentation/devicetree/bindings/display/msm/gmu.txt
+++ /dev/null
@@ -1,116 +0,0 @@
-Qualcomm adreno/snapdragon GMU (Graphics management unit)
-
-The GMU is a programmable power controller for the GPU. the CPU controls the
-GMU which in turn handles power controls for the GPU.
-
-Required properties:
-- compatible: "qcom,adreno-gmu-XYZ.W", "qcom,adreno-gmu"
- for example: "qcom,adreno-gmu-630.2", "qcom,adreno-gmu"
- Note that you need to list the less specific "qcom,adreno-gmu"
- for generic matches and the more specific identifier to identify
- the specific device.
-- reg: Physical base address and length of the GMU registers.
-- reg-names: Matching names for the register regions
- * "gmu"
- * "gmu_pdc"
- * "gmu_pdc_seg"
-- interrupts: The interrupt signals from the GMU.
-- interrupt-names: Matching names for the interrupts
- * "hfi"
- * "gmu"
-- clocks: phandles to the device clocks
-- clock-names: Matching names for the clocks
- * "gmu"
- * "cxo"
- * "axi"
- * "mnoc"
-- power-domains: should be:
- <&clock_gpucc GPU_CX_GDSC>
- <&clock_gpucc GPU_GX_GDSC>
-- power-domain-names: Matching names for the power domains
-- iommus: phandle to the adreno iommu
-- operating-points-v2: phandle to the OPP operating points
-
-Optional properties:
-- sram: phandle to the On Chip Memory (OCMEM) that's present on some Snapdragon
- SoCs. See Documentation/devicetree/bindings/sram/qcom,ocmem.yaml.
-
-Example:
-
-/ {
- ...
-
- gmu: gmu@506a000 {
- compatible="qcom,adreno-gmu-630.2", "qcom,adreno-gmu";
-
- reg = <0x506a000 0x30000>,
- <0xb280000 0x10000>,
- <0xb480000 0x10000>;
- reg-names = "gmu", "gmu_pdc", "gmu_pdc_seq";
-
- interrupts = <GIC_SPI 304 IRQ_TYPE_LEVEL_HIGH>,
- <GIC_SPI 305 IRQ_TYPE_LEVEL_HIGH>;
- interrupt-names = "hfi", "gmu";
-
- clocks = <&gpucc GPU_CC_CX_GMU_CLK>,
- <&gpucc GPU_CC_CXO_CLK>,
- <&gcc GCC_DDRSS_GPU_AXI_CLK>,
- <&gcc GCC_GPU_MEMNOC_GFX_CLK>;
- clock-names = "gmu", "cxo", "axi", "memnoc";
-
- power-domains = <&gpucc GPU_CX_GDSC>,
- <&gpucc GPU_GX_GDSC>;
- power-domain-names = "cx", "gx";
-
- iommus = <&adreno_smmu 5>;
-
- operating-points-v2 = <&gmu_opp_table>;
- };
-};
-
-a3xx example with OCMEM support:
-
-/ {
- ...
-
- gpu: adreno@fdb00000 {
- compatible = "qcom,adreno-330.2",
- "qcom,adreno";
- reg = <0xfdb00000 0x10000>;
- reg-names = "kgsl_3d0_reg_memory";
- interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
- interrupt-names = "kgsl_3d0_irq";
- clock-names = "core",
- "iface",
- "mem_iface";
- clocks = <&mmcc OXILI_GFX3D_CLK>,
- <&mmcc OXILICX_AHB_CLK>,
- <&mmcc OXILICX_AXI_CLK>;
- sram = <&gmu_sram>;
- power-domains = <&mmcc OXILICX_GDSC>;
- operating-points-v2 = <&gpu_opp_table>;
- iommus = <&gpu_iommu 0>;
- };
-
- ocmem@fdd00000 {
- compatible = "qcom,msm8974-ocmem";
-
- reg = <0xfdd00000 0x2000>,
- <0xfec00000 0x180000>;
- reg-names = "ctrl",
- "mem";
-
- clocks = <&rpmcc RPM_SMD_OCMEMGX_CLK>,
- <&mmcc OCMEMCX_OCMEMNOC_CLK>;
- clock-names = "core",
- "iface";
-
- #address-cells = <1>;
- #size-cells = <1>;
-
- gmu_sram: gmu-sram@0 {
- reg = <0x0 0x100000>;
- ranges = <0 0 0xfec00000 0x100000>;
- };
- };
-};
diff --git a/Documentation/devicetree/bindings/display/msm/gmu.yaml b/Documentation/devicetree/bindings/display/msm/gmu.yaml
new file mode 100644
index 0000000..776ff92
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/msm/gmu.yaml
@@ -0,0 +1,130 @@
+# SPDX-License-Identifier: GPL-2.0-only
+# Copyright 2019-2020, The Linux Foundation, All Rights Reserved
+%YAML 1.2
+---
+
+$id: "http://devicetree.org/schemas/display/msm/gmu.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Devicetree bindings for the GMU attached to certain Adreno GPUs
+
+maintainers:
+ - Rob Clark <robdclark@gmail.com>
+
+description: |
+ These bindings describe the Graphics Management Unit (GMU) that is attached
+ to members of the Adreno A6xx GPU family. The GMU provides on-device power
+ management and support to improve power efficiency and reduce the load on
+ the CPU.
+
+properties:
+ compatible:
+ items:
+ - enum:
+ - qcom,adreno-gmu-630.2
+ - const: qcom,adreno-gmu
+
+ reg:
+ items:
+ - description: Core GMU registers
+ - description: GMU PDC registers
+ - description: GMU PDC sequence registers
+
+ reg-names:
+ items:
+ - const: gmu
+ - const: gmu_pdc
+ - const: gmu_pdc_seq
+
+ clocks:
+ items:
+ - description: GMU clock
+ - description: GPU CX clock
+ - description: GPU AXI clock
+ - description: GPU MEMNOC clock
+
+ clock-names:
+ items:
+ - const: gmu
+ - const: cxo
+ - const: axi
+ - const: memnoc
+
+ interrupts:
+ items:
+ - description: GMU HFI interrupt
+ - description: GMU interrupt
+
+
+ interrupt-names:
+ items:
+ - const: hfi
+ - const: gmu
+
+ power-domains:
+ items:
+ - description: CX power domain
+ - description: GX power domain
+
+ power-domain-names:
+ items:
+ - const: cx
+ - const: gx
+
+ iommus:
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+ description:
+ Phandle to a IOMMU device and stream ID. Refer to ../../iommu/iommu.txt
+ for more information.
+
+ operating-points-v2:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description:
+ Phandle to the OPP table for the available GMU frequencies. Refer to
+ ../../opp/opp.txt for more information.
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - clocks
+ - clock-names
+ - interrupts
+ - interrupt-names
+ - power-domains
+ - power-domain-names
+ - iommus
+ - operating-points-v2
+
+examples:
+ - |
+ #include <dt-bindings/clock/qcom,gpucc-sdm845.h>
+ #include <dt-bindings/clock/qcom,gcc-sdm845.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ gmu: gmu@506a000 {
+ compatible="qcom,adreno-gmu-630.2", "qcom,adreno-gmu";
+
+ reg = <0x506a000 0x30000>,
+ <0xb280000 0x10000>,
+ <0xb480000 0x10000>;
+ reg-names = "gmu", "gmu_pdc", "gmu_pdc_seq";
+
+ clocks = <&gpucc GPU_CC_CX_GMU_CLK>,
+ <&gpucc GPU_CC_CXO_CLK>,
+ <&gcc GCC_DDRSS_GPU_AXI_CLK>,
+ <&gcc GCC_GPU_MEMNOC_GFX_CLK>;
+ clock-names = "gmu", "cxo", "axi", "memnoc";
+
+ interrupts = <GIC_SPI 304 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 305 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "hfi", "gmu";
+
+ power-domains = <&gpucc GPU_CX_GDSC>,
+ <&gpucc GPU_GX_GDSC>;
+ power-domain-names = "cx", "gx";
+
+ iommus = <&adreno_smmu 5>;
+ operating-points-v2 = <&gmu_opp_table>;
+ };
--
2.7.4
^ permalink raw reply related
* [PATCH v2 4/4] drm/msm/a6xx: Use the DMA API for GMU memory objects
From: Jordan Crouse @ 2020-02-20 18:26 UTC (permalink / raw)
To: linux-arm-msm
Cc: Douglas Anderson, David Airlie, freedreno, smasetty, linux-kernel,
dri-devel, Stephen Boyd, Sean Paul
In-Reply-To: <1582223216-23459-1-git-send-email-jcrouse@codeaurora.org>
The GMU has very few memory allocations and uses a flat memory space so
there is no good reason to go out of our way to bypass the DMA APIs which
were basically designed for this exact scenario.
v2: Pass force_dma false to of_dma_configure to require that the DMA
region be set up and return error from of_dma_configure to fail probe.
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 112 +++-------------------------------
drivers/gpu/drm/msm/adreno/a6xx_gmu.h | 5 +-
2 files changed, 11 insertions(+), 106 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index 983afea..c36b38b 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -2,6 +2,7 @@
/* Copyright (c) 2017-2019 The Linux Foundation. All rights reserved. */
#include <linux/clk.h>
+#include <linux/dma-mapping.h>
#include <linux/interconnect.h>
#include <linux/pm_domain.h>
#include <linux/pm_opp.h>
@@ -895,21 +896,10 @@ int a6xx_gmu_stop(struct a6xx_gpu *a6xx_gpu)
static void a6xx_gmu_memory_free(struct a6xx_gmu *gmu, struct a6xx_gmu_bo *bo)
{
- int count, i;
- u64 iova;
-
if (IS_ERR_OR_NULL(bo))
return;
- count = bo->size >> PAGE_SHIFT;
- iova = bo->iova;
-
- for (i = 0; i < count; i++, iova += PAGE_SIZE) {
- iommu_unmap(gmu->domain, iova, PAGE_SIZE);
- __free_pages(bo->pages[i], 0);
- }
-
- kfree(bo->pages);
+ dma_free_attrs(gmu->dev, bo->size, bo->virt, bo->iova, bo->attrs);
kfree(bo);
}
@@ -917,94 +907,23 @@ static struct a6xx_gmu_bo *a6xx_gmu_memory_alloc(struct a6xx_gmu *gmu,
size_t size)
{
struct a6xx_gmu_bo *bo;
- int ret, count, i;
bo = kzalloc(sizeof(*bo), GFP_KERNEL);
if (!bo)
return ERR_PTR(-ENOMEM);
bo->size = PAGE_ALIGN(size);
+ bo->attrs = DMA_ATTR_WRITE_COMBINE;
- count = bo->size >> PAGE_SHIFT;
+ bo->virt = dma_alloc_attrs(gmu->dev, bo->size, &bo->iova, GFP_KERNEL,
+ bo->attrs);
- bo->pages = kcalloc(count, sizeof(struct page *), GFP_KERNEL);
- if (!bo->pages) {
+ if (!bo->virt) {
kfree(bo);
return ERR_PTR(-ENOMEM);
}
- for (i = 0; i < count; i++) {
- bo->pages[i] = alloc_page(GFP_KERNEL);
- if (!bo->pages[i])
- goto err;
- }
-
- bo->iova = gmu->uncached_iova_base;
-
- for (i = 0; i < count; i++) {
- ret = iommu_map(gmu->domain,
- bo->iova + (PAGE_SIZE * i),
- page_to_phys(bo->pages[i]), PAGE_SIZE,
- IOMMU_READ | IOMMU_WRITE);
-
- if (ret) {
- DRM_DEV_ERROR(gmu->dev, "Unable to map GMU buffer object\n");
-
- for (i = i - 1 ; i >= 0; i--)
- iommu_unmap(gmu->domain,
- bo->iova + (PAGE_SIZE * i),
- PAGE_SIZE);
-
- goto err;
- }
- }
-
- bo->virt = vmap(bo->pages, count, VM_IOREMAP,
- pgprot_writecombine(PAGE_KERNEL));
- if (!bo->virt)
- goto err;
-
- /* Align future IOVA addresses on 1MB boundaries */
- gmu->uncached_iova_base += ALIGN(size, SZ_1M);
-
return bo;
-
-err:
- for (i = 0; i < count; i++) {
- if (bo->pages[i])
- __free_pages(bo->pages[i], 0);
- }
-
- kfree(bo->pages);
- kfree(bo);
-
- return ERR_PTR(-ENOMEM);
-}
-
-static int a6xx_gmu_memory_probe(struct a6xx_gmu *gmu)
-{
- int ret;
-
- /*
- * The GMU address space is hardcoded to treat the range
- * 0x60000000 - 0x80000000 as un-cached memory. All buffers shared
- * between the GMU and the CPU will live in this space
- */
- gmu->uncached_iova_base = 0x60000000;
-
-
- gmu->domain = iommu_domain_alloc(&platform_bus_type);
- if (!gmu->domain)
- return -ENODEV;
-
- ret = iommu_attach_device(gmu->domain, gmu->dev);
-
- if (ret) {
- iommu_domain_free(gmu->domain);
- gmu->domain = NULL;
- }
-
- return ret;
}
/* Return the 'arc-level' for the given frequency */
@@ -1264,10 +1183,6 @@ void a6xx_gmu_remove(struct a6xx_gpu *a6xx_gpu)
a6xx_gmu_memory_free(gmu, gmu->hfi);
- iommu_detach_device(gmu->domain, gmu->dev);
-
- iommu_domain_free(gmu->domain);
-
free_irq(gmu->gmu_irq, gmu);
free_irq(gmu->hfi_irq, gmu);
@@ -1288,7 +1203,10 @@ int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
gmu->dev = &pdev->dev;
- of_dma_configure(gmu->dev, node, true);
+ /* Pass force_dma false to require the DT to set the dma region */
+ ret = of_dma_configure(gmu->dev, node, false);
+ if (ret)
+ return ret;
/* Fow now, don't do anything fancy until we get our feet under us */
gmu->idle_level = GMU_IDLE_STATE_ACTIVE;
@@ -1300,11 +1218,6 @@ int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
if (ret)
goto err_put_device;
- /* Set up the IOMMU context bank */
- ret = a6xx_gmu_memory_probe(gmu);
- if (ret)
- goto err_put_device;
-
/* Allocate memory for for the HFI queues */
gmu->hfi = a6xx_gmu_memory_alloc(gmu, SZ_16K);
if (IS_ERR(gmu->hfi))
@@ -1350,11 +1263,6 @@ int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
err_memory:
a6xx_gmu_memory_free(gmu, gmu->hfi);
- if (gmu->domain) {
- iommu_detach_device(gmu->domain, gmu->dev);
-
- iommu_domain_free(gmu->domain);
- }
ret = -ENODEV;
err_put_device:
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
index 2af91ed..31bd1987 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
@@ -13,7 +13,7 @@ struct a6xx_gmu_bo {
void *virt;
size_t size;
u64 iova;
- struct page **pages;
+ unsigned long attrs;
};
/*
@@ -49,9 +49,6 @@ struct a6xx_gmu {
int hfi_irq;
int gmu_irq;
- struct iommu_domain *domain;
- u64 uncached_iova_base;
-
struct device *gxpd;
int idle_level;
--
2.7.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related
* [Buildroot] [PATCH 1/1] support/scripts/pkg-stats: iterate over CVEs in streaming
From: Titouan Christophe @ 2020-02-20 18:27 UTC (permalink / raw)
To: buildroot
The NVD files that are used to build the list of CVEs affecting
Buildroot packages are quite large (a few hundreds MB of json),
and cause the pkg-stats scripts to have a huge memory footprint
(a few GB with Python 2.7).
However, because we only need to iterate on CVE items one by one,
we can process them in streaming (ie decoding one CVE at a time
from the JSON representation). Because the json module from the
python standard library does not support such a mode of operation,
we switch to the third-party package ijson, which is compatible
with both Python 2 and Python3.
To run the script with these modifications, one should install
the ijson python package. This can be done with pip:
`pip install ijson`. On Debian based distributions, this can
also be done with the apt package manager:
`apt install python-ijson`.
Signed-off-by: Titouan Christophe <titouan.christophe@railnova.eu>
---
support/scripts/pkg-stats | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/support/scripts/pkg-stats b/support/scripts/pkg-stats
index c113cf9606..7721d98459 100755
--- a/support/scripts/pkg-stats
+++ b/support/scripts/pkg-stats
@@ -25,6 +25,7 @@ import re
import subprocess
import requests # URL checking
import json
+import ijson
import certifi
import distutils.version
import time
@@ -231,11 +232,11 @@ class CVE:
for year in range(NVD_START_YEAR, datetime.datetime.now().year + 1):
filename = CVE.download_nvd_year(nvd_dir, year)
try:
- content = json.load(gzip.GzipFile(filename))
+ content = ijson.items(gzip.GzipFile(filename), 'CVE_Items.item')
except:
print("ERROR: cannot read %s. Please remove the file then rerun this script" % filename)
raise
- for cve in content["CVE_Items"]:
+ for cve in content:
yield cls(cve['cve'])
def each_product(self):
--
2.24.1
^ permalink raw reply related
* [PATCH v2 2/4] dt-bindings: display: msm: Add required dma-range property
From: Jordan Crouse @ 2020-02-20 18:26 UTC (permalink / raw)
To: linux-arm-msm
Cc: Mark Rutland, devicetree, David Airlie, freedreno, smasetty,
linux-kernel, dri-devel, Rob Herring, Sean Paul
In-Reply-To: <1582223216-23459-1-git-send-email-jcrouse@codeaurora.org>
The GMU node now requires a specific dma-range property so that the driver
can use the DMA API to do the few memory allocations required by the GMU.
This sets the IOMMU iova allocator to match the 'uncached' part of the
GMU virtual address space.
v2: Fix the dma-ranges tag. The third pair should be the size.
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
Documentation/devicetree/bindings/display/msm/gmu.yaml | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/msm/gmu.yaml b/Documentation/devicetree/bindings/display/msm/gmu.yaml
index 776ff92..d11a073 100644
--- a/Documentation/devicetree/bindings/display/msm/gmu.yaml
+++ b/Documentation/devicetree/bindings/display/msm/gmu.yaml
@@ -83,6 +83,13 @@ properties:
Phandle to the OPP table for the available GMU frequencies. Refer to
../../opp/opp.txt for more information.
+ dma-ranges:
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ description:
+ Describe the dma-address range for the device. This should always
+ describe the range between 0x60000000 and 0x80000000 which represents
+ the uncached region of the GMU address space.
+
required:
- compatible
- reg
@@ -95,6 +102,7 @@ required:
- power-domain-names
- iommus
- operating-points-v2
+ - dma-ranges
examples:
- |
@@ -127,4 +135,6 @@ examples:
iommus = <&adreno_smmu 5>;
operating-points-v2 = <&gmu_opp_table>;
+
+ dma-ranges = <0 0x60000000 0 0x60000000 0 0x20000000>;
};
--
2.7.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related
* [PATCH v2 2/4] dt-bindings: display: msm: Add required dma-range property
From: Jordan Crouse @ 2020-02-20 18:26 UTC (permalink / raw)
To: linux-arm-msm
Cc: smasetty, John Stultz, Sean Paul, devicetree, linux-kernel,
dri-devel, Rob Herring, Rob Clark, David Airlie, Mark Rutland,
freedreno, Daniel Vetter
In-Reply-To: <1582223216-23459-1-git-send-email-jcrouse@codeaurora.org>
The GMU node now requires a specific dma-range property so that the driver
can use the DMA API to do the few memory allocations required by the GMU.
This sets the IOMMU iova allocator to match the 'uncached' part of the
GMU virtual address space.
v2: Fix the dma-ranges tag. The third pair should be the size.
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
Documentation/devicetree/bindings/display/msm/gmu.yaml | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/msm/gmu.yaml b/Documentation/devicetree/bindings/display/msm/gmu.yaml
index 776ff92..d11a073 100644
--- a/Documentation/devicetree/bindings/display/msm/gmu.yaml
+++ b/Documentation/devicetree/bindings/display/msm/gmu.yaml
@@ -83,6 +83,13 @@ properties:
Phandle to the OPP table for the available GMU frequencies. Refer to
../../opp/opp.txt for more information.
+ dma-ranges:
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ description:
+ Describe the dma-address range for the device. This should always
+ describe the range between 0x60000000 and 0x80000000 which represents
+ the uncached region of the GMU address space.
+
required:
- compatible
- reg
@@ -95,6 +102,7 @@ required:
- power-domain-names
- iommus
- operating-points-v2
+ - dma-ranges
examples:
- |
@@ -127,4 +135,6 @@ examples:
iommus = <&adreno_smmu 5>;
operating-points-v2 = <&gmu_opp_table>;
+
+ dma-ranges = <0 0x60000000 0 0x60000000 0 0x20000000>;
};
--
2.7.4
^ permalink raw reply related
* [PATCH v2 4/4] drm/msm/a6xx: Use the DMA API for GMU memory objects
From: Jordan Crouse @ 2020-02-20 18:26 UTC (permalink / raw)
To: linux-arm-msm
Cc: smasetty, John Stultz, Sean Paul, Stephen Boyd, dri-devel,
linux-kernel, Douglas Anderson, Rob Clark, David Airlie,
freedreno, Daniel Vetter
In-Reply-To: <1582223216-23459-1-git-send-email-jcrouse@codeaurora.org>
The GMU has very few memory allocations and uses a flat memory space so
there is no good reason to go out of our way to bypass the DMA APIs which
were basically designed for this exact scenario.
v2: Pass force_dma false to of_dma_configure to require that the DMA
region be set up and return error from of_dma_configure to fail probe.
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 112 +++-------------------------------
drivers/gpu/drm/msm/adreno/a6xx_gmu.h | 5 +-
2 files changed, 11 insertions(+), 106 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index 983afea..c36b38b 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -2,6 +2,7 @@
/* Copyright (c) 2017-2019 The Linux Foundation. All rights reserved. */
#include <linux/clk.h>
+#include <linux/dma-mapping.h>
#include <linux/interconnect.h>
#include <linux/pm_domain.h>
#include <linux/pm_opp.h>
@@ -895,21 +896,10 @@ int a6xx_gmu_stop(struct a6xx_gpu *a6xx_gpu)
static void a6xx_gmu_memory_free(struct a6xx_gmu *gmu, struct a6xx_gmu_bo *bo)
{
- int count, i;
- u64 iova;
-
if (IS_ERR_OR_NULL(bo))
return;
- count = bo->size >> PAGE_SHIFT;
- iova = bo->iova;
-
- for (i = 0; i < count; i++, iova += PAGE_SIZE) {
- iommu_unmap(gmu->domain, iova, PAGE_SIZE);
- __free_pages(bo->pages[i], 0);
- }
-
- kfree(bo->pages);
+ dma_free_attrs(gmu->dev, bo->size, bo->virt, bo->iova, bo->attrs);
kfree(bo);
}
@@ -917,94 +907,23 @@ static struct a6xx_gmu_bo *a6xx_gmu_memory_alloc(struct a6xx_gmu *gmu,
size_t size)
{
struct a6xx_gmu_bo *bo;
- int ret, count, i;
bo = kzalloc(sizeof(*bo), GFP_KERNEL);
if (!bo)
return ERR_PTR(-ENOMEM);
bo->size = PAGE_ALIGN(size);
+ bo->attrs = DMA_ATTR_WRITE_COMBINE;
- count = bo->size >> PAGE_SHIFT;
+ bo->virt = dma_alloc_attrs(gmu->dev, bo->size, &bo->iova, GFP_KERNEL,
+ bo->attrs);
- bo->pages = kcalloc(count, sizeof(struct page *), GFP_KERNEL);
- if (!bo->pages) {
+ if (!bo->virt) {
kfree(bo);
return ERR_PTR(-ENOMEM);
}
- for (i = 0; i < count; i++) {
- bo->pages[i] = alloc_page(GFP_KERNEL);
- if (!bo->pages[i])
- goto err;
- }
-
- bo->iova = gmu->uncached_iova_base;
-
- for (i = 0; i < count; i++) {
- ret = iommu_map(gmu->domain,
- bo->iova + (PAGE_SIZE * i),
- page_to_phys(bo->pages[i]), PAGE_SIZE,
- IOMMU_READ | IOMMU_WRITE);
-
- if (ret) {
- DRM_DEV_ERROR(gmu->dev, "Unable to map GMU buffer object\n");
-
- for (i = i - 1 ; i >= 0; i--)
- iommu_unmap(gmu->domain,
- bo->iova + (PAGE_SIZE * i),
- PAGE_SIZE);
-
- goto err;
- }
- }
-
- bo->virt = vmap(bo->pages, count, VM_IOREMAP,
- pgprot_writecombine(PAGE_KERNEL));
- if (!bo->virt)
- goto err;
-
- /* Align future IOVA addresses on 1MB boundaries */
- gmu->uncached_iova_base += ALIGN(size, SZ_1M);
-
return bo;
-
-err:
- for (i = 0; i < count; i++) {
- if (bo->pages[i])
- __free_pages(bo->pages[i], 0);
- }
-
- kfree(bo->pages);
- kfree(bo);
-
- return ERR_PTR(-ENOMEM);
-}
-
-static int a6xx_gmu_memory_probe(struct a6xx_gmu *gmu)
-{
- int ret;
-
- /*
- * The GMU address space is hardcoded to treat the range
- * 0x60000000 - 0x80000000 as un-cached memory. All buffers shared
- * between the GMU and the CPU will live in this space
- */
- gmu->uncached_iova_base = 0x60000000;
-
-
- gmu->domain = iommu_domain_alloc(&platform_bus_type);
- if (!gmu->domain)
- return -ENODEV;
-
- ret = iommu_attach_device(gmu->domain, gmu->dev);
-
- if (ret) {
- iommu_domain_free(gmu->domain);
- gmu->domain = NULL;
- }
-
- return ret;
}
/* Return the 'arc-level' for the given frequency */
@@ -1264,10 +1183,6 @@ void a6xx_gmu_remove(struct a6xx_gpu *a6xx_gpu)
a6xx_gmu_memory_free(gmu, gmu->hfi);
- iommu_detach_device(gmu->domain, gmu->dev);
-
- iommu_domain_free(gmu->domain);
-
free_irq(gmu->gmu_irq, gmu);
free_irq(gmu->hfi_irq, gmu);
@@ -1288,7 +1203,10 @@ int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
gmu->dev = &pdev->dev;
- of_dma_configure(gmu->dev, node, true);
+ /* Pass force_dma false to require the DT to set the dma region */
+ ret = of_dma_configure(gmu->dev, node, false);
+ if (ret)
+ return ret;
/* Fow now, don't do anything fancy until we get our feet under us */
gmu->idle_level = GMU_IDLE_STATE_ACTIVE;
@@ -1300,11 +1218,6 @@ int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
if (ret)
goto err_put_device;
- /* Set up the IOMMU context bank */
- ret = a6xx_gmu_memory_probe(gmu);
- if (ret)
- goto err_put_device;
-
/* Allocate memory for for the HFI queues */
gmu->hfi = a6xx_gmu_memory_alloc(gmu, SZ_16K);
if (IS_ERR(gmu->hfi))
@@ -1350,11 +1263,6 @@ int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
err_memory:
a6xx_gmu_memory_free(gmu, gmu->hfi);
- if (gmu->domain) {
- iommu_detach_device(gmu->domain, gmu->dev);
-
- iommu_domain_free(gmu->domain);
- }
ret = -ENODEV;
err_put_device:
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
index 2af91ed..31bd1987 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
@@ -13,7 +13,7 @@ struct a6xx_gmu_bo {
void *virt;
size_t size;
u64 iova;
- struct page **pages;
+ unsigned long attrs;
};
/*
@@ -49,9 +49,6 @@ struct a6xx_gmu {
int hfi_irq;
int gmu_irq;
- struct iommu_domain *domain;
- u64 uncached_iova_base;
-
struct device *gxpd;
int idle_level;
--
2.7.4
^ permalink raw reply related
* [PATCH v2 1/4] dt-bindings: display: msm: Convert GMU bindings to YAML
From: Jordan Crouse @ 2020-02-20 18:26 UTC (permalink / raw)
To: linux-arm-msm
Cc: Mark Rutland, devicetree, David Airlie, freedreno, smasetty,
linux-kernel, dri-devel, Rob Herring, Sean Paul
In-Reply-To: <1582223216-23459-1-git-send-email-jcrouse@codeaurora.org>
Convert display/msm/gmu.txt to display/msm/gmu.yaml and remove the old
text bindings.
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
.../devicetree/bindings/display/msm/gmu.txt | 116 ------------------
.../devicetree/bindings/display/msm/gmu.yaml | 130 +++++++++++++++++++++
2 files changed, 130 insertions(+), 116 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/display/msm/gmu.txt
create mode 100644 Documentation/devicetree/bindings/display/msm/gmu.yaml
diff --git a/Documentation/devicetree/bindings/display/msm/gmu.txt b/Documentation/devicetree/bindings/display/msm/gmu.txt
deleted file mode 100644
index bf9c7a2..0000000
--- a/Documentation/devicetree/bindings/display/msm/gmu.txt
+++ /dev/null
@@ -1,116 +0,0 @@
-Qualcomm adreno/snapdragon GMU (Graphics management unit)
-
-The GMU is a programmable power controller for the GPU. the CPU controls the
-GMU which in turn handles power controls for the GPU.
-
-Required properties:
-- compatible: "qcom,adreno-gmu-XYZ.W", "qcom,adreno-gmu"
- for example: "qcom,adreno-gmu-630.2", "qcom,adreno-gmu"
- Note that you need to list the less specific "qcom,adreno-gmu"
- for generic matches and the more specific identifier to identify
- the specific device.
-- reg: Physical base address and length of the GMU registers.
-- reg-names: Matching names for the register regions
- * "gmu"
- * "gmu_pdc"
- * "gmu_pdc_seg"
-- interrupts: The interrupt signals from the GMU.
-- interrupt-names: Matching names for the interrupts
- * "hfi"
- * "gmu"
-- clocks: phandles to the device clocks
-- clock-names: Matching names for the clocks
- * "gmu"
- * "cxo"
- * "axi"
- * "mnoc"
-- power-domains: should be:
- <&clock_gpucc GPU_CX_GDSC>
- <&clock_gpucc GPU_GX_GDSC>
-- power-domain-names: Matching names for the power domains
-- iommus: phandle to the adreno iommu
-- operating-points-v2: phandle to the OPP operating points
-
-Optional properties:
-- sram: phandle to the On Chip Memory (OCMEM) that's present on some Snapdragon
- SoCs. See Documentation/devicetree/bindings/sram/qcom,ocmem.yaml.
-
-Example:
-
-/ {
- ...
-
- gmu: gmu@506a000 {
- compatible="qcom,adreno-gmu-630.2", "qcom,adreno-gmu";
-
- reg = <0x506a000 0x30000>,
- <0xb280000 0x10000>,
- <0xb480000 0x10000>;
- reg-names = "gmu", "gmu_pdc", "gmu_pdc_seq";
-
- interrupts = <GIC_SPI 304 IRQ_TYPE_LEVEL_HIGH>,
- <GIC_SPI 305 IRQ_TYPE_LEVEL_HIGH>;
- interrupt-names = "hfi", "gmu";
-
- clocks = <&gpucc GPU_CC_CX_GMU_CLK>,
- <&gpucc GPU_CC_CXO_CLK>,
- <&gcc GCC_DDRSS_GPU_AXI_CLK>,
- <&gcc GCC_GPU_MEMNOC_GFX_CLK>;
- clock-names = "gmu", "cxo", "axi", "memnoc";
-
- power-domains = <&gpucc GPU_CX_GDSC>,
- <&gpucc GPU_GX_GDSC>;
- power-domain-names = "cx", "gx";
-
- iommus = <&adreno_smmu 5>;
-
- operating-points-v2 = <&gmu_opp_table>;
- };
-};
-
-a3xx example with OCMEM support:
-
-/ {
- ...
-
- gpu: adreno@fdb00000 {
- compatible = "qcom,adreno-330.2",
- "qcom,adreno";
- reg = <0xfdb00000 0x10000>;
- reg-names = "kgsl_3d0_reg_memory";
- interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
- interrupt-names = "kgsl_3d0_irq";
- clock-names = "core",
- "iface",
- "mem_iface";
- clocks = <&mmcc OXILI_GFX3D_CLK>,
- <&mmcc OXILICX_AHB_CLK>,
- <&mmcc OXILICX_AXI_CLK>;
- sram = <&gmu_sram>;
- power-domains = <&mmcc OXILICX_GDSC>;
- operating-points-v2 = <&gpu_opp_table>;
- iommus = <&gpu_iommu 0>;
- };
-
- ocmem@fdd00000 {
- compatible = "qcom,msm8974-ocmem";
-
- reg = <0xfdd00000 0x2000>,
- <0xfec00000 0x180000>;
- reg-names = "ctrl",
- "mem";
-
- clocks = <&rpmcc RPM_SMD_OCMEMGX_CLK>,
- <&mmcc OCMEMCX_OCMEMNOC_CLK>;
- clock-names = "core",
- "iface";
-
- #address-cells = <1>;
- #size-cells = <1>;
-
- gmu_sram: gmu-sram@0 {
- reg = <0x0 0x100000>;
- ranges = <0 0 0xfec00000 0x100000>;
- };
- };
-};
diff --git a/Documentation/devicetree/bindings/display/msm/gmu.yaml b/Documentation/devicetree/bindings/display/msm/gmu.yaml
new file mode 100644
index 0000000..776ff92
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/msm/gmu.yaml
@@ -0,0 +1,130 @@
+# SPDX-License-Identifier: GPL-2.0-only
+# Copyright 2019-2020, The Linux Foundation, All Rights Reserved
+%YAML 1.2
+---
+
+$id: "http://devicetree.org/schemas/display/msm/gmu.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Devicetree bindings for the GMU attached to certain Adreno GPUs
+
+maintainers:
+ - Rob Clark <robdclark@gmail.com>
+
+description: |
+ These bindings describe the Graphics Management Unit (GMU) that is attached
+ to members of the Adreno A6xx GPU family. The GMU provides on-device power
+ management and support to improve power efficiency and reduce the load on
+ the CPU.
+
+properties:
+ compatible:
+ items:
+ - enum:
+ - qcom,adreno-gmu-630.2
+ - const: qcom,adreno-gmu
+
+ reg:
+ items:
+ - description: Core GMU registers
+ - description: GMU PDC registers
+ - description: GMU PDC sequence registers
+
+ reg-names:
+ items:
+ - const: gmu
+ - const: gmu_pdc
+ - const: gmu_pdc_seq
+
+ clocks:
+ items:
+ - description: GMU clock
+ - description: GPU CX clock
+ - description: GPU AXI clock
+ - description: GPU MEMNOC clock
+
+ clock-names:
+ items:
+ - const: gmu
+ - const: cxo
+ - const: axi
+ - const: memnoc
+
+ interrupts:
+ items:
+ - description: GMU HFI interrupt
+ - description: GMU interrupt
+
+
+ interrupt-names:
+ items:
+ - const: hfi
+ - const: gmu
+
+ power-domains:
+ items:
+ - description: CX power domain
+ - description: GX power domain
+
+ power-domain-names:
+ items:
+ - const: cx
+ - const: gx
+
+ iommus:
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+ description:
+ Phandle to a IOMMU device and stream ID. Refer to ../../iommu/iommu.txt
+ for more information.
+
+ operating-points-v2:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description:
+ Phandle to the OPP table for the available GMU frequencies. Refer to
+ ../../opp/opp.txt for more information.
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - clocks
+ - clock-names
+ - interrupts
+ - interrupt-names
+ - power-domains
+ - power-domain-names
+ - iommus
+ - operating-points-v2
+
+examples:
+ - |
+ #include <dt-bindings/clock/qcom,gpucc-sdm845.h>
+ #include <dt-bindings/clock/qcom,gcc-sdm845.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ gmu: gmu@506a000 {
+ compatible="qcom,adreno-gmu-630.2", "qcom,adreno-gmu";
+
+ reg = <0x506a000 0x30000>,
+ <0xb280000 0x10000>,
+ <0xb480000 0x10000>;
+ reg-names = "gmu", "gmu_pdc", "gmu_pdc_seq";
+
+ clocks = <&gpucc GPU_CC_CX_GMU_CLK>,
+ <&gpucc GPU_CC_CXO_CLK>,
+ <&gcc GCC_DDRSS_GPU_AXI_CLK>,
+ <&gcc GCC_GPU_MEMNOC_GFX_CLK>;
+ clock-names = "gmu", "cxo", "axi", "memnoc";
+
+ interrupts = <GIC_SPI 304 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 305 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "hfi", "gmu";
+
+ power-domains = <&gpucc GPU_CX_GDSC>,
+ <&gpucc GPU_GX_GDSC>;
+ power-domain-names = "cx", "gx";
+
+ iommus = <&adreno_smmu 5>;
+ operating-points-v2 = <&gmu_opp_table>;
+ };
--
2.7.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related
* [PATCH v2 3/4] arm64: dts: sdm845: Set the virtual address range for GMU allocations
From: Jordan Crouse @ 2020-02-20 18:26 UTC (permalink / raw)
To: linux-arm-msm
Cc: smasetty, John Stultz, devicetree, Bjorn Andersson, linux-kernel,
Andy Gross, Rob Herring, Mark Rutland
In-Reply-To: <1582223216-23459-1-git-send-email-jcrouse@codeaurora.org>
GMU DMA allocations need to be mapped between 0x60000000 and 0x80000000 in
the GMU virtual address space. Specify the dma-range so the iova allocator
does the right thing.
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
arch/arm64/boot/dts/qcom/sdm845.dtsi | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index d42302b..3c766423 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -2911,6 +2911,8 @@
<&gpucc GPU_GX_GDSC>;
power-domain-names = "cx", "gx";
+ dma-ranges = <0 0x60000000 0 0x60000000 0 0x20000000>;
+
iommus = <&adreno_smmu 5>;
operating-points-v2 = <&gmu_opp_table>;
--
2.7.4
^ permalink raw reply related
* [PATCH v2 0/4] msm/gpu/a6xx: use the DMA-API for GMU memory allocations
From: Jordan Crouse @ 2020-02-20 18:26 UTC (permalink / raw)
To: linux-arm-msm
Cc: Mark Rutland, devicetree, David Airlie, linux-kernel, smasetty,
freedreno, Douglas Anderson, dri-devel, Stephen Boyd, Rob Herring,
Andy Gross, Bjorn Andersson, Sean Paul
When CONFIG_INIT_ON_ALLOC_DEFAULT_ON the GMU memory allocator runs afoul of
cache coherency issues because it is mapped as write-combine without clearing
the cache after it was zeroed.
Rather than duplicate the hacky workaround we use in the GEM allocator for the
same reason it turns out that we don't need to have a bespoke memory allocator
for the GMU anyway. It uses a flat, global address space and there are only
two relatively minor allocations anyway. In short, this is essentially what the
DMA API was created for so replace a bunch of memory management code with two
calls to allocate and free DMA memory and we're fine.
The only wrinkle is that the memory allocations need to be in a very specific
location in the GMU virtual address space so in order to get the iova allocator
to do the right thing we need to specify the dma-ranges property in the device
tree for the GMU node. Since we've not yet converted the GMU bindings over to
YAML two patches quickly turn into four but at the end of it we have at least
one bindings file converted to YAML and 99 less lines of code to worry about.
v2: Fix the example bindings for dma-ranges - the third item is the size
Pass false to of_dma_configure so that it fails probe if the DMA region is not
set up.
Jordan Crouse (4):
dt-bindings: display: msm: Convert GMU bindings to YAML
dt-bindings: display: msm: Add required dma-range property
arm64: dts: sdm845: Set the virtual address range for GMU allocations
drm/msm/a6xx: Use the DMA API for GMU memory objects
.../devicetree/bindings/display/msm/gmu.txt | 116 -----------------
.../devicetree/bindings/display/msm/gmu.yaml | 140 +++++++++++++++++++++
arch/arm64/boot/dts/qcom/sdm845.dtsi | 2 +
drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 112 ++---------------
drivers/gpu/drm/msm/adreno/a6xx_gmu.h | 5 +-
5 files changed, 153 insertions(+), 222 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/display/msm/gmu.txt
create mode 100644 Documentation/devicetree/bindings/display/msm/gmu.yaml
--
2.7.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* [MODERATED] Re: [PATCH 1/2] more sampling fun 1
From: Borislav Petkov @ 2020-02-20 18:26 UTC (permalink / raw)
To: speck
In-Reply-To: <20200220175340.GD58564@mtg-dev.jf.intel.com>
On Thu, Feb 20, 2020 at 09:53:40AM -0800, speck for mark gross wrote:
> Because its only needed in the context of the WIP mitigation patch that I'm
> having issues sending propperly.
>
> Sigh, I guess I'll just put it in a reply.
Actually, your 2/2 was being held by the ML because you've sent,
reportedly, not from the mail address you're subscribed with, to speck@.
It should be fixed now because Thomas added an alias for your other mail
address.
--
Regards/Gruss,
Boris.
SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg
--
^ permalink raw reply
* Re: [PATCH 6/8] drm/vram-helper: don't use ttm bo->offset v2
From: Nirmoy @ 2020-02-20 18:29 UTC (permalink / raw)
To: Daniel Vetter, Nirmoy Das
Cc: David1.Zhou, thellstrom, amd-gfx, airlied, kenny.ho, brian.welty,
maarten.lankhorst, dri-devel, nirmoy.das,
linux-graphics-maintainer, kraxel, alexander.deucher, sean,
christian.koenig, bskeggs
In-Reply-To: <20200220180955.GT2363188@phenom.ffwll.local>
On 2/20/20 7:09 PM, Daniel Vetter wrote:
> On Wed, Feb 19, 2020 at 02:53:20PM +0100, Nirmoy Das wrote:
>> Calculate GEM VRAM bo's offset within vram-helper without depending on
>> bo->offset
>>
>> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
>> ---
>> drivers/gpu/drm/drm_gem_vram_helper.c | 17 ++++++++++++++++-
>> 1 file changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
>> index 92a11bb42365..3edf5f241c15 100644
>> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
>> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
>> @@ -198,6 +198,21 @@ u64 drm_gem_vram_mmap_offset(struct drm_gem_vram_object *gbo)
>> }
>> EXPORT_SYMBOL(drm_gem_vram_mmap_offset);
>>
>> +/**
>> + * drm_gem_vram_pg_offset() - Returns a GEM VRAM object's page offset
>> + * @gbo: the GEM VRAM object
>> + *
>> + * Returns:
>> + * The buffer object's page offset, or
>> + * 0 with a warning when memory manager node of the buffer object is NULL
>> + * */
> We generally don't add full formal kerneldoc for internal functions like
> this. It won't get pulled into generated docs and generally just bitrots.
> Just informal comment if it's really tricky, but the function name here is
> clear enough I think.
Thanks for you review Daniel, I will remove that comment.
Nirmoy
> So with the comment removed:
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
>> +static s64 drm_gem_vram_pg_offset(struct drm_gem_vram_object *gbo)
>> +{
>> + if (WARN_ON_ONCE(!gbo->bo.mem.mm_node))
>> + return 0;
>> + return gbo->bo.mem.start;
>> +}
>> +
>> /**
>> * drm_gem_vram_offset() - \
>> Returns a GEM VRAM object's offset in video memory
>> @@ -214,7 +229,7 @@ s64 drm_gem_vram_offset(struct drm_gem_vram_object *gbo)
>> {
>> if (WARN_ON_ONCE(!gbo->pin_count))
>> return (s64)-ENODEV;
>> - return gbo->bo.offset;
>> + return drm_gem_vram_pg_offset(gbo) << PAGE_SHIFT;
>> }
>> EXPORT_SYMBOL(drm_gem_vram_offset);
>>
>> --
>> 2.25.0
>>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply
* Re: [PATCH 6/8] drm/vram-helper: don't use ttm bo->offset v2
From: Nirmoy @ 2020-02-20 18:29 UTC (permalink / raw)
To: Daniel Vetter, Nirmoy Das
Cc: thellstrom, amd-gfx, airlied, kenny.ho, brian.welty, dri-devel,
nirmoy.das, linux-graphics-maintainer, kraxel, alexander.deucher,
sean, christian.koenig, bskeggs
In-Reply-To: <20200220180955.GT2363188@phenom.ffwll.local>
On 2/20/20 7:09 PM, Daniel Vetter wrote:
> On Wed, Feb 19, 2020 at 02:53:20PM +0100, Nirmoy Das wrote:
>> Calculate GEM VRAM bo's offset within vram-helper without depending on
>> bo->offset
>>
>> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
>> ---
>> drivers/gpu/drm/drm_gem_vram_helper.c | 17 ++++++++++++++++-
>> 1 file changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
>> index 92a11bb42365..3edf5f241c15 100644
>> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
>> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
>> @@ -198,6 +198,21 @@ u64 drm_gem_vram_mmap_offset(struct drm_gem_vram_object *gbo)
>> }
>> EXPORT_SYMBOL(drm_gem_vram_mmap_offset);
>>
>> +/**
>> + * drm_gem_vram_pg_offset() - Returns a GEM VRAM object's page offset
>> + * @gbo: the GEM VRAM object
>> + *
>> + * Returns:
>> + * The buffer object's page offset, or
>> + * 0 with a warning when memory manager node of the buffer object is NULL
>> + * */
> We generally don't add full formal kerneldoc for internal functions like
> this. It won't get pulled into generated docs and generally just bitrots.
> Just informal comment if it's really tricky, but the function name here is
> clear enough I think.
Thanks for you review Daniel, I will remove that comment.
Nirmoy
> So with the comment removed:
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
>> +static s64 drm_gem_vram_pg_offset(struct drm_gem_vram_object *gbo)
>> +{
>> + if (WARN_ON_ONCE(!gbo->bo.mem.mm_node))
>> + return 0;
>> + return gbo->bo.mem.start;
>> +}
>> +
>> /**
>> * drm_gem_vram_offset() - \
>> Returns a GEM VRAM object's offset in video memory
>> @@ -214,7 +229,7 @@ s64 drm_gem_vram_offset(struct drm_gem_vram_object *gbo)
>> {
>> if (WARN_ON_ONCE(!gbo->pin_count))
>> return (s64)-ENODEV;
>> - return gbo->bo.offset;
>> + return drm_gem_vram_pg_offset(gbo) << PAGE_SHIFT;
>> }
>> EXPORT_SYMBOL(drm_gem_vram_offset);
>>
>> --
>> 2.25.0
>>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH 4/8] libxfs: enable tools to check that metadata updates have been committed
From: Darrick J. Wong @ 2020-02-20 18:26 UTC (permalink / raw)
To: Brian Foster; +Cc: sandeen, linux-xfs
In-Reply-To: <20200220175850.GH48977@bfoster>
On Thu, Feb 20, 2020 at 12:58:50PM -0500, Brian Foster wrote:
> On Thu, Feb 20, 2020 at 08:46:38AM -0800, Darrick J. Wong wrote:
> > On Thu, Feb 20, 2020 at 09:06:12AM -0500, Brian Foster wrote:
> > > On Wed, Feb 19, 2020 at 05:42:06PM -0800, Darrick J. Wong wrote:
> > > > From: Darrick J. Wong <darrick.wong@oracle.com>
> > > >
> > > > Add a new function that will ensure that everything we changed has
> > > > landed on stable media, and report the results. Subsequent commits will
> > > > teach the individual programs to report when things go wrong.
> > > >
> > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > > > ---
> > > > include/xfs_mount.h | 3 +++
> > > > libxfs/init.c | 43 +++++++++++++++++++++++++++++++++++++++++++
> > > > libxfs/libxfs_io.h | 2 ++
> > > > libxfs/rdwr.c | 27 +++++++++++++++++++++++++--
> > > > 4 files changed, 73 insertions(+), 2 deletions(-)
> > > >
> > > >
> > > > diff --git a/include/xfs_mount.h b/include/xfs_mount.h
> > > > index 29b3cc1b..c80aaf69 100644
> > > > --- a/include/xfs_mount.h
> > > > +++ b/include/xfs_mount.h
> > > > @@ -187,4 +187,7 @@ extern xfs_mount_t *libxfs_mount (xfs_mount_t *, xfs_sb_t *,
> > > > extern void libxfs_umount (xfs_mount_t *);
> > > > extern void libxfs_rtmount_destroy (xfs_mount_t *);
> > > >
> > > > +void libxfs_flush_devices(struct xfs_mount *mp, int *datadev, int *logdev,
> > > > + int *rtdev);
> > > > +
> > > > #endif /* __XFS_MOUNT_H__ */
> > > > diff --git a/libxfs/init.c b/libxfs/init.c
> > > > index a0d4b7f4..d1d3f4df 100644
> > > > --- a/libxfs/init.c
> > > > +++ b/libxfs/init.c
> > > > @@ -569,6 +569,8 @@ libxfs_buftarg_alloc(
> > > > }
> > > > btp->bt_mount = mp;
> > > > btp->dev = dev;
> > > > + btp->lost_writes = false;
> > > > +
> > > > return btp;
> > > > }
> > > >
> > > > @@ -791,6 +793,47 @@ libxfs_rtmount_destroy(xfs_mount_t *mp)
> > > > mp->m_rsumip = mp->m_rbmip = NULL;
> > > > }
> > > >
> > > > +static inline int
> > > > +libxfs_flush_buftarg(
> > > > + struct xfs_buftarg *btp)
> > > > +{
> > > > + if (btp->lost_writes)
> > > > + return -ENOTRECOVERABLE;
> > >
> > > I'm curious why we'd want to skip the flush just because some writes
> > > happened to fail..? I suppose the fs might be borked, but it seems a
> > > little strange to at least not try the flush, particularly since we
> > > might still flush any of the other two possible devices.
> >
> > My thinking here was that if the write verifiers (or the pwrite() calls
> > themselves) failed then there's no point in telling the disk to flush
> > its write cache since we already know it's not in sync with the buffer
> > cache.
> >
>
> I suppose, but it seems there is some value in flushing what we did
> write.. That's effectively historical behavior (since we ignored
> errors), right?
It's the historical behavior, yes. I don't think it makes much sense,
but OTOH I'm not opposed to restoring that.
> > > > +
> > > > + return libxfs_blkdev_issue_flush(btp);
> > > > +}
> > > > +
> > > > +/*
> > > > + * Purge the buffer cache to write all dirty buffers to disk and free all
> > > > + * incore buffers. Buffers that cannot be written will cause the lost_writes
> > > > + * flag to be set in the buftarg. If there were no lost writes, flush the
> > > > + * device to make sure the writes made it to stable storage.
> > > > + *
> > > > + * For each device, the return code will be set to -ENOTRECOVERABLE if we
> > > > + * couldn't write something to disk; or the results of the block device flush
> > > > + * operation.
> > >
> > > Why not -EIO?
> >
> > Originally I thought it might be useful to be able to distinguish
> > between "dirty buffers never even made it out of the buffer cache" vs.
> > "dirty buffers were sent to the disk but the disk sent back media
> > errors", though in the end the userspace tools don't make any
> > distinction.
> >
> > That said, looking at this again, maybe I should track write verifier
> > failure separately so that we can return EFSCORRUPTED for that?
> >
>
> It's not clear to me that anything application level would care much
> about verifier failure vs. I/O failure, but I've no objection to doing
> something like that either.
Yeah. The single usecase I can think of is where repair trips over a
write verifier and we should make it really obvious to the sysadmin that
repair is buggy and needs either (a) an upgrade or (b) a complaint filed
on linux-xfs.
--D
> Brian
>
> > --D
> >
> > >
> > > Brian
> > >
> > > > + */
> > > > +void
> > > > +libxfs_flush_devices(
> > > > + struct xfs_mount *mp,
> > > > + int *datadev,
> > > > + int *logdev,
> > > > + int *rtdev)
> > > > +{
> > > > + *datadev = *logdev = *rtdev = 0;
> > > > +
> > > > + libxfs_bcache_purge();
> > > > +
> > > > + if (mp->m_ddev_targp)
> > > > + *datadev = libxfs_flush_buftarg(mp->m_ddev_targp);
> > > > +
> > > > + if (mp->m_logdev_targp && mp->m_logdev_targp != mp->m_ddev_targp)
> > > > + *logdev = libxfs_flush_buftarg(mp->m_logdev_targp);
> > > > +
> > > > + if (mp->m_rtdev_targp)
> > > > + *rtdev = libxfs_flush_buftarg(mp->m_rtdev_targp);
> > > > +}
> > > > +
> > > > /*
> > > > * Release any resource obtained during a mount.
> > > > */
> > > > diff --git a/libxfs/libxfs_io.h b/libxfs/libxfs_io.h
> > > > index 579df52b..fc0fd060 100644
> > > > --- a/libxfs/libxfs_io.h
> > > > +++ b/libxfs/libxfs_io.h
> > > > @@ -23,10 +23,12 @@ struct xfs_perag;
> > > > struct xfs_buftarg {
> > > > struct xfs_mount *bt_mount;
> > > > dev_t dev;
> > > > + bool lost_writes;
> > > > };
> > > >
> > > > extern void libxfs_buftarg_init(struct xfs_mount *mp, dev_t ddev,
> > > > dev_t logdev, dev_t rtdev);
> > > > +int libxfs_blkdev_issue_flush(struct xfs_buftarg *btp);
> > > >
> > > > #define LIBXFS_BBTOOFF64(bbs) (((xfs_off_t)(bbs)) << BBSHIFT)
> > > >
> > > > diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
> > > > index 8b47d438..92e497f9 100644
> > > > --- a/libxfs/rdwr.c
> > > > +++ b/libxfs/rdwr.c
> > > > @@ -17,6 +17,7 @@
> > > > #include "xfs_inode_fork.h"
> > > > #include "xfs_inode.h"
> > > > #include "xfs_trans.h"
> > > > +#include "libfrog/platform.h"
> > > >
> > > > #include "libxfs.h" /* for LIBXFS_EXIT_ON_FAILURE */
> > > >
> > > > @@ -1227,9 +1228,11 @@ libxfs_brelse(
> > > >
> > > > if (!bp)
> > > > return;
> > > > - if (bp->b_flags & LIBXFS_B_DIRTY)
> > > > + if (bp->b_flags & LIBXFS_B_DIRTY) {
> > > > fprintf(stderr,
> > > > "releasing dirty buffer to free list!\n");
> > > > + bp->b_target->lost_writes = true;
> > > > + }
> > > >
> > > > pthread_mutex_lock(&xfs_buf_freelist.cm_mutex);
> > > > list_add(&bp->b_node.cn_mru, &xfs_buf_freelist.cm_list);
> > > > @@ -1248,9 +1251,11 @@ libxfs_bulkrelse(
> > > > return 0 ;
> > > >
> > > > list_for_each_entry(bp, list, b_node.cn_mru) {
> > > > - if (bp->b_flags & LIBXFS_B_DIRTY)
> > > > + if (bp->b_flags & LIBXFS_B_DIRTY) {
> > > > fprintf(stderr,
> > > > "releasing dirty buffer (bulk) to free list!\n");
> > > > + bp->b_target->lost_writes = true;
> > > > + }
> > > > count++;
> > > > }
> > > >
> > > > @@ -1479,6 +1484,24 @@ libxfs_irele(
> > > > kmem_cache_free(xfs_inode_zone, ip);
> > > > }
> > > >
> > > > +/*
> > > > + * Flush everything dirty in the kernel and disk write caches to stable media.
> > > > + * Returns 0 for success or a negative error code.
> > > > + */
> > > > +int
> > > > +libxfs_blkdev_issue_flush(
> > > > + struct xfs_buftarg *btp)
> > > > +{
> > > > + int fd, ret;
> > > > +
> > > > + if (btp->dev == 0)
> > > > + return 0;
> > > > +
> > > > + fd = libxfs_device_to_fd(btp->dev);
> > > > + ret = platform_flush_device(fd, btp->dev);
> > > > + return ret ? -errno : 0;
> > > > +}
> > > > +
> > > > /*
> > > > * Write out a buffer list synchronously.
> > > > *
> > > >
> > >
> >
>
^ permalink raw reply
* Re: [PATCH v2 0/6] NFS: Add support for the v4.2 READ_PLUS operation
From: Anna Schumaker @ 2020-02-20 18:25 UTC (permalink / raw)
To: Chuck Lever; +Cc: Trond.Myklebust, Linux NFS Mailing List
In-Reply-To: <34F6A8B4-3BC0-4C29-A6C1-176D3A866BFD@gmail.com>
On Thu, 2020-02-20 at 12:40 -0500, Chuck Lever wrote:
> Hi Anna-
>
> > On Feb 14, 2020, at 4:12 PM, schumaker.anna@gmail.com wrote:
> >
> > From: Anna Schumaker <Anna.Schumaker@Netapp.com>
> >
> > These patches add client support for the READ_PLUS operation, which
> > breaks read requests into several "data" and "hole" segments when
> > replying to the client. I also add a "noreadplus" mount option to allow
> > users to disable the new operation if it becomes a problem, similar to
> > the "nordirplus" mount option that we already have.
>
> Hrm, I went looking for the patch that adds "noreadplus", but I
> don't see it in this series?
You suggested dropping that patch in the v1 posting and waiting to see if
anybody asks for it.
>
> Wondering if, to start off, the default should be "noreadplus"
> until our feet are under us. Just a thought.
I could re-add the patch with this as the default if that's the way everybody
wants to go.
Anna
>
>
> > Here are the results of some performance tests I ran on Netapp lab
> > machines. I tested by reading various 2G files from a few different
> > undelying filesystems and across several NFS versions. I used the
> > `vmtouch` utility to make sure files were only cached when we wanted
> > them to be. In addition to 100% data and 100% hole cases, I also tested
> > with files that alternate between data and hole segments. These files
> > have either 4K, 8K, 16K, or 32K segment sizes and start with either data
> > or hole segments. So the file mixed-4d has a 4K segment size beginning
> > with a data segment, but mixed-32h hase 32K segments beginning with a
> > hole. The units are in seconds, with the first number for each NFS
> > version being the uncached read time and the second number is for when
> > the file is cached on the server.
> >
> > ext4 | v3 | v4.0 | v4.1 | v4.2
> > |
> > ----------|-----------------|-----------------|-----------------|-----------
> > ------|
> > data | 22.909 : 18.253 | 22.934 : 18.252 | 22.902 : 18.253 | 23.485 :
> > 18.253 |
> > hole | 18.256 : 18.253 | 18.255 : 18.252 | 18.256 : 18.253 | 0.708
> > : 0.709 |
> > mixed-4d | 28.261 : 18.253 | 29.616 : 18.252 | 28.341 : 18.252 | 24.508
> > : 9.150 |
> > mixed-8d | 27.956 : 18.253 | 28.404 : 18.252 | 28.320 : 18.252 | 23.967
> > : 9.140 |
> > mixed-16d | 28.172 : 18.253 | 27.946 : 18.252 | 27.627 : 18.252 | 23.043
> > : 9.134 |
> > mixed-32d | 25.350 : 18.253 | 24.406 : 18.252 | 24.384 : 18.253 | 20.698
> > : 9.132 |
> > mixed-4h | 28.913 : 18.253 | 28.564 : 18.252 | 27.996 : 18.252 | 21.837
> > : 9.150 |
> > mixed-8h | 28.625 : 18.253 | 27.833 : 18.252 | 27.798 : 18.253 | 21.710
> > : 9.140 |
> > mixed-16h | 27.975 : 18.253 | 27.662 : 18.252 | 27.795 : 18.253 | 20.585
> > : 9.134 |
> > mixed-32h | 25.958 : 18.253 | 25.491 : 18.252 | 24.856 : 18.252 | 21.018
> > : 9.132 |
> >
> > xfs | v3 | v4.0 | v4.1 | v4.2
> > |
> > ----------|-----------------|-----------------|-----------------|-----------
> > ------|
> > data | 22.041 : 18.253 | 22.618 : 18.252 | 23.067 : 18.253 | 23.496 :
> > 18.253 |
> > hole | 18.256 : 18.253 | 18.255 : 18.252 | 18.256 : 18.253 | 0.723
> > : 0.708 |
> > mixed-4d | 29.417 : 18.253 | 28.503 : 18.252 | 28.671 : 18.253 | 24.957
> > : 9.150 |
> > mixed-8d | 29.080 : 18.253 | 29.401 : 18.252 | 29.251 : 18.252 | 24.625
> > : 9.140 |
> > mixed-16d | 27.638 : 18.253 | 28.606 : 18.252 | 27.871 : 18.253 | 25.511
> > : 9.135 |
> > mixed-32d | 24.967 : 18.253 | 25.239 : 18.252 | 25.434 : 18.252 | 21.728
> > : 9.132 |
> > mixed-4h | 34.816 : 18.253 | 36.243 : 18.252 | 35.837 : 18.252 | 32.332
> > : 9.150 |
> > mixed-8h | 43.469 : 18.253 | 44.009 : 18.252 | 43.810 : 18.253 | 37.962
> > : 9.140 |
> > mixed-16h | 29.280 : 18.253 | 28.563 : 18.252 | 28.241 : 18.252 | 22.116
> > : 9.134 |
> > mixed-32h | 29.428 : 18.253 | 29.378 : 18.252 | 28.808 : 18.253 | 27.378
> > : 9.134 |
> >
> > btrfs | v3 | v4.0 | v4.1 | v4.2
> > |
> > ----------|-----------------|-----------------|-----------------|-----------
> > ------|
> > data | 25.547 : 18.253 | 25.053 : 18.252 | 24.209 : 18.253 | 32.121 :
> > 18.253 |
> > hole | 18.256 : 18.253 | 18.255 : 18.252 | 18.256 : 18.252 | 0.702
> > : 0.724 |
> > mixed-4d | 19.016 : 18.253 | 18.822 : 18.252 | 18.955 : 18.253 | 18.697
> > : 9.150 |
> > mixed-8d | 19.186 : 18.253 | 19.444 : 18.252 | 18.841 : 18.253 | 18.452
> > : 9.140 |
> > mixed-16d | 18.480 : 18.253 | 19.010 : 18.252 | 19.167 : 18.252 | 16.000
> > : 9.134 |
> > mixed-32d | 18.635 : 18.253 | 18.565 : 18.252 | 18.550 : 18.252 | 15.930
> > : 9.132 |
> > mixed-4h | 19.079 : 18.253 | 18.990 : 18.252 | 19.157 : 18.253 | 27.834
> > : 9.150 |
> > mixed-8h | 18.613 : 18.253 | 19.234 : 18.252 | 18.616 : 18.253 | 20.177
> > : 9.140 |
> > mixed-16h | 18.590 : 18.253 | 19.221 : 18.252 | 19.654 : 18.253 | 17.273
> > : 9.135 |
> > mixed-32h | 18.768 : 18.253 | 19.122 : 18.252 | 18.535 : 18.252 | 15.791
> > : 9.132 |
> >
> > ext3 | v3 | v4.0 | v4.1 | v4.2
> > |
> > ----------|-----------------|-----------------|-----------------|-----------
> > ------|
> > data | 34.292 : 18.253 | 33.810 : 18.252 | 33.450 : 18.253 | 33.390 :
> > 18.254 |
> > hole | 18.256 : 18.253 | 18.255 : 18.252 | 18.256 : 18.253 | 0.718
> > : 0.728 |
> > mixed-4d | 46.818 : 18.253 | 47.140 : 18.252 | 48.385 : 18.253 | 42.887
> > : 9.150 |
> > mixed-8d | 58.554 : 18.253 | 59.277 : 18.252 | 59.673 : 18.253 | 56.760
> > : 9.140 |
> > mixed-16d | 44.631 : 18.253 | 44.291 : 18.252 | 44.729 : 18.253 | 40.237
> > : 9.135 |
> > mixed-32d | 39.110 : 18.253 | 38.735 : 18.252 | 38.902 : 18.252 | 35.270
> > : 9.132 |
> > mixed-4h | 56.396 : 18.253 | 56.387 : 18.252 | 56.573 : 18.253 | 67.661
> > : 9.150 |
> > mixed-8h | 58.483 : 18.253 | 58.484 : 18.252 | 59.099 : 18.253 | 77.958
> > : 9.140 |
> > mixed-16h | 42.511 : 18.253 | 42.338 : 18.252 | 42.356 : 18.252 | 51.805
> > : 9.135 |
> > mixed-32h | 38.419 : 18.253 | 38.504 : 18.252 | 38.643 : 18.252 | 40.411
> > : 9.132 |
> >
> >
> > Changes since v1:
> > - Rebase to 5.6-rc1
> > - Drop the mount option patch for now
> > - Fix fallback to READ when the server doesn't support READ_PLUS
> >
> > Any questions?
> > Anna
> >
> >
> > Anna Schumaker (6):
> > SUNRPC: Split out a function for setting current page
> > SUNRPC: Add the ability to expand holes in data pages
> > SUNRPC: Add the ability to shift data to a specific offset
> > NFS: Add READ_PLUS data segment support
> > NFS: Add READ_PLUS hole segment decoding
> > NFS: Decode multiple READ_PLUS segments
> >
> > fs/nfs/nfs42xdr.c | 169 +++++++++++++++++++++++++
> > fs/nfs/nfs4proc.c | 43 ++++++-
> > fs/nfs/nfs4xdr.c | 1 +
> > include/linux/nfs4.h | 2 +-
> > include/linux/nfs_fs_sb.h | 1 +
> > include/linux/nfs_xdr.h | 2 +-
> > include/linux/sunrpc/xdr.h | 2 +
> > net/sunrpc/xdr.c | 244 ++++++++++++++++++++++++++++++++++++-
> > 8 files changed, 457 insertions(+), 7 deletions(-)
> >
> > --
> > 2.25.0
> >
>
> --
> Chuck Lever
> chucklever@gmail.com
>
>
>
^ permalink raw reply
* Re: [PATCH] mm: memcontrol: asynchronous reclaim for memory.high
From: Daniel Jordan @ 2020-02-20 18:23 UTC (permalink / raw)
To: Tejun Heo
Cc: Daniel Jordan, Johannes Weiner, Michal Hocko, Andrew Morton,
Roman Gushchin, linux-mm, cgroups, linux-kernel, kernel-team,
Peter Zijlstra
In-Reply-To: <20200220155651.GG698990@mtj.thefacebook.com>
On Thu, Feb 20, 2020 at 10:56:51AM -0500, Tejun Heo wrote:
> On Thu, Feb 20, 2020 at 10:45:24AM -0500, Daniel Jordan wrote:
> > Ok, consistency with io and memory is one advantage to doing it that way.
> > Creating kthreads in cgroups also seems viable so far, and it's unclear whether
> > either approach is significantly simpler or more maintainable than the other,
> > at least to me.
>
> The problem with separate kthread approach is that many of these work
> units are tiny, and cgroup membership might not be known or doesn't
> agree with the processing context from the beginning
The amount of work wouldn't seem to matter as long as the kernel thread stays
in the cgroup and lives long enough. There's only the one-time cost of
attaching it when it's forked. That seems doable for unbound workqueues (the
async reclaim), but may not be for the network packets.
The membership and context issues are pretty compelling though. Good to know,
I'll keep it in mind as I think this through.
> For example, the ownership of network packets can't be determined till
> processing has progressed quite a bit in shared contexts and each item
> too small to bounce around. The only viable way I can think of
> splitting aggregate overhead according to the number of packets (or
> some other trivially measureable quntity) processed.
>
> Anything sitting in reclaim layer is the same. Reclaim should be
> charged to the cgroup whose memory is reclaimed *but* shouldn't block
> other cgroups which are waiting for that memory. It has to happen in
> the context of the highest priority entity waiting for memory but the
> costs incurred must be charged to the memory owners.
>
> So, one way or the other, I think we'll need back charging and once
> back charging is needed for big ticket items like network and reclaim,
> it's kinda silly to use separate mechanisms for other stuff.
Yes, having both would appear to be redundant.
> > Is someone on your side working on remote charging right now? I was planning
> > to post an RFD comparing these soon and it would make sense to include them.
>
> It's been on the to do list but nobody is working on it yet.
Ok, thanks.
^ permalink raw reply
* [PATCH net-next] net: page_pool: Add documentation for page_pool API
From: Ilias Apalodimas @ 2020-02-20 18:25 UTC (permalink / raw)
To: brouer, davem, netdev; +Cc: lorenzo, rdunlap, toke, Ilias Apalodimas
Add documentation explaining the basic functionality and design
principles of the API
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
Documentation/networking/page_pool.rst | 159 +++++++++++++++++++++++++
1 file changed, 159 insertions(+)
create mode 100644 Documentation/networking/page_pool.rst
diff --git a/Documentation/networking/page_pool.rst b/Documentation/networking/page_pool.rst
new file mode 100644
index 000000000000..098d339ef272
--- /dev/null
+++ b/Documentation/networking/page_pool.rst
@@ -0,0 +1,159 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=============
+Page Pool API
+=============
+
+The page_pool allocator is optimized for the XDP mode that uses one frame
+per-page, but it can fallback on the regular page allocator APIs.
+
+Basic use involve replacing alloc_pages() calls with the
+page_pool_alloc_pages() call. Drivers should use page_pool_dev_alloc_pages()
+replacing dev_alloc_pages().
+
+API keeps track of inflight pages, in order to let API user know
+when it is safe to free a page_pool object. Thus, API users
+must run page_pool_release_page() when a page is leaving the page_pool or
+call page_pool_put_page() where appropriate in order to maintain correct
+accounting.
+
+API user must call page_pool_put_page() once on a page, as it
+will either recycle the page, or in case of refcnt > 1, it will
+release the DMA mapping and inflight state accounting.
+
+Architecture overview
+=====================
+
+.. code-block:: none
+
+ +------------------+
+ | Driver |
+ +------------------+
+ ^
+ |
+ |
+ |
+ v
+ +--------------------------------------------+
+ | request memory |
+ +--------------------------------------------+
+ ^ ^
+ | |
+ | Pool empty | Pool has entries
+ | |
+ v v
+ +-----------------------+ +------------------------+
+ | alloc (and map) pages | | get page from cache |
+ +-----------------------+ +------------------------+
+ ^ ^
+ | |
+ | cache available | No entries, refill
+ | | from ptr-ring
+ | |
+ v v
+ +-----------------+ +------------------+
+ | Fast cache | | ptr-ring cache |
+ +-----------------+ +------------------+
+
+API interface
+=============
+The number of pools created **must** match the number of hardware queues
+unless hardware restrictions make that impossible. This would otherwise beat the
+purpose of page pool, which is allocate pages fast from cache without locking.
+This lockless guarantee naturally comes from running under a NAPI softirq.
+The protection doesn't strictly have to be NAPI, any guarantee that allocating
+a page will cause no race conditions is enough.
+
+* page_pool_create(): Create a pool.
+ * flags: PP_FLAG_DMA_MAP, PP_FLAG_DMA_SYNC_DEV
+ * order: order^n pages on allocation
+ * pool_size: size of the ptr_ring
+ * nid: preferred NUMA node for allocation
+ * dev: struct device. Used on DMA operations
+ * dma_dir: DMA direction
+ * max_len: max DMA sync memory size
+ * offset: DMA address offset
+
+* page_pool_put_page(): The outcome of this depends on the page refcnt. If the
+ driver bumps the refcnt > 1 this will unmap the page. If the page refcnt is 1
+ the allocator owns the page and will try to recycle it in one of the pool
+ caches. If PP_FLAG_DMA_SYNC_DEV is set, the page will be synced for_device
+ using dma_sync_single_range_for_device().
+
+* page_pool_put_full_page(): Similar to page_pool_put_page(), but will DMA sync
+ for the entire memory area configured in area pool->max_len.
+
+* page_pool_recycle_direct(): Similar to page_pool_put_full_page() but caller
+ must guarantee safe context (e.g NAPI), since it will recycle the page
+ directly into the pool fast cache.
+
+* page_pool_release_page(): Unmap the page (if mapped) and account for it on
+ inflight counters.
+
+* page_pool_dev_alloc_pages(): Get a page from the page allocator or page_pool
+ caches.
+
+* page_pool_get_dma_addr(): Retrieve the stored DMA address.
+
+* page_pool_get_dma_dir(): Retrieve the stored DMA direction.
+
+Coding examples
+===============
+
+Registration
+------------
+
+.. code-block:: c
+
+ /* Page pool registration */
+ struct page_pool_params pp_params = { 0 };
+ struct xdp_rxq_info xdp_rxq;
+ int err;
+
+ pp_params.order = 0;
+ /* internal DMA mapping in page_pool */
+ pp_params.flags = PP_FLAG_DMA_MAP;
+ pp_params.pool_size = DESC_NUM;
+ pp_params.nid = NUMA_NO_NODE;
+ pp_params.dev = priv->dev;
+ pp_params.dma_dir = xdp_prog ? DMA_BIDIRECTIONAL : DMA_FROM_DEVICE;
+ page_pool = page_pool_create(&pp_params);
+
+ err = xdp_rxq_info_reg(&xdp_rxq, ndev, 0);
+ if (err)
+ goto err_out;
+
+ err = xdp_rxq_info_reg_mem_model(&xdp_rxq, MEM_TYPE_PAGE_POOL, page_pool);
+ if (err)
+ goto err_out;
+
+NAPI poller
+-----------
+
+
+.. code-block:: c
+
+ /* NAPI Rx poller */
+ enum dma_data_direction dma_dir;
+
+ dma_dir = page_pool_get_dma_dir(dring->page_pool);
+ while (done < budget) {
+ if (some error)
+ page_pool_recycle_direct(page_pool, page);
+ if (packet_is_xdp) {
+ if XDP_DROP:
+ page_pool_recycle_direct(page_pool, page);
+ } else (packet_is_skb) {
+ page_pool_release_page(page_pool, page);
+ new_page = page_pool_dev_alloc_pages(page_pool);
+ }
+ }
+
+Driver unload
+-------------
+
+.. code-block:: c
+
+ /* Driver unload */
+ page_pool_put_full_page(page_pool, page, false);
+ xdp_rxq_info_unreg(&xdp_rxq);
--
2.25.1
^ permalink raw reply related
* Re: [Xen-devel] [PATCH 3/3] xen/x86: Rename and simplify async_event_* infrastructure
From: Andrew Cooper @ 2020-02-20 18:24 UTC (permalink / raw)
To: Jan Beulich; +Cc: Xen-devel, Wei Liu, Roger Pau Monné
In-Reply-To: <8f3b2686-7d47-3899-2621-a8a45d2bbf02@suse.com>
On 18/02/2020 16:31, Jan Beulich wrote:
> On 17.02.2020 12:17, Andrew Cooper wrote:
>> --- a/xen/arch/x86/pv/iret.c
>> +++ b/xen/arch/x86/pv/iret.c
>> @@ -27,15 +27,15 @@ static void async_exception_cleanup(struct vcpu *curr)
>> {
>> unsigned int trap;
>>
>> - if ( !curr->arch.async_exception_mask )
>> + if ( !curr->arch.async_event_mask )
>> return;
>>
>> - if ( !(curr->arch.async_exception_mask & (curr->arch.async_exception_mask - 1)) )
>> - trap = __scanbit(curr->arch.async_exception_mask, VCPU_TRAP_NONE);
>> + if ( !(curr->arch.async_event_mask & (curr->arch.async_event_mask - 1)) )
>> + trap = __scanbit(curr->arch.async_event_mask, 0);
> The transformation just by itself is clearly not "no functional
> change"; it is together with the prior if(), but it took me a
> little to convince myself.
Well... It is no functional change, even if the fact isn't terribly obvious.
> I don't recall why VCPU_TRAP_NONE was
> used here originally (possibly just because of it being zero),
> but I think the latest now it would be better to use
> VCPU_TRAP_LAST + 1 instead, as 0 now has an actual meaning.
Half poking out of context is:
if ( unlikely(trap > VCPU_TRAP_LAST) )
{
ASSERT_UNREACHABLE();
return;
}
which would trigger on such an error path. That said, the following:
/* Restore previous asynchronous exception mask. */
curr->arch.async_event_mask = curr->arch.async_event[trap].old_mask;
will just pick the NMI old_mask in the case of something going wrong.
I deliberately made no adjustment here. I intend to replace it with
something which is correctly, rather than spending time trying to figure
out how some clearly broken logic was intended to "work".
>
>> @@ -557,12 +546,22 @@ struct arch_vcpu
>>
>> struct vpmu_struct vpmu;
>>
>> - struct {
>> - bool pending;
>> - uint8_t old_mask;
>> - } async_exception_state[VCPU_TRAP_LAST];
>> -#define async_exception_state(t) async_exception_state[(t)-1]
>> - uint8_t async_exception_mask;
>> + union {
>> +#define VCPU_TRAP_NMI 0
>> +#define VCPU_TRAP_MCE 1
>> +#define VCPU_TRAP_LAST VCPU_TRAP_MCE
>> + struct {
>> + bool pending;
>> + uint8_t old_mask;
>> + } async_event[VCPU_TRAP_LAST + 1];
>> + struct {
>> + bool nmi_pending;
>> + uint8_t nmi_old_mask;
>> + bool mce_pending;
>> + uint8_t mce_old_mask;
>> + };
>> + };
> How about
>
> union {
> #define VCPU_TRAP_NMI 0
> #define VCPU_TRAP_MCE 1
> #define VCPU_TRAP_LAST VCPU_TRAP_MCE
> struct async_event_state {
> bool pending;
> uint8_t old_mask;
> } async_event[VCPU_TRAP_LAST + 1];
> struct {
> struct async_event_state nmi;
> struct async_event_state mce;
> };
> };
>
> (structure tag subject to improvement)?
Can do. I don't expect any of this to survive, but I also don't yet
have a clear idea what form the eventual solution is going to take.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply
* Re: [PATCH] mm: memcontrol: asynchronous reclaim for memory.high
From: Daniel Jordan @ 2020-02-20 18:23 UTC (permalink / raw)
To: Tejun Heo
Cc: Daniel Jordan, Johannes Weiner, Michal Hocko, Andrew Morton,
Roman Gushchin, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
cgroups-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, kernel-team-b10kYP2dOMg,
Peter Zijlstra
In-Reply-To: <20200220155651.GG698990-146+VewaZzwNjtGbbfXrCEEOCMrvLtNR@public.gmane.org>
On Thu, Feb 20, 2020 at 10:56:51AM -0500, Tejun Heo wrote:
> On Thu, Feb 20, 2020 at 10:45:24AM -0500, Daniel Jordan wrote:
> > Ok, consistency with io and memory is one advantage to doing it that way.
> > Creating kthreads in cgroups also seems viable so far, and it's unclear whether
> > either approach is significantly simpler or more maintainable than the other,
> > at least to me.
>
> The problem with separate kthread approach is that many of these work
> units are tiny, and cgroup membership might not be known or doesn't
> agree with the processing context from the beginning
The amount of work wouldn't seem to matter as long as the kernel thread stays
in the cgroup and lives long enough. There's only the one-time cost of
attaching it when it's forked. That seems doable for unbound workqueues (the
async reclaim), but may not be for the network packets.
The membership and context issues are pretty compelling though. Good to know,
I'll keep it in mind as I think this through.
> For example, the ownership of network packets can't be determined till
> processing has progressed quite a bit in shared contexts and each item
> too small to bounce around. The only viable way I can think of
> splitting aggregate overhead according to the number of packets (or
> some other trivially measureable quntity) processed.
>
> Anything sitting in reclaim layer is the same. Reclaim should be
> charged to the cgroup whose memory is reclaimed *but* shouldn't block
> other cgroups which are waiting for that memory. It has to happen in
> the context of the highest priority entity waiting for memory but the
> costs incurred must be charged to the memory owners.
>
> So, one way or the other, I think we'll need back charging and once
> back charging is needed for big ticket items like network and reclaim,
> it's kinda silly to use separate mechanisms for other stuff.
Yes, having both would appear to be redundant.
> > Is someone on your side working on remote charging right now? I was planning
> > to post an RFD comparing these soon and it would make sense to include them.
>
> It's been on the to do list but nobody is working on it yet.
Ok, thanks.
^ 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.