2022-10-10 18:47:37

by Cleo John

[permalink] [raw]
Subject: [PATCH v2] riscv: fix styling in ucontext header

Change the two comments in ucontext.h by getting them up to
the coding style proposed by torvalds.

Signed-off-by: Cleo John <[email protected]>
---
In my opinion this also improves the readability so I think this is a useful change to do.
Please also tell me if you have a different opinion.

Changes in v2:
- change the styling of the top comments too

arch/riscv/include/uapi/asm/ucontext.h | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/riscv/include/uapi/asm/ucontext.h b/arch/riscv/include/uapi/asm/ucontext.h
index 44eb993950e5..516bd0bb0da5 100644
--- a/arch/riscv/include/uapi/asm/ucontext.h
+++ b/arch/riscv/include/uapi/asm/ucontext.h
@@ -15,19 +15,23 @@ struct ucontext {
struct ucontext *uc_link;
stack_t uc_stack;
sigset_t uc_sigmask;
- /* There's some padding here to allow sigset_t to be expanded in the
+ /*
+ * There's some padding here to allow sigset_t to be expanded in the
* future. Though this is unlikely, other architectures put uc_sigmask
* at the end of this structure and explicitly state it can be
- * expanded, so we didn't want to box ourselves in here. */
+ * expanded, so we didn't want to box ourselves in here.
+ */
__u8 __unused[1024 / 8 - sizeof(sigset_t)];
- /* We can't put uc_sigmask at the end of this structure because we need
+ /*
+ * We can't put uc_sigmask at the end of this structure because we need
* to be able to expand sigcontext in the future. For example, the
* vector ISA extension will almost certainly add ISA state. We want
* to ensure all user-visible ISA state can be saved and restored via a
* ucontext, so we're putting this at the end in order to allow for
* infinite extensibility. Since we know this will be extended and we
* assume sigset_t won't be extended an extreme amount, we're
- * prioritizing this. */
+ * prioritizing this.
+ */
struct sigcontext uc_mcontext;
};

--
2.25.1


2022-10-10 18:58:18

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2] riscv: fix styling in ucontext header

On Mon, Oct 10, 2022 at 08:28:48PM +0200, Cleo John wrote:
> Change the two comments in ucontext.h by getting them up to
> the coding style proposed by torvalds.
>
> Signed-off-by: Cleo John <[email protected]>
> ---
> In my opinion this also improves the readability so I think this is a useful change to do.
> Please also tell me if you have a different opinion.

I don't think it is all that /important/ of a change, but it does make
things match between this file and the other headers.
Reviewed-by: Conor Dooley <[email protected]>

Thanks.

>
> Changes in v2:
> - change the styling of the top comments too
>
> arch/riscv/include/uapi/asm/ucontext.h | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/arch/riscv/include/uapi/asm/ucontext.h b/arch/riscv/include/uapi/asm/ucontext.h
> index 44eb993950e5..516bd0bb0da5 100644
> --- a/arch/riscv/include/uapi/asm/ucontext.h
> +++ b/arch/riscv/include/uapi/asm/ucontext.h
> @@ -15,19 +15,23 @@ struct ucontext {
> struct ucontext *uc_link;
> stack_t uc_stack;
> sigset_t uc_sigmask;
> - /* There's some padding here to allow sigset_t to be expanded in the
> + /*
> + * There's some padding here to allow sigset_t to be expanded in the
> * future. Though this is unlikely, other architectures put uc_sigmask
> * at the end of this structure and explicitly state it can be
> - * expanded, so we didn't want to box ourselves in here. */
> + * expanded, so we didn't want to box ourselves in here.
> + */
> __u8 __unused[1024 / 8 - sizeof(sigset_t)];
> - /* We can't put uc_sigmask at the end of this structure because we need
> + /*
> + * We can't put uc_sigmask at the end of this structure because we need
> * to be able to expand sigcontext in the future. For example, the
> * vector ISA extension will almost certainly add ISA state. We want
> * to ensure all user-visible ISA state can be saved and restored via a
> * ucontext, so we're putting this at the end in order to allow for
> * infinite extensibility. Since we know this will be extended and we
> * assume sigset_t won't be extended an extreme amount, we're
> - * prioritizing this. */
> + * prioritizing this.
> + */
> struct sigcontext uc_mcontext;
> };
>
> --
> 2.25.1
>

2022-10-10 20:17:41

by Cleo John

[permalink] [raw]
Subject: Re: [PATCH v2] riscv: fix styling in ucontext header

Am Montag, 10. Oktober 2022, 20:50:56 CEST schrieb Conor Dooley:
> On Mon, Oct 10, 2022 at 08:28:48PM +0200, Cleo John wrote:
> > Change the two comments in ucontext.h by getting them up to
> > the coding style proposed by torvalds.
> >
> > Signed-off-by: Cleo John <[email protected]>
> > ---
> > In my opinion this also improves the readability so I think this is a useful change to do.
> > Please also tell me if you have a different opinion.
>
> I don't think it is all that /important/ of a change, but it does make
> things match between this file and the other headers.
> Reviewed-by: Conor Dooley <[email protected]>
>
> Thanks.
>

Yes, its not that important. Thats why I chose it. :D
To be honest this is my first commit to the kernel so
I wanted to do something simple to start things of
easy and to get more familiar with the procedure,
before getting my feet wet into some real kernel
additions.

Thanks for helping!

> >
> > Changes in v2:
> > - change the styling of the top comments too
> >
> > arch/riscv/include/uapi/asm/ucontext.h | 12 ++++++++----
> > 1 file changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/riscv/include/uapi/asm/ucontext.h b/arch/riscv/include/uapi/asm/ucontext.h
> > index 44eb993950e5..516bd0bb0da5 100644
> > --- a/arch/riscv/include/uapi/asm/ucontext.h
> > +++ b/arch/riscv/include/uapi/asm/ucontext.h
> > @@ -15,19 +15,23 @@ struct ucontext {
> > struct ucontext *uc_link;
> > stack_t uc_stack;
> > sigset_t uc_sigmask;
> > - /* There's some padding here to allow sigset_t to be expanded in the
> > + /*
> > + * There's some padding here to allow sigset_t to be expanded in the
> > * future. Though this is unlikely, other architectures put uc_sigmask
> > * at the end of this structure and explicitly state it can be
> > - * expanded, so we didn't want to box ourselves in here. */
> > + * expanded, so we didn't want to box ourselves in here.
> > + */
> > __u8 __unused[1024 / 8 - sizeof(sigset_t)];
> > - /* We can't put uc_sigmask at the end of this structure because we need
> > + /*
> > + * We can't put uc_sigmask at the end of this structure because we need
> > * to be able to expand sigcontext in the future. For example, the
> > * vector ISA extension will almost certainly add ISA state. We want
> > * to ensure all user-visible ISA state can be saved and restored via a
> > * ucontext, so we're putting this at the end in order to allow for
> > * infinite extensibility. Since we know this will be extended and we
> > * assume sigset_t won't be extended an extreme amount, we're
> > - * prioritizing this. */
> > + * prioritizing this.
> > + */
> > struct sigcontext uc_mcontext;
> > };
> >
>


Attachments:
signature.asc (849.00 B)
This is a digitally signed message part.

2022-10-10 20:57:54

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2] riscv: fix styling in ucontext header

On Mon, Oct 10, 2022 at 09:55:17PM +0200, Cleo John wrote:
> Am Montag, 10. Oktober 2022, 20:50:56 CEST schrieb Conor Dooley:
> > On Mon, Oct 10, 2022 at 08:28:48PM +0200, Cleo John wrote:
> > > Change the two comments in ucontext.h by getting them up to
> > > the coding style proposed by torvalds.
> > >
> > > Signed-off-by: Cleo John <[email protected]>
> > > ---
> > > In my opinion this also improves the readability so I think this is a useful change to do.
> > > Please also tell me if you have a different opinion.
> >
> > I don't think it is all that /important/ of a change, but it does make
> > things match between this file and the other headers.
> > Reviewed-by: Conor Dooley <[email protected]>
> >
> > Thanks.
> >
>
> Yes, its not that important. Thats why I chose it. :D

:)

> To be honest this is my first commit to the kernel so
> I wanted to do something simple to start things of
> easy and to get more familiar with the procedure,
> before getting my feet wet into some real kernel
> additions.

Cool, nice to have you & good luck!

> Thanks for helping!

nw, hopefully I wasn't too direct/negative.

Conor.

>
> > >
> > > Changes in v2:
> > > - change the styling of the top comments too
> > >
> > > arch/riscv/include/uapi/asm/ucontext.h | 12 ++++++++----
> > > 1 file changed, 8 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/arch/riscv/include/uapi/asm/ucontext.h b/arch/riscv/include/uapi/asm/ucontext.h
> > > index 44eb993950e5..516bd0bb0da5 100644
> > > --- a/arch/riscv/include/uapi/asm/ucontext.h
> > > +++ b/arch/riscv/include/uapi/asm/ucontext.h
> > > @@ -15,19 +15,23 @@ struct ucontext {
> > > struct ucontext *uc_link;
> > > stack_t uc_stack;
> > > sigset_t uc_sigmask;
> > > - /* There's some padding here to allow sigset_t to be expanded in the
> > > + /*
> > > + * There's some padding here to allow sigset_t to be expanded in the
> > > * future. Though this is unlikely, other architectures put uc_sigmask
> > > * at the end of this structure and explicitly state it can be
> > > - * expanded, so we didn't want to box ourselves in here. */
> > > + * expanded, so we didn't want to box ourselves in here.
> > > + */
> > > __u8 __unused[1024 / 8 - sizeof(sigset_t)];
> > > - /* We can't put uc_sigmask at the end of this structure because we need
> > > + /*
> > > + * We can't put uc_sigmask at the end of this structure because we need
> > > * to be able to expand sigcontext in the future. For example, the
> > > * vector ISA extension will almost certainly add ISA state. We want
> > > * to ensure all user-visible ISA state can be saved and restored via a
> > > * ucontext, so we're putting this at the end in order to allow for
> > > * infinite extensibility. Since we know this will be extended and we
> > > * assume sigset_t won't be extended an extreme amount, we're
> > > - * prioritizing this. */
> > > + * prioritizing this.
> > > + */
> > > struct sigcontext uc_mcontext;
> > > };
> > >
> >
>


2022-10-19 11:17:20

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2] riscv: fix styling in ucontext header

On Wed, Oct 19, 2022 at 12:07:58PM +0200, Cleo John wrote:
>
> Hey, because I am new to Kernel submissions I wanted to ask if there is a way for me to see / track how far this commit has gone in the pipeline of commits?

https://patchwork.kernel.org/project/linux-riscv/patch/20221010182848.GA28029@watet-ms7b87/

IIRC you sent the patch during the merge window, so it wouldn't be
applied for v6.1-rc1. You'll get an email from the patchwork-bot when it
gets applied.

HTH,
Conor.

2022-10-19 11:24:30

by Cleo John

[permalink] [raw]
Subject: Re: [PATCH v2] riscv: fix styling in ucontext header

On Mon, Oct 10, 2022 at 22:41:08 CEST, Conor Dooley wrote:
> On Mon, Oct 10, 2022 at 09:55:17PM +0200, Cleo John wrote:
> > Am Montag, 10. Oktober 2022, 20:50:56 CEST schrieb Conor Dooley:
> > > On Mon, Oct 10, 2022 at 08:28:48PM +0200, Cleo John wrote:
> > > > Change the two comments in ucontext.h by getting them up to
> > > > the coding style proposed by torvalds.
> > > >
> > > > Signed-off-by: Cleo John <[email protected]>
> > > > ---
> > > > In my opinion this also improves the readability so I think this is a useful change to do.
> > > > Please also tell me if you have a different opinion.
> > >
> > > I don't think it is all that /important/ of a change, but it does make
> > > things match between this file and the other headers.
> > > Reviewed-by: Conor Dooley <[email protected]>
> > >
> > > Thanks.
> > >
> >
> > Yes, its not that important. Thats why I chose it. :D
>
> :)
>
> > To be honest this is my first commit to the kernel so
> > I wanted to do something simple to start things of
> > easy and to get more familiar with the procedure,
> > before getting my feet wet into some real kernel
> > additions.
>
> Cool, nice to have you & good luck!
>
> > Thanks for helping!
>
> nw, hopefully I wasn't too direct/negative.
>
> Conor.
>
> >
> > > >
> > > > Changes in v2:
> > > > - change the styling of the top comments too
> > > >
> > > > arch/riscv/include/uapi/asm/ucontext.h | 12 ++++++++----
> > > > 1 file changed, 8 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/arch/riscv/include/uapi/asm/ucontext.h b/arch/riscv/include/uapi/asm/ucontext.h
> > > > index 44eb993950e5..516bd0bb0da5 100644
> > > > --- a/arch/riscv/include/uapi/asm/ucontext.h
> > > > +++ b/arch/riscv/include/uapi/asm/ucontext.h
> > > > @@ -15,19 +15,23 @@ struct ucontext {
> > > > struct ucontext *uc_link;
> > > > stack_t uc_stack;
> > > > sigset_t uc_sigmask;
> > > > - /* There's some padding here to allow sigset_t to be expanded in the
> > > > + /*
> > > > + * There's some padding here to allow sigset_t to be expanded in the
> > > > * future. Though this is unlikely, other architectures put uc_sigmask
> > > > * at the end of this structure and explicitly state it can be
> > > > - * expanded, so we didn't want to box ourselves in here. */
> > > > + * expanded, so we didn't want to box ourselves in here.
> > > > + */
> > > > __u8 __unused[1024 / 8 - sizeof(sigset_t)];
> > > > - /* We can't put uc_sigmask at the end of this structure because we need
> > > > + /*
> > > > + * We can't put uc_sigmask at the end of this structure because we need
> > > > * to be able to expand sigcontext in the future. For example, the
> > > > * vector ISA extension will almost certainly add ISA state. We want
> > > > * to ensure all user-visible ISA state can be saved and restored via a
> > > > * ucontext, so we're putting this at the end in order to allow for
> > > > * infinite extensibility. Since we know this will be extended and we
> > > > * assume sigset_t won't be extended an extreme amount, we're
> > > > - * prioritizing this. */
> > > > + * prioritizing this.
> > > > + */
> > > > struct sigcontext uc_mcontext;
> > > > };
> > > >
> > >
> >
>
>
>

Hey, because I am new to Kernel submissions I wanted to ask if there is a way for me to see / track how far this commit has gone in the pipeline of commits?

Thanks,
Cleo


Attachments:
signature.asc (849.00 B)
This is a digitally signed message part.

2022-10-28 22:44:09

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH v2] riscv: fix styling in ucontext header

On Wed, 19 Oct 2022 03:20:44 PDT (-0700), Conor Dooley wrote:
> On Wed, Oct 19, 2022 at 12:07:58PM +0200, Cleo John wrote:
>>
>> Hey, because I am new to Kernel submissions I wanted to ask if there is a way for me to see / track how far this commit has gone in the pipeline of commits?
>
> https://patchwork.kernel.org/project/linux-riscv/patch/20221010182848.GA28029@watet-ms7b87/
>
> IIRC you sent the patch during the merge window, so it wouldn't be
> applied for v6.1-rc1. You'll get an email from the patchwork-bot when it
> gets applied.

Yep, this is one of the more confusing parts of the Linux development
process for new folks (or at least was for me when I was new): you send
a patch and it's not super clear what's going to happen to it for a
while. The merge window is generally a pretty hectic time, so stuff
like this that's not fixing a bug or adding some frameware that other
patches depend on can kind of get lost in the shuffle.

I always feel kind of bad for folks about that, but patchwork should
help some here as at least we can get the smaller stuff called out.

This is on for-next, thanks!

2022-10-28 22:56:55

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2] riscv: fix styling in ucontext header

On Fri, Oct 28, 2022 at 03:23:21PM -0700, Palmer Dabbelt wrote:
> On Wed, 19 Oct 2022 03:20:44 PDT (-0700), Conor Dooley wrote:
> > On Wed, Oct 19, 2022 at 12:07:58PM +0200, Cleo John wrote:
> > >
> > > Hey, because I am new to Kernel submissions I wanted to ask if there is a way for me to see / track how far this commit has gone in the pipeline of commits?
> >
> > https://patchwork.kernel.org/project/linux-riscv/patch/20221010182848.GA28029@watet-ms7b87/
> >
> > IIRC you sent the patch during the merge window, so it wouldn't be
> > applied for v6.1-rc1. You'll get an email from the patchwork-bot when it
> > gets applied.
>
> Yep, this is one of the more confusing parts of the Linux development
> process for new folks (or at least was for me when I was new): you send a

Nope, was the case for me too. Same applies to most subsystems, think
probably places like netdev are good in that regard as their review
cadence is really good.

> patch and it's not super clear what's going to happen to it for a while.
> The merge window is generally a pretty hectic time, so stuff like this
> that's not fixing a bug or adding some frameware that other patches depend
> on can kind of get lost in the shuffle.
>
> I always feel kind of bad for folks about that, but patchwork should help
> some here as at least we can get the smaller stuff called out.

Yeah, hopefully patchwork helps. If we keep on top of it & over time
it'll get more useful for infrequent contributors - but for first time
contributors I'm not sure what to do other than for people that notice
it is a first time contributor to be helpful to them?

2022-10-28 22:58:14

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH v2] riscv: fix styling in ucontext header

On Mon, 10 Oct 2022 20:28:48 +0200, Cleo John wrote:
> Change the two comments in ucontext.h by getting them up to
> the coding style proposed by torvalds.
>
>

Applied, thanks!

[1/1] riscv: fix styling in ucontext header
https://git.kernel.org/palmer/c/3558927fc2b2

Best regards,
--
Palmer Dabbelt <[email protected]>