| Message ID | 20221010182848.GA28029@watet-ms7b87 (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 3558927fc2b2fd0af309648f4071035e08719866 |
| Delegated to: | Palmer Dabbelt |
| Headers | show |
| Series | [v2] riscv: fix styling in ucontext header | expand |
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 <waterdev@galaxycrow.de> > --- > 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 <conor.dooley@microchip.com> 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 >
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 <waterdev@galaxycrow.de> > > --- > > 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 <conor.dooley@microchip.com> > > 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; > > }; > > >
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 <waterdev@galaxycrow.de> > > > --- > > > 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 <conor.dooley@microchip.com> > > > > 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; > > > }; > > > > > >
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 <waterdev@galaxycrow.de> > > > > --- > > > > 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 <conor.dooley@microchip.com> > > > > > > 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
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.
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,
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!
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?
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; };
Change the two comments in ucontext.h by getting them up to the coding style proposed by torvalds. Signed-off-by: Cleo John <waterdev@galaxycrow.de> --- 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(-)