Re: [Intel-gfx] [PATCH 02/12] blk: use for_each_if

2018-07-13 Thread Vlastimil Babka
On 07/12/2018 08:45 AM, Joe Perches wrote:
> On Wed, 2018-07-11 at 20:50 +0200, Daniel Vetter wrote:
>> On Wed, Jul 11, 2018 at 8:30 PM, Jens Axboe  wrote:
>>> On 7/11/18 10:45 AM, Tejun Heo wrote:
 On Wed, Jul 11, 2018 at 09:40:58AM -0700, Tejun Heo wrote:
> On Mon, Jul 09, 2018 at 10:36:40AM +0200, Daniel Vetter wrote:
>> Makes the macros resilient against if {} else {} blocks right
>> afterwards.
>>
>> Signed-off-by: Daniel Vetter 
>> Cc: Tejun Heo 
>> Cc: Jens Axboe 
>> Cc: Shaohua Li 
>> Cc: Kate Stewart 
>> Cc: Greg Kroah-Hartman 
>> Cc: Joseph Qi 
>> Cc: Daniel Vetter 
>> Cc: Arnd Bergmann 
>
> Acked-by: Tejun Heo 
>
> Jens, it'd probably be best to route this through block tree.

 Oops, this requires an earlier patch to move the for_each_if def to a
 common header and should be routed together.
>>>
>>> Yeah, this is a problem with the submission.
>>>
>>> Always (ALWAYS) CC folks on at least the cover letter and generic
>>> earlier patches. Getting just one patch sent like this is mostly
>>> useless, and causes more harm than good.
>>
>> Ime sending a patch with more than 20 or so recipients means it gets
>> stuck everywhere in moderation queues. Or outright spam filters. I
>> thought the correct way to do this is to cc: mailing lists (lkml has
>> them all), but apparently that's not how it's done. Despite that all
>> the patch series I get never have the cover letter addressed to me
>> either.
>>
>> So what's the magic way to make this possible?
> 
> Jens' advice is crap.

This statement was rather offensive and totally uncalled for, AFAICS.
Why did you write it like that?

> There is no generic way to make this possible.
> 
> BCC's don't work, series that touch multiple subsystems
> get rejected when the recipient list is too large.

I don't know what's the usual limit for recipient list, probably never
hit it myself, but for series that are not so large, I use this approach
to make sure the cover letter is CC'd to everyone that's CC'd in any
patch in the series:

- add per-patch Cc:'s to the git commit logs
- clear out *.patch from the working dir
- git format-patch --cover-letter ...
- edit cover letter
- git send-email ... --cc-cmd=./cc.sh ...

where cc.sh contains this:

#/bin/sh
if [[ $1 == *cover-letter* ]]; then
grep '<.*@.*>' -h *.patch | sed 's/^.*: //' | sort | uniq
else
grep '<.*@.*>' -h $1 | sed 's/^.*: //' | sort | uniq
fi

That proceses all tags besides Cc (acked-by, reported-by etc) and turns
them to Cc's for each patch (or does git now do that by itself as well?)
and for cover letter, it accumulates that from all the patches.

Vlastimil

> I think you did it correctly.
> 

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 02/12] blk: use for_each_if

2018-07-12 Thread Joe Perches
On Thu, 2018-07-12 at 07:54 -0600, Jens Axboe wrote:
> 
> Thanks for your invaluable and useful feedback, sharing your vast
> experience in patchsets with dependencies.

I've probably more experience sending patchsets
with dependencies across subsystems than anyone.

There is no single style that works and I've
probably tried them all.

It's actually a somewhat significant issue within
this community that could use some arbitration.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 02/12] blk: use for_each_if

2018-07-12 Thread Jens Axboe
On 7/12/18 12:45 AM, Joe Perches wrote:
> On Wed, 2018-07-11 at 20:50 +0200, Daniel Vetter wrote:
>> On Wed, Jul 11, 2018 at 8:30 PM, Jens Axboe  wrote:
>>> On 7/11/18 10:45 AM, Tejun Heo wrote:
 On Wed, Jul 11, 2018 at 09:40:58AM -0700, Tejun Heo wrote:
> On Mon, Jul 09, 2018 at 10:36:40AM +0200, Daniel Vetter wrote:
>> Makes the macros resilient against if {} else {} blocks right
>> afterwards.
>>
>> Signed-off-by: Daniel Vetter 
>> Cc: Tejun Heo 
>> Cc: Jens Axboe 
>> Cc: Shaohua Li 
>> Cc: Kate Stewart 
>> Cc: Greg Kroah-Hartman 
>> Cc: Joseph Qi 
>> Cc: Daniel Vetter 
>> Cc: Arnd Bergmann 
>
> Acked-by: Tejun Heo 
>
> Jens, it'd probably be best to route this through block tree.

 Oops, this requires an earlier patch to move the for_each_if def to a
 common header and should be routed together.
>>>
>>> Yeah, this is a problem with the submission.
>>>
>>> Always (ALWAYS) CC folks on at least the cover letter and generic
>>> earlier patches. Getting just one patch sent like this is mostly
>>> useless, and causes more harm than good.
>>
>> Ime sending a patch with more than 20 or so recipients means it gets
>> stuck everywhere in moderation queues. Or outright spam filters. I
>> thought the correct way to do this is to cc: mailing lists (lkml has
>> them all), but apparently that's not how it's done. Despite that all
>> the patch series I get never have the cover letter addressed to me
>> either.
>>
>> So what's the magic way to make this possible?
> 
> Jens' advice is crap.
> 
> There is no generic way to make this possible.

Nobody claimed there was. And the advice is perfectly fine,
sending out patches to folks that have hidden dependencies on other
patches is a no-go.

> BCC's don't work, series that touch multiple subsystems
> get rejected when the recipient list is too large.
> 
> I think you did it correctly.

Clearly that's not the case, regardless of what you think.

Thanks for your invaluable and useful feedback, sharing your vast
experience in patchsets with dependencies.

-- 
Jens Axboe

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 02/12] blk: use for_each_if

2018-07-12 Thread Joe Perches
On Wed, 2018-07-11 at 20:50 +0200, Daniel Vetter wrote:
> On Wed, Jul 11, 2018 at 8:30 PM, Jens Axboe  wrote:
> > On 7/11/18 10:45 AM, Tejun Heo wrote:
> > > On Wed, Jul 11, 2018 at 09:40:58AM -0700, Tejun Heo wrote:
> > > > On Mon, Jul 09, 2018 at 10:36:40AM +0200, Daniel Vetter wrote:
> > > > > Makes the macros resilient against if {} else {} blocks right
> > > > > afterwards.
> > > > > 
> > > > > Signed-off-by: Daniel Vetter 
> > > > > Cc: Tejun Heo 
> > > > > Cc: Jens Axboe 
> > > > > Cc: Shaohua Li 
> > > > > Cc: Kate Stewart 
> > > > > Cc: Greg Kroah-Hartman 
> > > > > Cc: Joseph Qi 
> > > > > Cc: Daniel Vetter 
> > > > > Cc: Arnd Bergmann 
> > > > 
> > > > Acked-by: Tejun Heo 
> > > > 
> > > > Jens, it'd probably be best to route this through block tree.
> > > 
> > > Oops, this requires an earlier patch to move the for_each_if def to a
> > > common header and should be routed together.
> > 
> > Yeah, this is a problem with the submission.
> > 
> > Always (ALWAYS) CC folks on at least the cover letter and generic
> > earlier patches. Getting just one patch sent like this is mostly
> > useless, and causes more harm than good.
> 
> Ime sending a patch with more than 20 or so recipients means it gets
> stuck everywhere in moderation queues. Or outright spam filters. I
> thought the correct way to do this is to cc: mailing lists (lkml has
> them all), but apparently that's not how it's done. Despite that all
> the patch series I get never have the cover letter addressed to me
> either.
> 
> So what's the magic way to make this possible?

Jens' advice is crap.

There is no generic way to make this possible.

BCC's don't work, series that touch multiple subsystems
get rejected when the recipient list is too large.

I think you did it correctly.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 02/12] blk: use for_each_if

2018-07-12 Thread Daniel Vetter
On Wed, Jul 11, 2018 at 03:13:00PM -0600, Jens Axboe wrote:
> On 7/11/18 3:08 PM, Daniel Vetter wrote:
> > On Wed, Jul 11, 2018 at 10:06 PM, Tejun Heo  wrote:
> >> On Wed, Jul 11, 2018 at 01:31:51PM -0600, Jens Axboe wrote:
> >>> I don't think there's a git easy way of sending it out outside of
> >>> just ensuring that everybody is CC'ed on everything. I don't mind
> >>> that at all. I don't subscribe to lkml, and the patches weren't
> >>> sent to linux-block. Hence all I see is this stand-alone patch,
> >>> and logic would dictate that it's stand-alone (but it isn't).
> > 
> > Hm yeah I forgot to add linux-block. But others where there's no
> > dedicated list (or get_maintainers.pl didn't have one) also complained
> > about not getting Cc'ed, and I can't Cc everyone for sweeping changes.
> 
> I don't personally see a problem with just CC'ing everyone.
> 
> >> What I sometimes do is including a short blurb on each patch giving
> >> the overview and action hints (e.g. this is part of patchset doing XYZ
> >> and should be routed such and such).  It's a bit redundant but has
> >> worked pretty well for patchsets with dependenat & sweeping changes.
> > 
> > Yeah I guess I can just copypaste/summarize patch 1 to all the
> > subsequent patches, sounds like the best option.
> 
> Another approach might be to submit the first independent patch
> separately. Once that's in the kernel, you can send out the rest
> as independent patches instead of doing a cross-kernel series that
> all depend on one single patch. Seems to me that's where you run
> into issues, and it can be avoided quite easily.

Well patch 1 died in a bikeshed (or is on live support at most). I kinda
hoped that showing it's somewhat widely used pattern in the kernel would
help it's cause, but alas not going to happen.

Anyway for next time around I'll crank up the Cc: knob to the max :-)

Thanks anyway for comments and stuff.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 02/12] blk: use for_each_if

2018-07-11 Thread Jens Axboe
On 7/11/18 3:08 PM, Daniel Vetter wrote:
> On Wed, Jul 11, 2018 at 10:06 PM, Tejun Heo  wrote:
>> On Wed, Jul 11, 2018 at 01:31:51PM -0600, Jens Axboe wrote:
>>> I don't think there's a git easy way of sending it out outside of
>>> just ensuring that everybody is CC'ed on everything. I don't mind
>>> that at all. I don't subscribe to lkml, and the patches weren't
>>> sent to linux-block. Hence all I see is this stand-alone patch,
>>> and logic would dictate that it's stand-alone (but it isn't).
> 
> Hm yeah I forgot to add linux-block. But others where there's no
> dedicated list (or get_maintainers.pl didn't have one) also complained
> about not getting Cc'ed, and I can't Cc everyone for sweeping changes.

I don't personally see a problem with just CC'ing everyone.

>> What I sometimes do is including a short blurb on each patch giving
>> the overview and action hints (e.g. this is part of patchset doing XYZ
>> and should be routed such and such).  It's a bit redundant but has
>> worked pretty well for patchsets with dependenat & sweeping changes.
> 
> Yeah I guess I can just copypaste/summarize patch 1 to all the
> subsequent patches, sounds like the best option.

Another approach might be to submit the first independent patch
separately. Once that's in the kernel, you can send out the rest
as independent patches instead of doing a cross-kernel series that
all depend on one single patch. Seems to me that's where you run
into issues, and it can be avoided quite easily.

-- 
Jens Axboe

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 02/12] blk: use for_each_if

2018-07-11 Thread Daniel Vetter
On Wed, Jul 11, 2018 at 10:06 PM, Tejun Heo  wrote:
> On Wed, Jul 11, 2018 at 01:31:51PM -0600, Jens Axboe wrote:
>> I don't think there's a git easy way of sending it out outside of
>> just ensuring that everybody is CC'ed on everything. I don't mind
>> that at all. I don't subscribe to lkml, and the patches weren't
>> sent to linux-block. Hence all I see is this stand-alone patch,
>> and logic would dictate that it's stand-alone (but it isn't).

Hm yeah I forgot to add linux-block. But others where there's no
dedicated list (or get_maintainers.pl didn't have one) also complained
about not getting Cc'ed, and I can't Cc everyone for sweeping changes.

> What I sometimes do is including a short blurb on each patch giving
> the overview and action hints (e.g. this is part of patchset doing XYZ
> and should be routed such and such).  It's a bit redundant but has
> worked pretty well for patchsets with dependenat & sweeping changes.

Yeah I guess I can just copypaste/summarize patch 1 to all the
subsequent patches, sounds like the best option.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 02/12] blk: use for_each_if

2018-07-11 Thread Tejun Heo
On Wed, Jul 11, 2018 at 01:31:51PM -0600, Jens Axboe wrote:
> I don't think there's a git easy way of sending it out outside of
> just ensuring that everybody is CC'ed on everything. I don't mind
> that at all. I don't subscribe to lkml, and the patches weren't
> sent to linux-block. Hence all I see is this stand-alone patch,
> and logic would dictate that it's stand-alone (but it isn't).

What I sometimes do is including a short blurb on each patch giving
the overview and action hints (e.g. this is part of patchset doing XYZ
and should be routed such and such).  It's a bit redundant but has
worked pretty well for patchsets with dependenat & sweeping changes.

Thanks.

-- 
tejun
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 02/12] blk: use for_each_if

2018-07-11 Thread Jens Axboe
On 7/11/18 12:50 PM, Daniel Vetter wrote:
> On Wed, Jul 11, 2018 at 8:30 PM, Jens Axboe  wrote:
>> On 7/11/18 10:45 AM, Tejun Heo wrote:
>>> On Wed, Jul 11, 2018 at 09:40:58AM -0700, Tejun Heo wrote:
 On Mon, Jul 09, 2018 at 10:36:40AM +0200, Daniel Vetter wrote:
> Makes the macros resilient against if {} else {} blocks right
> afterwards.
>
> Signed-off-by: Daniel Vetter 
> Cc: Tejun Heo 
> Cc: Jens Axboe 
> Cc: Shaohua Li 
> Cc: Kate Stewart 
> Cc: Greg Kroah-Hartman 
> Cc: Joseph Qi 
> Cc: Daniel Vetter 
> Cc: Arnd Bergmann 

 Acked-by: Tejun Heo 

 Jens, it'd probably be best to route this through block tree.
>>>
>>> Oops, this requires an earlier patch to move the for_each_if def to a
>>> common header and should be routed together.
>>
>> Yeah, this is a problem with the submission.
>>
>> Always (ALWAYS) CC folks on at least the cover letter and generic
>> earlier patches. Getting just one patch sent like this is mostly
>> useless, and causes more harm than good.
> 
> Ime sending a patch with more than 20 or so recipients means it gets
> stuck everywhere in moderation queues. Or outright spam filters. I
> thought the correct way to do this is to cc: mailing lists (lkml has
> them all), but apparently that's not how it's done. Despite that all
> the patch series I get never have the cover letter addressed to me
> either.
> 
> So what's the magic way to make this possible?

I don't think there's a git easy way of sending it out outside of
just ensuring that everybody is CC'ed on everything. I don't mind
that at all. I don't subscribe to lkml, and the patches weren't
sent to linux-block. Hence all I see is this stand-alone patch,
and logic would dictate that it's stand-alone (but it isn't).

-- 
Jens Axboe

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 02/12] blk: use for_each_if

2018-07-11 Thread Daniel Vetter
On Wed, Jul 11, 2018 at 8:30 PM, Jens Axboe  wrote:
> On 7/11/18 10:45 AM, Tejun Heo wrote:
>> On Wed, Jul 11, 2018 at 09:40:58AM -0700, Tejun Heo wrote:
>>> On Mon, Jul 09, 2018 at 10:36:40AM +0200, Daniel Vetter wrote:
 Makes the macros resilient against if {} else {} blocks right
 afterwards.

 Signed-off-by: Daniel Vetter 
 Cc: Tejun Heo 
 Cc: Jens Axboe 
 Cc: Shaohua Li 
 Cc: Kate Stewart 
 Cc: Greg Kroah-Hartman 
 Cc: Joseph Qi 
 Cc: Daniel Vetter 
 Cc: Arnd Bergmann 
>>>
>>> Acked-by: Tejun Heo 
>>>
>>> Jens, it'd probably be best to route this through block tree.
>>
>> Oops, this requires an earlier patch to move the for_each_if def to a
>> common header and should be routed together.
>
> Yeah, this is a problem with the submission.
>
> Always (ALWAYS) CC folks on at least the cover letter and generic
> earlier patches. Getting just one patch sent like this is mostly
> useless, and causes more harm than good.

Ime sending a patch with more than 20 or so recipients means it gets
stuck everywhere in moderation queues. Or outright spam filters. I
thought the correct way to do this is to cc: mailing lists (lkml has
them all), but apparently that's not how it's done. Despite that all
the patch series I get never have the cover letter addressed to me
either.

So what's the magic way to make this possible?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 02/12] blk: use for_each_if

2018-07-11 Thread Jens Axboe
On 7/11/18 10:45 AM, Tejun Heo wrote:
> On Wed, Jul 11, 2018 at 09:40:58AM -0700, Tejun Heo wrote:
>> On Mon, Jul 09, 2018 at 10:36:40AM +0200, Daniel Vetter wrote:
>>> Makes the macros resilient against if {} else {} blocks right
>>> afterwards.
>>>
>>> Signed-off-by: Daniel Vetter 
>>> Cc: Tejun Heo 
>>> Cc: Jens Axboe 
>>> Cc: Shaohua Li 
>>> Cc: Kate Stewart 
>>> Cc: Greg Kroah-Hartman 
>>> Cc: Joseph Qi 
>>> Cc: Daniel Vetter 
>>> Cc: Arnd Bergmann 
>>
>> Acked-by: Tejun Heo 
>>
>> Jens, it'd probably be best to route this through block tree.
> 
> Oops, this requires an earlier patch to move the for_each_if def to a
> common header and should be routed together.

Yeah, this is a problem with the submission.

Always (ALWAYS) CC folks on at least the cover letter and generic
earlier patches. Getting just one patch sent like this is mostly
useless, and causes more harm than good.

-- 
Jens Axboe

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 02/12] blk: use for_each_if

2018-07-11 Thread Tejun Heo
On Wed, Jul 11, 2018 at 09:40:58AM -0700, Tejun Heo wrote:
> On Mon, Jul 09, 2018 at 10:36:40AM +0200, Daniel Vetter wrote:
> > Makes the macros resilient against if {} else {} blocks right
> > afterwards.
> > 
> > Signed-off-by: Daniel Vetter 
> > Cc: Tejun Heo 
> > Cc: Jens Axboe 
> > Cc: Shaohua Li 
> > Cc: Kate Stewart 
> > Cc: Greg Kroah-Hartman 
> > Cc: Joseph Qi 
> > Cc: Daniel Vetter 
> > Cc: Arnd Bergmann 
> 
> Acked-by: Tejun Heo 
> 
> Jens, it'd probably be best to route this through block tree.

Oops, this requires an earlier patch to move the for_each_if def to a
common header and should be routed together.

Thanks.

-- 
tejun
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 02/12] blk: use for_each_if

2018-07-11 Thread Tejun Heo
On Mon, Jul 09, 2018 at 10:36:40AM +0200, Daniel Vetter wrote:
> Makes the macros resilient against if {} else {} blocks right
> afterwards.
> 
> Signed-off-by: Daniel Vetter 
> Cc: Tejun Heo 
> Cc: Jens Axboe 
> Cc: Shaohua Li 
> Cc: Kate Stewart 
> Cc: Greg Kroah-Hartman 
> Cc: Joseph Qi 
> Cc: Daniel Vetter 
> Cc: Arnd Bergmann 

Acked-by: Tejun Heo 

Jens, it'd probably be best to route this through block tree.

Thanks.

-- 
tejun
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 02/12] blk: use for_each_if

2018-07-09 Thread Daniel Vetter
Makes the macros resilient against if {} else {} blocks right
afterwards.

Signed-off-by: Daniel Vetter 
Cc: Tejun Heo 
Cc: Jens Axboe 
Cc: Shaohua Li 
Cc: Kate Stewart 
Cc: Greg Kroah-Hartman 
Cc: Joseph Qi 
Cc: Daniel Vetter 
Cc: Arnd Bergmann 
---
 include/linux/blk-cgroup.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/blk-cgroup.h b/include/linux/blk-cgroup.h
index 6c666fd7de3c..f1c3afe42c26 100644
--- a/include/linux/blk-cgroup.h
+++ b/include/linux/blk-cgroup.h
@@ -382,7 +382,7 @@ static inline void blkg_put(struct blkcg_gq *blkg)
  */
 #define blkg_for_each_descendant_pre(d_blkg, pos_css, p_blkg)  \
css_for_each_descendant_pre((pos_css), &(p_blkg)->blkcg->css)   \
-   if (((d_blkg) = __blkg_lookup(css_to_blkcg(pos_css),\
+   for_each_if (((d_blkg) = __blkg_lookup(css_to_blkcg(pos_css), \
  (p_blkg)->q, false)))
 
 /**
@@ -397,7 +397,7 @@ static inline void blkg_put(struct blkcg_gq *blkg)
  */
 #define blkg_for_each_descendant_post(d_blkg, pos_css, p_blkg) \
css_for_each_descendant_post((pos_css), &(p_blkg)->blkcg->css)  \
-   if (((d_blkg) = __blkg_lookup(css_to_blkcg(pos_css),\
+   for_each_if (((d_blkg) = __blkg_lookup(css_to_blkcg(pos_css), \
  (p_blkg)->q, false)))
 
 /**
-- 
2.18.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx