From: Michael Ellerman <mpe@ellerman.id.au> To: Laurent Dufour <ldufour@linux.vnet.ibm.com>, linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org Cc: aneesh.kumar@linux.ibm.com, benh@kernel.crashing.org, paulus@samba.org, npiggin@gmail.com Subject: Re: [PATCH 3/3] powerpc/pseries/mm: call H_BLOCK_REMOVE Date: Mon, 30 Jul 2018 23:47:01 +1000 [thread overview] Message-ID: <877elcj0oa.fsf@concordia.ellerman.id.au> (raw) In-Reply-To: <1532699493-10883-4-git-send-email-ldufour@linux.vnet.ibm.com> Hi Laurent, Just one comment below. Laurent Dufour <ldufour@linux.vnet.ibm.com> writes: > diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platforms/pseries/lpar.c > index 96b8cd8a802d..41ed03245eb4 100644 > --- a/arch/powerpc/platforms/pseries/lpar.c > +++ b/arch/powerpc/platforms/pseries/lpar.c > @@ -418,6 +418,73 @@ static void pSeries_lpar_hpte_invalidate(unsigned long slot, unsigned long vpn, > BUG_ON(lpar_rc != H_SUCCESS); > } > > + > +/* > + * As defined in the PAPR's section 14.5.4.1.8 > + * The control mask doesn't include the returned reference and change bit from > + * the processed PTE. > + */ > +#define HBLKR_AVPN 0x0100000000000000UL > +#define HBLKR_CTRL_MASK 0xf800000000000000UL > +#define HBLKR_CTRL_SUCCESS 0x8000000000000000UL > +#define HBLKR_CTRL_ERRNOTFOUND 0x8800000000000000UL > +#define HBLKR_CTRL_ERRBUSY 0xa000000000000000UL > + > +/** > + * H_BLOCK_REMOVE caller. > + * @idx should point to the latest @param entry set with a PTEX. > + * If PTE cannot be processed because another CPUs has already locked that > + * group, those entries are put back in @param starting at index 1. > + * If entries has to be retried and @retry_busy is set to true, these entries > + * are retried until success. If @retry_busy is set to false, the returned > + * is the number of entries yet to process. > + */ > +static unsigned long call_block_remove(unsigned long idx, unsigned long *param, > + bool retry_busy) > +{ > + unsigned long i, rc, new_idx; > + unsigned long retbuf[PLPAR_HCALL9_BUFSIZE]; > + > +again: > + new_idx = 0; > + BUG_ON((idx < 2) || (idx > PLPAR_HCALL9_BUFSIZE)); I count 1 .. > + if (idx < PLPAR_HCALL9_BUFSIZE) > + param[idx] = HBR_END; > + > + rc = plpar_hcall9(H_BLOCK_REMOVE, retbuf, > + param[0], /* AVA */ > + param[1], param[2], param[3], param[4], /* TS0-7 */ > + param[5], param[6], param[7], param[8]); > + if (rc == H_SUCCESS) > + return 0; > + > + BUG_ON(rc != H_PARTIAL); 2 ... > + /* Check that the unprocessed entries were 'not found' or 'busy' */ > + for (i = 0; i < idx-1; i++) { > + unsigned long ctrl = retbuf[i] & HBLKR_CTRL_MASK; > + > + if (ctrl == HBLKR_CTRL_ERRBUSY) { > + param[++new_idx] = param[i+1]; > + continue; > + } > + > + BUG_ON(ctrl != HBLKR_CTRL_SUCCESS > + && ctrl != HBLKR_CTRL_ERRNOTFOUND); 3 ... BUG_ON()s. I know the code in this file is already pretty liberal with the use of BUG_ON() but I'd prefer if we don't make it any worse. Given this is an optimisation it seems like we should be able to fall back to the existing implementation in the case of error (which will probably then BUG_ON() 😂) If there's some reason we can't then I guess I can live with it. cheers
WARNING: multiple messages have this Message-ID (diff)
From: Michael Ellerman <mpe@ellerman.id.au> To: Laurent Dufour <ldufour@linux.vnet.ibm.com>, linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org Cc: aneesh.kumar@linux.ibm.com, benh@kernel.crashing.org, paulus@samba.org, npiggin@gmail.com Subject: Re: [PATCH 3/3] powerpc/pseries/mm: call H_BLOCK_REMOVE Date: Mon, 30 Jul 2018 23:47:01 +1000 [thread overview] Message-ID: <877elcj0oa.fsf@concordia.ellerman.id.au> (raw) In-Reply-To: <1532699493-10883-4-git-send-email-ldufour@linux.vnet.ibm.com> Hi Laurent, Just one comment below. Laurent Dufour <ldufour@linux.vnet.ibm.com> writes: > diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platfor= ms/pseries/lpar.c > index 96b8cd8a802d..41ed03245eb4 100644 > --- a/arch/powerpc/platforms/pseries/lpar.c > +++ b/arch/powerpc/platforms/pseries/lpar.c > @@ -418,6 +418,73 @@ static void pSeries_lpar_hpte_invalidate(unsigned lo= ng slot, unsigned long vpn, > BUG_ON(lpar_rc !=3D H_SUCCESS); > } >=20=20 > + > +/* > + * As defined in the PAPR's section 14.5.4.1.8 > + * The control mask doesn't include the returned reference and change bi= t from > + * the processed PTE. > + */ > +#define HBLKR_AVPN 0x0100000000000000UL > +#define HBLKR_CTRL_MASK 0xf800000000000000UL > +#define HBLKR_CTRL_SUCCESS 0x8000000000000000UL > +#define HBLKR_CTRL_ERRNOTFOUND 0x8800000000000000UL > +#define HBLKR_CTRL_ERRBUSY 0xa000000000000000UL > + > +/** > + * H_BLOCK_REMOVE caller. > + * @idx should point to the latest @param entry set with a PTEX. > + * If PTE cannot be processed because another CPUs has already locked th= at > + * group, those entries are put back in @param starting at index 1. > + * If entries has to be retried and @retry_busy is set to true, these en= tries > + * are retried until success. If @retry_busy is set to false, the return= ed > + * is the number of entries yet to process. > + */ > +static unsigned long call_block_remove(unsigned long idx, unsigned long = *param, > + bool retry_busy) > +{ > + unsigned long i, rc, new_idx; > + unsigned long retbuf[PLPAR_HCALL9_BUFSIZE]; > + > +again: > + new_idx =3D 0; > + BUG_ON((idx < 2) || (idx > PLPAR_HCALL9_BUFSIZE)); I count 1 .. > + if (idx < PLPAR_HCALL9_BUFSIZE) > + param[idx] =3D HBR_END; > + > + rc =3D plpar_hcall9(H_BLOCK_REMOVE, retbuf, > + param[0], /* AVA */ > + param[1], param[2], param[3], param[4], /* TS0-7 */ > + param[5], param[6], param[7], param[8]); > + if (rc =3D=3D H_SUCCESS) > + return 0; > + > + BUG_ON(rc !=3D H_PARTIAL); 2 ... > + /* Check that the unprocessed entries were 'not found' or 'busy' */ > + for (i =3D 0; i < idx-1; i++) { > + unsigned long ctrl =3D retbuf[i] & HBLKR_CTRL_MASK; > + > + if (ctrl =3D=3D HBLKR_CTRL_ERRBUSY) { > + param[++new_idx] =3D param[i+1]; > + continue; > + } > + > + BUG_ON(ctrl !=3D HBLKR_CTRL_SUCCESS > + && ctrl !=3D HBLKR_CTRL_ERRNOTFOUND); 3 ... BUG_ON()s. I know the code in this file is already pretty liberal with the use of BUG_ON() but I'd prefer if we don't make it any worse. Given this is an optimisation it seems like we should be able to fall back to the existing implementation in the case of error (which will probably then BUG_ON() =F0=9F=98=82) If there's some reason we can't then I guess I can live with it. cheers
next prev parent reply other threads:[~2018-07-30 13:47 UTC|newest] Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-07-27 13:51 [resend] [PATCH 0/3] powerpc/pseries: use H_BLOCK_REMOVE Laurent Dufour 2018-07-27 13:51 ` [PATCH 1/3] powerpc/pseries/mm: Introducing FW_FEATURE_BLOCK_REMOVE Laurent Dufour 2018-07-30 14:18 ` Aneesh Kumar K.V 2018-07-27 13:51 ` [PATCH 2/3] powerpc/pseries/mm: factorize PTE slot computation Laurent Dufour 2018-07-30 14:19 ` Aneesh Kumar K.V 2018-07-27 13:51 ` [PATCH 3/3] powerpc/pseries/mm: call H_BLOCK_REMOVE Laurent Dufour 2018-07-30 13:47 ` Michael Ellerman [this message] 2018-07-30 13:47 ` Michael Ellerman 2018-07-30 14:22 ` Aneesh Kumar K.V 2018-07-30 14:22 ` Aneesh Kumar K.V 2018-08-16 17:27 ` Laurent Dufour 2018-08-16 9:41 ` Laurent Dufour 2018-08-01 1:55 ` [resend] [PATCH 0/3] powerpc/pseries: use H_BLOCK_REMOVE Nicholas Piggin 2018-08-03 2:29 ` Michael Ellerman -- strict thread matches above, loose matches on Subject: below -- 2018-07-27 13:22 Laurent Dufour 2018-07-27 13:22 ` [PATCH 3/3] powerpc/pseries/mm: call H_BLOCK_REMOVE Laurent Dufour
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=877elcj0oa.fsf@concordia.ellerman.id.au \ --to=mpe@ellerman.id.au \ --cc=aneesh.kumar@linux.ibm.com \ --cc=benh@kernel.crashing.org \ --cc=ldufour@linux.vnet.ibm.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linuxppc-dev@lists.ozlabs.org \ --cc=npiggin@gmail.com \ --cc=paulus@samba.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.