Improve 'create_workqueue', 'create_freezable_workqueue' and
'create_singlethread_workqueue' so that they accept a format
specifier and a variable number of arguments.
This will put these macros more in line with 'alloc_ordered_workqueue' and
the underlying 'alloc_workqueue()' function.
This will also allow further code simplification.
Patch 1 is the modification of the macro.
Patch 2 is an example of simplification possible with this patch
Christophe JAILLET (2):
workqueue: Have 'alloc_workqueue()' like macros accept a format
specifier
net/mlx5: Simplify workqueue name creation
drivers/net/ethernet/mellanox/mlx5/core/health.c | 9 +--------
include/linux/workqueue.h | 14 +++++++-------
2 files changed, 8 insertions(+), 15 deletions(-)
--
2.27.0
There is no need to explicitly allocate, populate and free some memory
just to pass a workqueue name to 'create_singlethread_workqueue()'.
This macro can do all this for us, so keep the code simple.
Signed-off-by: Christophe JAILLET <[email protected]>
---
A similar patch has also been sent. It was replacing the kmalloc/strcpy/
strcat with a kasprintf.
Updating 'create_singlethread_workqueue' gives an even more elegant solution.
---
drivers/net/ethernet/mellanox/mlx5/core/health.c | 9 +--------
1 file changed, 2 insertion(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/health.c b/drivers/net/ethernet/mellanox/mlx5/core/health.c
index 9ff163c5bcde..160f852b7bbe 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/health.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/health.c
@@ -797,19 +797,13 @@ void mlx5_health_cleanup(struct mlx5_core_dev *dev)
int mlx5_health_init(struct mlx5_core_dev *dev)
{
struct mlx5_core_health *health;
- char *name;
mlx5_fw_reporters_create(dev);
health = &dev->priv.health;
- name = kmalloc(64, GFP_KERNEL);
- if (!name)
- goto out_err;
- strcpy(name, "mlx5_health");
- strcat(name, dev_name(dev->device));
- health->wq = create_singlethread_workqueue(name);
- kfree(name);
+ health->wq = create_singlethread_workqueue("mlx5_health%s",
+ dev_name(dev->device));
if (!health->wq)
goto out_err;
spin_lock_init(&health->wq_lock);
--
2.27.0
Improve 'create_workqueue', 'create_freezable_workqueue' and
'create_singlethread_workqueue' so that they accept a format
specifier and a variable number of arguments.
This will put these macros more in line with 'alloc_ordered_workqueue' and
the underlying 'alloc_workqueue()' function.
This will also allow further code simplification.
Suggested-by: Bart Van Assche <[email protected]>
Signed-off-by: Christophe JAILLET <[email protected]>
---
include/linux/workqueue.h | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index d15a7730ee18..145e756ff315 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -425,13 +425,13 @@ struct workqueue_struct *alloc_workqueue(const char *fmt,
alloc_workqueue(fmt, WQ_UNBOUND | __WQ_ORDERED | \
__WQ_ORDERED_EXPLICIT | (flags), 1, ##args)
-#define create_workqueue(name) \
- alloc_workqueue("%s", __WQ_LEGACY | WQ_MEM_RECLAIM, 1, (name))
-#define create_freezable_workqueue(name) \
- alloc_workqueue("%s", __WQ_LEGACY | WQ_FREEZABLE | WQ_UNBOUND | \
- WQ_MEM_RECLAIM, 1, (name))
-#define create_singlethread_workqueue(name) \
- alloc_ordered_workqueue("%s", __WQ_LEGACY | WQ_MEM_RECLAIM, name)
+#define create_workqueue(fmt, args...) \
+ alloc_workqueue(fmt, __WQ_LEGACY | WQ_MEM_RECLAIM, 1, ##args)
+#define create_freezable_workqueue(fmt, args...) \
+ alloc_workqueue(fmt, __WQ_LEGACY | WQ_FREEZABLE | WQ_UNBOUND | \
+ WQ_MEM_RECLAIM, 1, ##args)
+#define create_singlethread_workqueue(fmt, args...) \
+ alloc_ordered_workqueue(fmt, __WQ_LEGACY | WQ_MEM_RECLAIM, ##args)
extern void destroy_workqueue(struct workqueue_struct *wq);
--
2.27.0
On 4/18/21 2:26 PM, Christophe JAILLET wrote:
> Improve 'create_workqueue', 'create_freezable_workqueue' and
> 'create_singlethread_workqueue' so that they accept a format
> specifier and a variable number of arguments.
>
> This will put these macros more in line with 'alloc_ordered_workqueue' and
> the underlying 'alloc_workqueue()' function.
>
> This will also allow further code simplification.
Please Cc Tejun for workqueue changes since he maintains the workqueue code.
> diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
> index d15a7730ee18..145e756ff315 100644
> --- a/include/linux/workqueue.h
> +++ b/include/linux/workqueue.h
> @@ -425,13 +425,13 @@ struct workqueue_struct *alloc_workqueue(const char *fmt,
> alloc_workqueue(fmt, WQ_UNBOUND | __WQ_ORDERED | \
> __WQ_ORDERED_EXPLICIT | (flags), 1, ##args)
>
> -#define create_workqueue(name) \
> - alloc_workqueue("%s", __WQ_LEGACY | WQ_MEM_RECLAIM, 1, (name))
> -#define create_freezable_workqueue(name) \
> - alloc_workqueue("%s", __WQ_LEGACY | WQ_FREEZABLE | WQ_UNBOUND | \
> - WQ_MEM_RECLAIM, 1, (name))
> -#define create_singlethread_workqueue(name) \
> - alloc_ordered_workqueue("%s", __WQ_LEGACY | WQ_MEM_RECLAIM, name)
> +#define create_workqueue(fmt, args...) \
> + alloc_workqueue(fmt, __WQ_LEGACY | WQ_MEM_RECLAIM, 1, ##args)
> +#define create_freezable_workqueue(fmt, args...) \
> + alloc_workqueue(fmt, __WQ_LEGACY | WQ_FREEZABLE | WQ_UNBOUND | \
> + WQ_MEM_RECLAIM, 1, ##args)
> +#define create_singlethread_workqueue(fmt, args...) \
> + alloc_ordered_workqueue(fmt, __WQ_LEGACY | WQ_MEM_RECLAIM, ##args)
>
> extern void destroy_workqueue(struct workqueue_struct *wq);
>
>
> Message du 19/04/21 01:03
> De : "Bart Van Assche"
> A : "Christophe JAILLET" , [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], "Tejun Heo"
> Copie à : [email protected], [email protected], [email protected], [email protected]
> Objet : Re: [PATCH 1/2] workqueue: Have 'alloc_workqueue()' like macros accept a format specifier
>
> On 4/18/21 2:26 PM, Christophe JAILLET wrote:
> > Improve 'create_workqueue', 'create_freezable_workqueue' and
> > 'create_singlethread_workqueue' so that they accept a format
> > specifier and a variable number of arguments.
> >
> > This will put these macros more in line with 'alloc_ordered_workqueue' and
> > the underlying 'alloc_workqueue()' function.
> >
> > This will also allow further code simplification.
>
> Please Cc Tejun for workqueue changes since he maintains the workqueue code.
>
Hi,
The list in To: is the one given by get_maintainer.pl. Usualy, I only put the ML in Cc:
I've run the script on the 2 patches of the serie and merged the 2 lists. Everyone is in the To: of the cover letter and of the 2 patches.
If Théo is "Tejun Heo" ( (maintainer:WORKQUEUE) ), he is already in the To: line.
CJ
> > diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
> > index d15a7730ee18..145e756ff315 100644
> > --- a/include/linux/workqueue.h
> > +++ b/include/linux/workqueue.h
> > @@ -425,13 +425,13 @@ struct workqueue_struct *alloc_workqueue(const char *fmt,
> > alloc_workqueue(fmt, WQ_UNBOUND | __WQ_ORDERED | \
> > __WQ_ORDERED_EXPLICIT | (flags), 1, ##args)
> >
> > -#define create_workqueue(name) \
> > - alloc_workqueue("%s", __WQ_LEGACY | WQ_MEM_RECLAIM, 1, (name))
> > -#define create_freezable_workqueue(name) \
> > - alloc_workqueue("%s", __WQ_LEGACY | WQ_FREEZABLE | WQ_UNBOUND | \
> > - WQ_MEM_RECLAIM, 1, (name))
> > -#define create_singlethread_workqueue(name) \
> > - alloc_ordered_workqueue("%s", __WQ_LEGACY | WQ_MEM_RECLAIM, name)
> > +#define create_workqueue(fmt, args...) \
> > + alloc_workqueue(fmt, __WQ_LEGACY | WQ_MEM_RECLAIM, 1, ##args)
> > +#define create_freezable_workqueue(fmt, args...) \
> > + alloc_workqueue(fmt, __WQ_LEGACY | WQ_FREEZABLE | WQ_UNBOUND | \
> > + WQ_MEM_RECLAIM, 1, ##args)
> > +#define create_singlethread_workqueue(fmt, args...) \
> > + alloc_ordered_workqueue(fmt, __WQ_LEGACY | WQ_MEM_RECLAIM, ##args)
> >
> > extern void destroy_workqueue(struct workqueue_struct *wq);
> >
> >
>
>
On 18/04/2021 23.26, Christophe JAILLET wrote:
> Improve 'create_workqueue', 'create_freezable_workqueue' and
> 'create_singlethread_workqueue' so that they accept a format
> specifier and a variable number of arguments.
>
> This will put these macros more in line with 'alloc_ordered_workqueue' and
> the underlying 'alloc_workqueue()' function.
>
> This will also allow further code simplification.
>
> Suggested-by: Bart Van Assche <[email protected]>
> Signed-off-by: Christophe JAILLET <[email protected]>
> ---
> include/linux/workqueue.h | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
> index d15a7730ee18..145e756ff315 100644
> --- a/include/linux/workqueue.h
> +++ b/include/linux/workqueue.h
> @@ -425,13 +425,13 @@ struct workqueue_struct *alloc_workqueue(const char *fmt,
> alloc_workqueue(fmt, WQ_UNBOUND | __WQ_ORDERED | \
> __WQ_ORDERED_EXPLICIT | (flags), 1, ##args)
>
> -#define create_workqueue(name) \
> - alloc_workqueue("%s", __WQ_LEGACY | WQ_MEM_RECLAIM, 1, (name))
> +#define create_workqueue(fmt, args...) \
> + alloc_workqueue(fmt, __WQ_LEGACY | WQ_MEM_RECLAIM, 1, ##args)
The changes make sense, but are you sure that no current users of those
macros have some % character in the string they pass? If all users pass
string literals the compiler/0day bot should catch those, but as the
very example you give in 2/2 shows, not everybody passes string literals.
Maybe git grep would quickly tell that there's only 8 callers and they
are all audited quickly or something like that; in that case please
include a note to that effect in the commit log.
Rasmus
Hello, Christophe.
On Sun, Apr 18, 2021 at 11:25:52PM +0200, Christophe JAILLET wrote:
> Improve 'create_workqueue', 'create_freezable_workqueue' and
> 'create_singlethread_workqueue' so that they accept a format
> specifier and a variable number of arguments.
>
> This will put these macros more in line with 'alloc_ordered_workqueue' and
> the underlying 'alloc_workqueue()' function.
Those interfaces are deprecated and if you're doing anything with the users,
the right course of action would be converting them to use one of the
alloc_workqueue interfaces.
Thanks.
--
tejun
On 4/18/21 11:36 PM, Marion et Christophe JAILLET wrote:
> The list in To: is the one given by get_maintainer.pl. Usualy, I only
> put the ML in Cc: I've run the script on the 2 patches of the serie
> and merged the 2 lists. Everyone is in the To: of the cover letter
> and of the 2 patches.
>
> If Théo is "Tejun Heo" ( (maintainer:WORKQUEUE) ), he is already in
> the To: line.
Linus wants to see a "Cc: ${maintainer}" tag in patches that he receives
from a maintainer and that modify another subsystem than the subsystem
maintained by that maintainer.
Thanks,
Bart.
On Mon, Apr 19, 2021 at 01:02:34PM -0700, Bart Van Assche wrote:
> On 4/18/21 11:36 PM, Marion et Christophe JAILLET wrote:
> > The list in To: is the one given by get_maintainer.pl. Usualy, I only
> > put the ML in Cc: I've run the script on the 2 patches of the serie
> > and merged the 2 lists. Everyone is in the To: of the cover letter
> > and of the 2 patches.
> >
> > If Théo is "Tejun Heo" ( (maintainer:WORKQUEUE) ), he is already in
> > the To: line.
> Linus wants to see a "Cc: ${maintainer}" tag in patches that he receives
> from a maintainer and that modify another subsystem than the subsystem
> maintained by that maintainer.
Really? Do you remember a lore link for this?
Generally I've been junking the CC lines (vs Andrew at the other
extreme that often has 10's of CC lines)
Jason
On 4/22/21 5:24 AM, Jason Gunthorpe wrote:
> On Mon, Apr 19, 2021 at 01:02:34PM -0700, Bart Van Assche wrote:
>> On 4/18/21 11:36 PM, Marion et Christophe JAILLET wrote:
>>> The list in To: is the one given by get_maintainer.pl. Usualy, I only
>>> put the ML in Cc: I've run the script on the 2 patches of the serie
>>> and merged the 2 lists. Everyone is in the To: of the cover letter
>>> and of the 2 patches.
>>>
>>> If Théo is "Tejun Heo" ( (maintainer:WORKQUEUE) ), he is already in
>>> the To: line.
>> Linus wants to see a "Cc: ${maintainer}" tag in patches that he receives
>> from a maintainer and that modify another subsystem than the subsystem
>> maintained by that maintainer.
>
> Really? Do you remember a lore link for this?
Last time I saw Linus mentioning this was a few months ago.
Unfortunately I cannot find that message anymore.
> Generally I've been junking the CC lines (vs Andrew at the other
> extreme that often has 10's of CC lines)
Most entries in the MAINTAINERS file have one to three email addresses
so I'm surprised to read that Cc-ing maintainer(s) could result in tens
of Cc lines?
Thanks,
Bart.
On Thu, Apr 22, 2021 at 10:12:33AM -0700, Bart Van Assche wrote:
> On 4/22/21 5:24 AM, Jason Gunthorpe wrote:
> > On Mon, Apr 19, 2021 at 01:02:34PM -0700, Bart Van Assche wrote:
> >> On 4/18/21 11:36 PM, Marion et Christophe JAILLET wrote:
> >>> The list in To: is the one given by get_maintainer.pl. Usualy, I only
> >>> put the ML in Cc: I've run the script on the 2 patches of the serie
> >>> and merged the 2 lists. Everyone is in the To: of the cover letter
> >>> and of the 2 patches.
> >>>
> >>> If Th?o is "Tejun Heo" ( (maintainer:WORKQUEUE) ), he is already in
> >>> the To: line.
> >> Linus wants to see a "Cc: ${maintainer}" tag in patches that he receives
> >> from a maintainer and that modify another subsystem than the subsystem
> >> maintained by that maintainer.
> >
> > Really? Do you remember a lore link for this?
>
> Last time I saw Linus mentioning this was a few months ago.
> Unfortunately I cannot find that message anymore.
>
> > Generally I've been junking the CC lines (vs Andrew at the other
> > extreme that often has 10's of CC lines)
>
> Most entries in the MAINTAINERS file have one to three email addresses
> so I'm surprised to read that Cc-ing maintainer(s) could result in tens
> of Cc lines?
git log mm/
commit 2b8305260fb37fc20e13f71e13073304d0a031c8
Author: Alexander Potapenko <[email protected]>
Date: Thu Feb 25 17:19:21 2021 -0800
kfence, kasan: make KFENCE compatible with KASAN
Make KFENCE compatible with KASAN. Currently this helps test KFENCE
itself, where KASAN can catch potential corruptions to KFENCE state, or
other corruptions that may be a result of freepointer corruptions in the
main allocators.
[[email protected]: merge fixup]
[[email protected]: untag addresses for KFENCE]
Link: https://lkml.kernel.org/r/9dc196006921b191d25d10f6e611316db7da2efc.1611946152.git.andreyknvl@google.com
Link: https://lkml.kernel.org/r/[email protected]
Signed-off-by: Marco Elver <[email protected]>
Signed-off-by: Alexander Potapenko <[email protected]>
Signed-off-by: Andrey Konovalov <[email protected]>
Reviewed-by: Dmitry Vyukov <[email protected]>
Reviewed-by: Jann Horn <[email protected]>
Co-developed-by: Marco Elver <[email protected]>
Cc: Andrey Konovalov <[email protected]>
Cc: Andrey Ryabinin <[email protected]>
Cc: Andy Lutomirski <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Christopher Lameter <[email protected]>
Cc: Dave Hansen <[email protected]>
Cc: David Rientjes <[email protected]>
Cc: Eric Dumazet <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Hillf Danton <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Joern Engel <[email protected]>
Cc: Jonathan Corbet <[email protected]>
Cc: Joonsoo Kim <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Paul E. McKenney <[email protected]>
Cc: Pekka Enberg <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: SeongJae Park <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Vlastimil Babka <[email protected]>
Cc: Will Deacon <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
>
> Thanks,
>
> Bart.
On 4/22/21 11:00 AM, Leon Romanovsky wrote:
> On Thu, Apr 22, 2021 at 10:12:33AM -0700, Bart Van Assche wrote:
>> On 4/22/21 5:24 AM, Jason Gunthorpe wrote:
>>> On Mon, Apr 19, 2021 at 01:02:34PM -0700, Bart Van Assche wrote:
>>>> On 4/18/21 11:36 PM, Marion et Christophe JAILLET wrote:
>>>>> The list in To: is the one given by get_maintainer.pl. Usualy, I only
>>>>> put the ML in Cc: I've run the script on the 2 patches of the serie
>>>>> and merged the 2 lists. Everyone is in the To: of the cover letter
>>>>> and of the 2 patches.
>>>>>
>>>>> If Théo is "Tejun Heo" ( (maintainer:WORKQUEUE) ), he is already in
>>>>> the To: line.
>>>> Linus wants to see a "Cc: ${maintainer}" tag in patches that he receives
>>>> from a maintainer and that modify another subsystem than the subsystem
>>>> maintained by that maintainer.
>>>
>>> Really? Do you remember a lore link for this?
>>
>> Last time I saw Linus mentioning this was a few months ago.
>> Unfortunately I cannot find that message anymore.
>>
>>> Generally I've been junking the CC lines (vs Andrew at the other
>>> extreme that often has 10's of CC lines)
>>
>> Most entries in the MAINTAINERS file have one to three email addresses
>> so I'm surprised to read that Cc-ing maintainer(s) could result in tens
>> of Cc lines?
>
> git log mm/
>
> commit 2b8305260fb37fc20e13f71e13073304d0a031c8
> Author: Alexander Potapenko <[email protected]>
> Date: Thu Feb 25 17:19:21 2021 -0800
>
> kfence, kasan: make KFENCE compatible with KASAN
>
> Make KFENCE compatible with KASAN. Currently this helps test KFENCE
> itself, where KASAN can catch potential corruptions to KFENCE state, or
> other corruptions that may be a result of freepointer corruptions in the
> main allocators.
>
> [[email protected]: merge fixup]
> [[email protected]: untag addresses for KFENCE]
> Link: https://lkml.kernel.org/r/9dc196006921b191d25d10f6e611316db7da2efc.1611946152.git.andreyknvl@google.com
>
> Link: https://lkml.kernel.org/r/[email protected]
> Signed-off-by: Marco Elver <[email protected]>
> Signed-off-by: Alexander Potapenko <[email protected]>
> Signed-off-by: Andrey Konovalov <[email protected]>
> Reviewed-by: Dmitry Vyukov <[email protected]>
> Reviewed-by: Jann Horn <[email protected]>
> Co-developed-by: Marco Elver <[email protected]>
> Cc: Andrey Konovalov <[email protected]>
> Cc: Andrey Ryabinin <[email protected]>
> Cc: Andy Lutomirski <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: Christopher Lameter <[email protected]>
> Cc: Dave Hansen <[email protected]>
> Cc: David Rientjes <[email protected]>
> Cc: Eric Dumazet <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Hillf Danton <[email protected]>
> Cc: "H. Peter Anvin" <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Joern Engel <[email protected]>
> Cc: Jonathan Corbet <[email protected]>
> Cc: Joonsoo Kim <[email protected]>
> Cc: Kees Cook <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: Paul E. McKenney <[email protected]>
> Cc: Pekka Enberg <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: SeongJae Park <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Vlastimil Babka <[email protected]>
> Cc: Will Deacon <[email protected]>
> Signed-off-by: Andrew Morton <[email protected]>
> Signed-off-by: Linus Torvalds <[email protected]>
But where does that Cc-list come from? If I extract that patch and run
the get_maintainer.pl script, the following output appears:
$ git format-patch -1 2b8305260fb37fc20e13f71e13073304d0a031c8
0001-kfence-kasan-make-KFENCE-compatible-with-KASAN.patch
$ scripts/get_maintainer.pl
0001-kfence-kasan-make-KFENCE-compatible-with-KASAN.patch
Alexander Potapenko <[email protected]> (maintainer:KFENCE)
Marco Elver <[email protected]> (maintainer:KFENCE)
Dmitry Vyukov <[email protected]> (reviewer:KFENCE)
Andrey Ryabinin <[email protected]> (maintainer:KASAN)
Andrey Konovalov <[email protected]> (reviewer:KASAN)
Andrew Morton <[email protected]> (maintainer:MEMORY MANAGEMENT)
[email protected] (open list:KFENCE)
[email protected] (open list)
[email protected] (open list:MEMORY MANAGEMENT)
Additionally, please note that in my email I was referring to the
MAINTAINERS file. I do not use the get_maintainers.pl script but instead
select the Cc-list manually based on what I find in the MAINTAINERS file.
Thanks,
Bart.
On Thu, Apr 22, 2021 at 01:30PM -0700, Bart Van Assche wrote:
> On 4/22/21 11:00 AM, Leon Romanovsky wrote:
> > On Thu, Apr 22, 2021 at 10:12:33AM -0700, Bart Van Assche wrote:
> > > On 4/22/21 5:24 AM, Jason Gunthorpe wrote:
> > > > On Mon, Apr 19, 2021 at 01:02:34PM -0700, Bart Van Assche wrote:
> > > > > On 4/18/21 11:36 PM, Marion et Christophe JAILLET wrote:
> > > > > > The list in To: is the one given by get_maintainer.pl. Usualy, I only
> > > > > > put the ML in Cc: I've run the script on the 2 patches of the serie
> > > > > > and merged the 2 lists. Everyone is in the To: of the cover letter
> > > > > > and of the 2 patches.
> > > > > >
> > > > > > If Th?o is "Tejun Heo" ( (maintainer:WORKQUEUE) ), he is already in
> > > > > > the To: line.
> > > > > Linus wants to see a "Cc: ${maintainer}" tag in patches that he receives
> > > > > from a maintainer and that modify another subsystem than the subsystem
> > > > > maintained by that maintainer.
> > > >
> > > > Really? Do you remember a lore link for this?
> > >
> > > Last time I saw Linus mentioning this was a few months ago.
> > > Unfortunately I cannot find that message anymore.
> > >
> > > > Generally I've been junking the CC lines (vs Andrew at the other
> > > > extreme that often has 10's of CC lines)
> > >
> > > Most entries in the MAINTAINERS file have one to three email addresses
> > > so I'm surprised to read that Cc-ing maintainer(s) could result in tens
> > > of Cc lines?
> >
> > git log mm/
> >
> > commit 2b8305260fb37fc20e13f71e13073304d0a031c8
> > Author: Alexander Potapenko <[email protected]>
> > Date: Thu Feb 25 17:19:21 2021 -0800
> >
> > kfence, kasan: make KFENCE compatible with KASAN
> >
> > Make KFENCE compatible with KASAN. Currently this helps test KFENCE
> > itself, where KASAN can catch potential corruptions to KFENCE state, or
> > other corruptions that may be a result of freepointer corruptions in the
> > main allocators.
> >
> > [[email protected]: merge fixup]
> > [[email protected]: untag addresses for KFENCE]
> > Link: https://lkml.kernel.org/r/9dc196006921b191d25d10f6e611316db7da2efc.1611946152.git.andreyknvl@google.com
> >
> > Link: https://lkml.kernel.org/r/[email protected]
> > Signed-off-by: Marco Elver <[email protected]>
> > Signed-off-by: Alexander Potapenko <[email protected]>
> > Signed-off-by: Andrey Konovalov <[email protected]>
> > Reviewed-by: Dmitry Vyukov <[email protected]>
> > Reviewed-by: Jann Horn <[email protected]>
> > Co-developed-by: Marco Elver <[email protected]>
> > Cc: Andrey Konovalov <[email protected]>
> > Cc: Andrey Ryabinin <[email protected]>
> > Cc: Andy Lutomirski <[email protected]>
> > Cc: Borislav Petkov <[email protected]>
> > Cc: Catalin Marinas <[email protected]>
> > Cc: Christopher Lameter <[email protected]>
> > Cc: Dave Hansen <[email protected]>
> > Cc: David Rientjes <[email protected]>
> > Cc: Eric Dumazet <[email protected]>
> > Cc: Greg Kroah-Hartman <[email protected]>
> > Cc: Hillf Danton <[email protected]>
> > Cc: "H. Peter Anvin" <[email protected]>
> > Cc: Ingo Molnar <[email protected]>
> > Cc: Joern Engel <[email protected]>
> > Cc: Jonathan Corbet <[email protected]>
> > Cc: Joonsoo Kim <[email protected]>
> > Cc: Kees Cook <[email protected]>
> > Cc: Mark Rutland <[email protected]>
> > Cc: Paul E. McKenney <[email protected]>
> > Cc: Pekka Enberg <[email protected]>
> > Cc: Peter Zijlstra <[email protected]>
> > Cc: SeongJae Park <[email protected]>
> > Cc: Thomas Gleixner <[email protected]>
> > Cc: Vlastimil Babka <[email protected]>
> > Cc: Will Deacon <[email protected]>
> > Signed-off-by: Andrew Morton <[email protected]>
> > Signed-off-by: Linus Torvalds <[email protected]>
This is a special case probably as KFENCE touched various subsystems and
architectures.
That Cc list is from the original
https://lkml.kernel.org/r/[email protected]
It was determined based on the full series, mostly from a
get_maintainer.pl of 'reviewer' and 'maintainer' lists of the full
series diff, minus a some false positives to avoid spamming people, and
plus a few people get_maintainer.pl missed that had provided or could
provide useful input. So the list above is mostly maintainers+reviewers
of mm/, mm/kasan, arch/x86, and arch/arm64.
> But where does that Cc-list come from? If I extract that patch and run the
> get_maintainer.pl script, the following output appears:
>
> $ git format-patch -1 2b8305260fb37fc20e13f71e13073304d0a031c8
> 0001-kfence-kasan-make-KFENCE-compatible-with-KASAN.patch
> $ scripts/get_maintainer.pl
> 0001-kfence-kasan-make-KFENCE-compatible-with-KASAN.patch
> Alexander Potapenko <[email protected]> (maintainer:KFENCE)
> Marco Elver <[email protected]> (maintainer:KFENCE)
KFENCE did not yet exist when the patch the above series was part of was
posted... so chicken and egg situation here. ;-)
> Dmitry Vyukov <[email protected]> (reviewer:KFENCE)
> Andrey Ryabinin <[email protected]> (maintainer:KASAN)
> Andrey Konovalov <[email protected]> (reviewer:KASAN)
> Andrew Morton <[email protected]> (maintainer:MEMORY MANAGEMENT)
> [email protected] (open list:KFENCE)
> [email protected] (open list)
> [email protected] (open list:MEMORY MANAGEMENT)
Thanks,
-- Marco
On Thu, Apr 22, 2021 at 01:30:00PM -0700, Bart Van Assche wrote:
> But where does that Cc-list come from? If I extract that patch and run the
> get_maintainer.pl script, the following output appears:
Andrew takes it from the email CC list in the original email that the
patch came from
Jason
On Thu, Apr 22, 2021 at 09:24:19AM -0300, Jason Gunthorpe wrote:
> On Mon, Apr 19, 2021 at 01:02:34PM -0700, Bart Van Assche wrote:
> > On 4/18/21 11:36 PM, Marion et Christophe JAILLET wrote:
> > > The list in To: is the one given by get_maintainer.pl. Usualy, I only
> > > put the ML in Cc: I've run the script on the 2 patches of the serie
> > > and merged the 2 lists. Everyone is in the To: of the cover letter
> > > and of the 2 patches.
> > >
> > > If Th?o is "Tejun Heo" ( (maintainer:WORKQUEUE) ), he is already in
> > > the To: line.
> > Linus wants to see a "Cc: ${maintainer}" tag in patches that he receives
> > from a maintainer and that modify another subsystem than the subsystem
> > maintained by that maintainer.
>
> Really? Do you remember a lore link for this?
>
> Generally I've been junking the CC lines (vs Andrew at the other
> extreme that often has 10's of CC lines)
Of course this patch has already been NAKed but it wasn't clear to me
whose git tree it would have gone through. Surely if it were going
through your tree you would have required an Acked-by: from Tejun and
the CC: line would not be required. It would only be required if you
can't get a maintainer to respond.
regards,
dan carpenter