[RFC,3/3] libnvdimm, MAINTAINERS: Subsystem Profile

Commit Message

Dan Williams Nov. 15, 2018, 4:53 a.m. UTC

Document the basic policies of the libnvdimm subsystem and provide a
first example of a Subsystem Profile for others to duplicate and edit.

Cc: Ross Zwisler <[e-mail]>
Cc: Vishal Verma <[e-mail]>
Cc: Dave Jiang <[e-mail]>
Signed-off-by: Dan Williams <[e-mail]>
---
 Documentation/nvdimm/subsystem-profile.rst |   86 ++++++++++++++++++++++++++++
 MAINTAINERS                                |    4 +
 2 files changed, 90 insertions(+)
 create mode 100644 Documentation/nvdimm/subsystem-profile.rst

Comments

Geert Uytterhoeven Nov. 15, 2018, 8:03 a.m. UTC | #1

Hi Dan,

On Thu, Nov 15, 2018 at 6:06 AM Dan Williams <[e-mail]> wrote:
> Document the basic policies of the libnvdimm subsystem and provide a
> first example of a Subsystem Profile for others to duplicate and edit.
>
> Cc: Ross Zwisler <[e-mail]>
> Cc: Vishal Verma <[e-mail]>
> Cc: Dave Jiang <[e-mail]>
> Signed-off-by: Dan Williams <[e-mail]>

Thanks for your patch!

> --- /dev/null
> +++ b/Documentation/nvdimm/subsystem-profile.rst

> +Trusted Reviewers
> +-----------------
> +Johannes Thumshirn
> +Toshi Kani
> +Jeff Moyer
> +Robert Elliott

Don't you want to add email addresses?
Only the first one is listed in MAINTAINERS.

> +Time Zone / Office Hours
> +------------------------
> +8:00am to 5:00pm Pacific Time Zone

UTC-???

People are not familiar with all time zones.

Gr{oetje,eeting}s,

                        Geert

Mauro Carvalho Chehab Nov. 15, 2018, 2:10 p.m. UTC | #2

Em Thu, 15 Nov 2018 09:03:11 +0100
Geert Uytterhoeven <[e-mail]> escreveu:

> Hi Dan,
> 
> On Thu, Nov 15, 2018 at 6:06 AM Dan Williams <[e-mail]> wrote:
> > Document the basic policies of the libnvdimm subsystem and provide a
> > first example of a Subsystem Profile for others to duplicate and edit.
> >
> > Cc: Ross Zwisler <[e-mail]>
> > Cc: Vishal Verma <[e-mail]>
> > Cc: Dave Jiang <[e-mail]>
> > Signed-off-by: Dan Williams <[e-mail]>  
> 
> Thanks for your patch!
> 
> > --- /dev/null
> > +++ b/Documentation/nvdimm/subsystem-profile.rst  
> 
> > +Trusted Reviewers
> > +-----------------
> > +Johannes Thumshirn
> > +Toshi Kani
> > +Jeff Moyer
> > +Robert Elliott  
> 
> Don't you want to add email addresses?
> Only the first one is listed in MAINTAINERS.

IMO, it makes sense to have their e-mails here, in a way that it could
easily be parsed by get_maintainers.pl.

That's said, IMO a given subsystem should either use the R: tag at
MAINTAINERS or have a list of reviewers or sub-maintainers here, as
maintaining duplicated information will be painful.

If we go to this path, we should probably document it at MAINTAINERS
file.

> 
> > +Time Zone / Office Hours
> > +------------------------
> > +8:00am to 5:00pm Pacific Time Zone  
> 
> UTC-???
> 
> People are not familiar with all time zones.
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 




Cheers,
Mauro

Mauro Carvalho Chehab Nov. 15, 2018, 2:30 p.m. UTC | #3

Em Wed, 14 Nov 2018 20:53:30 -0800
Dan Williams <[e-mail]> escreveu:

> Document the basic policies of the libnvdimm subsystem and provide a
> first example of a Subsystem Profile for others to duplicate and edit.
> 
> Cc: Ross Zwisler <[e-mail]>
> Cc: Vishal Verma <[e-mail]>
> Cc: Dave Jiang <[e-mail]>
> Signed-off-by: Dan Williams <[e-mail]>
> ---
>  Documentation/nvdimm/subsystem-profile.rst |   86 ++++++++++++++++++++++++++++
>  MAINTAINERS                                |    4 +
>  2 files changed, 90 insertions(+)
>  create mode 100644 Documentation/nvdimm/subsystem-profile.rst
> 
> diff --git a/Documentation/nvdimm/subsystem-profile.rst b/Documentation/nvdimm/subsystem-profile.rst
> new file mode 100644
> index 000000000000..d3428be7528e
> --- /dev/null
> +++ b/Documentation/nvdimm/subsystem-profile.rst

Hmm... would it make sense to add a pointer at maintainer/index.rst (or to some
other .rst file) for those profiles too?

> @@ -0,0 +1,86 @@
> +LIBNVDIMM Subsystem Profile
> +===========================
> +
> +Overview
> +--------

A minor nitpick here: I would add a blank line after each topic/subtopic.

On some cases, Sphinx will do wrong without that blank line, and having
some places with that extra line and others without it sounds unbalanced
on my eyes ;-)

> +So, you have recently become a maintainer of the LIBNVDIMM subsystem,
> +condolences, it is a thankless job, here is the lay of the land. The git

My understanding that the main focus of this document is to help people to
submit patches to the subsystem.

With that in mind, I would never start the doc talking only to maintainers,
as developers will likely just stop reading it at the above paragraph.

> +tree, git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm.git/, is
> +writable by all the individuals listed in LIBNVDIMM section of
> +MAINTAINERS. Access is granted per the typical kernel.org account
> +management policies. Two branches in that tree are regularly pulled into
> +-next, libnvdimm-for-next, and libnvdimm-fixes. The submit rate of
> +patches is low, usually enough for one person to handle. There is a
> +patchwork instance at
> +https://patchwork.kernel.org/project/linux-nvdimm/list/, and it
> +historically is only used for ingesting patches and collecting
> +ack/review tags, i.e. no expectation to update the patch state after it
> +has been dispositioned, or merged.
> +
> +The most sensitive code area is the ACPI DSM (Device Specific Method)
> +path. In addition to the general fragility of an ioctl() ABI the ACPI
> +DSM scheme allows any vendor to implement any command without any prior
> +review by the ACPI committee. For this reason the LIBNVDIMM system seeks
> +to constrain the proliferation of vendor commands and at a minimum
> +requires any command support to be publicly documented. Over time the
> +submission rate of new vendor-specific commands is falling as more
> +commands are defined with named methods in the official ACPI
> +specification.

As Jani pointed, all the above stuff is for maintainers, but several other
stuff on this document are for developers. The best would likely to have
two separate files.

However, maintaining it on two separate files could be painful. Maybe
we could have an specific section, at the end of the document, with
maintainers-specific instructions.

> +
> +LIBNVDIMM sits at the intersection of device-drivers, the block-layer,
> +core memory-management, and filesystems. Be sure to re-route memory
> +management patches to the -mm tree, and otherwise pull-in fs-devel for
> +patches that touch anything related to DAX.

This is for developers, so it sounds OK!

> +
> +Core
> +----
> +F: drivers/nvdimm/\*_devs.c
> +F: drivers/acpi/nfit/\*.[ch]
> +
> +
> +Patches or Pull requests
> +------------------------
> +Patches only
> +
> +
> +Last day for new feature submissions
> +------------------------------------
> +Before -rc5
> +
> +
> +Last day to merge features
> +--------------------------
> +End of last -rc
> +
> +
> +Non-author Ack / Review Tags Required
> +-------------------------------------
> +Required
> +
> +
> +Test Suite
> +----------
> +Run ‘make check’ from https://github.com/pmem/ndctl
> +
> +
> +Trusted Reviewers
> +-----------------
> +Johannes Thumshirn
> +Toshi Kani
> +Jeff Moyer
> +Robert Elliott

See my other email commenting about that.

> +
> +
> +Resubmit Cadence
> +----------------
> +8 business days
> +
> +
> +Time Zone / Office Hours
> +------------------------
> +8:00am to 5:00pm Pacific Time Zone
> +
> +
> +Checkpatch / Style cleanups
> +---------------------------
> +Standalone style-cleanups are welcome.
> diff --git a/MAINTAINERS b/MAINTAINERS
> index bb4a83a7684d..ba2beedd4605 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8439,6 +8439,7 @@ M:	Dan Williams <[e-mail]>
>  M:	Vishal Verma <[e-mail]>
>  M:	Dave Jiang <[e-mail]>
>  L:	[e-mail]
> +P:	Documentation/nvdimm/subsystem-profile.rst
>  Q:	https://patchwork.kernel.org/project/linux-nvdimm/list/
>  S:	Supported
>  F:	drivers/nvdimm/blk.c
> @@ -8450,6 +8451,7 @@ M:	Dan Williams <[e-mail]>
>  M:	Ross Zwisler <[e-mail]>
>  M:	Dave Jiang <[e-mail]>
>  L:	[e-mail]
> +P:	Documentation/nvdimm/subsystem-profile.rst
>  Q:	https://patchwork.kernel.org/project/linux-nvdimm/list/
>  S:	Supported
>  F:	drivers/nvdimm/btt*
> @@ -8460,6 +8462,7 @@ M:	Dan Williams <[e-mail]>
>  M:	Vishal Verma <[e-mail]>
>  M:	Dave Jiang <[e-mail]>
>  L:	[e-mail]
> +P:	Documentation/nvdimm/subsystem-profile.rst
>  Q:	https://patchwork.kernel.org/project/linux-nvdimm/list/
>  S:	Supported
>  F:	drivers/nvdimm/pmem*
> @@ -8478,6 +8481,7 @@ M:	Ross Zwisler <[e-mail]>
>  M:	Vishal Verma <[e-mail]>
>  M:	Dave Jiang <[e-mail]>
>  L:	[e-mail]
> +P:	Documentation/nvdimm/subsystem-profile.rst
>  Q:	https://patchwork.kernel.org/project/linux-nvdimm/list/
>  T:	git git://git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm.git
>  S:	Supported
> 
> _______________________________________________
> Ksummit-discuss mailing list
> [e-mail]
> https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss




Cheers,
Mauro

Julia Lawall Nov. 15, 2018, 2:51 p.m. UTC | #4

On Thu, 15 Nov 2018, Mauro Carvalho Chehab wrote:

> Em Wed, 14 Nov 2018 20:53:30 -0800
> Dan Williams <[e-mail]> escreveu:
>
> > Document the basic policies of the libnvdimm subsystem and provide a
> > first example of a Subsystem Profile for others to duplicate and edit.
> >
> > Cc: Ross Zwisler <[e-mail]>
> > Cc: Vishal Verma <[e-mail]>
> > Cc: Dave Jiang <[e-mail]>
> > Signed-off-by: Dan Williams <[e-mail]>
> > ---
> >  Documentation/nvdimm/subsystem-profile.rst |   86 ++++++++++++++++++++++++++++
> >  MAINTAINERS                                |    4 +
> >  2 files changed, 90 insertions(+)
> >  create mode 100644 Documentation/nvdimm/subsystem-profile.rst
> >
> > diff --git a/Documentation/nvdimm/subsystem-profile.rst b/Documentation/nvdimm/subsystem-profile.rst
> > new file mode 100644
> > index 000000000000..d3428be7528e
> > --- /dev/null
> > +++ b/Documentation/nvdimm/subsystem-profile.rst
>
> Hmm... would it make sense to add a pointer at maintainer/index.rst (or to some
> other .rst file) for those profiles too?
>
> > @@ -0,0 +1,86 @@
> > +LIBNVDIMM Subsystem Profile
> > +===========================
> > +
> > +Overview
> > +--------
>
> A minor nitpick here: I would add a blank line after each topic/subtopic.
>
> On some cases, Sphinx will do wrong without that blank line, and having
> some places with that extra line and others without it sounds unbalanced
> on my eyes ;-)
>
> > +So, you have recently become a maintainer of the LIBNVDIMM subsystem,
> > +condolences, it is a thankless job, here is the lay of the land. The git
>
> My understanding that the main focus of this document is to help people to
> submit patches to the subsystem.
>
> With that in mind, I would never start the doc talking only to maintainers,
> as developers will likely just stop reading it at the above paragraph.

This seems like a good idea.  New maintainers will probably be directed to
this document by existing maintainers, so they will already have some
context.  On the other hand, developers may interact with it on their own,
so it is good that they know immediately that they are in the right place.

julia

>
> > +tree, git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm.git/, is
> > +writable by all the individuals listed in LIBNVDIMM section of
> > +MAINTAINERS. Access is granted per the typical kernel.org account
> > +management policies. Two branches in that tree are regularly pulled into
> > +-next, libnvdimm-for-next, and libnvdimm-fixes. The submit rate of
> > +patches is low, usually enough for one person to handle. There is a
> > +patchwork instance at
> > +https://patchwork.kernel.org/project/linux-nvdimm/list/, and it
> > +historically is only used for ingesting patches and collecting
> > +ack/review tags, i.e. no expectation to update the patch state after it
> > +has been dispositioned, or merged.
> > +
> > +The most sensitive code area is the ACPI DSM (Device Specific Method)
> > +path. In addition to the general fragility of an ioctl() ABI the ACPI
> > +DSM scheme allows any vendor to implement any command without any prior
> > +review by the ACPI committee. For this reason the LIBNVDIMM system seeks
> > +to constrain the proliferation of vendor commands and at a minimum
> > +requires any command support to be publicly documented. Over time the
> > +submission rate of new vendor-specific commands is falling as more
> > +commands are defined with named methods in the official ACPI
> > +specification.
>
> As Jani pointed, all the above stuff is for maintainers, but several other
> stuff on this document are for developers. The best would likely to have
> two separate files.
>
> However, maintaining it on two separate files could be painful. Maybe
> we could have an specific section, at the end of the document, with
> maintainers-specific instructions.
>
> > +
> > +LIBNVDIMM sits at the intersection of device-drivers, the block-layer,
> > +core memory-management, and filesystems. Be sure to re-route memory
> > +management patches to the -mm tree, and otherwise pull-in fs-devel for
> > +patches that touch anything related to DAX.
>
> This is for developers, so it sounds OK!
>
> > +
> > +Core
> > +----
> > +F: drivers/nvdimm/\*_devs.c
> > +F: drivers/acpi/nfit/\*.[ch]
> > +
> > +
> > +Patches or Pull requests
> > +------------------------
> > +Patches only
> > +
> > +
> > +Last day for new feature submissions
> > +------------------------------------
> > +Before -rc5
> > +
> > +
> > +Last day to merge features
> > +--------------------------
> > +End of last -rc
> > +
> > +
> > +Non-author Ack / Review Tags Required
> > +-------------------------------------
> > +Required
> > +
> > +
> > +Test Suite
> > +----------
> > +Run ‘make check’ from https://github.com/pmem/ndctl
> > +
> > +
> > +Trusted Reviewers
> > +-----------------
> > +Johannes Thumshirn
> > +Toshi Kani
> > +Jeff Moyer
> > +Robert Elliott
>
> See my other email commenting about that.
>
> > +
> > +
> > +Resubmit Cadence
> > +----------------
> > +8 business days
> > +
> > +
> > +Time Zone / Office Hours
> > +------------------------
> > +8:00am to 5:00pm Pacific Time Zone
> > +
> > +
> > +Checkpatch / Style cleanups
> > +---------------------------
> > +Standalone style-cleanups are welcome.
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index bb4a83a7684d..ba2beedd4605 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -8439,6 +8439,7 @@ M:	Dan Williams <[e-mail]>
> >  M:	Vishal Verma <[e-mail]>
> >  M:	Dave Jiang <[e-mail]>
> >  L:	[e-mail]
> > +P:	Documentation/nvdimm/subsystem-profile.rst
> >  Q:	https://patchwork.kernel.org/project/linux-nvdimm/list/
> >  S:	Supported
> >  F:	drivers/nvdimm/blk.c
> > @@ -8450,6 +8451,7 @@ M:	Dan Williams <[e-mail]>
> >  M:	Ross Zwisler <[e-mail]>
> >  M:	Dave Jiang <[e-mail]>
> >  L:	[e-mail]
> > +P:	Documentation/nvdimm/subsystem-profile.rst
> >  Q:	https://patchwork.kernel.org/project/linux-nvdimm/list/
> >  S:	Supported
> >  F:	drivers/nvdimm/btt*
> > @@ -8460,6 +8462,7 @@ M:	Dan Williams <[e-mail]>
> >  M:	Vishal Verma <[e-mail]>
> >  M:	Dave Jiang <[e-mail]>
> >  L:	[e-mail]
> > +P:	Documentation/nvdimm/subsystem-profile.rst
> >  Q:	https://patchwork.kernel.org/project/linux-nvdimm/list/
> >  S:	Supported
> >  F:	drivers/nvdimm/pmem*
> > @@ -8478,6 +8481,7 @@ M:	Ross Zwisler <[e-mail]>
> >  M:	Vishal Verma <[e-mail]>
> >  M:	Dave Jiang <[e-mail]>
> >  L:	[e-mail]
> > +P:	Documentation/nvdimm/subsystem-profile.rst
> >  Q:	https://patchwork.kernel.org/project/linux-nvdimm/list/
> >  T:	git git://git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm.git
> >  S:	Supported
> >
> > _______________________________________________
> > Ksummit-discuss mailing list
> > [e-mail]
> > https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss
>
>
>
>
> Cheers,
> Mauro
> _______________________________________________
> Ksummit-discuss mailing list
> [e-mail]
> https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss
>

Mauro Carvalho Chehab Nov. 15, 2018, 7:09 p.m. UTC | #5

Em Thu, 15 Nov 2018 18:20:08 +0200
Leon Romanovsky <[e-mail]> escreveu:

> On Thu, Nov 15, 2018 at 06:10:36AM -0800, Mauro Carvalho Chehab wrote:
> > Em Thu, 15 Nov 2018 09:03:11 +0100
> > Geert Uytterhoeven <[e-mail]> escreveu:
> >  
> > > Hi Dan,
> > >
> > > On Thu, Nov 15, 2018 at 6:06 AM Dan Williams <[e-mail]> wrote:  
> > > > Document the basic policies of the libnvdimm subsystem and provide a
> > > > first example of a Subsystem Profile for others to duplicate and edit.
> > > >
> > > > Cc: Ross Zwisler <[e-mail]>
> > > > Cc: Vishal Verma <[e-mail]>
> > > > Cc: Dave Jiang <[e-mail]>
> > > > Signed-off-by: Dan Williams <[e-mail]>  
> > >
> > > Thanks for your patch!
> > >  
> > > > --- /dev/null
> > > > +++ b/Documentation/nvdimm/subsystem-profile.rst  
> > >  
> > > > +Trusted Reviewers
> > > > +-----------------
> > > > +Johannes Thumshirn
> > > > +Toshi Kani
> > > > +Jeff Moyer
> > > > +Robert Elliott  
> > >
> > > Don't you want to add email addresses?
> > > Only the first one is listed in MAINTAINERS.  
> >
> > IMO, it makes sense to have their e-mails here, in a way that it could
> > easily be parsed by get_maintainers.pl.  
> 
> I personally think that list of "trusted reviewers" makes more harm than
> good. It creates unneeded negative feelings to those who wanted to be in
> this list, but for any reason they don't. Those reviewers will feel
> "untrusted".

Yeah, perhaps something like "most active reviewers" would sound
better.

Cheers,
Mauro

Luck, Tony Nov. 15, 2018, 7:40 p.m. UTC | #6

> I would recommend to remove this section at all.
> New maintainers won't come out of blue, but will be come
> from existing community and such individuals for sure will see
> and judge by themselves to whom they trust and to whom not.

Perhaps this is more of a hint to contributors than to maintainers
(see earlier discussion on who is the target audience for these documents).

It would help contributors know some names of useful reviewers (and
thus this list should be picked up by scripts/get_maintainer.pl to help
the user compose Cc: lists for e-mail patches).

-Tony

Joe Perches Nov. 15, 2018, 7:43 p.m. UTC | #7

On Thu, 2018-11-15 at 19:40 +0000, Luck, Tony wrote:
> > I would recommend to remove this section at all.
> > New maintainers won't come out of blue, but will be come
> > from existing community and such individuals for sure will see
> > and judge by themselves to whom they trust and to whom not.
> 
> Perhaps this is more of a hint to contributors than to maintainers
> (see earlier discussion on who is the target audience for these documents).
> 
> It would help contributors know some names of useful reviewers (and
> thus this list should be picked up by scripts/get_maintainer.pl to help
> the user compose Cc: lists for e-mail patches).

Trusted reviewers should be specifically listed
in the MAINTAINERS file with an "R:" entry.

get_maintainers should not look anywhere else.

Yasunori Gotou (Fujitsu) Nov. 16, 2018, 2:58 a.m. UTC | #8

Hi, Dan-san,

Thank you for your patch. I have one comment.

> ---
>  Documentation/nvdimm/subsystem-profile.rst |   86 ++++++++++++++++++++++++++++
>  MAINTAINERS                                |    4 +
>  2 files changed, 90 insertions(+)
>  create mode 100644 Documentation/nvdimm/subsystem-profile.rst
> 
> diff --git a/Documentation/nvdimm/subsystem-profile.rst b/Documentation/nvdimm/subsystem-profile.rst
> new file mode 100644
> index 000000000000..d3428be7528e
> --- /dev/null
> +++ b/Documentation/nvdimm/subsystem-profile.rst
> @@ -0,0 +1,86 @@
> +LIBNVDIMM Subsystem Profile
> +===========================
> +
> +Overview
> +--------
> +So, you have recently become a maintainer of the LIBNVDIMM subsystem,
> +condolences, it is a thankless job, here is the lay of the land. The git

Here, I think that more positive sentence is desirable to encourage new comer.

Certainly, I can imagine that maintainer is hard and tough work,
but I believe it is great work.
Even if this is irony or joke, then it may be cause of miss-understanding
(especially, non-native English speaker like me.)

In addition, new coming developer is sometimes nervous 
to post new mail/patch. If this text will be changed for new comer, 
then I think this text will be good to encourage them.
So, I think more positive expression is desirable.

Thanks,

Mauro Carvalho Chehab Nov. 16, 2018, 11:33 a.m. UTC | #9

Em Thu, 15 Nov 2018 21:35:20 +0200
Leon Romanovsky <[e-mail]> escreveu:

> On Thu, Nov 15, 2018 at 11:09:34AM -0800, Mauro Carvalho Chehab wrote:
> > Em Thu, 15 Nov 2018 18:20:08 +0200
> > Leon Romanovsky <[e-mail]> escreveu:
> >  
> > > On Thu, Nov 15, 2018 at 06:10:36AM -0800, Mauro Carvalho Chehab wrote:  
> > > > Em Thu, 15 Nov 2018 09:03:11 +0100
> > > > Geert Uytterhoeven <[e-mail]> escreveu:
> > > >  
> > > > > Hi Dan,
> > > > >
> > > > > On Thu, Nov 15, 2018 at 6:06 AM Dan Williams <[e-mail]> wrote:  
> > > > > > Document the basic policies of the libnvdimm subsystem and provide a
> > > > > > first example of a Subsystem Profile for others to duplicate and edit.
> > > > > >
> > > > > > Cc: Ross Zwisler <[e-mail]>
> > > > > > Cc: Vishal Verma <[e-mail]>
> > > > > > Cc: Dave Jiang <[e-mail]>
> > > > > > Signed-off-by: Dan Williams <[e-mail]>  
> > > > >
> > > > > Thanks for your patch!
> > > > >  
> > > > > > --- /dev/null
> > > > > > +++ b/Documentation/nvdimm/subsystem-profile.rst  
> > > > >  
> > > > > > +Trusted Reviewers
> > > > > > +-----------------
> > > > > > +Johannes Thumshirn
> > > > > > +Toshi Kani
> > > > > > +Jeff Moyer
> > > > > > +Robert Elliott  
> > > > >
> > > > > Don't you want to add email addresses?
> > > > > Only the first one is listed in MAINTAINERS.  
> > > >
> > > > IMO, it makes sense to have their e-mails here, in a way that it could
> > > > easily be parsed by get_maintainers.pl.  
> > >
> > > I personally think that list of "trusted reviewers" makes more harm than
> > > good. It creates unneeded negative feelings to those who wanted to be in
> > > this list, but for any reason they don't. Those reviewers will feel
> > > "untrusted".  
> >
> > Yeah, perhaps something like "most active reviewers" would sound
> > better.  
> 
> I would recommend to remove this section at all.
> New maintainers won't come out of blue, but will be come
> from existing community and such individuals for sure will see
> and judge by themselves to whom they trust and to whom not.

I see your point, but, on the other hand, having a list with the ones
that are actively doing reviews helps newcomers.

I would keep, but perhaps it makes sense to add some notice about a
criteria about how to be included at the "active reviewers" list,
(the criteria probably belongs to the subsystem profile), e. g. 
something like:

	"Active reviewers are developers that contribute with more than 25
	 patches per year and do more than 50 reviews per year on 
	 on patches written for drivers that they're not usual contributors" 
Cheers,
Mauro

Mauro Carvalho Chehab Nov. 16, 2018, 11:39 a.m. UTC | #10

Em Thu, 15 Nov 2018 11:43:51 -0800
Joe Perches <[e-mail]> escreveu:

> On Thu, 2018-11-15 at 19:40 +0000, Luck, Tony wrote:
> > > I would recommend to remove this section at all.
> > > New maintainers won't come out of blue, but will be come
> > > from existing community and such individuals for sure will see
> > > and judge by themselves to whom they trust and to whom not.  
> > 
> > Perhaps this is more of a hint to contributors than to maintainers
> > (see earlier discussion on who is the target audience for these documents).
> > 
> > It would help contributors know some names of useful reviewers (and
> > thus this list should be picked up by scripts/get_maintainer.pl to help
> > the user compose Cc: lists for e-mail patches).  
> 
> Trusted reviewers should be specifically listed
> in the MAINTAINERS file with an "R:" entry.
> 
> get_maintainers should not look anywhere else.

I know that making get_maintainers to look elsewhere can make it more
complex and slower, but IMHO, by having a per-subsystem profile, this is
unavoidable.

The thing is that touching at a single MAINTAINERS file every time a new
reviewer comes is painful. Also, MAINTAINERS file format doesn't allow
adding free text explaining the criteria for someone to become a
reviewer.

IMO, having reviewers on a per-subsystem file, where one could explain
the criteria for being added there will make easier to attract more
reviewers.

Cheers,
Mauro

Jan Kara Nov. 16, 2018, noon UTC | #11

On Fri 16-11-18 03:33:17, Mauro Carvalho Chehab wrote:
> Em Thu, 15 Nov 2018 21:35:20 +0200
> Leon Romanovsky <[e-mail]> escreveu:
> 
> > On Thu, Nov 15, 2018 at 11:09:34AM -0800, Mauro Carvalho Chehab wrote:
> > > Em Thu, 15 Nov 2018 18:20:08 +0200
> > > Leon Romanovsky <[e-mail]> escreveu:
> > >  
> > > > On Thu, Nov 15, 2018 at 06:10:36AM -0800, Mauro Carvalho Chehab wrote:  
> > > > > Em Thu, 15 Nov 2018 09:03:11 +0100
> > > > > Geert Uytterhoeven <[e-mail]> escreveu:
> > > > >  
> > > > > > Hi Dan,
> > > > > >
> > > > > > On Thu, Nov 15, 2018 at 6:06 AM Dan Williams <[e-mail]> wrote:  
> > > > > > > Document the basic policies of the libnvdimm subsystem and provide a
> > > > > > > first example of a Subsystem Profile for others to duplicate and edit.
> > > > > > >
> > > > > > > Cc: Ross Zwisler <[e-mail]>
> > > > > > > Cc: Vishal Verma <[e-mail]>
> > > > > > > Cc: Dave Jiang <[e-mail]>
> > > > > > > Signed-off-by: Dan Williams <[e-mail]>  
> > > > > >
> > > > > > Thanks for your patch!
> > > > > >  
> > > > > > > --- /dev/null
> > > > > > > +++ b/Documentation/nvdimm/subsystem-profile.rst  
> > > > > >  
> > > > > > > +Trusted Reviewers
> > > > > > > +-----------------
> > > > > > > +Johannes Thumshirn
> > > > > > > +Toshi Kani
> > > > > > > +Jeff Moyer
> > > > > > > +Robert Elliott  
> > > > > >
> > > > > > Don't you want to add email addresses?
> > > > > > Only the first one is listed in MAINTAINERS.  
> > > > >
> > > > > IMO, it makes sense to have their e-mails here, in a way that it could
> > > > > easily be parsed by get_maintainers.pl.  
> > > >
> > > > I personally think that list of "trusted reviewers" makes more harm than
> > > > good. It creates unneeded negative feelings to those who wanted to be in
> > > > this list, but for any reason they don't. Those reviewers will feel
> > > > "untrusted".  
> > >
> > > Yeah, perhaps something like "most active reviewers" would sound
> > > better.  
> > 
> > I would recommend to remove this section at all.
> > New maintainers won't come out of blue, but will be come
> > from existing community and such individuals for sure will see
> > and judge by themselves to whom they trust and to whom not.
> 
> I see your point, but, on the other hand, having a list with the ones
> that are actively doing reviews helps newcomers.

How exactly? Do you expect people to CC patches to these directly? And if
yes, why is not picking patches from the mailing list good enough?

								Honza

Dan Williams Nov. 16, 2018, 7:13 p.m. UTC | #12

On Thu, Nov 15, 2018 at 12:03 AM Geert Uytterhoeven
<[e-mail]> wrote:
>
> Hi Dan,
>
> On Thu, Nov 15, 2018 at 6:06 AM Dan Williams <[e-mail]> wrote:
> > Document the basic policies of the libnvdimm subsystem and provide a
> > first example of a Subsystem Profile for others to duplicate and edit.
> >
> > Cc: Ross Zwisler <[e-mail]>
> > Cc: Vishal Verma <[e-mail]>
> > Cc: Dave Jiang <[e-mail]>
> > Signed-off-by: Dan Williams <[e-mail]>
>
> Thanks for your patch!
>
> > --- /dev/null
> > +++ b/Documentation/nvdimm/subsystem-profile.rst
>
> > +Trusted Reviewers
> > +-----------------
> > +Johannes Thumshirn
> > +Toshi Kani
> > +Jeff Moyer
> > +Robert Elliott
>
> Don't you want to add email addresses?

I do, but I had not cleared this designation with these folks ahead of
time. Will resolve that for v2.

> Only the first one is listed in MAINTAINERS.

Correct, the intent is that these individuals can be sought out for
advice when the maintainer may not have the bandwidth to engage with a
contributor on a given timeframe. These are individuals that can
provide a second opinion.

> > +Time Zone / Office Hours
> > +------------------------
> > +8:00am to 5:00pm Pacific Time Zone
>
> UTC-???
>
> People are not familiar with all time zones.

Yes, will fix.

Dan Williams Nov. 16, 2018, 7:20 p.m. UTC | #13

On Thu, Nov 15, 2018 at 6:31 AM Mauro Carvalho Chehab
<[e-mail]> wrote:
>
> Em Wed, 14 Nov 2018 20:53:30 -0800
> Dan Williams <[e-mail]> escreveu:
>
> > Document the basic policies of the libnvdimm subsystem and provide a
> > first example of a Subsystem Profile for others to duplicate and edit.
> >
> > Cc: Ross Zwisler <[e-mail]>
> > Cc: Vishal Verma <[e-mail]>
> > Cc: Dave Jiang <[e-mail]>
> > Signed-off-by: Dan Williams <[e-mail]>
> > ---
> >  Documentation/nvdimm/subsystem-profile.rst |   86 ++++++++++++++++++++++++++++
> >  MAINTAINERS                                |    4 +
> >  2 files changed, 90 insertions(+)
> >  create mode 100644 Documentation/nvdimm/subsystem-profile.rst
> >
> > diff --git a/Documentation/nvdimm/subsystem-profile.rst b/Documentation/nvdimm/subsystem-profile.rst
> > new file mode 100644
> > index 000000000000..d3428be7528e
> > --- /dev/null
> > +++ b/Documentation/nvdimm/subsystem-profile.rst
>
> Hmm... would it make sense to add a pointer at maintainer/index.rst (or to some
> other .rst file) for those profiles too?
>
> > @@ -0,0 +1,86 @@
> > +LIBNVDIMM Subsystem Profile
> > +===========================
> > +
> > +Overview
> > +--------
>
> A minor nitpick here: I would add a blank line after each topic/subtopic.
>
> On some cases, Sphinx will do wrong without that blank line, and having
> some places with that extra line and others without it sounds unbalanced
> on my eyes ;-)
>
> > +So, you have recently become a maintainer of the LIBNVDIMM subsystem,
> > +condolences, it is a thankless job, here is the lay of the land. The git
>
> My understanding that the main focus of this document is to help people to
> submit patches to the subsystem.
>
> With that in mind, I would never start the doc talking only to maintainers,
> as developers will likely just stop reading it at the above paragraph.

Yes, I see this is confusing now. I'll fix up the Subsystem Profile
description text to be written with maintainers as the audience and
clarify that the per-subsystem instance is written with contributors
as the audience.

>
> > +tree, git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm.git/, is
> > +writable by all the individuals listed in LIBNVDIMM section of
> > +MAINTAINERS. Access is granted per the typical kernel.org account
> > +management policies. Two branches in that tree are regularly pulled into
> > +-next, libnvdimm-for-next, and libnvdimm-fixes. The submit rate of
> > +patches is low, usually enough for one person to handle. There is a
> > +patchwork instance at
> > +https://patchwork.kernel.org/project/linux-nvdimm/list/, and it
> > +historically is only used for ingesting patches and collecting
> > +ack/review tags, i.e. no expectation to update the patch state after it
> > +has been dispositioned, or merged.
> > +
> > +The most sensitive code area is the ACPI DSM (Device Specific Method)
> > +path. In addition to the general fragility of an ioctl() ABI the ACPI
> > +DSM scheme allows any vendor to implement any command without any prior
> > +review by the ACPI committee. For this reason the LIBNVDIMM system seeks
> > +to constrain the proliferation of vendor commands and at a minimum
> > +requires any command support to be publicly documented. Over time the
> > +submission rate of new vendor-specific commands is falling as more
> > +commands are defined with named methods in the official ACPI
> > +specification.
>
> As Jani pointed, all the above stuff is for maintainers, but several other
> stuff on this document are for developers. The best would likely to have
> two separate files.
>
> However, maintaining it on two separate files could be painful. Maybe
> we could have an specific section, at the end of the document, with
> maintainers-specific instructions.

Yes, good idea, will incorporate.

Rodrigo Vivi Nov. 16, 2018, 8:36 p.m. UTC | #14

On Thu, Nov 15, 2018 at 8:38 AM Leon Romanovsky <[e-mail]> wrote:
>
> On Thu, Nov 15, 2018 at 06:10:36AM -0800, Mauro Carvalho Chehab wrote:
> > Em Thu, 15 Nov 2018 09:03:11 +0100
> > Geert Uytterhoeven <[e-mail]> escreveu:
> >
> > > Hi Dan,
> > >
> > > On Thu, Nov 15, 2018 at 6:06 AM Dan Williams <[e-mail]> wrote:
> > > > Document the basic policies of the libnvdimm subsystem and provide a
> > > > first example of a Subsystem Profile for others to duplicate and edit.
> > > >
> > > > Cc: Ross Zwisler <[e-mail]>
> > > > Cc: Vishal Verma <[e-mail]>
> > > > Cc: Dave Jiang <[e-mail]>
> > > > Signed-off-by: Dan Williams <[e-mail]>
> > >
> > > Thanks for your patch!
> > >
> > > > --- /dev/null
> > > > +++ b/Documentation/nvdimm/subsystem-profile.rst
> > >
> > > > +Trusted Reviewers
> > > > +-----------------
> > > > +Johannes Thumshirn
> > > > +Toshi Kani
> > > > +Jeff Moyer
> > > > +Robert Elliott
> > >
> > > Don't you want to add email addresses?
> > > Only the first one is listed in MAINTAINERS.
> >
> > IMO, it makes sense to have their e-mails here, in a way that it could
> > easily be parsed by get_maintainers.pl.
>
> I personally think that list of "trusted reviewers" makes more harm than
> good. It creates unneeded negative feelings to those who wanted to be in
> this list, but for any reason they don't. Those reviewers will feel
> "untrusted".

I'd like to +1 on this concern here. Besides leaving all the other
people demotivated.

A small group of trusted reviewers doesn't scale. People will get overloaded.
Or you won't be able to enforce that all patches need to get Reviews.

Reviews should be coming from everywhere and commiters and maintainers
deciding on what to trust or re-review.

Also the list is hard to maintain and keep the lists updated.

>
> Thanks
> _______________________________________________
> Ksummit-discuss mailing list
> [e-mail]
> https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss

Dan Williams Nov. 16, 2018, 11:44 p.m. UTC | #15

On Fri, Nov 16, 2018 at 12:37 PM Rodrigo Vivi <[e-mail]> wrote:
>
> On Thu, Nov 15, 2018 at 8:38 AM Leon Romanovsky <[e-mail]> wrote:
> >
> > On Thu, Nov 15, 2018 at 06:10:36AM -0800, Mauro Carvalho Chehab wrote:
> > > Em Thu, 15 Nov 2018 09:03:11 +0100
> > > Geert Uytterhoeven <[e-mail]> escreveu:
> > >
> > > > Hi Dan,
> > > >
> > > > On Thu, Nov 15, 2018 at 6:06 AM Dan Williams <[e-mail]> wrote:
> > > > > Document the basic policies of the libnvdimm subsystem and provide a
> > > > > first example of a Subsystem Profile for others to duplicate and edit.
> > > > >
> > > > > Cc: Ross Zwisler <[e-mail]>
> > > > > Cc: Vishal Verma <[e-mail]>
> > > > > Cc: Dave Jiang <[e-mail]>
> > > > > Signed-off-by: Dan Williams <[e-mail]>
> > > >
> > > > Thanks for your patch!
> > > >
> > > > > --- /dev/null
> > > > > +++ b/Documentation/nvdimm/subsystem-profile.rst
> > > >
> > > > > +Trusted Reviewers
> > > > > +-----------------
> > > > > +Johannes Thumshirn
> > > > > +Toshi Kani
> > > > > +Jeff Moyer
> > > > > +Robert Elliott
> > > >
> > > > Don't you want to add email addresses?
> > > > Only the first one is listed in MAINTAINERS.
> > >
> > > IMO, it makes sense to have their e-mails here, in a way that it could
> > > easily be parsed by get_maintainers.pl.
> >
> > I personally think that list of "trusted reviewers" makes more harm than
> > good. It creates unneeded negative feelings to those who wanted to be in
> > this list, but for any reason they don't. Those reviewers will feel
> > "untrusted".
>
> I'd like to +1 on this concern here. Besides leaving all the other
> people demotivated.

Yes, that's a valid concern, I overlooked that unfortunate interpretation.

>
> A small group of trusted reviewers doesn't scale. People will get overloaded.
> Or you won't be able to enforce that all patches need to get Reviews.
>
> Reviews should be coming from everywhere and commiters and maintainers
> deciding on what to trust or re-review.
>
> Also the list is hard to maintain and keep the lists updated.

I understand the concern, and as I saw feedback come in I realized
there were more people that I would add to that reviewer list for
libnvdimm.

Stepping back the end goal is to have an initial list of recommended
people to follow up with directly to seek a second opinion, or help in
cases where a contributor otherwise needs some direction / engagement
that they are not readily receiving from the maintainer. Typically
someone just lurks on the mailing list for a few weeks to get a feel
for who the usual suspects are in the subsystem, but for a new
contributor identifying those individuals may be difficult.

One of the contributing factors of lack of response to a patchset is
that they are sent with the implicit expectation that the maintainer
will get to eventually, and typically other people feel content to sit
back and watch. If instead a contributor sent a direct mail to a
"trusted reviewer" saying, "Hey, Alice, Bob seems busy can you help me
out?" that seems more likely to rope in additional review help.

Mauro Carvalho Chehab Nov. 18, 2018, 1:03 p.m. UTC | #16

Em Fri, 16 Nov 2018 15:44:45 -0800
Dan Williams <[e-mail]> escreveu:

> On Fri, Nov 16, 2018 at 12:37 PM Rodrigo Vivi <[e-mail]> wrote:
> >
> > On Thu, Nov 15, 2018 at 8:38 AM Leon Romanovsky <[e-mail]> wrote:  
> > >
> > > On Thu, Nov 15, 2018 at 06:10:36AM -0800, Mauro Carvalho Chehab wrote:  
> > > > Em Thu, 15 Nov 2018 09:03:11 +0100
> > > > Geert Uytterhoeven <[e-mail]> escreveu:
> > > >  
> > > > > Hi Dan,
> > > > >
> > > > > On Thu, Nov 15, 2018 at 6:06 AM Dan Williams <[e-mail]> wrote:  
> > > > > > Document the basic policies of the libnvdimm subsystem and provide a
> > > > > > first example of a Subsystem Profile for others to duplicate and edit.
> > > > > >
> > > > > > Cc: Ross Zwisler <[e-mail]>
> > > > > > Cc: Vishal Verma <[e-mail]>
> > > > > > Cc: Dave Jiang <[e-mail]>
> > > > > > Signed-off-by: Dan Williams <[e-mail]>  
> > > > >
> > > > > Thanks for your patch!
> > > > >  
> > > > > > --- /dev/null
> > > > > > +++ b/Documentation/nvdimm/subsystem-profile.rst  
> > > > >  
> > > > > > +Trusted Reviewers
> > > > > > +-----------------
> > > > > > +Johannes Thumshirn
> > > > > > +Toshi Kani
> > > > > > +Jeff Moyer
> > > > > > +Robert Elliott  
> > > > >
> > > > > Don't you want to add email addresses?
> > > > > Only the first one is listed in MAINTAINERS.  
> > > >
> > > > IMO, it makes sense to have their e-mails here, in a way that it could
> > > > easily be parsed by get_maintainers.pl.  
> > >
> > > I personally think that list of "trusted reviewers" makes more harm than
> > > good. It creates unneeded negative feelings to those who wanted to be in
> > > this list, but for any reason they don't. Those reviewers will feel
> > > "untrusted".  
> >
> > I'd like to +1 on this concern here. Besides leaving all the other
> > people demotivated.  
> 
> Yes, that's a valid concern, I overlooked that unfortunate interpretation.
> 
> >
> > A small group of trusted reviewers doesn't scale. People will get overloaded.
> > Or you won't be able to enforce that all patches need to get Reviews.
> >
> > Reviews should be coming from everywhere and commiters and maintainers
> > deciding on what to trust or re-review.
> >
> > Also the list is hard to maintain and keep the lists updated.  
> 
> I understand the concern, and as I saw feedback come in I realized
> there were more people that I would add to that reviewer list for
> libnvdimm.
> 
> Stepping back the end goal is to have an initial list of recommended
> people to follow up with directly to seek a second opinion, or help in
> cases where a contributor otherwise needs some direction / engagement
> that they are not readily receiving from the maintainer. Typically
> someone just lurks on the mailing list for a few weeks to get a feel
> for who the usual suspects are in the subsystem, but for a new
> contributor identifying those individuals may be difficult.
> 
> One of the contributing factors of lack of response to a patchset is
> that they are sent with the implicit expectation that the maintainer
> will get to eventually, and typically other people feel content to sit
> back and watch. If instead a contributor sent a direct mail to a
> "trusted reviewer" saying, "Hey, Alice, Bob seems busy can you help me
> out?" that seems more likely to rope in additional review help.

Hmm.. Perhaps the subsystem profile should point to IRC channels if any?
Several subsystems use them in order to provide newbies some directions
and to discuss other development issues.

Thanks,
Mauro

Mauro Carvalho Chehab Nov. 18, 2018, 1:11 p.m. UTC | #17

Em Sat, 17 Nov 2018 11:38:50 +1100
NeilBrown <[e-mail]> escreveu:

> On Fri, Nov 16 2018, Dan Williams wrote:
> 
> > On Fri, Nov 16, 2018 at 12:37 PM Rodrigo Vivi <[e-mail]> wrote:  
> >>
> >> On Thu, Nov 15, 2018 at 8:38 AM Leon Romanovsky <[e-mail]> wrote:  
> >> >
> >> > On Thu, Nov 15, 2018 at 06:10:36AM -0800, Mauro Carvalho Chehab wrote:  
> >> > > Em Thu, 15 Nov 2018 09:03:11 +0100
> >> > > Geert Uytterhoeven <[e-mail]> escreveu:
> >> > >  
> >> > > > Hi Dan,
> >> > > >
> >> > > > On Thu, Nov 15, 2018 at 6:06 AM Dan Williams <[e-mail]> wrote:  
> >> > > > > Document the basic policies of the libnvdimm subsystem and provide a
> >> > > > > first example of a Subsystem Profile for others to duplicate and edit.
> >> > > > >
> >> > > > > Cc: Ross Zwisler <[e-mail]>
> >> > > > > Cc: Vishal Verma <[e-mail]>
> >> > > > > Cc: Dave Jiang <[e-mail]>
> >> > > > > Signed-off-by: Dan Williams <[e-mail]>  
> >> > > >
> >> > > > Thanks for your patch!
> >> > > >  
> >> > > > > --- /dev/null
> >> > > > > +++ b/Documentation/nvdimm/subsystem-profile.rst  
> >> > > >  
> >> > > > > +Trusted Reviewers
> >> > > > > +-----------------
> >> > > > > +Johannes Thumshirn
> >> > > > > +Toshi Kani
> >> > > > > +Jeff Moyer
> >> > > > > +Robert Elliott  
> >> > > >
> >> > > > Don't you want to add email addresses?
> >> > > > Only the first one is listed in MAINTAINERS.  
> >> > >
> >> > > IMO, it makes sense to have their e-mails here, in a way that it could
> >> > > easily be parsed by get_maintainers.pl.  
> >> >
> >> > I personally think that list of "trusted reviewers" makes more harm than
> >> > good. It creates unneeded negative feelings to those who wanted to be in
> >> > this list, but for any reason they don't. Those reviewers will feel
> >> > "untrusted".  
> >>
> >> I'd like to +1 on this concern here. Besides leaving all the other
> >> people demotivated.  
> >
> > Yes, that's a valid concern, I overlooked that unfortunate interpretation.
> >  
> >>
> >> A small group of trusted reviewers doesn't scale. People will get overloaded.
> >> Or you won't be able to enforce that all patches need to get Reviews.
> >>
> >> Reviews should be coming from everywhere and commiters and maintainers
> >> deciding on what to trust or re-review.
> >>
> >> Also the list is hard to maintain and keep the lists updated.  
> >
> > I understand the concern, and as I saw feedback come in I realized
> > there were more people that I would add to that reviewer list for
> > libnvdimm.
> >
> > Stepping back the end goal is to have an initial list of recommended
> > people to follow up with directly to seek a second opinion, or help in
> > cases where a contributor otherwise needs some direction / engagement
> > that they are not readily receiving from the maintainer. Typically
> > someone just lurks on the mailing list for a few weeks to get a feel
> > for who the usual suspects are in the subsystem, but for a new
> > contributor identifying those individuals may be difficult.
> >
> > One of the contributing factors of lack of response to a patchset is
> > that they are sent with the implicit expectation that the maintainer
> > will get to eventually, and typically other people feel content to sit
> > back and watch. If instead a contributor sent a direct mail to a
> > "trusted reviewer" saying, "Hey, Alice, Bob seems busy can you help me
> > out?" that seems more likely to rope in additional review help.  
> 
> In here is, I think, a real issue that listing "trusted reviewers" might
> help address.
> As you say, people don't feel the need to comment - particularly if they
> don't see anything wrong (often best to insert a bug to encourage
> responses!).
> Maybe if we list people, it will make them feel that their opinion is
> valuable (trusted!) and that will encourage them to Ack or Review a
> patch.
> I have found that being given a title of responsibility can change my
> thinking from "someone should" to "I should".

I heard a similar feedback from some subsystems: giving someone a 
formal credit may actually help to get more reviews.

However, as Leon pointed later in this tread:

Em Sun, 18 Nov 2018 09:12:54 +0200
Leon Romanovsky <[e-mail]> escreveu:

> On Fri, Nov 16, 2018 at 03:39:47AM -0800, Mauro Carvalho Chehab wrote:
> > Em Thu, 15 Nov 2018 11:43:51 -0800
> > Joe Perches <[e-mail]> escreveu:
> >  
> > > On Thu, 2018-11-15 at 19:40 +0000, Luck, Tony wrote:  
> > > > > I would recommend to remove this section at all.
> > > > > New maintainers won't come out of blue, but will be come
> > > > > from existing community and such individuals for sure will see
> > > > > and judge by themselves to whom they trust and to whom not.  
> > > >
> > > > Perhaps this is more of a hint to contributors than to maintainers
> > > > (see earlier discussion on who is the target audience for these documents).
> > > >
> > > > It would help contributors know some names of useful reviewers (and
> > > > thus this list should be picked up by scripts/get_maintainer.pl to help
> > > > the user compose Cc: lists for e-mail patches).  
> > >
> > > Trusted reviewers should be specifically listed
> > > in the MAINTAINERS file with an "R:" entry.
> > >
> > > get_maintainers should not look anywhere else.  
> >
> > I know that making get_maintainers to look elsewhere can make it more
> > complex and slower, but IMHO, by having a per-subsystem profile, this is
> > unavoidable.
> >
> > The thing is that touching at a single MAINTAINERS file every time a new
> > reviewer comes is painful. Also, MAINTAINERS file format doesn't allow
> > adding free text explaining the criteria for someone to become a
> > reviewer.  
> 
> You are pointing to the actual problem -> someone needs to maintain such
> lists, Removal of persons from that list won't be easy task too.

While adding new reviewers is easy (someone just need to send a patch,
with the Acked-by from the reviewer to be included), getting someone 
removed from it can be very painful (and will likely require some written
policy about how to do that).

Thanks,
Mauro

Jani Nikula Nov. 20, 2018, 8:10 a.m. UTC | #18

On Sun, 18 Nov 2018, Mauro Carvalho Chehab <[e-mail]> wrote:
> Hmm.. Perhaps the subsystem profile should point to IRC channels if any?
> Several subsystems use them in order to provide newbies some directions
> and to discuss other development issues.

MAINTAINERS:

	C: URI for chat protocol, server and channel where developers
	   usually hang out, for example irc://server/channel.

Alas very few subsystems have added that.

I actually fear very few subsystems will add a subsystem profile for
that matter.


BR,
Jani.

Dan Williams Nov. 20, 2018, 7:31 p.m. UTC | #19

On Tue, Nov 20, 2018 at 12:09 AM Jani Nikula <[e-mail]> wrote:
>
> On Sun, 18 Nov 2018, Mauro Carvalho Chehab <[e-mail]> wrote:
> > Hmm.. Perhaps the subsystem profile should point to IRC channels if any?
> > Several subsystems use them in order to provide newbies some directions
> > and to discuss other development issues.
>
> MAINTAINERS:
>
>         C: URI for chat protocol, server and channel where developers
>            usually hang out, for example irc://server/channel.
>
> Alas very few subsystems have added that.
>
> I actually fear very few subsystems will add a subsystem profile for
> that matter.

I'm not too worried about the adoption rate of the Subsystem Profile.
If a few subsystems pick it up and contributors find it useful then it
will naturally proliferate, if not then perhaps the initial concern
about contributor pain stumbling through the discovery of these local
policy details was misplaced.

Mauro Carvalho Chehab Nov. 26, 2018, 11:12 a.m. UTC | #20

Hi Jani,

Em Tue, 20 Nov 2018 10:10:25 +0200
Jani Nikula <[e-mail]> escreveu:

> On Sun, 18 Nov 2018, Mauro Carvalho Chehab <[e-mail]> wrote:
> > Hmm.. Perhaps the subsystem profile should point to IRC channels if any?
> > Several subsystems use them in order to provide newbies some directions
> > and to discuss other development issues.  
> 
> MAINTAINERS:
> 
> 	C: URI for chat protocol, server and channel where developers
> 	   usually hang out, for example irc://server/channel.
> 
> Alas very few subsystems have added that.
> 
> I actually fear very few subsystems will add a subsystem profile for
> that matter.

Sorry for a late reply. In the case of the media subsystem, we
actually use 2 different channels, one when the matter is related
to digital TV (#linuxtv) and another one for the rest (#v4l), both
at freenode.

That's something that wouldn't fit well into MAINTAINERS file, as
we can't explain what should be used for each.

So, the way it is it won't work well for us.

Thanks,
Mauro

Joe Perches Nov. 26, 2018, 3:55 p.m. UTC | #21

On Mon, 2018-11-26 at 09:12 -0200, Mauro Carvalho Chehab wrote:
> Hi Jani,
> 
> Em Tue, 20 Nov 2018 10:10:25 +0200
> Jani Nikula <[e-mail]> escreveu:
> 
> > On Sun, 18 Nov 2018, Mauro Carvalho Chehab <[e-mail]> wrote:
> > > Hmm.. Perhaps the subsystem profile should point to IRC channels if any?
> > > Several subsystems use them in order to provide newbies some directions
> > > and to discuss other development issues.  
> > 
> > MAINTAINERS:
> > 
> > 	C: URI for chat protocol, server and channel where developers
> > 	   usually hang out, for example irc://server/channel.
> > 
> > Alas very few subsystems have added that.
> > 
> > I actually fear very few subsystems will add a subsystem profile for
> > that matter.
> 
> Sorry for a late reply. In the case of the media subsystem, we
> actually use 2 different channels, one when the matter is related
> to digital TV (#linuxtv) and another one for the rest (#v4l), both
> at freenode.
> 
> That's something that wouldn't fit well into MAINTAINERS file, as
> we can't explain what should be used for each.
> 
> So, the way it is it won't work well for us.

So maybe add a comment to both like:

C:	<URI1>		# only for digital tv #linuxtv
C:	<URI2>		# everything else

Patch

diff --git a/Documentation/nvdimm/subsystem-profile.rst b/Documentation/nvdimm/subsystem-profile.rst
new file mode 100644
index 000000000000..d3428be7528e
--- /dev/null
+++ b/Documentation/nvdimm/subsystem-profile.rst
@@ -0,0 +1,86 @@ 
+LIBNVDIMM Subsystem Profile
+===========================
+
+Overview
+--------
+So, you have recently become a maintainer of the LIBNVDIMM subsystem,
+condolences, it is a thankless job, here is the lay of the land. The git
+tree, git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm.git/, is
+writable by all the individuals listed in LIBNVDIMM section of
+MAINTAINERS. Access is granted per the typical kernel.org account
+management policies. Two branches in that tree are regularly pulled into
+-next, libnvdimm-for-next, and libnvdimm-fixes. The submit rate of
+patches is low, usually enough for one person to handle. There is a
+patchwork instance at
+https://patchwork.kernel.org/project/linux-nvdimm/list/, and it
+historically is only used for ingesting patches and collecting
+ack/review tags, i.e. no expectation to update the patch state after it
+has been dispositioned, or merged.
+
+The most sensitive code area is the ACPI DSM (Device Specific Method)
+path. In addition to the general fragility of an ioctl() ABI the ACPI
+DSM scheme allows any vendor to implement any command without any prior
+review by the ACPI committee. For this reason the LIBNVDIMM system seeks
+to constrain the proliferation of vendor commands and at a minimum
+requires any command support to be publicly documented. Over time the
+submission rate of new vendor-specific commands is falling as more
+commands are defined with named methods in the official ACPI
+specification.
+
+LIBNVDIMM sits at the intersection of device-drivers, the block-layer,
+core memory-management, and filesystems. Be sure to re-route memory
+management patches to the -mm tree, and otherwise pull-in fs-devel for
+patches that touch anything related to DAX.
+
+Core
+----
+F: drivers/nvdimm/\*_devs.c
+F: drivers/acpi/nfit/\*.[ch]
+
+
+Patches or Pull requests
+------------------------
+Patches only
+
+
+Last day for new feature submissions
+------------------------------------
+Before -rc5
+
+
+Last day to merge features
+--------------------------
+End of last -rc
+
+
+Non-author Ack / Review Tags Required
+-------------------------------------
+Required
+
+
+Test Suite
+----------
+Run ‘make check’ from https://github.com/pmem/ndctl
+
+
+Trusted Reviewers
+-----------------
+Johannes Thumshirn
+Toshi Kani
+Jeff Moyer
+Robert Elliott
+
+
+Resubmit Cadence
+----------------
+8 business days
+
+
+Time Zone / Office Hours
+------------------------
+8:00am to 5:00pm Pacific Time Zone
+
+
+Checkpatch / Style cleanups
+---------------------------
+Standalone style-cleanups are welcome.
diff --git a/MAINTAINERS b/MAINTAINERS
index bb4a83a7684d..ba2beedd4605 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8439,6 +8439,7 @@  M:	Dan Williams <[e-mail]>
 M:	Vishal Verma <[e-mail]>
 M:	Dave Jiang <[e-mail]>
 L:	[e-mail]
+P:	Documentation/nvdimm/subsystem-profile.rst
 Q:	https://patchwork.kernel.org/project/linux-nvdimm/list/
 S:	Supported
 F:	drivers/nvdimm/blk.c
@@ -8450,6 +8451,7 @@  M:	Dan Williams <[e-mail]>
 M:	Ross Zwisler <[e-mail]>
 M:	Dave Jiang <[e-mail]>
 L:	[e-mail]
+P:	Documentation/nvdimm/subsystem-profile.rst
 Q:	https://patchwork.kernel.org/project/linux-nvdimm/list/
 S:	Supported
 F:	drivers/nvdimm/btt*
@@ -8460,6 +8462,7 @@  M:	Dan Williams <[e-mail]>
 M:	Vishal Verma <[e-mail]>
 M:	Dave Jiang <[e-mail]>
 L:	[e-mail]
+P:	Documentation/nvdimm/subsystem-profile.rst
 Q:	https://patchwork.kernel.org/project/linux-nvdimm/list/
 S:	Supported
 F:	drivers/nvdimm/pmem*
@@ -8478,6 +8481,7 @@  M:	Ross Zwisler <[e-mail]>
 M:	Vishal Verma <[e-mail]>
 M:	Dave Jiang <[e-mail]>
 L:	[e-mail]
+P:	Documentation/nvdimm/subsystem-profile.rst
 Q:	https://patchwork.kernel.org/project/linux-nvdimm/list/
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm.git
 S:	Supported